[S3] Characterize TypeScript client lifecycle boundaries #137

Goal

Add focused, deterministic characterization tests for the existing TypeScript client lifecycle before changing activation or restart orchestration.

This is S3 of the refactoring roadmap. S1 and S2 have merged; S2 (#134, closing #132) protects the Python/settings boundary. S3 protects the asynchronous client-side behavior. The deliverable is a tests-only PR, not a new lifecycle API or a scheduler fix.

Baseline and files

Inspection baseline: f99929bded23913259fa293ccf7f785c7606449c, the merged S2 commit. Start from current main and recheck existing coverage.

Expected change locations: typescript/tests/activation.test.ts and typescript/tests/server.test.ts. Make a small test-helper/mock adjustment only if needed; no new test framework.

Existing coverage: do not duplicate it

There are already tests for basic configured/missing-interpreter startup, clearing an interpreter, two rapid restart calls, package-change subscription, deactivation, and formatter registration/crash/recovery/already-Running/no-client cases.

The gaps are primarily controlled asynchronous boundaries, ordering, timer cancellation and ownership, not an absence of lifecycle tests.

Two examples:

• The existing test named stops old client before creating new one asserts that stop() was called, but does not hold that promise pending to prove that replacement waits for it.
• The existing in-flight-disposal test checks that lsClient remains undefined, but does not establish that the restart stub was entered before disposal. It can exercise an earlier disposed guard instead of late-result cleanup.

Focused scope

1. Restart ordering and failure outcomes

Exercise the real restartServer orchestration with the existing mocked client/API dependencies:

• Hold the old client's stop() pending. Assert that the replacement has not started before old stop completes.
• Hold the replacement's start() pending. Assert busy status while starting and ready/non-busy only after successful startup/trace setup.
• Protect the explicit success-status update even when the mock's initial start does not emit a state event. The current mock changes its state directly in start().
• Characterize creation failure and start rejection: returned client is undefined, error status is reported, and a created client is disposed after start failure. Failure must not install a successful-running listener.
• Strengthen the existing old-stop-rejection case only as needed: it currently logs/continues rather than rejecting the entire restart.

Do not stub the exported createServer and assume same-module local calls are replaced. Use existing dependency/prototype stubs or an input that actually exercises the creation-failure branch.

2. Overlap, debounce and cancellation

• Hold a restart genuinely pending; prove that another runServer() request resolves before that restart finishes.
• Verify multiple overlapping requests reset/coalesce the deferred retry rather than launch concurrent restarts.
• After completing the active restart, advance fake time to the expected trailing edge and observe the queued restart.
• Protect disposal cancellation for queued work: no restart from a pending retry or package-change debounce after the context is disposed.
• For package-change bursts, exercise the captured subscription callback and verify the trailing-edge behavior, not just subscription registration.

Preserve the current contract: await runServer() on the overlapping-request path acknowledges scheduling; it does not promise a fresh Running client.

3. Real in-flight disposal and resource ownership

Strengthen the existing disposal case using an explicit "restart entered" barrier. Dispose only after the stub is known to be waiting, then resolve it with a client and disposable:

• The late client is not assigned to ctx.lsClient.
• Its successful cleanup path calls stop() and dispose().
• Returned per-client disposables are released.
• Further calls/timers cannot resurrect the disposed context.

Also strengthen the existing deactivation test to protect context disposal before awaiting client stop, rather than just asserting both were called. Reuse existing cases/helpers where possible.

Do not introduce a new promise that deactivation waits for all pending starts; that would be later lifecycle design work.

4. Formatter ownership across an asynchronous replacement

Add only missing boundary assertions around the existing formatter suite:

• With a healthy Running old client and a held replacement, do not register an extra placeholder early.
• At the relevant existing completion/state transitions, preserve fallback registration and healthy-client removal.
• After listener disposal/replacement, old-client state events must not re-register the placeholder or affect the current generation.

Use the existing stateful client mock and real listener-disposal behavior. Do not equate disposal of one client with disposal of the extension-lifetime fallback feature.

Assert actual observed boundaries, not an idealized gap-free state machine. If inspection exposes a lifecycle gap, record it separately rather than silently fixing production behavior to satisfy this issue.

Concrete test sketches

These are illustrative structure, not prescribed helper APIs.

An overlap is not an awaited restart

const entered = deferred<void>();
const finished = deferred<RestartServerResult>();
restartStub.callsFake(() => {
    entered.resolve();
    return finished.promise;
});

const first = ctx.runServer();
await entered.promise;
await ctx.runServer(); // Current compatibility contract: resolves while first is pending.
assert.isUndefined(ctx.lsClient);
assert.strictEqual(restartStub.callCount, 1);

finished.resolve({ client, disposables: [] });
await first;
// Advance bounded fake time and assert the queued retry separately.

In the real test, track that first is still unsettled at the overlap assertion. Do not infer this only from lsClient.

Observable lifecycle trace

old.stop requested
  [hold old stop: no new start]
old.stop completed
status busy
new.start requested
  [hold start: no ready status]
new.start completed
trace configured
status ready

A simple event array or Sinon ordering assertions are enough. Protect relevant relative ordering without snapshotting every log line, localization string, internal await or private variable.

A disposal test must actually reach the in-flight boundary

const running = ctx.runServer();
await restartEntered.promise;
ctx.dispose();
restartFinished.resolve({ client: lateClient, disposables: [listener] });
await running;

assert.isUndefined(ctx.lsClient);
sinon.assert.calledOnce(lateClient.stop);
sinon.assert.calledOnce(lateClient.dispose);
sinon.assert.calledOnce(listener.dispose);

Do not add a blanket new cleanup guarantee for exceptional stop/dispose paths that the current implementation does not provide.

Implementation and validation

• Use existing Mocha TDD, Chai, Sinon sandboxes and fake timers.
• Prefer deferred promises signaling meaningful entry/completion points over arbitrary sleeps or a fixed number of microtask flushes.
• Advance timers by bounded intervals. Do not drain all timers while a held restart keeps rescheduling itself.
• Dispose contexts, restore clocks/stubs/listeners and settle owned promises in cleanup so cases do not leak into each other.
• Keep helpers local and small. No lifecycle tracing framework, production test hooks, real VS Code host or new dependencies.

Suggested checks from the repository root:

npm --prefix .\typescript test -- --grep 'createToolContext|restartServer|deactivateServer'
npm --prefix .\typescript test
npm --prefix .\typescript run build
npm --prefix .\typescript run lint
git diff --check

Run the targeted suite first, then the full suite/build/lint. Follow .nvmrc where available and report actual local runtime/platform. Also lint changed test files with the existing ESLint configuration if the standard script only includes src.

S2's reference baseline was 148 passing TypeScript tests; do not hardcode a count. S1 already runs the test command in CI. The 14 pre-existing production-source formatting failures are outside this issue.

Acceptance criteria

• Tests cover the four focused areas above without duplicating already adequate cases.
• Deferred barriers prove operations are actually in flight at the assertion/disposal point.
• The overlap test explicitly preserves request-acceptance versus completion semantics.
• Retry/package timers cannot fire after context disposal; burst/reset behavior is deterministic.
• Lifecycle ordering, status outcomes, late-result cleanup and stale-listener isolation are observable assertions.
• Existing and new TS tests, build and relevant lint checks pass.
• No changes to production code, API/payload shape, scheduling delays, defaults, dependencies, lockfiles or workflow behavior.
• Any newly discovered production issue is documented separately rather than fixed or concealed by mocks/blanket skips in this PR.

Non-goals

No activation facade, scheduler/state-machine rewrite, new restart-readiness API, behavior fix, logging/status ownership refactor, executor/runner work, consumer migration, Python/settings changes, version-automation cleanup (#135), directory moves or S4 packaged-consumer infrastructure.