🤖 refactor: adopt handlerGen as the oRPC router default and convert gateway OAuth procedures - #4032
Merged
Merged
Conversation
…ment handlerGen convention
This comment has been minimized.
This comment has been minimized.
Member
Author
|
@codex review |
Member
Author
|
@codex security review |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This was referenced Sep 1, 2026
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 6, 2026
…w Scope (coder#4033) ## Summary Phase 5 of the progressive Effect migration (first phase of Wave 2, the wave's gating item deferred since coder#4027): converts `OAuthFlowManager` internals to Effect with real resource safety. Every registered desktop OAuth flow now owns a per-flow `Scope` whose release finalizers guarantee cleanup (registration-timeout clear, deferred settlement, loopback-server close) on every termination path — finish, cancel, caller-timeout race, duplicate registration, `shutdownAll`, and defects. The Promise-based public API is preserved as thin `Effect.runPromise` facades, so the three not-yet-converted OAuth services (`coderOauthService`, `codexOauthService`, `muxGovernorOauthService`) and all existing tests work unchanged. ## Background Wave 1 (coder#4022, coder#4025, coder#4027, coder#4028, coder#4030, coder#4031, coder#4032) established the house pattern: `Effect.gen` internals, thin `runPromise` facades, `handlerGen` for oRPC procedures. coder#4027 converted `muxGatewayOauthService` but explicitly deferred the shared flow-lifecycle manager: its resources (loopback `http.Server`, registration `setTimeout`, result deferred) were cleaned up via ad-hoc `try/catch` + fire-and-forget `void closeServer(...)`, and a defect while resolving the deferred silently skipped the server close. This PR is the acquire/release case that deferral pointed at, and unblocks Phase 6 (batch conversion of the sibling OAuth services). ## Implementation **Per-flow Scope design** — `register` creates a `Scope.makeUnsafe()` per flow and moves ownership of the caller-acquired resources into it via one `Effect.acquireRelease` per resource (a combined acquisition would install its finalizer only after every step succeeded, leaking earlier resources on a later defect — the coder#4031 Codex P2 lesson). Release runs in reverse acquisition order, preserving the pre-Effect `finish` ordering: clear registration timeout → settle deferred (waiters unblock before the async close) → close loopback server (awaited). **Deferred settlement via finalizer** — each `ActiveFlow` carries a mutable `finalResult` staged by the terminating path (finish/cancel/shutdown/replace); the settle finalizer resolves the caller's deferred with it. Settlement is therefore scope-guaranteed rather than an ad-hoc `resolve` call, with a defensive fallback result so waiters can never hang. **Caller-facing timeout race** — `waitFor` maps to `Effect.timeout` over `Effect.promise` on the shared deferred: the local wait timer is fiber-managed (interruption clears it), stays separate from the registration-time timeout, and on any error result runs `finish` for shared cleanup. The cleanup's synchronous bookkeeping (map removal, completed-result recording) runs before `waitFor` resolves — exact parity with the old sync prefix — while the async release runs in an `Effect.forkDetach` fiber, replacing the old `void this.finish(...)` fire-and-forget with a supervised fiber that survives the caller's completion (verified by a live-runtime probe: detached fibers outlive the parent, `runFork`/`runPromise` execute synchronously to first suspension, and a throwing finalizer does not skip its siblings). **shutdownAll contract** — preserved as async (`Promise<void>` facade): `serviceContainer.dispose` awaits it, and loopback-server closes are bounded by the server's force-finish socket handling. It never rejects; release defects are caught (`Effect.catchDefect`) and logged at debug level, per the startup/shutdown-must-never-crash rule. **Effect-native surface** — `waitForEffect` / `cancelEffect` / `finishEffect` / `cancelAllEffect` / `shutdownAllEffect` are public (wire-shaped, never-failing — same shape as coder#4032's Effect surfaces). `muxGatewayOauthService`'s Effect pipeline now yields `finishEffect` directly instead of `Effect.promise(() => …finish(...))`, and its registration-timeout callback uses `Effect.runFork(finishEffect(...))` instead of `void finish(...)`. **Not converted to Effect `Deferred`** — the result deferred's identity is part of the public caller-owned `OAuthFlowEntry` (the three unconverted services construct entries with `createDeferred`), so swapping it would break the "existing callers unchanged" contract; revisit when Phase 6 converts entry construction. ## Validation - All 18 pre-existing `oauthFlowManager` tests pass byte-identical, plus all OAuth service suites (194 tests: coder/codex/muxGateway/muxGovernor/mcp/copilot/codexOauthAuth) and loopback-server/oauthUtils suites. - Two new behavioral tests for the genuinely-new guarantees: (1) server close + timeout clear still happen when the deferred `resolve` throws (the pre-Effect code skipped the close — this test fails on the old implementation), and (2) the detached cleanup fiber completes after `waitFor` has already returned on the timeout path (guards against accidental child-fiber supervision, where the release would be interrupted with the caller). - A standalone Effect v4 runtime probe validated the semantics the design relies on (finalizer independence under defects, reverse sequential release order, eager sync-prefix execution of `runPromise`/`runFork`, `forkDetach` outliving the parent, `Effect.timeout` + `Effect.catch` over `Effect.promise`). - `make static-check` green. ## Risks Low-to-moderate: this is shared lifecycle code under four OAuth login flows (Gateway, Governor, Codex, Coder). The public API, observable ordering (map removal before `finish` resolves, deferred settlement before server close, synchronous `register`), and error strings are preserved exactly; regressions would surface as leaked loopback listeners, hung `waitFor` calls, or unsettled deferreds — all covered by the existing + new suites. ## Lessons for Phase 6 Phase 6 is the batch conversion of `coderOauthService`, `codexOauthService`, `muxGovernorOauthService`, `copilotOauthService`, plus their ~20 router sites. Notes to make it mechanical: - The manager now exposes never-failing, wire-shaped `waitForEffect`/`cancelEffect`/`finishEffect`/`shutdownAllEffect`, so converted service pipelines can yield them directly (see `desktopCallbackPipeline` in `muxGatewayOauthService` as the template), and registration-timeout callbacks should use `Effect.runFork(manager.finishEffect(...))`. - `beginFinish`'s sync-bookkeeping/async-release split is the pattern to reach for wherever a service needs "unregister now, release in background" semantics. - Each sibling's `startDesktopFlow` should become uninterruptible like the gateway's (coder#4032): a client abort between loopback acquisition and `register` would otherwise leak the server. - `coderOauthService` is the outlier: it has extra commit-path liveness checks (`has`) and multi-step persist/commit finish calls (~10 `desktopFlows.*` sites vs ~5 in the others) — expect most of the Phase 6 effort there. - **Recommendation: two PRs.** PR A: codex + governor + copilot service internals (near-identical DesktopFlow shape, mechanical) together with their router procedures moving to `handlerGen` (the `waitFor`/`cancel` handlers for the gateway can join here — the router comment at `muxGatewayOauth` already points at this). PR B: `coderOauthService` alone — its commit/persist liveness semantics deserve isolated review, and a combined PR would bury it under the mechanical churn. --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `anthropic:claude-fable-5` • Thinking: `xhigh`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 6, 2026
…acquireRelease + scope-tied debounce fiber) (coder#4040) ## Summary Phase 10 (final chartered phase of Wave 2 of the progressive Effect migration) converts the four incremental resource seams inside `StreamManager` that Phase 4 (coder#4032) mapped and coder#4039 refined — temp-dir lifecycle, partial-write debounce, error-categorization state, and usage accounting — without touching the stream engine core, `StreamManager`'s public API, or any crash-recovery semantics. ## Background Phase 4 deliberately deferred wholesale `streamManager.ts` conversion (5,281 lines; fibers suspend on I/O; AbortController-centric; 30+ field `WorkspaceStreamInfo`) but identified four seams that convert cleanly. coder#4039 established the implementation templates this PR applies: per-resource `Effect.acquireRelease`, scope-tied fibers via `Effect.forkIn`, the plain-mutables rule for AI-SDK-callback state, and the zero-suspension foreign-callsite emit pattern. ## Implementation ### Seam 1 — Temp-dir lifecycle → per-resource `Effect.acquireRelease` (converted) Each stream now creates a `Scope` (`resourceScope`) in `startStream`. The temp dir is acquired through `Effect.acquireRelease`: the release finalizer (the existing fire-and-forget `cleanupStreamTempDir`) registers only after acquisition succeeds. `Scope.close` replaces the previous two hand-coordinated cleanup sites (`startStream`'s `!streamRegistered` finally and `processStreamWithCleanup`'s finally); close is idempotent, so ownership transfer at registration can never double-release or leak. The falsy guard for whitebox `providedRuntimeTempDir: ""` fixtures is preserved inside the finalizer. ### Seam 2 — Partial-write debounce → scope-tied fiber (converted) `partialWriteTimer` (`setTimeout` handle + three manual `clearTimeout` sites) becomes `partialWriteFiber`, forked into the stream's `resourceScope` (coder#4039's scope-tied ticker template, adapted to debounce semantics). Re-arm and explicit-flush cancellation go through one `interruptPartialWriteFiber` helper; the stream-teardown cancellation is now implicit — closing the scope interrupts a pending flush, so a debounced write can never fire after the stream ends and resurrect `partial.json` for a dead stream. `Effect.runSync(Effect.forkIn(...))`/`runFork` execute synchronously up to the sleep, preserving the previous `setTimeout` registration ordering; Effect's clock registers a plain `setTimeout` under the hood, so scheduling semantics are unchanged. ### Seam 3 — Error categorization / `lostResponseIds` (stayed plain, documented) Applying coder#4039's plain-mutables rule honestly: `lostResponseIds` is mutated exclusively from non-Effect contexts (AI-SDK error paths inside `processStreamWithCleanup`) and read synchronously by `isResponseIdLost` (a spy-pinned public seam used by `TurnRequestBuilder`). No fiber reads or mutates it, so a `Ref` would add ceremony without adding safety. `categorizeError` is pure synchronous classification — nothing to convert. Both stay plain; the rationale is now documented at the declaration site. This seam is smaller than the coder#4032 map suggested, by design. ### Seam 4 — Usage accounting (stayed plain, documented; zero-suspension invariant pinned in comments) `cumulativeUsage` / `lastStepUsage` / `cumulativeProviderMetadata` are mutated only by the AI-SDK `fullStream` loop (`finish-step`) and plain retry/reset methods — foreign callsites, never fibers — so they stay plain mutables per the rule. The SubscriptionEmit-transfer property already holds structurally and is now documented: the `usage-delta` emit runs with zero suspension after the mutation (`emitTurnEvent` invokes the sink synchronously), so downstream Effect queue bridges (coder#4039's `SubscriptionEmit.push` → `Queue.offerUnsafe`) observe usage events in mutation order. ### Interruption-posture audit - The debounce fiber is interruptible by design (cancelling a pending checkpoint write is exactly what `clearTimeout` did). If interrupted mid-flush, the underlying `flushPartialWrite` promise still runs to completion — no torn write; writes stay serialized via `partialWritePromise`. - Temp-dir release and fiber interruption are synchronous finalizers on a root fiber (`Effect.runFork(Scope.close(...))`); nothing can interrupt teardown mid-way. - No awaits were added between liveness checks and writes; every abort-signal checkpoint in `startStream` keeps its position. - Verified empirically against effect@4.0.0-rc.112: `forkIn` into a closed scope never throws and the fiber never runs; double `Scope.close` releases exactly once; closing a scope with a sleeping fiber interrupts it and still runs later finalizers. ### What stayed out (per charter) The stream engine core (`fullStream` loop, AbortController seams, `WorkspaceStreamInfo` restructure), `StreamManager`'s public API, and all spy-pinned seams (`createTempDirForStream` mockResolvedValue spies, `cleanupStreamTempDir` Reflect-extraction, chaos-test `Reflect.set` internals) are untouched. ## Validation - `streamManager.test.ts` (119 existing + 2 new), `streamManager.chaos.test.ts`, `streamManager.modelOnlyNotifications.test.ts`, `streamSimulation`, `replayBufferedStreamMessageRelay`, `agentSession.preStreamError`, `agentSession.resumeStreamEmptyHistory`, `aiService`, `hooks`, `turnRequestBuilder` — all pass unchanged. - New tests cover the two genuinely new invariants: temp-dir release runs exactly once across the ownership transfer, and a pending debounced partial write is interrupted at stream end (deterministically armed via stream-state gating, not sleeps). - `make static-check` green. - Standalone probe validated the Effect v4 scope/fiber edge semantics the design relies on (closed-scope fork, double-close idempotence, sleeping-fiber interruption). ## Risks `streamManager.ts` is the most crash-sensitive file in the repo. The regression surface here is partial-write scheduling and temp-dir cleanup: - **Partial-write loss window**: unchanged — a pending debounce cancelled at teardown was also dropped by the old `clearTimeout`; terminal paths still perform their own awaited flush/commit. - **Partial resurrection after stream end**: strictly improved and now test-pinned (scope close interrupts the pending flush; previously relied on `clearTimeout` placement). - **Temp-dir leak/double-free**: strictly improved (idempotent single-owner release vs. two coordinated call sites + flag). - Crash-recovery semantics (repairable incomplete state, malformed-history tolerance) are untouched: no persistence formats, no request-building paths, no commit predicates changed. ## Wave 2 completion state Wave 2 (coder#4033 OAuth flow scopes, coder#4034 OAuth services + router sites, coder#4035 coderOauthService, coder#4036 Config semaphore, coder#4038 consolidation/status generator, coder#4039 oRPC subscription Stream bridge, this PR) is complete. What remains Promise-based repo-wide, in suggested Wave 3 order: 1. **StreamManager engine core** — the `fullStream` consumption loop, AbortController lifecycle, and `WorkspaceStreamInfo` restructure; deliberately deferred (I/O-suspended fiber teardown needs async `Scope.close`, which the current sync teardown contract can't host). Revisit after a ManagedRuntime/Layer DI RFC. 2. **workspace/project/task services** — lock-heavy CRUD services; natural next targets for the wrap-around-locks + `Effect.gen`-behind-facade house pattern. 3. **WorkflowService internal event seam** (deferred from coder#4039) and `createTickIterable` (no resource to manage; convert opportunistically). 4. **ManagedRuntime/Layer dependency injection** — an RFC-scale change replacing constructor wiring; unlocks `TestClock` for the timing-sensitive suites and app-lifetime scopes. 5. **Schema at persistence boundaries** — `Schema.decodeUnknown` for config/history/session artifacts. 6. **OAuth token-refresh worker** (optional; needs product sign-off) and effect v4 GA + oRPC lockstep upgrades. --- _Generated with `xum` • Model: `anthropic:claude-fable-5` • Thinking: `xhigh` • Cost: `$n/a`_ <!-- mux-attribution: model=anthropic:claude-fable-5 thinking=xhigh costs=n/a -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 6, 2026
…aborts and awaits in-flight streams (Wave 4 PR 1) (coder#4070) ## Summary `StreamManager` becomes the first (and intended) occupant of `AppFiberScope`: every started stream is wrapped in one supervisor fiber, so `ServiceContainer.dispose()` (desktop, `xum server`, ACP) and the CLI cleanup lists now **abort and await in-flight streams** — partial flushed with usage, `stream-abort` (`abortReason: "system"`) delivered, the interrupted assistant message committed into `chat.jsonl`, `partial.json` removed — *before* the bridges and sessions are torn down. The same PR closes the two adjacent cancel races the shutdown path would have widened (Wave 4 plan D3): a cancel landing after the stream loop finished no longer resurrects `partial.json` or emits a second terminal event, and concurrent cancellers (user stop racing dispose) join one cleanup. ## Background Until now `dispose()` had no `streamManager` step: an in-flight stream died with the process and was reconciled from `partial.json` (≤ 500 ms stale) on the *next* load, leaving an empty assistant placeholder row in `chat.jsonl` until then. `AppFiberScope` (Phase 11) was built for exactly this and had no occupant. This is PR 1 of the Effect migration Wave 4 plan (appended below): D1 — the fiber is the ownership/supervision unit, the `AbortSignal` stays the cancellation transport; D2 — supervisor topology; D3 — adjacent cancel races; D4 — the 2 s close bound stays (outer budgets are 5 s). ## Implementation - **`StreamManager.superviseEngine`** (D2): after `streamInfo.processingPromise` is assigned (byte-identical) it forks `Effect.promise(() => processingPromise)` (zero-arity thunk → rc.112 allocates no internal AbortController) into `engineScope` with `{ startImmediately: true }`. `onInterrupt` → `Effect.uninterruptible(Effect.promise(async () => { await cancelStreamSafely(ws, info, "system"); await info.completionController.promise; }))`; `catchDefect` → `log.warn`. A stream that finishes on its own exits the fiber, which removes its scope finalizer (no residue). A stream started after the scope closed is interrupted synchronously by `forkIn` and aborted right away (fail-closed during shutdown, pinned in `appFiberScope.test.ts`). The pre-registration window (`pendingStreamStarts`) stays unsupervised (documented). - **`engineScope?: Scope.Closeable`** is a trailing optional 6th constructor parameter, wired from `AppFiberScopeTag` in `StreamManagerLive` (`di/layers/core.ts`; `AppFiberScopeTag` added to `CoreInputTags` — both roots already provide it via `runtimeSeams`). Default `undefined` = today's behavior for every direct construction (tests, `aiService.ts` compat path). - **D3(a)** `cleanupAbortedStream`: after `await processingPromise`, return if `terminalCompletion !== undefined` (completed/failed while the cancel was in flight). **Plus** a `partialRetired` marker set in the completion path right before `deletePartial`: `flushPartialWrite` becomes a no-op afterwards, because the *pre-abort flush* in `cancelStreamSafely` (not only the abort bookkeeping) re-created `partial.json` when the cancel landed between `deletePartial` and `COMPLETED`. - **D3(b)** `cancelStreamSafely`: per-stream `cancelPromise` latch, checked and assigned synchronously at entry (after the existing `COMPLETED` early return, before the first `await`). `checkSoftCancelStream` shares the latch (`??=`) so a soft interrupt racing a hard cancel/dispose also yields exactly one `stream-abort`. - Docs: `appRuntime.ts` contract ("Deliberately not done" updated; step 2 now says what it aborts), `appFiberScope.ts`, `serviceContainer.ts` step comment. New `[shutdown] streamManager.abortStream { workspaceId, messageId, ms }` debug line per supervised stream (shutdownStep style). Net product change: 5 files, +196/−42 (≈ 70 non-comment lines added). ## PR 1 notes ### Pre-work findings 1. **`processStreamWithCleanup` never rejects** — body is `try { … } catch { await handleStreamFailure } finally { … }`; a throw from `handleStreamFailure`/`finally` *could* still reject, but `startStream` assigns `processingPromise = processStreamWithCleanup(...).catch(log.error)`, so the promise the supervisor wraps never rejects. `catchDefect` stays as belt-and-braces. 2. **Probe test** (`appFiberScope.test.ts`, +2 cases): `closeScopeBounded` interrupts a fiber suspended on `Effect.promise` **without settling the wrapped promise**, runs the async `onInterrupt` finalizer to completion, and resolves only afterwards (`["finalizer-start", "finalizer-end"]`). 3. **Abort observers after the finalizer** — the finalizer resolves only after `completionController.promise`, i.e. after `cleanupAbortedStream` (`writePartial` with usage) → `emitStreamAbort` → AIService sink (`readPartial` → `commitPartial` → `deletePartial` → `emit("stream-abort")` to agentSession/taskService/analytics listeners) → `.finally(settle)`. The only work left after it is the EventEmitter listeners' own async continuations (turn-phase transitions, task-handle settlement) — the same as today's `system` aborts from `taskService`. Durable order `writePartial → commitPartial → deletePartial` holds; a force-exit between any two steps leaves either `partial.json` (recovered by `agentSession.init`'s `commitPartial`) or an already-committed row whose `historySequence` update-or-append is idempotent. **No OFF-RAMP condition fired.** 4. **Measured `[shutdown] AppFiberScope closed { ms }`** (`xum server`, SIGTERM, `XUM_LOG_LEVEL=debug`, `script -f`): idle **1 ms**; one flowing Sonnet 5 stream **56 ms**; two flowing streams **95 ms** (per-stream `abortStream` 91 / 94 ms, parallel); wedged provider (scratch env-gated `fullStream` that never yields and ignores abort, not committed) **2050 ms** with the `teardown timed out` warning, `AppRuntime disposed` 5 ms (idempotent re-close), process exit **2.10 s** after SIGTERM — inside the 5 s force-exit. Baseline `main`: 1 ms (nothing supervised). 5. **Closed-scope `forkIn` pinned** (`appFiberScope.test.ts`): `startImmediately: true` into an already-closed scope runs the body to its first async boundary and then the `onInterrupt` finalizer synchronously (`["body-started", "interrupted"]`, fiber exit is a failure). `streamManager.test.ts` pins the product consequence (late start → `{ status: "aborted", abortReason: "system" }`, one `stream-abort`, registry empty). ### Deviations from the plan (all additive) - The supervisor finalizer also awaits `completionController.promise` (plan: `cancelStreamSafely` only). `cancelStreamSafely` returns *before* abort delivery — the `stream-abort` sink is where AIService commits the partial — so without this the close would resolve with `partial.json` still present and the commit racing `desktopBridgeServer.stop()`. This is what makes STOP criterion #1 ("`partial.json` absent immediately after exit") true. - `partialRetired` (see D3(a) above): the plan's guard alone left the pre-abort flush as a resurrection path; the D3(a) test was red against the guard-only variant on exactly that assertion. - `checkSoftCancelStream` joins the latch: dispose during a pending soft interrupt (queued-message preemption at tool-end) is the same double-cleanup shape D3(b) fixes for hard cancels. - `[shutdown] streamManager.abortStream` debug line — there was no log line for the abort at all, so the transcript could not show the plan's `stream-abort → AppFiberScope closed → desktopBridgeServer.stop` order; it now does. - Plan §7 (5) `xum run` Ctrl-C transcript: `xum run` installs **no** SIGINT handler (only `cli/server.ts` does), so Ctrl-C is Node's default immediate termination (exit 130) and the cleanup list — including `appFiberScope.close` — runs only on normal completion. The CLI-root path is pinned by the new `coreServicesRoot.test.ts` case instead; adding a signal handler to `xum run` is out of scope. ### Dogfooding evidence (transcripts + before/after in the first comment) | Scenario (`xum server`, SIGTERM 3 s after first text) | `AppFiberScope closed` | `partial.json` right after exit | last `chat.jsonl` row | |---|---|---|---| | `main` @ 3c06630a7 (baseline) | 1 ms | **present** (1065 chars of text) | empty assistant placeholder (0 chars) | | this branch, 1 stream | 56 ms | **absent** | assistant row, `partial: true`, 1172 chars | | this branch, 2 streams | 95 ms | absent | assistant rows committed (933 chars for the new one) | | this branch, wedged provider | 2050 ms (warn) | n/a (no text produced) | exit 2.10 s after SIGTERM | Restart UX (screenshot below, branch): the persisted interrupted text is shown above the `INTERRUPTED` barrier; the workspace then **auto-resumed the turn on startup** (agentSession startup auto-retry — `"system"` aborts do not suppress it). Verified the baseline does the same after it commits the partial on load (`[STREAM MESSAGE]` right after startup on `main`), so the only behavioral difference is *when* the partial is committed (at shutdown vs. next load). Not exercised headless: Electron `before-quit` (same `dispose()`; CI e2e).  ### Lessons for PR 3 (`initialize()` as a startup effect) - Effect v4 `runPromise`/facades reject with the raw error, so error identity is free — but any `Effect.promise` thunk wrapping an `await`ed step must be `async` and the whole pipeline needs `catchDefect` if the facade must never reject. - Bounded waits inside `dispose()` are cheap (`disposeAppRuntime` after a timed-out scope close took 5 ms because `Scope.close` is idempotent); a startup timeout should equally *not* try to unwind the abandoned step — parity with the existing "throwing step" path is the whole design. - `[startup] <step> { ms }` lines are already the right measurement surface; this run's cold start showed `taskService.initialize` 114 ms and `workspaceService.initialize` 83 ms, so a 60 s per-step bound is ≥ 500× the slowest observed step. - Keep the `xum run` parity caveat in mind: the CLI roots have no signal handling, so any "dispose on failed initialize()" work only applies where a dispose path exists (`cli/server.ts` is the one PR 3 adds). ## Validation - New behavioral tests: `streamManager.test.ts` (6: flowing close, wedged bound, late start, D3(a), D3(b) race, 50-stream residue), `streamManager.chaos.test.ts` (1 new fuzz variant, existing cases untouched; also ran 7 extra seeds locally), `serviceContainer.test.ts` (dispose aborts + awaits a real stream before `desktopBridgeServer.stop()`, checks `partial.json`/`chat.jsonl`), `coreServicesRoot.test.ts` (same via `closeScopeBounded(appFiberScope)`), `appFiberScope.test.ts` (2 probes). - Red checks: the D3(a) test fails without the two guards (2 terminal events) and the D3(b) test fails without the latch (2 `stream-abort`s). - Gate suites green: `streamManager.test.ts`, `streamManager.chaos.test.ts`, `streamManager.modelOnlyNotifications.test.ts`, `aiService.test.ts`, `agentSession.disposeRace.test.ts`, `agentSession.sinceReplayContract.test.ts`, `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `taskService.test.ts`, `workspaceService.test.ts` (one unrelated `FakeAIService` metadata test flaked once in a combined run, green isolated and on rerun), `turnRequestBuilder.test.ts`; `make static-check`. - Pre-review audits: interruption posture (the supervisor's only suspension is the promise; finalizer uninterruptible end-to-end), no defect escapes (`catchDefect`; async thunks), spy seams unchanged (`processStreamWithCleanup`, `createStreamResult`, `createStreamAtomically`, `startStream`; constructor arity trailing-optional; `Reflect.set/get` targets valid), sync-start (`processingPromise` assigned before the fork; `runSync(forkIn)` synchronous), no constructor side effects, zero-suspension latch. ## Risks - **Shutdown path** (desktop/`xum server`/ACP): a flowing stream now costs one chunk (~50–100 ms) at dispose; a wedged provider costs the existing 2 s bound and a warning — identical outcome to today's process exit. Rollback: revert the one `core.ts` wiring line (`engineScope` undefined → today's behavior); the D3 guards are independent bug fixes. - **Cancel semantics**: with the latch, the *first* canceller's `abortReason`/`abandonPartial` win; a discard racing a system cancel by a few ms keeps the partial instead of dropping it (previously both cleanups ran and emitted two aborts). - **Streams started mid-shutdown** are aborted as `"system"` (fail-closed): the caller sees a normal `Ok(handle)` whose completion settles `aborted`, and the turn is recovered on next load exactly like any other `system` abort. --- <details> <summary>📋 Implementation Plan</summary> # Effect migration — Wave 4: finish the concurrency/lifecycle core Bounded wave: **4 PRs (PR 4 optional), explicit STOP criterion, explicit OFF-RAMPs.** Plan only; nothing here is implemented. > **Review status:** Independently reviewed (adversarial Reviewer sub-agent, advisor unavailable): APPROVE WITH REQUIRED EDITS — both edits applied; verified claims: Effect.promise 0-arity thunk allocates no AbortController (internal/effect.js:741–776); closed-scope forkIn+startImmediately runs onInterrupt (2237–2274, 391–409); forkIn observer removes the scope finalizer on exit (2270–2271); Effect.timeoutOrElse exists (Effect.d.ts:7833); 0 line drift at b87f62729; 11 settleWorkspaceTurn callers confirmed; 'aborted' ∈ NON_RETRYABLE_STREAM_ERRORS. Line references are to `main` @ `b87f62729`. ## 0. Thesis check (coordinator's judgment vs. evidence) **Thesis:** Effect's payoff in this app is structured concurrency + interruption-safe lifecycles in the orchestration core (still Promise + AbortController). **Verdict: holds for the stream engine; only half-holds for turn handles.** - Stream engine — **holds.** `ServiceContainer.dispose()` never stops or awaits in-flight streams (`serviceContainer.ts:478–537` has no `streamManager` step); an in-flight stream dies with the process and is recovered on next load from `partial.json` (≤ 500 ms stale, `PARTIAL_WRITE_THROTTLE_MS`, `streamManager.ts:776`). `AppFiberScope` exists precisely for this and has no occupant. A supervised per-stream fiber is the right tool. - Turn handles — **half-holds.** The 7× "superseded by an uncorrelated workspace stream-end" false-settle is a *correlation-predicate* bug (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `workspaceTurnManager.ts:4220–4307`: any uncorrelated stream-end after the prompt index settles the handle `interrupted`), not a Promise-vs-fiber structure bug. Turn handles are **persisted records** (`taskHandleStore.upsertWorkspaceTurn`) spanning **multiple streams** (tool-call continuations are deferred via `hasSameTurnContinuation`, `:4491`) and surviving restarts; a fiber/Deferred can only model the in-process waiter and would not fix correlation. Open PR **#3949** fixes the predicate in Promise idiom and is Codex-green. Wave 4's turn-handle PR therefore becomes **"codify the settlement invariant + prove the class is gone"**, not "fiberize handles" (D5 below). Corrected baseline numbers (measured this workspace): 46/470 `src/node` non-test files import `effect` (coordinator said 35); 113 direct `Effect.run*` sites outside `di/` in 16 files; 226 `Effect.gen`; 9 `TaggedError` classes; effect `4.0.0-rc.112`, `@orpc/*` `1.14.11`; **effect v4 is not GA** (rc line still current). ## 1. Verified current state (evidence the design rests on) <details> <summary>Stream engine (streamManager.ts)</summary> - `startStream` (`:4723–4901`): per-workspace mutex → `new AbortController()` + `linkAbortSignal` (`:4771–4772`) → `resourceScope = Scope.makeUnsafe()` (`:4777`) → temp-dir `Effect.acquireRelease` (`:4802–4824`) → `createStreamAtomically` → `streamText` (`:2244`, `abortSignal: abortController.signal` `:2250`) → registered in `workspaceStreams` (`:2463`) → **`streamInfo.processingPromise = this.processStreamWithCleanup(...)` fire-and-forget (`:4876–4882`)** → returns `Ok({ messageId, completion })`. - `processStreamWithCleanup` (`:3331–4089`, plain async): `while(true)` retry loop; `for await (part of fullStream)` (`:3358–3837`) with abort check at loop head (`:3361`); post-loop `if (!signal.aborted)` gate (`:3849`) → completion path (`deletePartial` `:3981`, `updateHistory` `:3989`, `recordSessionUsage` `:4001`, `state = COMPLETED` `:4017`, emit `stream-end` `:4023`, `terminalCompletion` `:4024`); error path → `handleStreamFailure` (`:4094–4112`) → `persistStreamError` writes error partial; `finally` (`:4052–4088`): release MCP lease, `Effect.runFork(Scope.close(resourceScope))` (`:4064–4066`), unlink abort, `workspaceStreams.delete`, `eventSpine.emit("stream.end")`, `completionController.settle`. - Cancellation: `stopStream` (`:5043–5111`) → `cancelStreamSafely` (`:1766–1800`): `if (state === COMPLETED) { await processingPromise; return }` → `state = STOPPING` → `flushPartialWrite` → `abortController.abort()` → `cleanupAbortedStream` (`:1828–1951`): `await processingPromise` → usage → `writePartial` (`:1876–1910`) → `emitStreamAbort` → `settle({status:"aborted"})`. **No completed-guard after the await** (verified `:1838–1951`): a cancel landing between `:3849` and `:4017` re-writes `partial.json` after `deletePartial` and emits `stream-abort` after `stream-end` (pre-existing window; dispose() will widen its exposure). `cancelStreamSafely` is also not idempotent for concurrent callers (only `COMPLETED` is checked). - AIService on `stream-abort` (`aiService.ts:355–377`): `abandonPartial ? deletePartial : commitPartial → deletePartial` (fire-and-forget listener). - Crash recovery: `HistoryService.commitPartial` (`historyService.ts:1963–2061`) — strips error metadata, `hasCommitWorthyParts`, stale-epoch check, update-or-append by `historySequence`, delete partial; invoked from `agentSession.init` (`:5002`), `aiService.streamMessage` (`:886`), stream-abort (`:364`), `duplicateWorkspace`. - `StreamAbortReason = "user" | "startup" | "system"` (`src/common/orpc/schemas/stream.ts:295`). - Pinned seams: chaos test `Reflect.set(streamManager, "tokenTracker" | "createStreamResult")` (`streamManager.chaos.test.ts:130–134, 238–242`); `streamManager.test.ts` `Reflect.set` on `processStreamWithCleanup` (`:2787`), `createStreamAtomically` (`:2783`), `createTempDirForStream`, `cleanupStreamTempDir`, `Reflect.get` on `workspaceStreams`, `schedulePartialWrite`, …; `modelOnlyNotifications.test.ts` calls `processStreamWithCleanup` directly (`:93, :187`); `aiService.test.ts` spies `startStream`, `generateStreamToken`, `createTempDirForStream`, `isResponseIdLost`. Constructor: `(historyService, sessionUsageService?, getProvidersConfig?, eventSink = noop, runner = defaultEffectRunner)` (`:801–813`); `effectRunner` used at `:1152, :1154, :1169` only. - Every stream event carries `workspaceId` + `messageId`; `stream-end`/`stream-abort`/`error` carry `metadata.muxMetadata` when the prompt had it. </details> <details> <summary>DI / shutdown / startup</summary> - `AppFiberScopeLive` is in `CoreLive`'s `runtimeSeams` (`di/layers/core.ts:644`), so both roots have it; `StreamManagerLive` (`core.ts:226–238`, stage S2b) already yields `EffectRunnerTag`. CLI cleanup lists include `appFiberScope.close` (`cli/run.ts:1579`, `cli/workflow.ts:286`). - Bounds: `APP_FIBER_SCOPE_CLOSE_TIMEOUT_MS = 2000`, `APP_RUNTIME_DISPOSE_TIMEOUT_MS = 2000`; outer budgets are **5000 ms** on both desktop (`desktop/main.ts:1297` `Promise.race` vs `setTimeout(5000)`) and `xum server` (`cli/server.ts:236–243` force-exit). **The scope bound cannot grow** without changing outer budgets. - rc.112 semantics verified in `node_modules/effect/dist/internal/effect.js:2264`: `forkIn` registers a scope finalizer and **removes it when the fiber completes** (no leak), and **interrupts immediately if the scope is already closed** (streams starting mid-shutdown fail closed). `Effect.promise(evaluate: (signal) => PromiseLike)`, `Effect.onInterrupt`, `Effect.forkIn(_, scope, { startImmediately? })`, `Stream.toAsyncIterableWith(context)`, `Stream.provideContext` all exist. - `ServiceContainer.initialize()` (`serviceContainer.ts:297–362`): six awaited `initialize()`s wrapped in `recordStep` (durations only, no catch, no timeout) + three sync `start()`s + two fire-and-forget sweeps. Failure handling: desktop `Startup Failed` dialog + `app.quit()` (`desktop/main.ts:1249–1265`); `cli/server.ts:136` uncontained; ACP `serverConnection.ts:205–216` dispose + rethrow; `tests/ipc/setup.ts:85` no catch. No outer timeout anywhere. - `streamBridge.subscriptionIterable` (`orpc/streamBridge.ts:176`) → `Stream.toAsyncIterable(...)` on the global runtime, 19 call sites in `routerSubscriptions.ts`; heartbeat via `Effect.sleep` in `forkScoped` (`:145–152`). `streamBridge.test.ts` has 11 real-time waits, but only **3 are clock-bound** (`:207` 1 ms initial delay, `:241` heartbeat 10 ms, `:255` 10 ms laziness); 8 are `waitFor(listenerCount…)` readiness polls that TestClock cannot replace. </details> <details> <summary>Turn handles + open PRs</summary> - Handle record `{ handleId "wst_…", ownerWorkspaceId, workspaceId, turnId, messageId, status, attentionPolicy, disposableWorkspace }`; prompt carries `muxMetadata: { type:"workspace-turn-task", taskHandleId, ownerWorkspaceId, turnId }` (`workspaceTurnManager.ts:1420`). `TaskService` forwards `aiService` `stream-end`/`stream-abort`/`error` to `finalizeWorkspaceTurnFromStreamEnd` (`:4442–4544`): correlated branch matches `record.workspaceId && record.turnId` (`:4472`); **uncorrelated branch** (`metadata == null`, not `agentId === "compact"`) → `interruptWorkspaceTurnFromUncorrelatedStreamEnd` → settles `interrupted` whenever `streamEndIndex >= promptIndex` (`:4293–4305`). Producers of such uncorrelated ends: bash-monitor wake continuations, child terminal-attention deliveries, heartbeat, peer messages, parent auto-resume. - Cascade: disposable child → `cleanupDisposableWorkspaceTurn` → `workspaceService.remove(…, true)` kills its background processes; persistent child → parent sees `interrupted` → `task_stop` → `backgroundProcessManager.stopMonitor(…, "canceled")`. This is the observed "monitors died afterwards". - Settlement chokepoint: `settleWorkspaceTurn(params)` (`:2085`), **11 callers**, guarded by `workspaceTurnSettlementLocks.withLock(handleId)`; waiters in `pendingWorkspaceTurnWaitersByHandleId` with `setTimeout` timeouts (`:2425–2496`). - **#3949** "preserve turns across synthetic wake ends" (coadler): rewrites the uncorrelated branch — walks history from the turn anchor to the stream-end and settles *only if a manual child input intervened* (`isManualChildWorkspaceInput`); otherwise ignores the end. Touches `:281–295, :4217–4355` + tests (+286/−31). Codex: "Didn't find any major issues" + clean security on `f9baa2fc9`. `mergeable: MERGEABLE`, but `Test / Unit` and `Codex Comments` red, 19 commits behind main. - **#3915** "correlate workspace-turn liveness" (coadler): creation reservations + `getWorkspaceTurnLiveness`/`getWorkspaceTurnRuntimeActivity` (identity-matches the active stream's `muxMetadata` against the record) for staleness/capacity. Touches `:442–486, :1298, :2573, :3761, :3800–4064` (+494/−60). `BLOCKED`, latest Codex review has open findings, `Test / Unit` red, 19 behind. - Together they are the identity-correlated model the coordinator wants: #3915 = identity-correlated *liveness*, #3949 = identity-gated *settlement*. </details> ## 2. Design decisions **D1 — Fibers WRAP the AbortController; they do not replace it.** The AI SDK is cancelled only via `AbortSignal`; the `for await` loop, soft-interrupt at step boundaries, retry/fallback re-creation of `streamResult`, and ~30 abort touchpoints (#4032) all key off the signal. Converting the 750-line loop to `Stream.fromAsyncIterable` + fiber interruption would touch hundreds of `WorkspaceStreamInfo` transitions and break the `processStreamWithCleanup`/`createStreamResult` spy seams. Instead: **the fiber is the ownership/supervision unit; the signal stays the cancellation transport.** The dual-cancellation glue #4032 feared is confined to **one** point — the supervisor's `onInterrupt` — which routes through the existing user-stop path (`cancelStreamSafely`), so shutdown ≡ "user pressed stop" semantically (partial flushed with usage, `stream-abort` emitted, `completion` settles `aborted`, AIService commits the partial). **D2 — Supervisor topology: one supervisor fiber per stream in `AppFiberScope`, wrapping the already-started `processingPromise`.** `streamInfo.processingPromise = this.processStreamWithCleanup(...)` stays byte-identical (sync-start preserved; `Reflect.set(processStreamWithCleanup)` seam preserved; `cleanupAbortedStream`'s `await processingPromise` unchanged). Immediately after it: ```ts // startStream, after processingPromise is assigned (unsupervised path unchanged when no scope) this.superviseEngine(typedWorkspaceId, streamInfo); private superviseEngine(workspaceId: WorkspaceId, streamInfo: WorkspaceStreamInfo): void { if (this.engineScope === undefined) return; // direct construction / CLI tests: today's behavior assert(streamInfo.engineFiber === undefined, "engine already supervised"); // Zero-arity thunk on purpose: rc.112 allocates an internal AbortController only // when `evaluate.length !== 0`; the stream's own controller stays the sole signal. const supervisor = Effect.promise(() => streamInfo.processingPromise).pipe( Effect.onInterrupt(() => Effect.uninterruptible( // explicit, per house doctrine (finalizers are already uninterruptible) Effect.promise(async () => this.cancelStreamSafely(workspaceId, streamInfo, "system")) ) ), Effect.catchDefect((d) => Effect.sync(() => log.warn("[stream] engine supervisor defect", { workspaceId, error: d }))) ); streamInfo.engineFiber = this.effectRunner.runSync( Effect.forkIn(supervisor, this.engineScope, { startImmediately: true }) ); } ``` - `Effect.promise` is interruptible while suspended (`internal/effect.js:741–801`, Async op); `onInterrupt` = `onErrorFilter(causeFilterInterruptors, …)` (`:1762`); `forkIn` registers `fiberInterrupt(fiber)` as the scope finalizer (`:2264–2275`), `fiberInterrupt` awaits the fiber (`:635–642`), and parallel `scopeClose` awaits all finalizers via `fiberAwaitAll` (`:1590–1601`) → `closeScopeBounded` at dispose step 2 gives "interrupt **and** await" while `historyService`/`sessionUsage`/`eventSink → AIService → bridge servers` are still alive (bridges stop in step 3, so clients receive `stream-abort`). - Normal completion: fiber exits → `forkIn`'s observer removes the scope finalizer (verified) → no per-stream residue. - Stream started after step 2: `forkIn` on a closed scope calls `fiber.interruptUnsafe` synchronously and returns the fiber (`:2272–2274`, `runSync` does not defect). With `startImmediately: true`, `forkUnsafe` runs `child.evaluate` synchronously (`:2233–2247`) up to the `Effect.promise` Async op (`:772–801`), so the fiber is suspended (`_running=false`) when the interrupt lands and `interruptUnsafe` (`:391–409`) unwinds the stack through the `onInterrupt` handler → the stream is aborted as `system` (fail-closed during shutdown). Verified in rc.112 internals (`effect.js:2233–2247, 391–409`); **pin with a test** ("stream started after scope close is aborted") so an RC bump cannot silently change it. - `"system"` is semantically exact: `"user"`/`"startup"` suppress next-startup recovery (`retryEligibility.ts:114–118, 284–287`), `"system"` marks an involuntary backend interruption (as `taskService.ts:8100, 8223` use it). **No in-session retry loop is possible:** the `stream-abort` handler (`agentSession.ts:6010`) routes `{ type: "aborted" }` to `retryManager.handleStreamFailure`, and `"aborted"` is in `NON_RETRYABLE_STREAM_ERRORS` (`retryEligibility.ts:49–59, 106`) → `retryManager.ts:99–104` abandons immediately, never schedules a fiber. Dogfooding still checks the *restart* UX (the recovered partial is shown as interrupted; note whether any next-startup recovery re-sends — same class as today's `system` aborts from `taskService`). - `engineScope` arrives as an **optional 6th constructor parameter** (`engineScope?: Scope.Closeable`), wired from `AppFiberScopeTag` in `StreamManagerLive` (`core.ts:226`). Default `undefined` keeps every direct-construction test and `aiService.ts:174` path identical (I4). `AppFiberScopeLive` already sits beneath S2b in `runtimeSeams`, so no staging change (I6). - Abort reason: reuse **`"system"`** — no wire/schema change; UI copy for `system` already exists. - Pending-start window (`pendingStreamStarts`, before registration) is **not** supervised: nothing is persisted for it yet, and `stopStream` already aborts pending controllers. Documented, not fixed. **D3 — Fix the two adjacent cancel races in the same PR (closely-related bugs, not deferrals).** (a) `cleanupAbortedStream`: after `await processingPromise`, if `streamInfo.terminalCompletion !== undefined` (completed/failed while the cancel was in flight) → return without abort bookkeeping (prevents `partial.json` resurrection after `deletePartial` and a `stream-abort` after `stream-end`). (b) `cancelStreamSafely` (`:1766`): latch a per-stream `cancelPromise` so concurrent cancellers (user stop racing dispose) join one cleanup → exactly one `stream-abort`, one `settle`. **Zero-suspension requirement:** the latch must be checked and assigned **synchronously at function entry, before any `await`** (the current first await is `flushPartialWrite` at `:1789`) — otherwise racing callers can both enter `cleanupAbortedStream`. Shape: ```ts if (streamInfo.cancelPromise) return streamInfo.cancelPromise; streamInfo.cancelPromise = (async () => { /* existing body, unchanged */ })(); return streamInfo.cancelPromise; ``` Both are ≤ 10 LoC and get behavioral tests. **D4 — Shutdown bound stays 2 s; the finalizer must be fast or abandoned.** Outer budgets are 5 s; 2 s + 2 s already consume 4 s. A flowing stream aborts within one chunk; a wedged provider (no chunks, ignores abort) hits the existing `boundedTeardown` timeout: warning, continue, process exit — identical to today's outcome. Dogfooding measures the actual `[shutdown] AppFiberScope closed { ms }` with a live stream. **D5 — Turn handles: codify the settlement invariant; do not fiberize.** Invariant: *a workspace-turn handle settles terminally only by (i) a stream terminal event whose `muxMetadata` correlates `{taskHandleId, ownerWorkspaceId, turnId}` to the record; (ii) an explicit interrupt (`task_stop`/`interruptWorkspaceTurn`); (iii) manual supersession — a manual child input after the turn anchor; (iv) stale-liveness reconciliation.* An uncorrelated stream-end is **never** terminal by itself. #3949 makes (iii) the only uncorrelated outcome; #3915 implements (iv) by identity. Wave 4 adds a `cause` discriminant to `settleWorkspaceTurn` (the single chokepoint) with a runtime assertion, plus the regression harness. Rationale for not converting waiters to `Deferred`/fibers: no behavioral gain, 4.9k-line file, and the coordinator's "settle only on the owning stream's termination" is over-specified — a turn owns *several* streams. **D6 — Startup: `initialize()` stays a Promise facade over a runtime-run startup effect; timeout ⇒ same failure path as a thrown step.** Each step is `Effect.tryPromise({ try: async () => step(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, ms)) }))` (`timeoutOrElse` exists in rc.112, `Effect.d.ts:7833`; chosen over `timeout` + `catchTag` because the step's error channel is `unknown`, which `catchTag` cannot narrow). No `forkDetach` needed: a Promise step keeps running on its own when the waiting fiber times out (not inside an uninterruptible region, so the timeout interrupts the wait directly). `StartupStepTimeoutError extends Error` with `name = "StartupStepTimeoutError"` set in the constructor and message `"<step> exceeded <ms> ms"` (so the desktop dialog's error formatting shows both the class and the step name) → desktop shows it in the existing `Startup Failed` dialog; CLI/ACP/tests paths unchanged. Step **errors keep their identity** (v4 `runPromise` rejects with the raw failure). Downgrading any step to best-effort is a **policy change, out of scope** (audit of the six implementations: extensionMetadata/telemetry/experiments are local fs, <50 ms; policy has its own 10 s fetch timeout; workspaceService bounds its sync internally; only `taskService.initialize` — config scan + `editConfig` + recovery `sendMessage`s — is potentially unbounded). The three `start()`s stay sync (`Effect.sync`), the two fire-and-forget sweeps stay outside the effect. `stepDurationsMs` is preserved. **Abandon-and-quit safety:** an abandoned `taskService.initialize` may be mid-`editConfig` when the root exits. Parity requirement for PR 3: after a rejected `initialize()`, every root runs the bounded `dispose()` before exiting. Verified: desktop already does — `services` is assigned before the await (`main.ts:653–656`), the catch calls `app.quit()`, and the `before-quit` listener (`:1271–1305`, guard `if (isDisposing || !services) return`) races `services.dispose()` against 5 s; ACP does (`serverConnection.ts:205–216`); **`cli/server.ts` does not** (`:133–136` awaited at top level, `main().catch` at `:282` only logs) → PR 3 adds a bounded `dispose()` there (≤ 10 LoC, same 5 s budget). **D7 — streamBridge: thread the runtime context, not a runner.** `subscriptionIterable` gains `context?: Context.Context<never>` → `Stream.toAsyncIterableWith(context)`; `routerSubscriptions` passes the handler's `"effect/context"`. Production behavior identical; heartbeat sleeps on the runtime `Clock`; tests can run the 3 clock-bound waits on `TestClock`. Honest scope: the 8 readiness polls stay. ## 3. PRs (ordered by value ÷ risk; each independently mergeable) ### PR 1 — StreamManager engine core becomes the first `AppFiberScope` occupant **Value:** high (the only remaining shutdown data-integrity gap; the reason `AppFiberScope` exists). **Risk:** medium → low with D1/D2. **Net product LoC ≈ +55** (`superviseEngine` ~25, ctor param/field ~5, `engineFiber` field ~2, D3 guards ~12, `core.ts` wiring ~2, doc updates in `appRuntime.ts`/`appFiberScope.ts` "occupant" text ~10). Files: `src/node/services/streamManager.ts`, `src/node/services/di/layers/core.ts`, `src/node/services/di/appRuntime.ts` + `appFiberScope.ts` (docs), tests below. Pre-work (before writing product code; each yields a note in the PR body): 1. Confirm `processStreamWithCleanup` never rejects (try/catch/finally shape `:3331–4089`); else the supervisor must fold rejections (it already `catchDefect`s). 2. Confirm `Effect.promise` interruption + `onInterrupt` await ordering under `Scope.close` in a 20-line probe test (pattern of `appFiberScope.test.ts:27–47`). 3. Enumerate abort observers that run *after* the finalizer resolves (AIService `stream-abort` listener → `commitPartial`; agentSession completion continuations) and confirm the durable order (`writePartial` → commit → `deletePartial`) makes a mid-flight `process.exit` recoverable on next load (it is: partial survives until commit completes). 4. Measure: `[shutdown] AppFiberScope closed { ms }` with a live stream in the sandbox (D4). 5. Pin (same probe test): a fiber forked with `startImmediately: true` into an already-closed scope still runs its `onInterrupt` finalizer (reviewer-verified in rc.112 internals; the test guards RC bumps). Acceptance (behavioral tests only): - `streamManager.test.ts` (new cases; existing cases untouched): with `engineScope = Scope.makeUnsafe("parallel")` and a fake `createStreamResult` whose `fullStream` yields one `text-delta` then blocks until its `AbortSignal` fires — `closeScopeBounded(engineScope)` resolves; `writePartial` was called with the streamed text; exactly one `stream-abort` (`abortReason: "system"`) and zero `stream-end`; `completion` settles `{status:"aborted"}`; `workspaceStreams` is empty. - Wedged provider (fullStream never yields, ignores abort): `closeScopeBounded` resolves within the bound, never rejects, warns once (assert the returned promise resolves and no throw; do **not** assert log text). - No-scope construction: identical event sequence to today (guards existing suites; no new assertions needed beyond the unchanged suites passing). - D3(a): cancel issued after the loop exits but before `COMPLETED` → history has exactly one final message, `partial.json` absent, event order `stream-end` only. - D3(b): `stopStream` + `closeScopeBounded` racing on one stream → exactly one `stream-abort`, one settle. - Fiber residue: after 50 completed streams, `closeScopeBounded(engineScope)` emits zero `stream-abort` and completes in the same tick class as an empty scope (assert no aborts and `workspaceStreams.size === 0`). - `streamManager.chaos.test.ts` — existing cases byte-identical; **one new fuzz variant** constructs with an engine scope and closes it at a random iteration: every stream settles **exactly once** (count terminal events per `messageId` ≤ 1, all `completion` promises settle). - `serviceContainer.test.ts`: "dispose() aborts and awaits an in-flight stream before `desktopBridgeServer.stop()`" (extend the ordering harness at `:295–323`); `coreServicesRoot.test.ts`: `xum run` cleanup list does the same via `appFiberScope.close`. Gate suites: `streamManager.test.ts`, `streamManager.chaos.test.ts`, `streamManager.modelOnlyNotifications.test.ts`, `aiService.test.ts`, `agentSession.disposeRace.test.ts`, `agentSession.sinceReplayContract.test.ts`, `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `taskService.test.ts`, `workspaceService.test.ts`, `turnRequestBuilder.test.ts`; `make static-check`. House pre-review audits: interruption posture (supervisor's only suspension is the promise; finalizer uninterruptible end-to-end incl. `cancelStreamSafely` → `cleanupAbortedStream`); no defect escapes (`catchDefect` on the supervisor; `Effect.promise` thunks `async`); spy-seam check (`processStreamWithCleanup`, `createStreamResult`, `createStreamAtomically`, `startStream` signatures unchanged; constructor arity unchanged, trailing optional); sync-start (`processingPromise` assigned before fork; `runSync(forkIn)` completes synchronously); no constructor side-effects added; zero-suspension check on D3(b) latch (`cancelPromise` checked-and-assigned synchronously at `cancelStreamSafely` entry, before the first `await` at `:1789`; review the diff for any inserted `await`/lookup ahead of the assignment). Rollback: revert the `core.ts` wiring line → `engineScope` undefined → today's behavior; D3 guards can stay (independent bug fixes). ### PR 2 — Turn-settlement invariant + false-settle regression harness (gated on #3949) **Value:** high (7× production race). **Risk:** low. **Net product LoC ≈ +40** (`WorkspaceTurnSettlementCause` union + `cause` on `settleWorkspaceTurn` params + assert ~10; 11 call sites × 1–3 lines). Relationship to open PRs — explicit: - **#3949 is the fix and a hard prerequisite.** PR 2 rebases on it, changes none of its logic (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `isWorkspaceTurnAnchorForRecord`, `isManualChildWorkspaceInput`), and adds the invariant + proof on top. If #3949 has not merged when PRs 1/3 are done: **do not fork a competing fix**; report to the coordinator, offer the regression test file to #3949's author as a review artifact, and hold PR 2 (it is not on any other PR's critical path). - **#3915 is a soft prerequisite.** Its diff (`:3800–4064` incl. `settleStaleWorkspaceTurn`, a `settleWorkspaceTurn` caller) overlaps PR 2's one-line-per-caller change. Prefer landing after it; if PR 2 must go first, the conflict is a one-line `cause:` addition per caller. PR 2 never edits liveness/reservation code. - Wave 4 does not otherwise touch `workspaceTurnManager.ts`. Design: `type WorkspaceTurnSettlementCause` enumerated from the 11 callers (audited at `main` @ `b87f62729`): `:1373` creation validation failure; `:1476` pre-stream interrupt during launch; `:1500`/`:1518` pre-stream send failure; `:3834`/`:3863` stale-liveness recovery / restart timeout (`settleStaleWorkspaceTurn` — #3915's region); `:4301` **uncorrelated-stream-end manual supersession** (the only uncorrelated settle in the codebase; the path #3949 rewrites); `:4529` correlated terminal; `:4571` stream-abort; `:4675` deferred stream error; `:4736` terminal stream error. `settleWorkspaceTurn` asserts `params.cause` is a member and, for `manual-supersession`, that the superseding input's `messageId` is supplied — turning D5 into an exhaustive `Record<Cause, …>` check rather than prose, so a future "settle on uncorrelated end" cannot be added without naming (and justifying) a cause. Acceptance: - New `workspaceTurnManager.uncorrelatedStreamEnd.test.ts` (real `WorkspaceTurnManager` + `TaskHandleStore` + fake `aiService` emitter, following the existing suite's harness): (1) create turn → correlated `stream-start` → **synthetic wake stream** on the same child ends uncorrelated after the anchor → handle stays `running`, waiter unresolved, no disposable cleanup, no terminal attention → correlated `stream-end` → `completed`. (2) same with `finishReason:"tool-calls"` continuation in between. (3) manual child input between anchor and end → `interrupted` with `cause: manual-supersession`. (4) explicit `interruptWorkspaceTurn` → `interrupted`, `cause: explicit-interrupt`. Case (1) is the **scripted reproduction**: it must **fail on the pre-#3949 merge-base** (run the file from a sibling worktree at `git merge-base origin/main <#3949 head>`; record the failing assertion in the PR body) and pass after. - Existing 113 `workspaceTurnManager.test.ts` cases and `taskService.test.ts` turn cases unchanged. Gate suites: `workspaceTurnManager.test.ts`, `taskService.test.ts`, `taskHandleStore.test.ts`, `tools/task*.test.ts`; `make static-check`. Audits: spy-seam (`getWorkspaceTurn`, `listAllWorkspaceTurns`, `enqueueTerminalAttention`, `deliverPersistentChildWorkspaceTurnResult` untouched); settlement lock held across the assert; no new suspension inside `withLock`. Rollback: revert; the test file stays valid against #3949 alone (drop the `cause` assertions). ### PR 3 — `ServiceContainer.initialize()` as a runtime-run startup effect with per-step timeouts **Value:** medium (a hung `taskService.initialize()` currently pins the splash screen forever; deterministic TestClock tests of startup). **Risk:** low–medium. **Net product LoC ≈ +80** (step table ~20, timed-step helper ~15, `StartupStepTimeoutError` ~8, constant ~3, facade ~10, root dispose-on-failure parity ≤ 10, doc update ~10). Files: `serviceContainer.ts`, `src/constants/terminationTimeouts.ts` (keep with the termination constants so the budget doc stays in one place), `cli/server.ts` (dispose in the startup catch if missing), `di/appRuntime.ts` doc ("Deliberately not done" → remove the initialize() line; add startup contract). Design (D6): `initialize(): Promise<void>` → `this.runtime.managed.runPromise(this.startupEffect())`. `startupEffect = Effect.gen` over an ordered `readonly steps: ReadonlyArray<{ name, run: () => Promise<void> }>` (assert names unique); each step: `recordStep` timing kept, `Effect.tryPromise({ try: async () => run(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, STARTUP_STEP_TIMEOUT_MS)) }))`. Then `Effect.sync` for the three `start()`s; the sweeps remain after `runPromise`. Constant `STARTUP_STEP_TIMEOUT_MS` — pre-work measures `[startup] <step> { ms }` across sandbox cold starts and picks ≥ 10× the slowest observed (propose 60 s; must be generous — a false timeout turns a slow-but-fine start into a crash). Roots dispose after a rejected `initialize()` (D6 abandon-and-quit safety). Acceptance (all in `serviceContainer.test.ts`, TestClock via the existing `AppLive` spy at `:355–395`): - A step that never resolves → `initialize()` rejects with `StartupStepTimeoutError` naming the step after exactly `TestClock.adjust(STARTUP_STEP_TIMEOUT_MS)`; later steps did not run. - A rejecting step → `initialize()` rejects with **the same error object** (identity), later steps did not run (parity with today). - Happy path → `stepDurationsMs` has all six keys; `start()`s called once each; second `initialize()` call behavior unchanged from today (verify whether re-entry is guarded today; preserve). - `tests/ipc` harness and ACP entry still pass unchanged. Gate suites: `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `src/node/acp/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (smoke subset); `make static-check`. Audits: I1 untouched (no layer body changes); I2 (only the composition root touches the runtime); error identity preserved (no wrapping); abandoned-step safety — every root runs bounded `dispose()` after a rejected `initialize()` (D6; `cli/server.ts` gains it in this PR); no sweep moved into the effect; the six-step order and `[startup] <step>` names unchanged. Rollback: revert; constant removal. ### PR 4 (optional — cut if budget is exhausted) — `streamBridge` on the runtime context **Value:** low (closes the last documented "global runtime" exception; enables TestClock for the heartbeat). **Risk:** low. **Net product LoC ≈ +30** (`context?` option + `toAsyncIterableWith` ~8; 19 call sites × 1 line via one shared helper in `routerSubscriptions.ts` ~3). Acceptance: the 3 clock-bound waits (`:207, :241, :255`) run on `TestClock`; the heartbeat test asserts N heartbeats after `TestClock.adjust(N × interval)` with zero real time; existing behavioral assertions unchanged; `tests/ipc` subscription tests pass. Do not rewrite the 8 readiness polls. Gate suites: `streamBridge.test.ts`, `routerSubscriptions*.test.ts`, `orpc/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (subscription subset); `make static-check`. Audits: `Stream.toAsyncIterableWith` preserves double-close safety (pin with the existing test); `Cause.Done` typing unchanged; no `Scope`/`MemoMap`/`Scheduler` captured (pass the oRPC `effect/context`, which the DI layer already strips per `EffectRunnerLive`); `context` stays optional so direct callers/tests without a runtime keep today's global-runtime path. Rollback: revert; the optional `context` default (`Context.empty()`) is exactly today's `toAsyncIterable`, so a partial revert of call sites is also safe. ### Execution order and size Net product LoC for the wave ≈ **+205** (PR 1 ≈ +55, PR 2 ≈ +40, PR 3 ≈ +80, PR 4 ≈ +30); tests ≈ +600–800. PR 1 starts immediately. PR 2 starts the moment #3949 merges (parallel with PR 1/3 — disjoint files). PR 3 after PR 1 merges (both touch `appRuntime.ts` docs; PR 3 also touches `serviceContainer.ts`). PR 4 last, only if PRs 1–3 landed and no OFF-RAMP fired. Each PR: Codex dual review, `Codex Comments` minimization, merge queue; commit WIP early (`/tmp` wipes). ## 4. STOP criterion (measurable) and OFF-RAMPs Wave 4 is **done** — and the Effect migration line **stops** without a new RFC — when all hold: 1. **dispose() awaits in-flight streams:** `serviceContainer.test.ts` ordering test + `coreServicesRoot.test.ts` pass on main; a sandbox `script -f` transcript of `xum server` receiving SIGTERM mid-stream shows `stream-abort` → `[shutdown] AppFiberScope closed { ms }` → `[shutdown] desktopBridgeServer.stop`, and immediately after exit `partial.json` is absent while `chat.jsonl` contains the interrupted assistant message (baseline on `main`: `partial.json` present, message absent until next load). `{ ms }` < 2000 in the flowing-stream case. 2. **False-settle class eliminated:** the scripted reproduction fails on the pre-#3949 merge-base and passes on main after PR 2; `settleWorkspaceTurn` rejects any settlement without an enumerated cause; the coordinator's own Mux sessions show zero "superseded by an uncorrelated workspace stream-end" in the two weeks after PR 2 (soft signal, logged in the wave summary). 3. **Startup:** timeout and error-identity tests pass under TestClock; `[startup]` per-step lines unchanged in the sandbox transcript; a throwaway build with the constant set to 1 ms shows `Startup failed: StartupStepTimeoutError: <step> exceeded 1 ms` and a clean exit. 4. **No new lifecycle flakes:** 0 failures attributable to the touched suites across **N = 20** consecutive *completed* `Test / Unit` runs on `main` after the last Wave 4 merge — query `gh run list --workflow pr.yml --branch main --limit 60 --json databaseId,status,conclusion,event,headSha` (note: `gh run list --json` serializes these fields in **lowercase**, e.g. `{"status":"completed","conclusion":"success"}`, unlike `statusCheckRollup`), keep `status === "completed"` (pending runs have an empty `conclusion`, not null), take the newest 20, and for any run with `conclusion !== "success"` (case-insensitive normalization acceptable) inspect the failing job's log for the touched suite names (job `timeout`/`cancelled` from the 15-min budget is not a flake); plus green merge-queue runs for each PR. Any attributable flake → fix or revert before declaring done. **OFF-RAMP (PR 1):** fires if pre-work 1–3 shows (a) routing shutdown through `cancelStreamSafely` cannot preserve crash-recovery semantics without changing `cleanupAbortedStream`'s contract beyond D3, (b) the chaos variant exposes a double-settle not closable by D3(b), or (c) the finalizer cannot fit the 2 s bound for flowing streams. Then: stop PR 1, keep `AppFiberScope` unoccupied, update `appRuntime.ts` "Deliberately not done" with the concrete blocker and the measured evidence, land D3 alone as a bug-fix PR. PRs 2–4 are independent and proceed. **OFF-RAMP (PR 3):** if error identity or the `tests/ipc`/ACP paths cannot be preserved, keep `initialize()` as is and record why. **OFF-RAMP (PR 2):** #3949 not merged → hold (see PR 2). ## 5. Risk register | Risk | Likelihood | Mitigation | |---|---|---| | Crash-recovery regression: double commit / partial resurrection when shutdown-abort races completion | medium (window widens with dispose()) | D3(a) guard + test; `commitPartial`'s `historySequence` update-or-append is idempotent (`historyService.ts:2036–2041`) | | Provider abort emits an `error` chunk → error path instead of abort path | low | identical to today's user-stop path (parity); chaos variant covers hostile streams | | Wedged provider pins the 2 s bound → warning every shutdown | low | `boundedTeardown` already bounds; transcript measures; no budget change possible (5 s outer) | | AIService `stream-abort` listener (`commitPartial`) still in flight when `process.exit` runs | low | durable order writePartial → commit → deletePartial; next-load recovery; PR 1 transcript checks `partial.json` is already gone when `cli/server.ts` logs its final cleanup line before `process.exit(0)` (`:252–266`) | | Chaos-test seams (`createStreamResult`, `tokenTracker`) | none if scope-less construction stays default | new variant added, old cases untouched | | Collision with #3915/#3949 | medium | PR 2 gated; no edits to their regions; one-line `cause:` conflicts only | | RC churn (rc.113+ renames `forkIn`/`onInterrupt`/`toAsyncIterableWith`) | low | all Effect imports already in `streamManager.ts`/`streamBridge.ts`; pins fixed; GA upgrade is a separate lockstep PR (§6) | | Startup false timeout on slow hosts | medium if constant too small | measure first; ≥ 10× slowest observed; generous default (60 s) | | Sync-start assumptions in tests that `Reflect.set(processStreamWithCleanup)` | low | promise assigned before fork; forkIn `runSync` synchronous | | `shutdown()` (desktop second `before-quit` listener) still does not await streams | accepted | contract says `shutdown()` never touches the runtime; desktop's dispose race is the covered path | | Streams starting during shutdown | covered | `forkIn` on closed scope interrupts immediately → `system` abort (`startImmediately` semantics verified in rc.112; pinned by test) | | `system` abort triggers an in-session RetryManager retry during shutdown | **unreachable** (verified) | `"aborted"` ∈ `NON_RETRYABLE_STREAM_ERRORS` → `retryManager.ts:99–104` abandons; no fiber scheduled | | PR 3: abandoned `taskService.initialize` mid-`editConfig` when the root exits after a timeout | low | desktop/ACP already dispose on startup failure; PR 3 adds the missing `cli/server.ts` dispose; config writes are lock/journal-protected | | `startImmediately`/`onInterrupt` semantics differ in a later RC | low | pinned by the probe test in `appFiberScope.test.ts` style; RC bumps are a separate lockstep PR | ## 6. Standing item — effect v4 GA + `@orpc/experimental-effect` lockstep (analysis only) v4 is **not GA** (rc.112 is current; v3 `3.x` remains the stable line). No PR this wave. When GA ships: one lockstep PR bumping `effect` + all `@orpc/*` (`1.14.11` today; check the GA-compatible `@orpc/experimental-effect`), canary gates = `di/*.test.ts`, `streamBridge.test.ts`, `streamManager.test.ts`, `serviceContainer.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc`, `make static-check`. The `Context → ServiceMap` rename risk is firewalled: `Context.Service` tags, `Context.omit/get`, `Layer`, `ManagedRuntime`, `TestClock` live only under `di/` + `orpc/effectContext.ts`; `streamManager.ts`/`streamBridge.ts` use `Effect`/`Scope`/`Fiber`/`Exit`/`Stream`/`Queue`/`Cause` only. PR 4 adds one `Context.Context<never>` type reference to `streamBridge.ts` — keep it as a type-only import so a rename is a one-line fix. ## 7. Dogfooding (per PR; evidence attached to the PR with `gh … --attach`) Common setup: `make dev-server-sandbox DEV_SERVER_SANDBOX_ARGS="--clean-projects"` (or `xum server` on a temp `XUM_ROOT`) with `XUM_LOG_LEVEL=debug`, run under `script -f ~/wave4-scratch/<pr>-<scenario>.log`; scratch under `$HOME/wave4-scratch/` (never `/tmp`). Drive the UI with `agent-browser` (`open` → `snapshot -i` → click the explicit "Send message" ref; re-snapshot after typing). Screenshots are primary evidence; record WebM and finalize with `ffmpeg -c copy`. - **PR 1:** (1) start a long stream (prompt that streams ~30 s), wait 3–5 s, `kill -TERM <server pid>`; transcript must show the order in STOP #1 and `[shutdown] AppFiberScope closed { ms }`; (2) `ls <XUM_ROOT>/sessions/<ws>/partial.json` (absent) + `tail -n 1 chat.jsonl` (interrupted assistant message); (3) restart, open the workspace in agent-browser, screenshot the persisted interrupted message; (4) same scenario on `main` for the baseline diff; (5) `xum run` Ctrl-C mid-stream transcript (CLI root parity); (6) quality gate between phases: gate suites green before the sandbox run, sandbox evidence before requesting review. - **PR 2:** primary evidence is the regression file run on both the merge-base worktree (failing output) and the branch (passing). Secondary: sandbox parent workspace delegates via `task` kind=workspace to a child that arms a background bash monitor firing within ~10 s and keeps working ~60 s; screenshot the parent's task result (baseline `main`: `interrupted … uncorrelated workspace stream-end`; after: `completed`) and the child's log lines. - **PR 3:** `xum server` cold start transcript with `[startup] <step> { ms }` for the six steps (parity); throwaway worktree build with `STARTUP_STEP_TIMEOUT_MS = 1` → transcript of `Startup failed: StartupStepTimeoutError …` and exit code (do not ship); desktop dialog cannot be shown headless — cite the unchanged `desktop/main.ts:1249–1265` catch. - **PR 4:** agent-browser session left idle 60 s with the connection indicator visible (heartbeats keep it green) + screenshot; `streamBridge.test.ts` on TestClock. ## 8. Non-goals (restated; out of this wave) Typed-error propagation sweep / removing the ~113 facades; converting services to `yield*`-based Effect services; PubSub for the internal EventEmitter bus; Schema at persistence boundaries; Effect observability; converting sync read paths, AI-SDK per-request callbacks, cross-process lock interiors, or deterministic try-lock funnels; replacing `AbortController` as the SDK cancellation transport; converting the `fullStream` loop to `Stream`; fiberizing turn-handle waiters; downgrading startup steps to best-effort; changing outer quit budgets; supervising the pre-registration stream-start window; `shutdown()` semantics; the effect GA bump (standing analysis only). </details> --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `xhigh` • Cost: `$34.90`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=xhigh costs=34.90 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 6, 2026
…r-step timeouts (Wave 4 PR 3) (coder#4076) ## Summary Wave 4 PR 3 (plan §2 D6 / §3 "PR 3"): `ServiceContainer.initializeCore()` is now a Promise facade over a **startup effect run on the app runtime** — one `Effect.gen` over an ordered table of the five hard startup steps, each `Effect.tryPromise` (identity catch) bounded by `Effect.timeoutOrElse(STARTUP_STEP_TIMEOUT_MS)` on the runtime's `Clock`. A hung step no longer pins the splash screen / listener bind forever: after 60 s it fails startup with a `StartupStepTimeoutError` (`"<step> exceeded <ms> ms"`, `step`/`timeoutMs` fields) through the roots' **unchanged** failure paths. Step errors keep their identity; later steps do not run; the five step names/order and the `[startup] ServiceContainer.initialize completed { stepDurationsMs }` payload are unchanged. Every process root now runs the bounded `dispose()` before exiting on a rejected startup (D6 abandon-and-quit safety): `cli/server.ts` gains it, and the ACP root's existing catch **did not** dispose after a rejected `initialize()` (`if (initialized)` guard — the plan's claim that it did was wrong), so it does now. ## Background - Plan: `~/.xum/plans/mux/effect-wave4-lifecycle-core-plan.md` (collapsed below); PR 1 = coder#4070; the plan predates coder#4058, which split `initialize()` into `initializeCore()` (hard steps, gate the server listener) + `runStartupHousekeeping()` (best-effort, abort-signal cancellable). This PR targets `initializeCore()` only. - `runStartupHousekeeping()` is deliberately untouched: its steps are already cancellable through the dispose abort signal and non-fatal by policy, so per-step timeouts there would be a policy change the plan forbids (recorded in the `appRuntime.ts` startup contract). ## Implementation - `serviceContainer.ts`: `startupCoreSteps: readonly StartupStep[]` (names asserted unique in the constructor — they key `stepDurationsMs`); `initializeCore()` = `assert(not disposed)` + `runtime.managed.runPromise(startupCoreEffect())`; `timedStartupStep` = `Effect.suspend` → `tryPromise({ try: async () => run(), catch: identity })` → `timeoutOrElse` → `Effect.ensuring` (duration recorded when the *wait* ends — settled, failed, or abandoned — never by the abandoned step later). `timeoutOrElse` rather than `timeout` + `catchTag` because the error channel is `unknown`. No `forkDetach`: the zero-arity thunk gets no AbortSignal, so the step promise keeps running; its late settlement is a no-op on the exited fiber and `tryPromise` keeps a rejection handler attached (pinned by test). - `StartupStepTimeoutError extends Error` with `name` set in the constructor, so the desktop `Startup Failed` dialog and `Failed to initialize server:` print class + step without formatting changes. - `src/constants/terminationTimeouts.ts`: `STARTUP_STEP_TIMEOUT_MS = 60 s` (evidence below) and `SERVICE_TEARDOWN_BUDGET_MS = 5 s` (the outer teardown budget `cli/server.ts` already used as a literal, now shared with the ACP root). - `cli/server.ts`: `main().catch` runs `dispose()` bounded by the same 5 s budget as the SIGTERM cleanup when the container was constructed, then exits 1. `acp/serverConnection.ts`: the catch disposes unconditionally (bounded). `desktop/main.ts` unchanged — the catch at `:1249–1265` quits and the `before-quit` listener (`:1271–1305`) races `dispose()` against 5 s. - `appRuntime.ts`: new "Startup" contract section; "Deliberately not done" no longer lists `initialize()`. - Decisions pinned by tests: re-entry is **not** guarded (parity with the promise chain); `initializeCore()` after `dispose()` fails fast with an assertion instead of the disposed runtime's bare `"ManagedRuntime disposed"` string defect. ## Validation - `serviceContainer.test.ts` (+5): TestClock timeout — rejects with `StartupStepTimeoutError` naming the step **exactly** at `TestClock.adjust(STARTUP_STEP_TIMEOUT_MS)` (still pending at −1 ms), later steps not called, the abandoned step's late rejection is not unhandled and changes nothing on the container; error identity (`toBe`) for a rejecting and for a synchronously throwing step with later steps skipped; happy path records the five keys in order and a second call re-runs the steps; after `dispose()` fails fast without running a step. **Red check** against the old promise chain: the timeout test (hangs → 5 s test timeout) and the after-dispose test fail; the identity/sync/re-run tests are parity pins (green on both, by design). - Gates: `serviceContainer.test.ts` (23), `di/*.test.ts` + `coreServicesRoot.test.ts` + `acp/*.test.ts` (55), `TEST_INTEGRATION=1 bun x jest tests/ipc/{doubleRegister,windowTitle,savedQueries,acp.sessionMethods,acp.disconnectCleanup}` (32), `bun test src/cli/` (185), `make static-check`. - Dogfooding (first comment, with screenshots): 3 + 3 `xum server` cold starts on `main` vs branch — same ten `stepDurationsMs` keys in the same order, slowest core step `taskService.recoverInterruptedTasks` 49–61 ms; a throwaway build with the constant forced to 1 ms on `xum server` and `xum acp` shows `Failed to initialize server: StartupStepTimeoutError: extensionMetadata.initialize exceeded 1 ms`, the full bounded `[shutdown]` dispose sequence (28 ms / 36 ms), exit code 1. - `cli/server.ts` has no unit seam for `main().catch` (`server.test.ts` exercises `createOrpcServer`); the throwaway transcript is the evidence for that path. ## Risks - False timeout on a pathologically slow host turns a slow-but-fine start into a crash — bounded at 60 s, ≥ 1000× the measured maximum and above the policy service's own 10 s fetch timeout; the only potentially unbounded step, `taskService.recoverInterruptedTasks`, scales with active agent tasks, not deployment size. - An abandoned step (e.g. mid-`editConfig`) keeps running until the process exits; the bounded `dispose()` applies the same latches as a quit (`beginShutdown`s) and config writes are lock/journal protected — identical exposure to a SIGTERM mid-startup today. - Disposing the runtime concurrently with an in-flight `initializeCore()` does not interrupt the startup fiber (root fibers are not scope children) — same as the promise chain it replaces; documented, not changed. OFF-RAMP did not fire: error identity, `tests/ipc` and the ACP path are preserved. --- <details> <summary>📋 Implementation Plan (Wave 4 — effect-wave4-lifecycle-core-plan.md)</summary> # Effect migration — Wave 4: finish the concurrency/lifecycle core Bounded wave: **4 PRs (PR 4 optional), explicit STOP criterion, explicit OFF-RAMPs.** Plan only; nothing here is implemented. > **Review status:** Independently reviewed (adversarial Reviewer sub-agent, advisor unavailable): APPROVE WITH REQUIRED EDITS — both edits applied; verified claims: Effect.promise 0-arity thunk allocates no AbortController (internal/effect.js:741–776); closed-scope forkIn+startImmediately runs onInterrupt (2237–2274, 391–409); forkIn observer removes the scope finalizer on exit (2270–2271); Effect.timeoutOrElse exists (Effect.d.ts:7833); 0 line drift at b87f627; 11 settleWorkspaceTurn callers confirmed; 'aborted' ∈ NON_RETRYABLE_STREAM_ERRORS. Line references are to `main` @ `b87f62729`. ## 0. Thesis check (coordinator's judgment vs. evidence) **Thesis:** Effect's payoff in this app is structured concurrency + interruption-safe lifecycles in the orchestration core (still Promise + AbortController). **Verdict: holds for the stream engine; only half-holds for turn handles.** - Stream engine — **holds.** `ServiceContainer.dispose()` never stops or awaits in-flight streams (`serviceContainer.ts:478–537` has no `streamManager` step); an in-flight stream dies with the process and is recovered on next load from `partial.json` (≤ 500 ms stale, `PARTIAL_WRITE_THROTTLE_MS`, `streamManager.ts:776`). `AppFiberScope` exists precisely for this and has no occupant. A supervised per-stream fiber is the right tool. - Turn handles — **half-holds.** The 7× "superseded by an uncorrelated workspace stream-end" false-settle is a *correlation-predicate* bug (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `workspaceTurnManager.ts:4220–4307`: any uncorrelated stream-end after the prompt index settles the handle `interrupted`), not a Promise-vs-fiber structure bug. Turn handles are **persisted records** (`taskHandleStore.upsertWorkspaceTurn`) spanning **multiple streams** (tool-call continuations are deferred via `hasSameTurnContinuation`, `:4491`) and surviving restarts; a fiber/Deferred can only model the in-process waiter and would not fix correlation. Open PR **coder#3949** fixes the predicate in Promise idiom and is Codex-green. Wave 4's turn-handle PR therefore becomes **"codify the settlement invariant + prove the class is gone"**, not "fiberize handles" (D5 below). Corrected baseline numbers (measured this workspace): 46/470 `src/node` non-test files import `effect` (coordinator said 35); 113 direct `Effect.run*` sites outside `di/` in 16 files; 226 `Effect.gen`; 9 `TaggedError` classes; effect `4.0.0-rc.112`, `@orpc/*` `1.14.11`; **effect v4 is not GA** (rc line still current). ## 1. Verified current state (evidence the design rests on) <details> <summary>Stream engine (streamManager.ts)</summary> - `startStream` (`:4723–4901`): per-workspace mutex → `new AbortController()` + `linkAbortSignal` (`:4771–4772`) → `resourceScope = Scope.makeUnsafe()` (`:4777`) → temp-dir `Effect.acquireRelease` (`:4802–4824`) → `createStreamAtomically` → `streamText` (`:2244`, `abortSignal: abortController.signal` `:2250`) → registered in `workspaceStreams` (`:2463`) → **`streamInfo.processingPromise = this.processStreamWithCleanup(...)` fire-and-forget (`:4876–4882`)** → returns `Ok({ messageId, completion })`. - `processStreamWithCleanup` (`:3331–4089`, plain async): `while(true)` retry loop; `for await (part of fullStream)` (`:3358–3837`) with abort check at loop head (`:3361`); post-loop `if (!signal.aborted)` gate (`:3849`) → completion path (`deletePartial` `:3981`, `updateHistory` `:3989`, `recordSessionUsage` `:4001`, `state = COMPLETED` `:4017`, emit `stream-end` `:4023`, `terminalCompletion` `:4024`); error path → `handleStreamFailure` (`:4094–4112`) → `persistStreamError` writes error partial; `finally` (`:4052–4088`): release MCP lease, `Effect.runFork(Scope.close(resourceScope))` (`:4064–4066`), unlink abort, `workspaceStreams.delete`, `eventSpine.emit("stream.end")`, `completionController.settle`. - Cancellation: `stopStream` (`:5043–5111`) → `cancelStreamSafely` (`:1766–1800`): `if (state === COMPLETED) { await processingPromise; return }` → `state = STOPPING` → `flushPartialWrite` → `abortController.abort()` → `cleanupAbortedStream` (`:1828–1951`): `await processingPromise` → usage → `writePartial` (`:1876–1910`) → `emitStreamAbort` → `settle({status:"aborted"})`. **No completed-guard after the await** (verified `:1838–1951`): a cancel landing between `:3849` and `:4017` re-writes `partial.json` after `deletePartial` and emits `stream-abort` after `stream-end` (pre-existing window; dispose() will widen its exposure). `cancelStreamSafely` is also not idempotent for concurrent callers (only `COMPLETED` is checked). - AIService on `stream-abort` (`aiService.ts:355–377`): `abandonPartial ? deletePartial : commitPartial → deletePartial` (fire-and-forget listener). - Crash recovery: `HistoryService.commitPartial` (`historyService.ts:1963–2061`) — strips error metadata, `hasCommitWorthyParts`, stale-epoch check, update-or-append by `historySequence`, delete partial; invoked from `agentSession.init` (`:5002`), `aiService.streamMessage` (`:886`), stream-abort (`:364`), `duplicateWorkspace`. - `StreamAbortReason = "user" | "startup" | "system"` (`src/common/orpc/schemas/stream.ts:295`). - Pinned seams: chaos test `Reflect.set(streamManager, "tokenTracker" | "createStreamResult")` (`streamManager.chaos.test.ts:130–134, 238–242`); `streamManager.test.ts` `Reflect.set` on `processStreamWithCleanup` (`:2787`), `createStreamAtomically` (`:2783`), `createTempDirForStream`, `cleanupStreamTempDir`, `Reflect.get` on `workspaceStreams`, `schedulePartialWrite`, …; `modelOnlyNotifications.test.ts` calls `processStreamWithCleanup` directly (`:93, :187`); `aiService.test.ts` spies `startStream`, `generateStreamToken`, `createTempDirForStream`, `isResponseIdLost`. Constructor: `(historyService, sessionUsageService?, getProvidersConfig?, eventSink = noop, runner = defaultEffectRunner)` (`:801–813`); `effectRunner` used at `:1152, :1154, :1169` only. - Every stream event carries `workspaceId` + `messageId`; `stream-end`/`stream-abort`/`error` carry `metadata.muxMetadata` when the prompt had it. </details> <details> <summary>DI / shutdown / startup</summary> - `AppFiberScopeLive` is in `CoreLive`'s `runtimeSeams` (`di/layers/core.ts:644`), so both roots have it; `StreamManagerLive` (`core.ts:226–238`, stage S2b) already yields `EffectRunnerTag`. CLI cleanup lists include `appFiberScope.close` (`cli/run.ts:1579`, `cli/workflow.ts:286`). - Bounds: `APP_FIBER_SCOPE_CLOSE_TIMEOUT_MS = 2000`, `APP_RUNTIME_DISPOSE_TIMEOUT_MS = 2000`; outer budgets are **5000 ms** on both desktop (`desktop/main.ts:1297` `Promise.race` vs `setTimeout(5000)`) and `xum server` (`cli/server.ts:236–243` force-exit). **The scope bound cannot grow** without changing outer budgets. - rc.112 semantics verified in `node_modules/effect/dist/internal/effect.js:2264`: `forkIn` registers a scope finalizer and **removes it when the fiber completes** (no leak), and **interrupts immediately if the scope is already closed** (streams starting mid-shutdown fail closed). `Effect.promise(evaluate: (signal) => PromiseLike)`, `Effect.onInterrupt`, `Effect.forkIn(_, scope, { startImmediately? })`, `Stream.toAsyncIterableWith(context)`, `Stream.provideContext` all exist. - `ServiceContainer.initialize()` (`serviceContainer.ts:297–362`): six awaited `initialize()`s wrapped in `recordStep` (durations only, no catch, no timeout) + three sync `start()`s + two fire-and-forget sweeps. Failure handling: desktop `Startup Failed` dialog + `app.quit()` (`desktop/main.ts:1249–1265`); `cli/server.ts:136` uncontained; ACP `serverConnection.ts:205–216` dispose + rethrow; `tests/ipc/setup.ts:85` no catch. No outer timeout anywhere. - `streamBridge.subscriptionIterable` (`orpc/streamBridge.ts:176`) → `Stream.toAsyncIterable(...)` on the global runtime, 19 call sites in `routerSubscriptions.ts`; heartbeat via `Effect.sleep` in `forkScoped` (`:145–152`). `streamBridge.test.ts` has 11 real-time waits, but only **3 are clock-bound** (`:207` 1 ms initial delay, `:241` heartbeat 10 ms, `:255` 10 ms laziness); 8 are `waitFor(listenerCount…)` readiness polls that TestClock cannot replace. </details> <details> <summary>Turn handles + open PRs</summary> - Handle record `{ handleId "wst_…", ownerWorkspaceId, workspaceId, turnId, messageId, status, attentionPolicy, disposableWorkspace }`; prompt carries `muxMetadata: { type:"workspace-turn-task", taskHandleId, ownerWorkspaceId, turnId }` (`workspaceTurnManager.ts:1420`). `TaskService` forwards `aiService` `stream-end`/`stream-abort`/`error` to `finalizeWorkspaceTurnFromStreamEnd` (`:4442–4544`): correlated branch matches `record.workspaceId && record.turnId` (`:4472`); **uncorrelated branch** (`metadata == null`, not `agentId === "compact"`) → `interruptWorkspaceTurnFromUncorrelatedStreamEnd` → settles `interrupted` whenever `streamEndIndex >= promptIndex` (`:4293–4305`). Producers of such uncorrelated ends: bash-monitor wake continuations, child terminal-attention deliveries, heartbeat, peer messages, parent auto-resume. - Cascade: disposable child → `cleanupDisposableWorkspaceTurn` → `workspaceService.remove(…, true)` kills its background processes; persistent child → parent sees `interrupted` → `task_stop` → `backgroundProcessManager.stopMonitor(…, "canceled")`. This is the observed "monitors died afterwards". - Settlement chokepoint: `settleWorkspaceTurn(params)` (`:2085`), **11 callers**, guarded by `workspaceTurnSettlementLocks.withLock(handleId)`; waiters in `pendingWorkspaceTurnWaitersByHandleId` with `setTimeout` timeouts (`:2425–2496`). - **coder#3949** "preserve turns across synthetic wake ends" (coadler): rewrites the uncorrelated branch — walks history from the turn anchor to the stream-end and settles *only if a manual child input intervened* (`isManualChildWorkspaceInput`); otherwise ignores the end. Touches `:281–295, :4217–4355` + tests (+286/−31). Codex: "Didn't find any major issues" + clean security on `f9baa2fc9`. `mergeable: MERGEABLE`, but `Test / Unit` and `Codex Comments` red, 19 commits behind main. - **coder#3915** "correlate workspace-turn liveness" (coadler): creation reservations + `getWorkspaceTurnLiveness`/`getWorkspaceTurnRuntimeActivity` (identity-matches the active stream's `muxMetadata` against the record) for staleness/capacity. Touches `:442–486, :1298, :2573, :3761, :3800–4064` (+494/−60). `BLOCKED`, latest Codex review has open findings, `Test / Unit` red, 19 behind. - Together they are the identity-correlated model the coordinator wants: coder#3915 = identity-correlated *liveness*, coder#3949 = identity-gated *settlement*. </details> ## 2. Design decisions **D1 — Fibers WRAP the AbortController; they do not replace it.** The AI SDK is cancelled only via `AbortSignal`; the `for await` loop, soft-interrupt at step boundaries, retry/fallback re-creation of `streamResult`, and ~30 abort touchpoints (coder#4032) all key off the signal. Converting the 750-line loop to `Stream.fromAsyncIterable` + fiber interruption would touch hundreds of `WorkspaceStreamInfo` transitions and break the `processStreamWithCleanup`/`createStreamResult` spy seams. Instead: **the fiber is the ownership/supervision unit; the signal stays the cancellation transport.** The dual-cancellation glue coder#4032 feared is confined to **one** point — the supervisor's `onInterrupt` — which routes through the existing user-stop path (`cancelStreamSafely`), so shutdown ≡ "user pressed stop" semantically (partial flushed with usage, `stream-abort` emitted, `completion` settles `aborted`, AIService commits the partial). **D2 — Supervisor topology: one supervisor fiber per stream in `AppFiberScope`, wrapping the already-started `processingPromise`.** `streamInfo.processingPromise = this.processStreamWithCleanup(...)` stays byte-identical (sync-start preserved; `Reflect.set(processStreamWithCleanup)` seam preserved; `cleanupAbortedStream`'s `await processingPromise` unchanged). Immediately after it: ```ts // startStream, after processingPromise is assigned (unsupervised path unchanged when no scope) this.superviseEngine(typedWorkspaceId, streamInfo); private superviseEngine(workspaceId: WorkspaceId, streamInfo: WorkspaceStreamInfo): void { if (this.engineScope === undefined) return; // direct construction / CLI tests: today's behavior assert(streamInfo.engineFiber === undefined, "engine already supervised"); // Zero-arity thunk on purpose: rc.112 allocates an internal AbortController only // when `evaluate.length !== 0`; the stream's own controller stays the sole signal. const supervisor = Effect.promise(() => streamInfo.processingPromise).pipe( Effect.onInterrupt(() => Effect.uninterruptible( // explicit, per house doctrine (finalizers are already uninterruptible) Effect.promise(async () => this.cancelStreamSafely(workspaceId, streamInfo, "system")) ) ), Effect.catchDefect((d) => Effect.sync(() => log.warn("[stream] engine supervisor defect", { workspaceId, error: d }))) ); streamInfo.engineFiber = this.effectRunner.runSync( Effect.forkIn(supervisor, this.engineScope, { startImmediately: true }) ); } ``` - `Effect.promise` is interruptible while suspended (`internal/effect.js:741–801`, Async op); `onInterrupt` = `onErrorFilter(causeFilterInterruptors, …)` (`:1762`); `forkIn` registers `fiberInterrupt(fiber)` as the scope finalizer (`:2264–2275`), `fiberInterrupt` awaits the fiber (`:635–642`), and parallel `scopeClose` awaits all finalizers via `fiberAwaitAll` (`:1590–1601`) → `closeScopeBounded` at dispose step 2 gives "interrupt **and** await" while `historyService`/`sessionUsage`/`eventSink → AIService → bridge servers` are still alive (bridges stop in step 3, so clients receive `stream-abort`). - Normal completion: fiber exits → `forkIn`'s observer removes the scope finalizer (verified) → no per-stream residue. - Stream started after step 2: `forkIn` on a closed scope calls `fiber.interruptUnsafe` synchronously and returns the fiber (`:2272–2274`, `runSync` does not defect). With `startImmediately: true`, `forkUnsafe` runs `child.evaluate` synchronously (`:2233–2247`) up to the `Effect.promise` Async op (`:772–801`), so the fiber is suspended (`_running=false`) when the interrupt lands and `interruptUnsafe` (`:391–409`) unwinds the stack through the `onInterrupt` handler → the stream is aborted as `system` (fail-closed during shutdown). Verified in rc.112 internals (`effect.js:2233–2247, 391–409`); **pin with a test** ("stream started after scope close is aborted") so an RC bump cannot silently change it. - `"system"` is semantically exact: `"user"`/`"startup"` suppress next-startup recovery (`retryEligibility.ts:114–118, 284–287`), `"system"` marks an involuntary backend interruption (as `taskService.ts:8100, 8223` use it). **No in-session retry loop is possible:** the `stream-abort` handler (`agentSession.ts:6010`) routes `{ type: "aborted" }` to `retryManager.handleStreamFailure`, and `"aborted"` is in `NON_RETRYABLE_STREAM_ERRORS` (`retryEligibility.ts:49–59, 106`) → `retryManager.ts:99–104` abandons immediately, never schedules a fiber. Dogfooding still checks the *restart* UX (the recovered partial is shown as interrupted; note whether any next-startup recovery re-sends — same class as today's `system` aborts from `taskService`). - `engineScope` arrives as an **optional 6th constructor parameter** (`engineScope?: Scope.Closeable`), wired from `AppFiberScopeTag` in `StreamManagerLive` (`core.ts:226`). Default `undefined` keeps every direct-construction test and `aiService.ts:174` path identical (I4). `AppFiberScopeLive` already sits beneath S2b in `runtimeSeams`, so no staging change (I6). - Abort reason: reuse **`"system"`** — no wire/schema change; UI copy for `system` already exists. - Pending-start window (`pendingStreamStarts`, before registration) is **not** supervised: nothing is persisted for it yet, and `stopStream` already aborts pending controllers. Documented, not fixed. **D3 — Fix the two adjacent cancel races in the same PR (closely-related bugs, not deferrals).** (a) `cleanupAbortedStream`: after `await processingPromise`, if `streamInfo.terminalCompletion !== undefined` (completed/failed while the cancel was in flight) → return without abort bookkeeping (prevents `partial.json` resurrection after `deletePartial` and a `stream-abort` after `stream-end`). (b) `cancelStreamSafely` (`:1766`): latch a per-stream `cancelPromise` so concurrent cancellers (user stop racing dispose) join one cleanup → exactly one `stream-abort`, one `settle`. **Zero-suspension requirement:** the latch must be checked and assigned **synchronously at function entry, before any `await`** (the current first await is `flushPartialWrite` at `:1789`) — otherwise racing callers can both enter `cleanupAbortedStream`. Shape: ```ts if (streamInfo.cancelPromise) return streamInfo.cancelPromise; streamInfo.cancelPromise = (async () => { /* existing body, unchanged */ })(); return streamInfo.cancelPromise; ``` Both are ≤ 10 LoC and get behavioral tests. **D4 — Shutdown bound stays 2 s; the finalizer must be fast or abandoned.** Outer budgets are 5 s; 2 s + 2 s already consume 4 s. A flowing stream aborts within one chunk; a wedged provider (no chunks, ignores abort) hits the existing `boundedTeardown` timeout: warning, continue, process exit — identical to today's outcome. Dogfooding measures the actual `[shutdown] AppFiberScope closed { ms }` with a live stream. **D5 — Turn handles: codify the settlement invariant; do not fiberize.** Invariant: *a workspace-turn handle settles terminally only by (i) a stream terminal event whose `muxMetadata` correlates `{taskHandleId, ownerWorkspaceId, turnId}` to the record; (ii) an explicit interrupt (`task_stop`/`interruptWorkspaceTurn`); (iii) manual supersession — a manual child input after the turn anchor; (iv) stale-liveness reconciliation.* An uncorrelated stream-end is **never** terminal by itself. coder#3949 makes (iii) the only uncorrelated outcome; coder#3915 implements (iv) by identity. Wave 4 adds a `cause` discriminant to `settleWorkspaceTurn` (the single chokepoint) with a runtime assertion, plus the regression harness. Rationale for not converting waiters to `Deferred`/fibers: no behavioral gain, 4.9k-line file, and the coordinator's "settle only on the owning stream's termination" is over-specified — a turn owns *several* streams. **D6 — Startup: `initialize()` stays a Promise facade over a runtime-run startup effect; timeout ⇒ same failure path as a thrown step.** Each step is `Effect.tryPromise({ try: async () => step(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, ms)) }))` (`timeoutOrElse` exists in rc.112, `Effect.d.ts:7833`; chosen over `timeout` + `catchTag` because the step's error channel is `unknown`, which `catchTag` cannot narrow). No `forkDetach` needed: a Promise step keeps running on its own when the waiting fiber times out (not inside an uninterruptible region, so the timeout interrupts the wait directly). `StartupStepTimeoutError extends Error` with `name = "StartupStepTimeoutError"` set in the constructor and message `"<step> exceeded <ms> ms"` (so the desktop dialog's error formatting shows both the class and the step name) → desktop shows it in the existing `Startup Failed` dialog; CLI/ACP/tests paths unchanged. Step **errors keep their identity** (v4 `runPromise` rejects with the raw failure). Downgrading any step to best-effort is a **policy change, out of scope** (audit of the six implementations: extensionMetadata/telemetry/experiments are local fs, <50 ms; policy has its own 10 s fetch timeout; workspaceService bounds its sync internally; only `taskService.initialize` — config scan + `editConfig` + recovery `sendMessage`s — is potentially unbounded). The three `start()`s stay sync (`Effect.sync`), the two fire-and-forget sweeps stay outside the effect. `stepDurationsMs` is preserved. **Abandon-and-quit safety:** an abandoned `taskService.initialize` may be mid-`editConfig` when the root exits. Parity requirement for PR 3: after a rejected `initialize()`, every root runs the bounded `dispose()` before exiting. Verified: desktop already does — `services` is assigned before the await (`main.ts:653–656`), the catch calls `app.quit()`, and the `before-quit` listener (`:1271–1305`, guard `if (isDisposing || !services) return`) races `services.dispose()` against 5 s; ACP does (`serverConnection.ts:205–216`); **`cli/server.ts` does not** (`:133–136` awaited at top level, `main().catch` at `:282` only logs) → PR 3 adds a bounded `dispose()` there (≤ 10 LoC, same 5 s budget). **D7 — streamBridge: thread the runtime context, not a runner.** `subscriptionIterable` gains `context?: Context.Context<never>` → `Stream.toAsyncIterableWith(context)`; `routerSubscriptions` passes the handler's `"effect/context"`. Production behavior identical; heartbeat sleeps on the runtime `Clock`; tests can run the 3 clock-bound waits on `TestClock`. Honest scope: the 8 readiness polls stay. ## 3. PRs (ordered by value ÷ risk; each independently mergeable) ### PR 1 — StreamManager engine core becomes the first `AppFiberScope` occupant **Value:** high (the only remaining shutdown data-integrity gap; the reason `AppFiberScope` exists). **Risk:** medium → low with D1/D2. **Net product LoC ≈ +55** (`superviseEngine` ~25, ctor param/field ~5, `engineFiber` field ~2, D3 guards ~12, `core.ts` wiring ~2, doc updates in `appRuntime.ts`/`appFiberScope.ts` "occupant" text ~10). Files: `src/node/services/streamManager.ts`, `src/node/services/di/layers/core.ts`, `src/node/services/di/appRuntime.ts` + `appFiberScope.ts` (docs), tests below. Pre-work (before writing product code; each yields a note in the PR body): 1. Confirm `processStreamWithCleanup` never rejects (try/catch/finally shape `:3331–4089`); else the supervisor must fold rejections (it already `catchDefect`s). 2. Confirm `Effect.promise` interruption + `onInterrupt` await ordering under `Scope.close` in a 20-line probe test (pattern of `appFiberScope.test.ts:27–47`). 3. Enumerate abort observers that run *after* the finalizer resolves (AIService `stream-abort` listener → `commitPartial`; agentSession completion continuations) and confirm the durable order (`writePartial` → commit → `deletePartial`) makes a mid-flight `process.exit` recoverable on next load (it is: partial survives until commit completes). 4. Measure: `[shutdown] AppFiberScope closed { ms }` with a live stream in the sandbox (D4). 5. Pin (same probe test): a fiber forked with `startImmediately: true` into an already-closed scope still runs its `onInterrupt` finalizer (reviewer-verified in rc.112 internals; the test guards RC bumps). Acceptance (behavioral tests only): - `streamManager.test.ts` (new cases; existing cases untouched): with `engineScope = Scope.makeUnsafe("parallel")` and a fake `createStreamResult` whose `fullStream` yields one `text-delta` then blocks until its `AbortSignal` fires — `closeScopeBounded(engineScope)` resolves; `writePartial` was called with the streamed text; exactly one `stream-abort` (`abortReason: "system"`) and zero `stream-end`; `completion` settles `{status:"aborted"}`; `workspaceStreams` is empty. - Wedged provider (fullStream never yields, ignores abort): `closeScopeBounded` resolves within the bound, never rejects, warns once (assert the returned promise resolves and no throw; do **not** assert log text). - No-scope construction: identical event sequence to today (guards existing suites; no new assertions needed beyond the unchanged suites passing). - D3(a): cancel issued after the loop exits but before `COMPLETED` → history has exactly one final message, `partial.json` absent, event order `stream-end` only. - D3(b): `stopStream` + `closeScopeBounded` racing on one stream → exactly one `stream-abort`, one settle. - Fiber residue: after 50 completed streams, `closeScopeBounded(engineScope)` emits zero `stream-abort` and completes in the same tick class as an empty scope (assert no aborts and `workspaceStreams.size === 0`). - `streamManager.chaos.test.ts` — existing cases byte-identical; **one new fuzz variant** constructs with an engine scope and closes it at a random iteration: every stream settles **exactly once** (count terminal events per `messageId` ≤ 1, all `completion` promises settle). - `serviceContainer.test.ts`: "dispose() aborts and awaits an in-flight stream before `desktopBridgeServer.stop()`" (extend the ordering harness at `:295–323`); `coreServicesRoot.test.ts`: `xum run` cleanup list does the same via `appFiberScope.close`. Gate suites: `streamManager.test.ts`, `streamManager.chaos.test.ts`, `streamManager.modelOnlyNotifications.test.ts`, `aiService.test.ts`, `agentSession.disposeRace.test.ts`, `agentSession.sinceReplayContract.test.ts`, `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `taskService.test.ts`, `workspaceService.test.ts`, `turnRequestBuilder.test.ts`; `make static-check`. House pre-review audits: interruption posture (supervisor's only suspension is the promise; finalizer uninterruptible end-to-end incl. `cancelStreamSafely` → `cleanupAbortedStream`); no defect escapes (`catchDefect` on the supervisor; `Effect.promise` thunks `async`); spy-seam check (`processStreamWithCleanup`, `createStreamResult`, `createStreamAtomically`, `startStream` signatures unchanged; constructor arity unchanged, trailing optional); sync-start (`processingPromise` assigned before fork; `runSync(forkIn)` completes synchronously); no constructor side-effects added; zero-suspension check on D3(b) latch (`cancelPromise` checked-and-assigned synchronously at `cancelStreamSafely` entry, before the first `await` at `:1789`; review the diff for any inserted `await`/lookup ahead of the assignment). Rollback: revert the `core.ts` wiring line → `engineScope` undefined → today's behavior; D3 guards can stay (independent bug fixes). ### PR 2 — Turn-settlement invariant + false-settle regression harness (gated on coder#3949) **Value:** high (7× production race). **Risk:** low. **Net product LoC ≈ +40** (`WorkspaceTurnSettlementCause` union + `cause` on `settleWorkspaceTurn` params + assert ~10; 11 call sites × 1–3 lines). Relationship to open PRs — explicit: - **coder#3949 is the fix and a hard prerequisite.** PR 2 rebases on it, changes none of its logic (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `isWorkspaceTurnAnchorForRecord`, `isManualChildWorkspaceInput`), and adds the invariant + proof on top. If coder#3949 has not merged when PRs 1/3 are done: **do not fork a competing fix**; report to the coordinator, offer the regression test file to coder#3949's author as a review artifact, and hold PR 2 (it is not on any other PR's critical path). - **coder#3915 is a soft prerequisite.** Its diff (`:3800–4064` incl. `settleStaleWorkspaceTurn`, a `settleWorkspaceTurn` caller) overlaps PR 2's one-line-per-caller change. Prefer landing after it; if PR 2 must go first, the conflict is a one-line `cause:` addition per caller. PR 2 never edits liveness/reservation code. - Wave 4 does not otherwise touch `workspaceTurnManager.ts`. Design: `type WorkspaceTurnSettlementCause` enumerated from the 11 callers (audited at `main` @ `b87f62729`): `:1373` creation validation failure; `:1476` pre-stream interrupt during launch; `:1500`/`:1518` pre-stream send failure; `:3834`/`:3863` stale-liveness recovery / restart timeout (`settleStaleWorkspaceTurn` — coder#3915's region); `:4301` **uncorrelated-stream-end manual supersession** (the only uncorrelated settle in the codebase; the path coder#3949 rewrites); `:4529` correlated terminal; `:4571` stream-abort; `:4675` deferred stream error; `:4736` terminal stream error. `settleWorkspaceTurn` asserts `params.cause` is a member and, for `manual-supersession`, that the superseding input's `messageId` is supplied — turning D5 into an exhaustive `Record<Cause, …>` check rather than prose, so a future "settle on uncorrelated end" cannot be added without naming (and justifying) a cause. Acceptance: - New `workspaceTurnManager.uncorrelatedStreamEnd.test.ts` (real `WorkspaceTurnManager` + `TaskHandleStore` + fake `aiService` emitter, following the existing suite's harness): (1) create turn → correlated `stream-start` → **synthetic wake stream** on the same child ends uncorrelated after the anchor → handle stays `running`, waiter unresolved, no disposable cleanup, no terminal attention → correlated `stream-end` → `completed`. (2) same with `finishReason:"tool-calls"` continuation in between. (3) manual child input between anchor and end → `interrupted` with `cause: manual-supersession`. (4) explicit `interruptWorkspaceTurn` → `interrupted`, `cause: explicit-interrupt`. Case (1) is the **scripted reproduction**: it must **fail on the pre-coder#3949 merge-base** (run the file from a sibling worktree at `git merge-base origin/main <coder#3949 head>`; record the failing assertion in the PR body) and pass after. - Existing 113 `workspaceTurnManager.test.ts` cases and `taskService.test.ts` turn cases unchanged. Gate suites: `workspaceTurnManager.test.ts`, `taskService.test.ts`, `taskHandleStore.test.ts`, `tools/task*.test.ts`; `make static-check`. Audits: spy-seam (`getWorkspaceTurn`, `listAllWorkspaceTurns`, `enqueueTerminalAttention`, `deliverPersistentChildWorkspaceTurnResult` untouched); settlement lock held across the assert; no new suspension inside `withLock`. Rollback: revert; the test file stays valid against coder#3949 alone (drop the `cause` assertions). ### PR 3 — `ServiceContainer.initialize()` as a runtime-run startup effect with per-step timeouts **Value:** medium (a hung `taskService.initialize()` currently pins the splash screen forever; deterministic TestClock tests of startup). **Risk:** low–medium. **Net product LoC ≈ +80** (step table ~20, timed-step helper ~15, `StartupStepTimeoutError` ~8, constant ~3, facade ~10, root dispose-on-failure parity ≤ 10, doc update ~10). Files: `serviceContainer.ts`, `src/constants/terminationTimeouts.ts` (keep with the termination constants so the budget doc stays in one place), `cli/server.ts` (dispose in the startup catch if missing), `di/appRuntime.ts` doc ("Deliberately not done" → remove the initialize() line; add startup contract). Design (D6): `initialize(): Promise<void>` → `this.runtime.managed.runPromise(this.startupEffect())`. `startupEffect = Effect.gen` over an ordered `readonly steps: ReadonlyArray<{ name, run: () => Promise<void> }>` (assert names unique); each step: `recordStep` timing kept, `Effect.tryPromise({ try: async () => run(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, STARTUP_STEP_TIMEOUT_MS)) }))`. Then `Effect.sync` for the three `start()`s; the sweeps remain after `runPromise`. Constant `STARTUP_STEP_TIMEOUT_MS` — pre-work measures `[startup] <step> { ms }` across sandbox cold starts and picks ≥ 10× the slowest observed (propose 60 s; must be generous — a false timeout turns a slow-but-fine start into a crash). Roots dispose after a rejected `initialize()` (D6 abandon-and-quit safety). Acceptance (all in `serviceContainer.test.ts`, TestClock via the existing `AppLive` spy at `:355–395`): - A step that never resolves → `initialize()` rejects with `StartupStepTimeoutError` naming the step after exactly `TestClock.adjust(STARTUP_STEP_TIMEOUT_MS)`; later steps did not run. - A rejecting step → `initialize()` rejects with **the same error object** (identity), later steps did not run (parity with today). - Happy path → `stepDurationsMs` has all six keys; `start()`s called once each; second `initialize()` call behavior unchanged from today (verify whether re-entry is guarded today; preserve). - `tests/ipc` harness and ACP entry still pass unchanged. Gate suites: `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `src/node/acp/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (smoke subset); `make static-check`. Audits: I1 untouched (no layer body changes); I2 (only the composition root touches the runtime); error identity preserved (no wrapping); abandoned-step safety — every root runs bounded `dispose()` after a rejected `initialize()` (D6; `cli/server.ts` gains it in this PR); no sweep moved into the effect; the six-step order and `[startup] <step>` names unchanged. Rollback: revert; constant removal. ### PR 4 (optional — cut if budget is exhausted) — `streamBridge` on the runtime context **Value:** low (closes the last documented "global runtime" exception; enables TestClock for the heartbeat). **Risk:** low. **Net product LoC ≈ +30** (`context?` option + `toAsyncIterableWith` ~8; 19 call sites × 1 line via one shared helper in `routerSubscriptions.ts` ~3). Acceptance: the 3 clock-bound waits (`:207, :241, :255`) run on `TestClock`; the heartbeat test asserts N heartbeats after `TestClock.adjust(N × interval)` with zero real time; existing behavioral assertions unchanged; `tests/ipc` subscription tests pass. Do not rewrite the 8 readiness polls. Gate suites: `streamBridge.test.ts`, `routerSubscriptions*.test.ts`, `orpc/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (subscription subset); `make static-check`. Audits: `Stream.toAsyncIterableWith` preserves double-close safety (pin with the existing test); `Cause.Done` typing unchanged; no `Scope`/`MemoMap`/`Scheduler` captured (pass the oRPC `effect/context`, which the DI layer already strips per `EffectRunnerLive`); `context` stays optional so direct callers/tests without a runtime keep today's global-runtime path. Rollback: revert; the optional `context` default (`Context.empty()`) is exactly today's `toAsyncIterable`, so a partial revert of call sites is also safe. ### Execution order and size Net product LoC for the wave ≈ **+205** (PR 1 ≈ +55, PR 2 ≈ +40, PR 3 ≈ +80, PR 4 ≈ +30); tests ≈ +600–800. PR 1 starts immediately. PR 2 starts the moment coder#3949 merges (parallel with PR 1/3 — disjoint files). PR 3 after PR 1 merges (both touch `appRuntime.ts` docs; PR 3 also touches `serviceContainer.ts`). PR 4 last, only if PRs 1–3 landed and no OFF-RAMP fired. Each PR: Codex dual review, `Codex Comments` minimization, merge queue; commit WIP early (`/tmp` wipes). ## 4. STOP criterion (measurable) and OFF-RAMPs Wave 4 is **done** — and the Effect migration line **stops** without a new RFC — when all hold: 1. **dispose() awaits in-flight streams:** `serviceContainer.test.ts` ordering test + `coreServicesRoot.test.ts` pass on main; a sandbox `script -f` transcript of `xum server` receiving SIGTERM mid-stream shows `stream-abort` → `[shutdown] AppFiberScope closed { ms }` → `[shutdown] desktopBridgeServer.stop`, and immediately after exit `partial.json` is absent while `chat.jsonl` contains the interrupted assistant message (baseline on `main`: `partial.json` present, message absent until next load). `{ ms }` < 2000 in the flowing-stream case. 2. **False-settle class eliminated:** the scripted reproduction fails on the pre-coder#3949 merge-base and passes on main after PR 2; `settleWorkspaceTurn` rejects any settlement without an enumerated cause; the coordinator's own Mux sessions show zero "superseded by an uncorrelated workspace stream-end" in the two weeks after PR 2 (soft signal, logged in the wave summary). 3. **Startup:** timeout and error-identity tests pass under TestClock; `[startup]` per-step lines unchanged in the sandbox transcript; a throwaway build with the constant set to 1 ms shows `Startup failed: StartupStepTimeoutError: <step> exceeded 1 ms` and a clean exit. 4. **No new lifecycle flakes:** 0 failures attributable to the touched suites across **N = 20** consecutive *completed* `Test / Unit` runs on `main` after the last Wave 4 merge — query `gh run list --workflow pr.yml --branch main --limit 60 --json databaseId,status,conclusion,event,headSha` (note: `gh run list --json` serializes these fields in **lowercase**, e.g. `{"status":"completed","conclusion":"success"}`, unlike `statusCheckRollup`), keep `status === "completed"` (pending runs have an empty `conclusion`, not null), take the newest 20, and for any run with `conclusion !== "success"` (case-insensitive normalization acceptable) inspect the failing job's log for the touched suite names (job `timeout`/`cancelled` from the 15-min budget is not a flake); plus green merge-queue runs for each PR. Any attributable flake → fix or revert before declaring done. **OFF-RAMP (PR 1):** fires if pre-work 1–3 shows (a) routing shutdown through `cancelStreamSafely` cannot preserve crash-recovery semantics without changing `cleanupAbortedStream`'s contract beyond D3, (b) the chaos variant exposes a double-settle not closable by D3(b), or (c) the finalizer cannot fit the 2 s bound for flowing streams. Then: stop PR 1, keep `AppFiberScope` unoccupied, update `appRuntime.ts` "Deliberately not done" with the concrete blocker and the measured evidence, land D3 alone as a bug-fix PR. PRs 2–4 are independent and proceed. **OFF-RAMP (PR 3):** if error identity or the `tests/ipc`/ACP paths cannot be preserved, keep `initialize()` as is and record why. **OFF-RAMP (PR 2):** coder#3949 not merged → hold (see PR 2). ## 5. Risk register | Risk | Likelihood | Mitigation | |---|---|---| | Crash-recovery regression: double commit / partial resurrection when shutdown-abort races completion | medium (window widens with dispose()) | D3(a) guard + test; `commitPartial`'s `historySequence` update-or-append is idempotent (`historyService.ts:2036–2041`) | | Provider abort emits an `error` chunk → error path instead of abort path | low | identical to today's user-stop path (parity); chaos variant covers hostile streams | | Wedged provider pins the 2 s bound → warning every shutdown | low | `boundedTeardown` already bounds; transcript measures; no budget change possible (5 s outer) | | AIService `stream-abort` listener (`commitPartial`) still in flight when `process.exit` runs | low | durable order writePartial → commit → deletePartial; next-load recovery; PR 1 transcript checks `partial.json` is already gone when `cli/server.ts` logs its final cleanup line before `process.exit(0)` (`:252–266`) | | Chaos-test seams (`createStreamResult`, `tokenTracker`) | none if scope-less construction stays default | new variant added, old cases untouched | | Collision with coder#3915/coder#3949 | medium | PR 2 gated; no edits to their regions; one-line `cause:` conflicts only | | RC churn (rc.113+ renames `forkIn`/`onInterrupt`/`toAsyncIterableWith`) | low | all Effect imports already in `streamManager.ts`/`streamBridge.ts`; pins fixed; GA upgrade is a separate lockstep PR (§6) | | Startup false timeout on slow hosts | medium if constant too small | measure first; ≥ 10× slowest observed; generous default (60 s) | | Sync-start assumptions in tests that `Reflect.set(processStreamWithCleanup)` | low | promise assigned before fork; forkIn `runSync` synchronous | | `shutdown()` (desktop second `before-quit` listener) still does not await streams | accepted | contract says `shutdown()` never touches the runtime; desktop's dispose race is the covered path | | Streams starting during shutdown | covered | `forkIn` on closed scope interrupts immediately → `system` abort (`startImmediately` semantics verified in rc.112; pinned by test) | | `system` abort triggers an in-session RetryManager retry during shutdown | **unreachable** (verified) | `"aborted"` ∈ `NON_RETRYABLE_STREAM_ERRORS` → `retryManager.ts:99–104` abandons; no fiber scheduled | | PR 3: abandoned `taskService.initialize` mid-`editConfig` when the root exits after a timeout | low | desktop/ACP already dispose on startup failure; PR 3 adds the missing `cli/server.ts` dispose; config writes are lock/journal-protected | | `startImmediately`/`onInterrupt` semantics differ in a later RC | low | pinned by the probe test in `appFiberScope.test.ts` style; RC bumps are a separate lockstep PR | ## 6. Standing item — effect v4 GA + `@orpc/experimental-effect` lockstep (analysis only) v4 is **not GA** (rc.112 is current; v3 `3.x` remains the stable line). No PR this wave. When GA ships: one lockstep PR bumping `effect` + all `@orpc/*` (`1.14.11` today; check the GA-compatible `@orpc/experimental-effect`), canary gates = `di/*.test.ts`, `streamBridge.test.ts`, `streamManager.test.ts`, `serviceContainer.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc`, `make static-check`. The `Context → ServiceMap` rename risk is firewalled: `Context.Service` tags, `Context.omit/get`, `Layer`, `ManagedRuntime`, `TestClock` live only under `di/` + `orpc/effectContext.ts`; `streamManager.ts`/`streamBridge.ts` use `Effect`/`Scope`/`Fiber`/`Exit`/`Stream`/`Queue`/`Cause` only. PR 4 adds one `Context.Context<never>` type reference to `streamBridge.ts` — keep it as a type-only import so a rename is a one-line fix. ## 7. Dogfooding (per PR; evidence attached to the PR with `gh … --attach`) Common setup: `make dev-server-sandbox DEV_SERVER_SANDBOX_ARGS="--clean-projects"` (or `xum server` on a temp `XUM_ROOT`) with `XUM_LOG_LEVEL=debug`, run under `script -f ~/wave4-scratch/<pr>-<scenario>.log`; scratch under `$HOME/wave4-scratch/` (never `/tmp`). Drive the UI with `agent-browser` (`open` → `snapshot -i` → click the explicit "Send message" ref; re-snapshot after typing). Screenshots are primary evidence; record WebM and finalize with `ffmpeg -c copy`. - **PR 1:** (1) start a long stream (prompt that streams ~30 s), wait 3–5 s, `kill -TERM <server pid>`; transcript must show the order in STOP coder#1 and `[shutdown] AppFiberScope closed { ms }`; (2) `ls <XUM_ROOT>/sessions/<ws>/partial.json` (absent) + `tail -n 1 chat.jsonl` (interrupted assistant message); (3) restart, open the workspace in agent-browser, screenshot the persisted interrupted message; (4) same scenario on `main` for the baseline diff; (5) `xum run` Ctrl-C mid-stream transcript (CLI root parity); (6) quality gate between phases: gate suites green before the sandbox run, sandbox evidence before requesting review. - **PR 2:** primary evidence is the regression file run on both the merge-base worktree (failing output) and the branch (passing). Secondary: sandbox parent workspace delegates via `task` kind=workspace to a child that arms a background bash monitor firing within ~10 s and keeps working ~60 s; screenshot the parent's task result (baseline `main`: `interrupted … uncorrelated workspace stream-end`; after: `completed`) and the child's log lines. - **PR 3:** `xum server` cold start transcript with `[startup] <step> { ms }` for the six steps (parity); throwaway worktree build with `STARTUP_STEP_TIMEOUT_MS = 1` → transcript of `Startup failed: StartupStepTimeoutError …` and exit code (do not ship); desktop dialog cannot be shown headless — cite the unchanged `desktop/main.ts:1249–1265` catch. - **PR 4:** agent-browser session left idle 60 s with the connection indicator visible (heartbeats keep it green) + screenshot; `streamBridge.test.ts` on TestClock. ## 8. Non-goals (restated; out of this wave) Typed-error propagation sweep / removing the ~113 facades; converting services to `yield*`-based Effect services; PubSub for the internal EventEmitter bus; Schema at persistence boundaries; Effect observability; converting sync read paths, AI-SDK per-request callbacks, cross-process lock interiors, or deterministic try-lock funnels; replacing `AbortController` as the SDK cancellation transport; converting the `fullStream` loop to `Stream`; fiberizing turn-handle waiters; downgrading startup steps to best-effort; changing outer quit budgets; supervising the pre-registration stream-start window; `shutdown()` semantics; the effect GA bump (standing analysis only). </details> --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `xhigh` • Cost: `$13.39`_ <!-- mux-attribution: model=anthropic:claude-fable-5-1 thinking=xhigh costs=13.39 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 6, 2026
… PR 4) (coder#4085) ## Summary Wave 4 PR 4 (D7): subscription streams now inherit the app runtime's context, so their heartbeat sleeps use its Clock. **+30 net production lines**, no payload, queue, ordering, or completion changes. Direct callers without a context retain `Stream.toAsyncIterable` exactly. ## Implementation and audits - Optional, type-only `Context.Context<never>` in `streamBridge.ts`; the provided-context branch uses rc.112 `Stream.toAsyncIterableWith`. - All **19** subscription handlers use one forwarding helper. `subscribeLogs` lacked a context argument, so its router entry now forwards it too. - The baseline `runtime.context` was **not clean**: it captured `Layer.CurrentMemoMap` and the eager build's scheduler (not Scope). A five-line `Context.omit` projection removes all three before export. Its narrow type assertion preserves the DI contract that these artifacts are not declared app services; no layer bodies changed. `managed.cachedContext` is not an alternative: it still contains CurrentMemoMap. - `Cause.Done`, synchronous `Queue.offerUnsafe`, listener ownership, and abort/finally close logic are unchanged. Existing readiness polls remain. IPC and ACP both obtain the context through `ServiceContainer.toORPCContext()`. - No OFF-RAMP. No changes to streamManager, workspaceTurnManager, or serviceContainer startup. ## Validation - `bun test src/node/orpc src/node/services/di src/node/services/serviceContainer.test.ts`: **136 pass, 3 existing skips**. - TestClock replaces the three clock-bound waits. Heartbeat baseline **23 ms** (two real 10 ms intervals); after **2 ms** (three virtual 1 s intervals), with a <50 ms wall-clock assertion. Teardown runs with and without context; abort during pull plus repeated return detaches exactly once. - Red checks: context-artifact test fails before projection; heartbeat test receives zero ticks when the context adapter is disabled. - New handler-level TestClock regression pins oRPC context forwarding. - `make static-check`: green (including both typechecks). - Focused IPC subscriptions: **5 pass** (history replay, terminal output, exit, attach snapshot and live ordering). Command: `TEST_INTEGRATION=1 bun x jest tests/ipc/streaming/websocketHistoryReplay.test.ts tests/ipc/terminal/terminal.test.ts --runInBand -t 'getHistory|send command|exit event|^terminal PTY attach should'`. - Broader IPC run: **7 pass, 1 fail**. The extra terminal reattach test sees an empty serialized screen; reproduced identically with all four production files restored to unchanged base `c470d61e9`. Building the initially missing worker artifacts and changing TERM did not fix it. Recorded separately, not patched or silently counted as passing. - Sandbox: connected API context before/after **65.096 s idle**, successful smoke reply, no reconnect/disconnect toast; **72.2 s** verified recording. Screenshots/video attached in the evidence comment. The app intentionally renders no healthy connection badge, so positive state is recorded alongside screenshots rather than claiming a green indicator. A listener-threshold warning appeared around the recorder’s extra-page setup; no leak was demonstrated. No heartbeat debug lines exist, and none were added. ## Risk / plan deviations Low: context binding only; explicit fallback keeps partial rollback safe. The plan's clean-context and every-handler-already-has-context assumptions were false, requiring the small projection and logs forwarding. PR 2 remains independently gated; this file-disjoint optional PR proceeds under the task's explicit instruction. --- <details> <summary>📋 Implementation Plan</summary> # Effect migration — Wave 4: finish the concurrency/lifecycle core Bounded wave: **4 PRs (PR 4 optional), explicit STOP criterion, explicit OFF-RAMPs.** Plan only; nothing here is implemented. > **Review status:** Independently reviewed (adversarial Reviewer sub-agent, advisor unavailable): APPROVE WITH REQUIRED EDITS — both edits applied; verified claims: Effect.promise 0-arity thunk allocates no AbortController (internal/effect.js:741–776); closed-scope forkIn+startImmediately runs onInterrupt (2237–2274, 391–409); forkIn observer removes the scope finalizer on exit (2270–2271); Effect.timeoutOrElse exists (Effect.d.ts:7833); 0 line drift at b87f627; 11 settleWorkspaceTurn callers confirmed; 'aborted' ∈ NON_RETRYABLE_STREAM_ERRORS. Line references are to `main` @ `b87f62729`. ## 0. Thesis check (coordinator's judgment vs. evidence) **Thesis:** Effect's payoff in this app is structured concurrency + interruption-safe lifecycles in the orchestration core (still Promise + AbortController). **Verdict: holds for the stream engine; only half-holds for turn handles.** - Stream engine — **holds.** `ServiceContainer.dispose()` never stops or awaits in-flight streams (`serviceContainer.ts:478–537` has no `streamManager` step); an in-flight stream dies with the process and is recovered on next load from `partial.json` (≤ 500 ms stale, `PARTIAL_WRITE_THROTTLE_MS`, `streamManager.ts:776`). `AppFiberScope` exists precisely for this and has no occupant. A supervised per-stream fiber is the right tool. - Turn handles — **half-holds.** The 7× "superseded by an uncorrelated workspace stream-end" false-settle is a *correlation-predicate* bug (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `workspaceTurnManager.ts:4220–4307`: any uncorrelated stream-end after the prompt index settles the handle `interrupted`), not a Promise-vs-fiber structure bug. Turn handles are **persisted records** (`taskHandleStore.upsertWorkspaceTurn`) spanning **multiple streams** (tool-call continuations are deferred via `hasSameTurnContinuation`, `:4491`) and surviving restarts; a fiber/Deferred can only model the in-process waiter and would not fix correlation. Open PR **coder#3949** fixes the predicate in Promise idiom and is Codex-green. Wave 4's turn-handle PR therefore becomes **"codify the settlement invariant + prove the class is gone"**, not "fiberize handles" (D5 below). Corrected baseline numbers (measured this workspace): 46/470 `src/node` non-test files import `effect` (coordinator said 35); 113 direct `Effect.run*` sites outside `di/` in 16 files; 226 `Effect.gen`; 9 `TaggedError` classes; effect `4.0.0-rc.112`, `@orpc/*` `1.14.11`; **effect v4 is not GA** (rc line still current). ## 1. Verified current state (evidence the design rests on) <details> <summary>Stream engine (streamManager.ts)</summary> - `startStream` (`:4723–4901`): per-workspace mutex → `new AbortController()` + `linkAbortSignal` (`:4771–4772`) → `resourceScope = Scope.makeUnsafe()` (`:4777`) → temp-dir `Effect.acquireRelease` (`:4802–4824`) → `createStreamAtomically` → `streamText` (`:2244`, `abortSignal: abortController.signal` `:2250`) → registered in `workspaceStreams` (`:2463`) → **`streamInfo.processingPromise = this.processStreamWithCleanup(...)` fire-and-forget (`:4876–4882`)** → returns `Ok({ messageId, completion })`. - `processStreamWithCleanup` (`:3331–4089`, plain async): `while(true)` retry loop; `for await (part of fullStream)` (`:3358–3837`) with abort check at loop head (`:3361`); post-loop `if (!signal.aborted)` gate (`:3849`) → completion path (`deletePartial` `:3981`, `updateHistory` `:3989`, `recordSessionUsage` `:4001`, `state = COMPLETED` `:4017`, emit `stream-end` `:4023`, `terminalCompletion` `:4024`); error path → `handleStreamFailure` (`:4094–4112`) → `persistStreamError` writes error partial; `finally` (`:4052–4088`): release MCP lease, `Effect.runFork(Scope.close(resourceScope))` (`:4064–4066`), unlink abort, `workspaceStreams.delete`, `eventSpine.emit("stream.end")`, `completionController.settle`. - Cancellation: `stopStream` (`:5043–5111`) → `cancelStreamSafely` (`:1766–1800`): `if (state === COMPLETED) { await processingPromise; return }` → `state = STOPPING` → `flushPartialWrite` → `abortController.abort()` → `cleanupAbortedStream` (`:1828–1951`): `await processingPromise` → usage → `writePartial` (`:1876–1910`) → `emitStreamAbort` → `settle({status:"aborted"})`. **No completed-guard after the await** (verified `:1838–1951`): a cancel landing between `:3849` and `:4017` re-writes `partial.json` after `deletePartial` and emits `stream-abort` after `stream-end` (pre-existing window; dispose() will widen its exposure). `cancelStreamSafely` is also not idempotent for concurrent callers (only `COMPLETED` is checked). - AIService on `stream-abort` (`aiService.ts:355–377`): `abandonPartial ? deletePartial : commitPartial → deletePartial` (fire-and-forget listener). - Crash recovery: `HistoryService.commitPartial` (`historyService.ts:1963–2061`) — strips error metadata, `hasCommitWorthyParts`, stale-epoch check, update-or-append by `historySequence`, delete partial; invoked from `agentSession.init` (`:5002`), `aiService.streamMessage` (`:886`), stream-abort (`:364`), `duplicateWorkspace`. - `StreamAbortReason = "user" | "startup" | "system"` (`src/common/orpc/schemas/stream.ts:295`). - Pinned seams: chaos test `Reflect.set(streamManager, "tokenTracker" | "createStreamResult")` (`streamManager.chaos.test.ts:130–134, 238–242`); `streamManager.test.ts` `Reflect.set` on `processStreamWithCleanup` (`:2787`), `createStreamAtomically` (`:2783`), `createTempDirForStream`, `cleanupStreamTempDir`, `Reflect.get` on `workspaceStreams`, `schedulePartialWrite`, …; `modelOnlyNotifications.test.ts` calls `processStreamWithCleanup` directly (`:93, :187`); `aiService.test.ts` spies `startStream`, `generateStreamToken`, `createTempDirForStream`, `isResponseIdLost`. Constructor: `(historyService, sessionUsageService?, getProvidersConfig?, eventSink = noop, runner = defaultEffectRunner)` (`:801–813`); `effectRunner` used at `:1152, :1154, :1169` only. - Every stream event carries `workspaceId` + `messageId`; `stream-end`/`stream-abort`/`error` carry `metadata.muxMetadata` when the prompt had it. </details> <details> <summary>DI / shutdown / startup</summary> - `AppFiberScopeLive` is in `CoreLive`'s `runtimeSeams` (`di/layers/core.ts:644`), so both roots have it; `StreamManagerLive` (`core.ts:226–238`, stage S2b) already yields `EffectRunnerTag`. CLI cleanup lists include `appFiberScope.close` (`cli/run.ts:1579`, `cli/workflow.ts:286`). - Bounds: `APP_FIBER_SCOPE_CLOSE_TIMEOUT_MS = 2000`, `APP_RUNTIME_DISPOSE_TIMEOUT_MS = 2000`; outer budgets are **5000 ms** on both desktop (`desktop/main.ts:1297` `Promise.race` vs `setTimeout(5000)`) and `xum server` (`cli/server.ts:236–243` force-exit). **The scope bound cannot grow** without changing outer budgets. - rc.112 semantics verified in `node_modules/effect/dist/internal/effect.js:2264`: `forkIn` registers a scope finalizer and **removes it when the fiber completes** (no leak), and **interrupts immediately if the scope is already closed** (streams starting mid-shutdown fail closed). `Effect.promise(evaluate: (signal) => PromiseLike)`, `Effect.onInterrupt`, `Effect.forkIn(_, scope, { startImmediately? })`, `Stream.toAsyncIterableWith(context)`, `Stream.provideContext` all exist. - `ServiceContainer.initialize()` (`serviceContainer.ts:297–362`): six awaited `initialize()`s wrapped in `recordStep` (durations only, no catch, no timeout) + three sync `start()`s + two fire-and-forget sweeps. Failure handling: desktop `Startup Failed` dialog + `app.quit()` (`desktop/main.ts:1249–1265`); `cli/server.ts:136` uncontained; ACP `serverConnection.ts:205–216` dispose + rethrow; `tests/ipc/setup.ts:85` no catch. No outer timeout anywhere. - `streamBridge.subscriptionIterable` (`orpc/streamBridge.ts:176`) → `Stream.toAsyncIterable(...)` on the global runtime, 19 call sites in `routerSubscriptions.ts`; heartbeat via `Effect.sleep` in `forkScoped` (`:145–152`). `streamBridge.test.ts` has 11 real-time waits, but only **3 are clock-bound** (`:207` 1 ms initial delay, `:241` heartbeat 10 ms, `:255` 10 ms laziness); 8 are `waitFor(listenerCount…)` readiness polls that TestClock cannot replace. </details> <details> <summary>Turn handles + open PRs</summary> - Handle record `{ handleId "wst_…", ownerWorkspaceId, workspaceId, turnId, messageId, status, attentionPolicy, disposableWorkspace }`; prompt carries `muxMetadata: { type:"workspace-turn-task", taskHandleId, ownerWorkspaceId, turnId }` (`workspaceTurnManager.ts:1420`). `TaskService` forwards `aiService` `stream-end`/`stream-abort`/`error` to `finalizeWorkspaceTurnFromStreamEnd` (`:4442–4544`): correlated branch matches `record.workspaceId && record.turnId` (`:4472`); **uncorrelated branch** (`metadata == null`, not `agentId === "compact"`) → `interruptWorkspaceTurnFromUncorrelatedStreamEnd` → settles `interrupted` whenever `streamEndIndex >= promptIndex` (`:4293–4305`). Producers of such uncorrelated ends: bash-monitor wake continuations, child terminal-attention deliveries, heartbeat, peer messages, parent auto-resume. - Cascade: disposable child → `cleanupDisposableWorkspaceTurn` → `workspaceService.remove(…, true)` kills its background processes; persistent child → parent sees `interrupted` → `task_stop` → `backgroundProcessManager.stopMonitor(…, "canceled")`. This is the observed "monitors died afterwards". - Settlement chokepoint: `settleWorkspaceTurn(params)` (`:2085`), **11 callers**, guarded by `workspaceTurnSettlementLocks.withLock(handleId)`; waiters in `pendingWorkspaceTurnWaitersByHandleId` with `setTimeout` timeouts (`:2425–2496`). - **coder#3949** "preserve turns across synthetic wake ends" (coadler): rewrites the uncorrelated branch — walks history from the turn anchor to the stream-end and settles *only if a manual child input intervened* (`isManualChildWorkspaceInput`); otherwise ignores the end. Touches `:281–295, :4217–4355` + tests (+286/−31). Codex: "Didn't find any major issues" + clean security on `f9baa2fc9`. `mergeable: MERGEABLE`, but `Test / Unit` and `Codex Comments` red, 19 commits behind main. - **coder#3915** "correlate workspace-turn liveness" (coadler): creation reservations + `getWorkspaceTurnLiveness`/`getWorkspaceTurnRuntimeActivity` (identity-matches the active stream's `muxMetadata` against the record) for staleness/capacity. Touches `:442–486, :1298, :2573, :3761, :3800–4064` (+494/−60). `BLOCKED`, latest Codex review has open findings, `Test / Unit` red, 19 behind. - Together they are the identity-correlated model the coordinator wants: coder#3915 = identity-correlated *liveness*, coder#3949 = identity-gated *settlement*. </details> ## 2. Design decisions **D1 — Fibers WRAP the AbortController; they do not replace it.** The AI SDK is cancelled only via `AbortSignal`; the `for await` loop, soft-interrupt at step boundaries, retry/fallback re-creation of `streamResult`, and ~30 abort touchpoints (coder#4032) all key off the signal. Converting the 750-line loop to `Stream.fromAsyncIterable` + fiber interruption would touch hundreds of `WorkspaceStreamInfo` transitions and break the `processStreamWithCleanup`/`createStreamResult` spy seams. Instead: **the fiber is the ownership/supervision unit; the signal stays the cancellation transport.** The dual-cancellation glue coder#4032 feared is confined to **one** point — the supervisor's `onInterrupt` — which routes through the existing user-stop path (`cancelStreamSafely`), so shutdown ≡ "user pressed stop" semantically (partial flushed with usage, `stream-abort` emitted, `completion` settles `aborted`, AIService commits the partial). **D2 — Supervisor topology: one supervisor fiber per stream in `AppFiberScope`, wrapping the already-started `processingPromise`.** `streamInfo.processingPromise = this.processStreamWithCleanup(...)` stays byte-identical (sync-start preserved; `Reflect.set(processStreamWithCleanup)` seam preserved; `cleanupAbortedStream`'s `await processingPromise` unchanged). Immediately after it: ```ts // startStream, after processingPromise is assigned (unsupervised path unchanged when no scope) this.superviseEngine(typedWorkspaceId, streamInfo); private superviseEngine(workspaceId: WorkspaceId, streamInfo: WorkspaceStreamInfo): void { if (this.engineScope === undefined) return; // direct construction / CLI tests: today's behavior assert(streamInfo.engineFiber === undefined, "engine already supervised"); // Zero-arity thunk on purpose: rc.112 allocates an internal AbortController only // when `evaluate.length !== 0`; the stream's own controller stays the sole signal. const supervisor = Effect.promise(() => streamInfo.processingPromise).pipe( Effect.onInterrupt(() => Effect.uninterruptible( // explicit, per house doctrine (finalizers are already uninterruptible) Effect.promise(async () => this.cancelStreamSafely(workspaceId, streamInfo, "system")) ) ), Effect.catchDefect((d) => Effect.sync(() => log.warn("[stream] engine supervisor defect", { workspaceId, error: d }))) ); streamInfo.engineFiber = this.effectRunner.runSync( Effect.forkIn(supervisor, this.engineScope, { startImmediately: true }) ); } ``` - `Effect.promise` is interruptible while suspended (`internal/effect.js:741–801`, Async op); `onInterrupt` = `onErrorFilter(causeFilterInterruptors, …)` (`:1762`); `forkIn` registers `fiberInterrupt(fiber)` as the scope finalizer (`:2264–2275`), `fiberInterrupt` awaits the fiber (`:635–642`), and parallel `scopeClose` awaits all finalizers via `fiberAwaitAll` (`:1590–1601`) → `closeScopeBounded` at dispose step 2 gives "interrupt **and** await" while `historyService`/`sessionUsage`/`eventSink → AIService → bridge servers` are still alive (bridges stop in step 3, so clients receive `stream-abort`). - Normal completion: fiber exits → `forkIn`'s observer removes the scope finalizer (verified) → no per-stream residue. - Stream started after step 2: `forkIn` on a closed scope calls `fiber.interruptUnsafe` synchronously and returns the fiber (`:2272–2274`, `runSync` does not defect). With `startImmediately: true`, `forkUnsafe` runs `child.evaluate` synchronously (`:2233–2247`) up to the `Effect.promise` Async op (`:772–801`), so the fiber is suspended (`_running=false`) when the interrupt lands and `interruptUnsafe` (`:391–409`) unwinds the stack through the `onInterrupt` handler → the stream is aborted as `system` (fail-closed during shutdown). Verified in rc.112 internals (`effect.js:2233–2247, 391–409`); **pin with a test** ("stream started after scope close is aborted") so an RC bump cannot silently change it. - `"system"` is semantically exact: `"user"`/`"startup"` suppress next-startup recovery (`retryEligibility.ts:114–118, 284–287`), `"system"` marks an involuntary backend interruption (as `taskService.ts:8100, 8223` use it). **No in-session retry loop is possible:** the `stream-abort` handler (`agentSession.ts:6010`) routes `{ type: "aborted" }` to `retryManager.handleStreamFailure`, and `"aborted"` is in `NON_RETRYABLE_STREAM_ERRORS` (`retryEligibility.ts:49–59, 106`) → `retryManager.ts:99–104` abandons immediately, never schedules a fiber. Dogfooding still checks the *restart* UX (the recovered partial is shown as interrupted; note whether any next-startup recovery re-sends — same class as today's `system` aborts from `taskService`). - `engineScope` arrives as an **optional 6th constructor parameter** (`engineScope?: Scope.Closeable`), wired from `AppFiberScopeTag` in `StreamManagerLive` (`core.ts:226`). Default `undefined` keeps every direct-construction test and `aiService.ts:174` path identical (I4). `AppFiberScopeLive` already sits beneath S2b in `runtimeSeams`, so no staging change (I6). - Abort reason: reuse **`"system"`** — no wire/schema change; UI copy for `system` already exists. - Pending-start window (`pendingStreamStarts`, before registration) is **not** supervised: nothing is persisted for it yet, and `stopStream` already aborts pending controllers. Documented, not fixed. **D3 — Fix the two adjacent cancel races in the same PR (closely-related bugs, not deferrals).** (a) `cleanupAbortedStream`: after `await processingPromise`, if `streamInfo.terminalCompletion !== undefined` (completed/failed while the cancel was in flight) → return without abort bookkeeping (prevents `partial.json` resurrection after `deletePartial` and a `stream-abort` after `stream-end`). (b) `cancelStreamSafely` (`:1766`): latch a per-stream `cancelPromise` so concurrent cancellers (user stop racing dispose) join one cleanup → exactly one `stream-abort`, one `settle`. **Zero-suspension requirement:** the latch must be checked and assigned **synchronously at function entry, before any `await`** (the current first await is `flushPartialWrite` at `:1789`) — otherwise racing callers can both enter `cleanupAbortedStream`. Shape: ```ts if (streamInfo.cancelPromise) return streamInfo.cancelPromise; streamInfo.cancelPromise = (async () => { /* existing body, unchanged */ })(); return streamInfo.cancelPromise; ``` Both are ≤ 10 LoC and get behavioral tests. **D4 — Shutdown bound stays 2 s; the finalizer must be fast or abandoned.** Outer budgets are 5 s; 2 s + 2 s already consume 4 s. A flowing stream aborts within one chunk; a wedged provider (no chunks, ignores abort) hits the existing `boundedTeardown` timeout: warning, continue, process exit — identical to today's outcome. Dogfooding measures the actual `[shutdown] AppFiberScope closed { ms }` with a live stream. **D5 — Turn handles: codify the settlement invariant; do not fiberize.** Invariant: *a workspace-turn handle settles terminally only by (i) a stream terminal event whose `muxMetadata` correlates `{taskHandleId, ownerWorkspaceId, turnId}` to the record; (ii) an explicit interrupt (`task_stop`/`interruptWorkspaceTurn`); (iii) manual supersession — a manual child input after the turn anchor; (iv) stale-liveness reconciliation.* An uncorrelated stream-end is **never** terminal by itself. coder#3949 makes (iii) the only uncorrelated outcome; coder#3915 implements (iv) by identity. Wave 4 adds a `cause` discriminant to `settleWorkspaceTurn` (the single chokepoint) with a runtime assertion, plus the regression harness. Rationale for not converting waiters to `Deferred`/fibers: no behavioral gain, 4.9k-line file, and the coordinator's "settle only on the owning stream's termination" is over-specified — a turn owns *several* streams. **D6 — Startup: `initialize()` stays a Promise facade over a runtime-run startup effect; timeout ⇒ same failure path as a thrown step.** Each step is `Effect.tryPromise({ try: async () => step(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, ms)) }))` (`timeoutOrElse` exists in rc.112, `Effect.d.ts:7833`; chosen over `timeout` + `catchTag` because the step's error channel is `unknown`, which `catchTag` cannot narrow). No `forkDetach` needed: a Promise step keeps running on its own when the waiting fiber times out (not inside an uninterruptible region, so the timeout interrupts the wait directly). `StartupStepTimeoutError extends Error` with `name = "StartupStepTimeoutError"` set in the constructor and message `"<step> exceeded <ms> ms"` (so the desktop dialog's error formatting shows both the class and the step name) → desktop shows it in the existing `Startup Failed` dialog; CLI/ACP/tests paths unchanged. Step **errors keep their identity** (v4 `runPromise` rejects with the raw failure). Downgrading any step to best-effort is a **policy change, out of scope** (audit of the six implementations: extensionMetadata/telemetry/experiments are local fs, <50 ms; policy has its own 10 s fetch timeout; workspaceService bounds its sync internally; only `taskService.initialize` — config scan + `editConfig` + recovery `sendMessage`s — is potentially unbounded). The three `start()`s stay sync (`Effect.sync`), the two fire-and-forget sweeps stay outside the effect. `stepDurationsMs` is preserved. **Abandon-and-quit safety:** an abandoned `taskService.initialize` may be mid-`editConfig` when the root exits. Parity requirement for PR 3: after a rejected `initialize()`, every root runs the bounded `dispose()` before exiting. Verified: desktop already does — `services` is assigned before the await (`main.ts:653–656`), the catch calls `app.quit()`, and the `before-quit` listener (`:1271–1305`, guard `if (isDisposing || !services) return`) races `services.dispose()` against 5 s; ACP does (`serverConnection.ts:205–216`); **`cli/server.ts` does not** (`:133–136` awaited at top level, `main().catch` at `:282` only logs) → PR 3 adds a bounded `dispose()` there (≤ 10 LoC, same 5 s budget). **D7 — streamBridge: thread the runtime context, not a runner.** `subscriptionIterable` gains `context?: Context.Context<never>` → `Stream.toAsyncIterableWith(context)`; `routerSubscriptions` passes the handler's `"effect/context"`. Production behavior identical; heartbeat sleeps on the runtime `Clock`; tests can run the 3 clock-bound waits on `TestClock`. Honest scope: the 8 readiness polls stay. ## 3. PRs (ordered by value ÷ risk; each independently mergeable) ### PR 1 — StreamManager engine core becomes the first `AppFiberScope` occupant **Value:** high (the only remaining shutdown data-integrity gap; the reason `AppFiberScope` exists). **Risk:** medium → low with D1/D2. **Net product LoC ≈ +55** (`superviseEngine` ~25, ctor param/field ~5, `engineFiber` field ~2, D3 guards ~12, `core.ts` wiring ~2, doc updates in `appRuntime.ts`/`appFiberScope.ts` "occupant" text ~10). Files: `src/node/services/streamManager.ts`, `src/node/services/di/layers/core.ts`, `src/node/services/di/appRuntime.ts` + `appFiberScope.ts` (docs), tests below. Pre-work (before writing product code; each yields a note in the PR body): 1. Confirm `processStreamWithCleanup` never rejects (try/catch/finally shape `:3331–4089`); else the supervisor must fold rejections (it already `catchDefect`s). 2. Confirm `Effect.promise` interruption + `onInterrupt` await ordering under `Scope.close` in a 20-line probe test (pattern of `appFiberScope.test.ts:27–47`). 3. Enumerate abort observers that run *after* the finalizer resolves (AIService `stream-abort` listener → `commitPartial`; agentSession completion continuations) and confirm the durable order (`writePartial` → commit → `deletePartial`) makes a mid-flight `process.exit` recoverable on next load (it is: partial survives until commit completes). 4. Measure: `[shutdown] AppFiberScope closed { ms }` with a live stream in the sandbox (D4). 5. Pin (same probe test): a fiber forked with `startImmediately: true` into an already-closed scope still runs its `onInterrupt` finalizer (reviewer-verified in rc.112 internals; the test guards RC bumps). Acceptance (behavioral tests only): - `streamManager.test.ts` (new cases; existing cases untouched): with `engineScope = Scope.makeUnsafe("parallel")` and a fake `createStreamResult` whose `fullStream` yields one `text-delta` then blocks until its `AbortSignal` fires — `closeScopeBounded(engineScope)` resolves; `writePartial` was called with the streamed text; exactly one `stream-abort` (`abortReason: "system"`) and zero `stream-end`; `completion` settles `{status:"aborted"}`; `workspaceStreams` is empty. - Wedged provider (fullStream never yields, ignores abort): `closeScopeBounded` resolves within the bound, never rejects, warns once (assert the returned promise resolves and no throw; do **not** assert log text). - No-scope construction: identical event sequence to today (guards existing suites; no new assertions needed beyond the unchanged suites passing). - D3(a): cancel issued after the loop exits but before `COMPLETED` → history has exactly one final message, `partial.json` absent, event order `stream-end` only. - D3(b): `stopStream` + `closeScopeBounded` racing on one stream → exactly one `stream-abort`, one settle. - Fiber residue: after 50 completed streams, `closeScopeBounded(engineScope)` emits zero `stream-abort` and completes in the same tick class as an empty scope (assert no aborts and `workspaceStreams.size === 0`). - `streamManager.chaos.test.ts` — existing cases byte-identical; **one new fuzz variant** constructs with an engine scope and closes it at a random iteration: every stream settles **exactly once** (count terminal events per `messageId` ≤ 1, all `completion` promises settle). - `serviceContainer.test.ts`: "dispose() aborts and awaits an in-flight stream before `desktopBridgeServer.stop()`" (extend the ordering harness at `:295–323`); `coreServicesRoot.test.ts`: `xum run` cleanup list does the same via `appFiberScope.close`. Gate suites: `streamManager.test.ts`, `streamManager.chaos.test.ts`, `streamManager.modelOnlyNotifications.test.ts`, `aiService.test.ts`, `agentSession.disposeRace.test.ts`, `agentSession.sinceReplayContract.test.ts`, `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `taskService.test.ts`, `workspaceService.test.ts`, `turnRequestBuilder.test.ts`; `make static-check`. House pre-review audits: interruption posture (supervisor's only suspension is the promise; finalizer uninterruptible end-to-end incl. `cancelStreamSafely` → `cleanupAbortedStream`); no defect escapes (`catchDefect` on the supervisor; `Effect.promise` thunks `async`); spy-seam check (`processStreamWithCleanup`, `createStreamResult`, `createStreamAtomically`, `startStream` signatures unchanged; constructor arity unchanged, trailing optional); sync-start (`processingPromise` assigned before fork; `runSync(forkIn)` completes synchronously); no constructor side-effects added; zero-suspension check on D3(b) latch (`cancelPromise` checked-and-assigned synchronously at `cancelStreamSafely` entry, before the first `await` at `:1789`; review the diff for any inserted `await`/lookup ahead of the assignment). Rollback: revert the `core.ts` wiring line → `engineScope` undefined → today's behavior; D3 guards can stay (independent bug fixes). ### PR 2 — Turn-settlement invariant + false-settle regression harness (gated on coder#3949) **Value:** high (7× production race). **Risk:** low. **Net product LoC ≈ +40** (`WorkspaceTurnSettlementCause` union + `cause` on `settleWorkspaceTurn` params + assert ~10; 11 call sites × 1–3 lines). Relationship to open PRs — explicit: - **coder#3949 is the fix and a hard prerequisite.** PR 2 rebases on it, changes none of its logic (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `isWorkspaceTurnAnchorForRecord`, `isManualChildWorkspaceInput`), and adds the invariant + proof on top. If coder#3949 has not merged when PRs 1/3 are done: **do not fork a competing fix**; report to the coordinator, offer the regression test file to coder#3949's author as a review artifact, and hold PR 2 (it is not on any other PR's critical path). - **coder#3915 is a soft prerequisite.** Its diff (`:3800–4064` incl. `settleStaleWorkspaceTurn`, a `settleWorkspaceTurn` caller) overlaps PR 2's one-line-per-caller change. Prefer landing after it; if PR 2 must go first, the conflict is a one-line `cause:` addition per caller. PR 2 never edits liveness/reservation code. - Wave 4 does not otherwise touch `workspaceTurnManager.ts`. Design: `type WorkspaceTurnSettlementCause` enumerated from the 11 callers (audited at `main` @ `b87f62729`): `:1373` creation validation failure; `:1476` pre-stream interrupt during launch; `:1500`/`:1518` pre-stream send failure; `:3834`/`:3863` stale-liveness recovery / restart timeout (`settleStaleWorkspaceTurn` — coder#3915's region); `:4301` **uncorrelated-stream-end manual supersession** (the only uncorrelated settle in the codebase; the path coder#3949 rewrites); `:4529` correlated terminal; `:4571` stream-abort; `:4675` deferred stream error; `:4736` terminal stream error. `settleWorkspaceTurn` asserts `params.cause` is a member and, for `manual-supersession`, that the superseding input's `messageId` is supplied — turning D5 into an exhaustive `Record<Cause, …>` check rather than prose, so a future "settle on uncorrelated end" cannot be added without naming (and justifying) a cause. Acceptance: - New `workspaceTurnManager.uncorrelatedStreamEnd.test.ts` (real `WorkspaceTurnManager` + `TaskHandleStore` + fake `aiService` emitter, following the existing suite's harness): (1) create turn → correlated `stream-start` → **synthetic wake stream** on the same child ends uncorrelated after the anchor → handle stays `running`, waiter unresolved, no disposable cleanup, no terminal attention → correlated `stream-end` → `completed`. (2) same with `finishReason:"tool-calls"` continuation in between. (3) manual child input between anchor and end → `interrupted` with `cause: manual-supersession`. (4) explicit `interruptWorkspaceTurn` → `interrupted`, `cause: explicit-interrupt`. Case (1) is the **scripted reproduction**: it must **fail on the pre-coder#3949 merge-base** (run the file from a sibling worktree at `git merge-base origin/main <coder#3949 head>`; record the failing assertion in the PR body) and pass after. - Existing 113 `workspaceTurnManager.test.ts` cases and `taskService.test.ts` turn cases unchanged. Gate suites: `workspaceTurnManager.test.ts`, `taskService.test.ts`, `taskHandleStore.test.ts`, `tools/task*.test.ts`; `make static-check`. Audits: spy-seam (`getWorkspaceTurn`, `listAllWorkspaceTurns`, `enqueueTerminalAttention`, `deliverPersistentChildWorkspaceTurnResult` untouched); settlement lock held across the assert; no new suspension inside `withLock`. Rollback: revert; the test file stays valid against coder#3949 alone (drop the `cause` assertions). ### PR 3 — `ServiceContainer.initialize()` as a runtime-run startup effect with per-step timeouts **Value:** medium (a hung `taskService.initialize()` currently pins the splash screen forever; deterministic TestClock tests of startup). **Risk:** low–medium. **Net product LoC ≈ +80** (step table ~20, timed-step helper ~15, `StartupStepTimeoutError` ~8, constant ~3, facade ~10, root dispose-on-failure parity ≤ 10, doc update ~10). Files: `serviceContainer.ts`, `src/constants/terminationTimeouts.ts` (keep with the termination constants so the budget doc stays in one place), `cli/server.ts` (dispose in the startup catch if missing), `di/appRuntime.ts` doc ("Deliberately not done" → remove the initialize() line; add startup contract). Design (D6): `initialize(): Promise<void>` → `this.runtime.managed.runPromise(this.startupEffect())`. `startupEffect = Effect.gen` over an ordered `readonly steps: ReadonlyArray<{ name, run: () => Promise<void> }>` (assert names unique); each step: `recordStep` timing kept, `Effect.tryPromise({ try: async () => run(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, STARTUP_STEP_TIMEOUT_MS)) }))`. Then `Effect.sync` for the three `start()`s; the sweeps remain after `runPromise`. Constant `STARTUP_STEP_TIMEOUT_MS` — pre-work measures `[startup] <step> { ms }` across sandbox cold starts and picks ≥ 10× the slowest observed (propose 60 s; must be generous — a false timeout turns a slow-but-fine start into a crash). Roots dispose after a rejected `initialize()` (D6 abandon-and-quit safety). Acceptance (all in `serviceContainer.test.ts`, TestClock via the existing `AppLive` spy at `:355–395`): - A step that never resolves → `initialize()` rejects with `StartupStepTimeoutError` naming the step after exactly `TestClock.adjust(STARTUP_STEP_TIMEOUT_MS)`; later steps did not run. - A rejecting step → `initialize()` rejects with **the same error object** (identity), later steps did not run (parity with today). - Happy path → `stepDurationsMs` has all six keys; `start()`s called once each; second `initialize()` call behavior unchanged from today (verify whether re-entry is guarded today; preserve). - `tests/ipc` harness and ACP entry still pass unchanged. Gate suites: `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `src/node/acp/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (smoke subset); `make static-check`. Audits: I1 untouched (no layer body changes); I2 (only the composition root touches the runtime); error identity preserved (no wrapping); abandoned-step safety — every root runs bounded `dispose()` after a rejected `initialize()` (D6; `cli/server.ts` gains it in this PR); no sweep moved into the effect; the six-step order and `[startup] <step>` names unchanged. Rollback: revert; constant removal. ### PR 4 (optional — cut if budget is exhausted) — `streamBridge` on the runtime context **Value:** low (closes the last documented "global runtime" exception; enables TestClock for the heartbeat). **Risk:** low. **Net product LoC ≈ +30** (`context?` option + `toAsyncIterableWith` ~8; 19 call sites × 1 line via one shared helper in `routerSubscriptions.ts` ~3). Acceptance: the 3 clock-bound waits (`:207, :241, :255`) run on `TestClock`; the heartbeat test asserts N heartbeats after `TestClock.adjust(N × interval)` with zero real time; existing behavioral assertions unchanged; `tests/ipc` subscription tests pass. Do not rewrite the 8 readiness polls. Gate suites: `streamBridge.test.ts`, `routerSubscriptions*.test.ts`, `orpc/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (subscription subset); `make static-check`. Audits: `Stream.toAsyncIterableWith` preserves double-close safety (pin with the existing test); `Cause.Done` typing unchanged; no `Scope`/`MemoMap`/`Scheduler` captured (pass the oRPC `effect/context`, which the DI layer already strips per `EffectRunnerLive`); `context` stays optional so direct callers/tests without a runtime keep today's global-runtime path. Rollback: revert; the optional `context` default (`Context.empty()`) is exactly today's `toAsyncIterable`, so a partial revert of call sites is also safe. ### Execution order and size Net product LoC for the wave ≈ **+205** (PR 1 ≈ +55, PR 2 ≈ +40, PR 3 ≈ +80, PR 4 ≈ +30); tests ≈ +600–800. PR 1 starts immediately. PR 2 starts the moment coder#3949 merges (parallel with PR 1/3 — disjoint files). PR 3 after PR 1 merges (both touch `appRuntime.ts` docs; PR 3 also touches `serviceContainer.ts`). PR 4 last, only if PRs 1–3 landed and no OFF-RAMP fired. Each PR: Codex dual review, `Codex Comments` minimization, merge queue; commit WIP early (`/tmp` wipes). ## 4. STOP criterion (measurable) and OFF-RAMPs Wave 4 is **done** — and the Effect migration line **stops** without a new RFC — when all hold: 1. **dispose() awaits in-flight streams:** `serviceContainer.test.ts` ordering test + `coreServicesRoot.test.ts` pass on main; a sandbox `script -f` transcript of `xum server` receiving SIGTERM mid-stream shows `stream-abort` → `[shutdown] AppFiberScope closed { ms }` → `[shutdown] desktopBridgeServer.stop`, and immediately after exit `partial.json` is absent while `chat.jsonl` contains the interrupted assistant message (baseline on `main`: `partial.json` present, message absent until next load). `{ ms }` < 2000 in the flowing-stream case. 2. **False-settle class eliminated:** the scripted reproduction fails on the pre-coder#3949 merge-base and passes on main after PR 2; `settleWorkspaceTurn` rejects any settlement without an enumerated cause; the coordinator's own Mux sessions show zero "superseded by an uncorrelated workspace stream-end" in the two weeks after PR 2 (soft signal, logged in the wave summary). 3. **Startup:** timeout and error-identity tests pass under TestClock; `[startup]` per-step lines unchanged in the sandbox transcript; a throwaway build with the constant set to 1 ms shows `Startup failed: StartupStepTimeoutError: <step> exceeded 1 ms` and a clean exit. 4. **No new lifecycle flakes:** 0 failures attributable to the touched suites across **N = 20** consecutive *completed* `Test / Unit` runs on `main` after the last Wave 4 merge — query `gh run list --workflow pr.yml --branch main --limit 60 --json databaseId,status,conclusion,event,headSha` (note: `gh run list --json` serializes these fields in **lowercase**, e.g. `{"status":"completed","conclusion":"success"}`, unlike `statusCheckRollup`), keep `status === "completed"` (pending runs have an empty `conclusion`, not null), take the newest 20, and for any run with `conclusion !== "success"` (case-insensitive normalization acceptable) inspect the failing job's log for the touched suite names (job `timeout`/`cancelled` from the 15-min budget is not a flake); plus green merge-queue runs for each PR. Any attributable flake → fix or revert before declaring done. **OFF-RAMP (PR 1):** fires if pre-work 1–3 shows (a) routing shutdown through `cancelStreamSafely` cannot preserve crash-recovery semantics without changing `cleanupAbortedStream`'s contract beyond D3, (b) the chaos variant exposes a double-settle not closable by D3(b), or (c) the finalizer cannot fit the 2 s bound for flowing streams. Then: stop PR 1, keep `AppFiberScope` unoccupied, update `appRuntime.ts` "Deliberately not done" with the concrete blocker and the measured evidence, land D3 alone as a bug-fix PR. PRs 2–4 are independent and proceed. **OFF-RAMP (PR 3):** if error identity or the `tests/ipc`/ACP paths cannot be preserved, keep `initialize()` as is and record why. **OFF-RAMP (PR 2):** coder#3949 not merged → hold (see PR 2). ## 5. Risk register | Risk | Likelihood | Mitigation | |---|---|---| | Crash-recovery regression: double commit / partial resurrection when shutdown-abort races completion | medium (window widens with dispose()) | D3(a) guard + test; `commitPartial`'s `historySequence` update-or-append is idempotent (`historyService.ts:2036–2041`) | | Provider abort emits an `error` chunk → error path instead of abort path | low | identical to today's user-stop path (parity); chaos variant covers hostile streams | | Wedged provider pins the 2 s bound → warning every shutdown | low | `boundedTeardown` already bounds; transcript measures; no budget change possible (5 s outer) | | AIService `stream-abort` listener (`commitPartial`) still in flight when `process.exit` runs | low | durable order writePartial → commit → deletePartial; next-load recovery; PR 1 transcript checks `partial.json` is already gone when `cli/server.ts` logs its final cleanup line before `process.exit(0)` (`:252–266`) | | Chaos-test seams (`createStreamResult`, `tokenTracker`) | none if scope-less construction stays default | new variant added, old cases untouched | | Collision with coder#3915/coder#3949 | medium | PR 2 gated; no edits to their regions; one-line `cause:` conflicts only | | RC churn (rc.113+ renames `forkIn`/`onInterrupt`/`toAsyncIterableWith`) | low | all Effect imports already in `streamManager.ts`/`streamBridge.ts`; pins fixed; GA upgrade is a separate lockstep PR (§6) | | Startup false timeout on slow hosts | medium if constant too small | measure first; ≥ 10× slowest observed; generous default (60 s) | | Sync-start assumptions in tests that `Reflect.set(processStreamWithCleanup)` | low | promise assigned before fork; forkIn `runSync` synchronous | | `shutdown()` (desktop second `before-quit` listener) still does not await streams | accepted | contract says `shutdown()` never touches the runtime; desktop's dispose race is the covered path | | Streams starting during shutdown | covered | `forkIn` on closed scope interrupts immediately → `system` abort (`startImmediately` semantics verified in rc.112; pinned by test) | | `system` abort triggers an in-session RetryManager retry during shutdown | **unreachable** (verified) | `"aborted"` ∈ `NON_RETRYABLE_STREAM_ERRORS` → `retryManager.ts:99–104` abandons; no fiber scheduled | | PR 3: abandoned `taskService.initialize` mid-`editConfig` when the root exits after a timeout | low | desktop/ACP already dispose on startup failure; PR 3 adds the missing `cli/server.ts` dispose; config writes are lock/journal-protected | | `startImmediately`/`onInterrupt` semantics differ in a later RC | low | pinned by the probe test in `appFiberScope.test.ts` style; RC bumps are a separate lockstep PR | ## 6. Standing item — effect v4 GA + `@orpc/experimental-effect` lockstep (analysis only) v4 is **not GA** (rc.112 is current; v3 `3.x` remains the stable line). No PR this wave. When GA ships: one lockstep PR bumping `effect` + all `@orpc/*` (`1.14.11` today; check the GA-compatible `@orpc/experimental-effect`), canary gates = `di/*.test.ts`, `streamBridge.test.ts`, `streamManager.test.ts`, `serviceContainer.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc`, `make static-check`. The `Context → ServiceMap` rename risk is firewalled: `Context.Service` tags, `Context.omit/get`, `Layer`, `ManagedRuntime`, `TestClock` live only under `di/` + `orpc/effectContext.ts`; `streamManager.ts`/`streamBridge.ts` use `Effect`/`Scope`/`Fiber`/`Exit`/`Stream`/`Queue`/`Cause` only. PR 4 adds one `Context.Context<never>` type reference to `streamBridge.ts` — keep it as a type-only import so a rename is a one-line fix. ## 7. Dogfooding (per PR; evidence attached to the PR with `gh … --attach`) Common setup: `make dev-server-sandbox DEV_SERVER_SANDBOX_ARGS="--clean-projects"` (or `xum server` on a temp `XUM_ROOT`) with `XUM_LOG_LEVEL=debug`, run under `script -f ~/wave4-scratch/<pr>-<scenario>.log`; scratch under `$HOME/wave4-scratch/` (never `/tmp`). Drive the UI with `agent-browser` (`open` → `snapshot -i` → click the explicit "Send message" ref; re-snapshot after typing). Screenshots are primary evidence; record WebM and finalize with `ffmpeg -c copy`. - **PR 1:** (1) start a long stream (prompt that streams ~30 s), wait 3–5 s, `kill -TERM <server pid>`; transcript must show the order in STOP coder#1 and `[shutdown] AppFiberScope closed { ms }`; (2) `ls <XUM_ROOT>/sessions/<ws>/partial.json` (absent) + `tail -n 1 chat.jsonl` (interrupted assistant message); (3) restart, open the workspace in agent-browser, screenshot the persisted interrupted message; (4) same scenario on `main` for the baseline diff; (5) `xum run` Ctrl-C mid-stream transcript (CLI root parity); (6) quality gate between phases: gate suites green before the sandbox run, sandbox evidence before requesting review. - **PR 2:** primary evidence is the regression file run on both the merge-base worktree (failing output) and the branch (passing). Secondary: sandbox parent workspace delegates via `task` kind=workspace to a child that arms a background bash monitor firing within ~10 s and keeps working ~60 s; screenshot the parent's task result (baseline `main`: `interrupted … uncorrelated workspace stream-end`; after: `completed`) and the child's log lines. - **PR 3:** `xum server` cold start transcript with `[startup] <step> { ms }` for the six steps (parity); throwaway worktree build with `STARTUP_STEP_TIMEOUT_MS = 1` → transcript of `Startup failed: StartupStepTimeoutError …` and exit code (do not ship); desktop dialog cannot be shown headless — cite the unchanged `desktop/main.ts:1249–1265` catch. - **PR 4:** agent-browser session left idle 60 s with the connection indicator visible (heartbeats keep it green) + screenshot; `streamBridge.test.ts` on TestClock. ## 8. Non-goals (restated; out of this wave) Typed-error propagation sweep / removing the ~113 facades; converting services to `yield*`-based Effect services; PubSub for the internal EventEmitter bus; Schema at persistence boundaries; Effect observability; converting sync read paths, AI-SDK per-request callbacks, cross-process lock interiors, or deterministic try-lock funnels; replacing `AbortController` as the SDK cancellation transport; converting the `fullStream` loop to `Stream`; fiberizing turn-handle waiters; downgrading startup steps to best-effort; changing outer quit budgets; supervising the pre-registration stream-start window; `shutdown()` semantics; the effect GA bump (standing analysis only). </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$0.00`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=0.00 -->
yermakoffivan
pushed a commit
to yermakoffivan/mux
that referenced
this pull request
Sep 6, 2026
…er#4096) ## Summary Make workspace-turn terminal transitions name an internal, validated cause, and pin the synthetic-wake false-settlement regression fixed by #3949. **No lifecycle-policy, persisted schema, wire, liveness, or Effect-fiber changes.** - Cover all 13 central settlement call sites plus the independent explicit-interrupt path with 14 cause kinds. The same synchronous validator runs inside each existing lock before terminal persistence; explicit stop retains its epoch/latch/mirror ordering. - Carry the actual superseding input ID for manual supersession. Preserve conservative interruption on unreadable history, missing stream-end, missing anchor, or invalid persisted manual-input ID under a separate cause with an explicit reason; no fabricated manual evidence or extra history reads. - Add 19 real-manager/real-store/real-HistoryService regression cases covering pending waiters, disposable cleanup, terminal attention, continuation, malformed causes, and explicit-stop ordering. Reuse the existing test harness without changing existing assertions. ## Evidence - **Exact pre-fix main `3937abd4e96ab7475f5f702bd40118016f83e5c4`:** behavior-only reproduction compiles and fails at the `running` assertion: actual `interrupted`, “Workspace turn superseded by an uncorrelated workspace stream-end.” The new cause assertions are not involved. - **PR2:** 791 tests pass across the manager, TaskService, TaskHandleStore, task tools, and workspace-turn inheritance suites; `make static-check` and a fresh `make build` pass on Bun 1.3.5. Detailed logs, reproduction, and isolated live evidence are in the first comment. **Live dogfood limitation:** two fresh isolated parent/child monitor attempts failed the requested waiting-through-final scenario: the parent received an early **correlated** `finishReason: "tool-calls"` error. With a retained child, the 60-second script then exited successfully and the child completed with the same turn correlation; the handle self-healed to completed after the parent had already failed. No uncorrelated assistant end or new cause assertion failure occurred. The untouched monitor-withdrawal/queue-cut continuation path is documented with screenshots/video in the first comment; per the addendum, this PR does not broaden into that lifecycle-policy fix. **Live success is not claimed.** ## Scope / risk The Sep 5 addendum supersedes the original single-chokepoint/manual-only/11-caller assumptions. Production is **+104 net lines** rather than the stale ~40 estimate. The manager owns no AI emitter routing: its regression fixture uses actual creation/admission, the established AI fake with correlated stream state, and the manager's direct event entry point. Existing private-settlement tests receive causes only; all existing assertions remain intact. The validator is an internal developer contract, not a new authorization or recovery policy. Arbitrary uncorrelated synthetic output is never authoritative completion; readable anchored history without manual supersession stays nonterminal. Unknown-history conservative interruption remains intentional. #3915 remains a soft dependency; the two-week live-host observation and post-wave N20 main-run check remain follow-up signals, not claims established by this PR. --- <details> <summary>📋 Accepted implementation plan (including superseding Sep 5 addendum)</summary> # Effect migration — Wave 4: finish the concurrency/lifecycle core Bounded wave: **4 PRs (PR 4 optional), explicit STOP criterion, explicit OFF-RAMPs.** Plan only; nothing here is implemented. > **Review status:** Independently reviewed (adversarial Reviewer sub-agent, advisor unavailable): APPROVE WITH REQUIRED EDITS — both edits applied; verified claims: Effect.promise 0-arity thunk allocates no AbortController (internal/effect.js:741–776); closed-scope forkIn+startImmediately runs onInterrupt (2237–2274, 391–409); forkIn observer removes the scope finalizer on exit (2270–2271); Effect.timeoutOrElse exists (Effect.d.ts:7833); 0 line drift at b87f62729; 11 settleWorkspaceTurn callers confirmed; 'aborted' ∈ NON_RETRYABLE_STREAM_ERRORS. Line references are to `main` @ `b87f62729`. ## 0. Thesis check (coordinator's judgment vs. evidence) **Thesis:** Effect's payoff in this app is structured concurrency + interruption-safe lifecycles in the orchestration core (still Promise + AbortController). **Verdict: holds for the stream engine; only half-holds for turn handles.** - Stream engine — **holds.** `ServiceContainer.dispose()` never stops or awaits in-flight streams (`serviceContainer.ts:478–537` has no `streamManager` step); an in-flight stream dies with the process and is recovered on next load from `partial.json` (≤ 500 ms stale, `PARTIAL_WRITE_THROTTLE_MS`, `streamManager.ts:776`). `AppFiberScope` exists precisely for this and has no occupant. A supervised per-stream fiber is the right tool. - Turn handles — **half-holds.** The 7× "superseded by an uncorrelated workspace stream-end" false-settle is a _correlation-predicate_ bug (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `workspaceTurnManager.ts:4220–4307`: any uncorrelated stream-end after the prompt index settles the handle `interrupted`), not a Promise-vs-fiber structure bug. Turn handles are **persisted records** (`taskHandleStore.upsertWorkspaceTurn`) spanning **multiple streams** (tool-call continuations are deferred via `hasSameTurnContinuation`, `:4491`) and surviving restarts; a fiber/Deferred can only model the in-process waiter and would not fix correlation. Open PR **#3949** fixes the predicate in Promise idiom and is Codex-green. Wave 4's turn-handle PR therefore becomes **"codify the settlement invariant + prove the class is gone"**, not "fiberize handles" (D5 below). Corrected baseline numbers (measured this workspace): 46/470 `src/node` non-test files import `effect` (coordinator said 35); 113 direct `Effect.run*` sites outside `di/` in 16 files; 226 `Effect.gen`; 9 `TaggedError` classes; effect `4.0.0-rc.112`, `@orpc/*` `1.14.11`; **effect v4 is not GA** (rc line still current). ## 1. Verified current state (evidence the design rests on) <details> <summary>Stream engine (streamManager.ts)</summary> - `startStream` (`:4723–4901`): per-workspace mutex → `new AbortController()` + `linkAbortSignal` (`:4771–4772`) → `resourceScope = Scope.makeUnsafe()` (`:4777`) → temp-dir `Effect.acquireRelease` (`:4802–4824`) → `createStreamAtomically` → `streamText` (`:2244`, `abortSignal: abortController.signal` `:2250`) → registered in `workspaceStreams` (`:2463`) → **`streamInfo.processingPromise = this.processStreamWithCleanup(...)` fire-and-forget (`:4876–4882`)** → returns `Ok({ messageId, completion })`. - `processStreamWithCleanup` (`:3331–4089`, plain async): `while(true)` retry loop; `for await (part of fullStream)` (`:3358–3837`) with abort check at loop head (`:3361`); post-loop `if (!signal.aborted)` gate (`:3849`) → completion path (`deletePartial` `:3981`, `updateHistory` `:3989`, `recordSessionUsage` `:4001`, `state = COMPLETED` `:4017`, emit `stream-end` `:4023`, `terminalCompletion` `:4024`); error path → `handleStreamFailure` (`:4094–4112`) → `persistStreamError` writes error partial; `finally` (`:4052–4088`): release MCP lease, `Effect.runFork(Scope.close(resourceScope))` (`:4064–4066`), unlink abort, `workspaceStreams.delete`, `eventSpine.emit("stream.end")`, `completionController.settle`. - Cancellation: `stopStream` (`:5043–5111`) → `cancelStreamSafely` (`:1766–1800`): `if (state === COMPLETED) { await processingPromise; return }` → `state = STOPPING` → `flushPartialWrite` → `abortController.abort()` → `cleanupAbortedStream` (`:1828–1951`): `await processingPromise` → usage → `writePartial` (`:1876–1910`) → `emitStreamAbort` → `settle({status:"aborted"})`. **No completed-guard after the await** (verified `:1838–1951`): a cancel landing between `:3849` and `:4017` re-writes `partial.json` after `deletePartial` and emits `stream-abort` after `stream-end` (pre-existing window; dispose() will widen its exposure). `cancelStreamSafely` is also not idempotent for concurrent callers (only `COMPLETED` is checked). - AIService on `stream-abort` (`aiService.ts:355–377`): `abandonPartial ? deletePartial : commitPartial → deletePartial` (fire-and-forget listener). - Crash recovery: `HistoryService.commitPartial` (`historyService.ts:1963–2061`) — strips error metadata, `hasCommitWorthyParts`, stale-epoch check, update-or-append by `historySequence`, delete partial; invoked from `agentSession.init` (`:5002`), `aiService.streamMessage` (`:886`), stream-abort (`:364`), `duplicateWorkspace`. - `StreamAbortReason = "user" | "startup" | "system"` (`src/common/orpc/schemas/stream.ts:295`). - Pinned seams: chaos test `Reflect.set(streamManager, "tokenTracker" | "createStreamResult")` (`streamManager.chaos.test.ts:130–134, 238–242`); `streamManager.test.ts` `Reflect.set` on `processStreamWithCleanup` (`:2787`), `createStreamAtomically` (`:2783`), `createTempDirForStream`, `cleanupStreamTempDir`, `Reflect.get` on `workspaceStreams`, `schedulePartialWrite`, …; `modelOnlyNotifications.test.ts` calls `processStreamWithCleanup` directly (`:93, :187`); `aiService.test.ts` spies `startStream`, `generateStreamToken`, `createTempDirForStream`, `isResponseIdLost`. Constructor: `(historyService, sessionUsageService?, getProvidersConfig?, eventSink = noop, runner = defaultEffectRunner)` (`:801–813`); `effectRunner` used at `:1152, :1154, :1169` only. - Every stream event carries `workspaceId` + `messageId`; `stream-end`/`stream-abort`/`error` carry `metadata.muxMetadata` when the prompt had it. </details> <details> <summary>DI / shutdown / startup</summary> - `AppFiberScopeLive` is in `CoreLive`'s `runtimeSeams` (`di/layers/core.ts:644`), so both roots have it; `StreamManagerLive` (`core.ts:226–238`, stage S2b) already yields `EffectRunnerTag`. CLI cleanup lists include `appFiberScope.close` (`cli/run.ts:1579`, `cli/workflow.ts:286`). - Bounds: `APP_FIBER_SCOPE_CLOSE_TIMEOUT_MS = 2000`, `APP_RUNTIME_DISPOSE_TIMEOUT_MS = 2000`; outer budgets are **5000 ms** on both desktop (`desktop/main.ts:1297` `Promise.race` vs `setTimeout(5000)`) and `xum server` (`cli/server.ts:236–243` force-exit). **The scope bound cannot grow** without changing outer budgets. - rc.112 semantics verified in `node_modules/effect/dist/internal/effect.js:2264`: `forkIn` registers a scope finalizer and **removes it when the fiber completes** (no leak), and **interrupts immediately if the scope is already closed** (streams starting mid-shutdown fail closed). `Effect.promise(evaluate: (signal) => PromiseLike)`, `Effect.onInterrupt`, `Effect.forkIn(_, scope, { startImmediately? })`, `Stream.toAsyncIterableWith(context)`, `Stream.provideContext` all exist. - `ServiceContainer.initialize()` (`serviceContainer.ts:297–362`): six awaited `initialize()`s wrapped in `recordStep` (durations only, no catch, no timeout) + three sync `start()`s + two fire-and-forget sweeps. Failure handling: desktop `Startup Failed` dialog + `app.quit()` (`desktop/main.ts:1249–1265`); `cli/server.ts:136` uncontained; ACP `serverConnection.ts:205–216` dispose + rethrow; `tests/ipc/setup.ts:85` no catch. No outer timeout anywhere. - `streamBridge.subscriptionIterable` (`orpc/streamBridge.ts:176`) → `Stream.toAsyncIterable(...)` on the global runtime, 19 call sites in `routerSubscriptions.ts`; heartbeat via `Effect.sleep` in `forkScoped` (`:145–152`). `streamBridge.test.ts` has 11 real-time waits, but only **3 are clock-bound** (`:207` 1 ms initial delay, `:241` heartbeat 10 ms, `:255` 10 ms laziness); 8 are `waitFor(listenerCount…)` readiness polls that TestClock cannot replace. </details> <details> <summary>Turn handles + open PRs</summary> - Handle record `{ handleId "wst_…", ownerWorkspaceId, workspaceId, turnId, messageId, status, attentionPolicy, disposableWorkspace }`; prompt carries `muxMetadata: { type:"workspace-turn-task", taskHandleId, ownerWorkspaceId, turnId }` (`workspaceTurnManager.ts:1420`). `TaskService` forwards `aiService` `stream-end`/`stream-abort`/`error` to `finalizeWorkspaceTurnFromStreamEnd` (`:4442–4544`): correlated branch matches `record.workspaceId && record.turnId` (`:4472`); **uncorrelated branch** (`metadata == null`, not `agentId === "compact"`) → `interruptWorkspaceTurnFromUncorrelatedStreamEnd` → settles `interrupted` whenever `streamEndIndex >= promptIndex` (`:4293–4305`). Producers of such uncorrelated ends: bash-monitor wake continuations, child terminal-attention deliveries, heartbeat, peer messages, parent auto-resume. - Cascade: disposable child → `cleanupDisposableWorkspaceTurn` → `workspaceService.remove(…, true)` kills its background processes; persistent child → parent sees `interrupted` → `task_stop` → `backgroundProcessManager.stopMonitor(…, "canceled")`. This is the observed "monitors died afterwards". - Settlement chokepoint: `settleWorkspaceTurn(params)` (`:2085`), **11 callers**, guarded by `workspaceTurnSettlementLocks.withLock(handleId)`; waiters in `pendingWorkspaceTurnWaitersByHandleId` with `setTimeout` timeouts (`:2425–2496`). - **#3949** "preserve turns across synthetic wake ends" (coadler): rewrites the uncorrelated branch — walks history from the turn anchor to the stream-end and settles _only if a manual child input intervened_ (`isManualChildWorkspaceInput`); otherwise ignores the end. Touches `:281–295, :4217–4355` + tests (+286/−31). Codex: "Didn't find any major issues" + clean security on `f9baa2fc9`. `mergeable: MERGEABLE`, but `Test / Unit` and `Codex Comments` red, 19 commits behind main. - **#3915** "correlate workspace-turn liveness" (coadler): creation reservations + `getWorkspaceTurnLiveness`/`getWorkspaceTurnRuntimeActivity` (identity-matches the active stream's `muxMetadata` against the record) for staleness/capacity. Touches `:442–486, :1298, :2573, :3761, :3800–4064` (+494/−60). `BLOCKED`, latest Codex review has open findings, `Test / Unit` red, 19 behind. - Together they are the identity-correlated model the coordinator wants: #3915 = identity-correlated _liveness_, #3949 = identity-gated _settlement_. </details> ## 2. Design decisions **D1 — Fibers WRAP the AbortController; they do not replace it.** The AI SDK is cancelled only via `AbortSignal`; the `for await` loop, soft-interrupt at step boundaries, retry/fallback re-creation of `streamResult`, and ~30 abort touchpoints (#4032) all key off the signal. Converting the 750-line loop to `Stream.fromAsyncIterable` + fiber interruption would touch hundreds of `WorkspaceStreamInfo` transitions and break the `processStreamWithCleanup`/`createStreamResult` spy seams. Instead: **the fiber is the ownership/supervision unit; the signal stays the cancellation transport.** The dual-cancellation glue #4032 feared is confined to **one** point — the supervisor's `onInterrupt` — which routes through the existing user-stop path (`cancelStreamSafely`), so shutdown ≡ "user pressed stop" semantically (partial flushed with usage, `stream-abort` emitted, `completion` settles `aborted`, AIService commits the partial). **D2 — Supervisor topology: one supervisor fiber per stream in `AppFiberScope`, wrapping the already-started `processingPromise`.** `streamInfo.processingPromise = this.processStreamWithCleanup(...)` stays byte-identical (sync-start preserved; `Reflect.set(processStreamWithCleanup)` seam preserved; `cleanupAbortedStream`'s `await processingPromise` unchanged). Immediately after it: ```ts // startStream, after processingPromise is assigned (unsupervised path unchanged when no scope) this.superviseEngine(typedWorkspaceId, streamInfo); private superviseEngine(workspaceId: WorkspaceId, streamInfo: WorkspaceStreamInfo): void { if (this.engineScope === undefined) return; // direct construction / CLI tests: today's behavior assert(streamInfo.engineFiber === undefined, "engine already supervised"); // Zero-arity thunk on purpose: rc.112 allocates an internal AbortController only // when `evaluate.length !== 0`; the stream's own controller stays the sole signal. const supervisor = Effect.promise(() => streamInfo.processingPromise).pipe( Effect.onInterrupt(() => Effect.uninterruptible( // explicit, per house doctrine (finalizers are already uninterruptible) Effect.promise(async () => this.cancelStreamSafely(workspaceId, streamInfo, "system")) ) ), Effect.catchDefect((d) => Effect.sync(() => log.warn("[stream] engine supervisor defect", { workspaceId, error: d }))) ); streamInfo.engineFiber = this.effectRunner.runSync( Effect.forkIn(supervisor, this.engineScope, { startImmediately: true }) ); } ``` - `Effect.promise` is interruptible while suspended (`internal/effect.js:741–801`, Async op); `onInterrupt` = `onErrorFilter(causeFilterInterruptors, …)` (`:1762`); `forkIn` registers `fiberInterrupt(fiber)` as the scope finalizer (`:2264–2275`), `fiberInterrupt` awaits the fiber (`:635–642`), and parallel `scopeClose` awaits all finalizers via `fiberAwaitAll` (`:1590–1601`) → `closeScopeBounded` at dispose step 2 gives "interrupt **and** await" while `historyService`/`sessionUsage`/`eventSink → AIService → bridge servers` are still alive (bridges stop in step 3, so clients receive `stream-abort`). - Normal completion: fiber exits → `forkIn`'s observer removes the scope finalizer (verified) → no per-stream residue. - Stream started after step 2: `forkIn` on a closed scope calls `fiber.interruptUnsafe` synchronously and returns the fiber (`:2272–2274`, `runSync` does not defect). With `startImmediately: true`, `forkUnsafe` runs `child.evaluate` synchronously (`:2233–2247`) up to the `Effect.promise` Async op (`:772–801`), so the fiber is suspended (`_running=false`) when the interrupt lands and `interruptUnsafe` (`:391–409`) unwinds the stack through the `onInterrupt` handler → the stream is aborted as `system` (fail-closed during shutdown). Verified in rc.112 internals (`effect.js:2233–2247, 391–409`); **pin with a test** ("stream started after scope close is aborted") so an RC bump cannot silently change it. - `"system"` is semantically exact: `"user"`/`"startup"` suppress next-startup recovery (`retryEligibility.ts:114–118, 284–287`), `"system"` marks an involuntary backend interruption (as `taskService.ts:8100, 8223` use it). **No in-session retry loop is possible:** the `stream-abort` handler (`agentSession.ts:6010`) routes `{ type: "aborted" }` to `retryManager.handleStreamFailure`, and `"aborted"` is in `NON_RETRYABLE_STREAM_ERRORS` (`retryEligibility.ts:49–59, 106`) → `retryManager.ts:99–104` abandons immediately, never schedules a fiber. Dogfooding still checks the _restart_ UX (the recovered partial is shown as interrupted; note whether any next-startup recovery re-sends — same class as today's `system` aborts from `taskService`). - `engineScope` arrives as an **optional 6th constructor parameter** (`engineScope?: Scope.Closeable`), wired from `AppFiberScopeTag` in `StreamManagerLive` (`core.ts:226`). Default `undefined` keeps every direct-construction test and `aiService.ts:174` path identical (I4). `AppFiberScopeLive` already sits beneath S2b in `runtimeSeams`, so no staging change (I6). - Abort reason: reuse **`"system"`** — no wire/schema change; UI copy for `system` already exists. - Pending-start window (`pendingStreamStarts`, before registration) is **not** supervised: nothing is persisted for it yet, and `stopStream` already aborts pending controllers. Documented, not fixed. **D3 — Fix the two adjacent cancel races in the same PR (closely-related bugs, not deferrals).** (a) `cleanupAbortedStream`: after `await processingPromise`, if `streamInfo.terminalCompletion !== undefined` (completed/failed while the cancel was in flight) → return without abort bookkeeping (prevents `partial.json` resurrection after `deletePartial` and a `stream-abort` after `stream-end`). (b) `cancelStreamSafely` (`:1766`): latch a per-stream `cancelPromise` so concurrent cancellers (user stop racing dispose) join one cleanup → exactly one `stream-abort`, one `settle`. **Zero-suspension requirement:** the latch must be checked and assigned **synchronously at function entry, before any `await`** (the current first await is `flushPartialWrite` at `:1789`) — otherwise racing callers can both enter `cleanupAbortedStream`. Shape: ```ts if (streamInfo.cancelPromise) return streamInfo.cancelPromise; streamInfo.cancelPromise = (async () => { /* existing body, unchanged */ })(); return streamInfo.cancelPromise; ``` Both are ≤ 10 LoC and get behavioral tests. **D4 — Shutdown bound stays 2 s; the finalizer must be fast or abandoned.** Outer budgets are 5 s; 2 s + 2 s already consume 4 s. A flowing stream aborts within one chunk; a wedged provider (no chunks, ignores abort) hits the existing `boundedTeardown` timeout: warning, continue, process exit — identical to today's outcome. Dogfooding measures the actual `[shutdown] AppFiberScope closed { ms }` with a live stream. **D5 — Turn handles: codify the settlement invariant; do not fiberize.** Invariant: _a workspace-turn handle settles terminally only by (i) a stream terminal event whose `muxMetadata` correlates `{taskHandleId, ownerWorkspaceId, turnId}` to the record; (ii) an explicit interrupt (`task_stop`/`interruptWorkspaceTurn`); (iii) manual supersession — a manual child input after the turn anchor; (iv) stale-liveness reconciliation._ An uncorrelated stream-end is **never** terminal by itself. #3949 makes (iii) the only uncorrelated outcome; #3915 implements (iv) by identity. Wave 4 adds a `cause` discriminant to `settleWorkspaceTurn` (the single chokepoint) with a runtime assertion, plus the regression harness. Rationale for not converting waiters to `Deferred`/fibers: no behavioral gain, 4.9k-line file, and the coordinator's "settle only on the owning stream's termination" is over-specified — a turn owns _several_ streams. **D6 — Startup: `initialize()` stays a Promise facade over a runtime-run startup effect; timeout ⇒ same failure path as a thrown step.** Each step is `Effect.tryPromise({ try: async () => step(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, ms)) }))` (`timeoutOrElse` exists in rc.112, `Effect.d.ts:7833`; chosen over `timeout` + `catchTag` because the step's error channel is `unknown`, which `catchTag` cannot narrow). No `forkDetach` needed: a Promise step keeps running on its own when the waiting fiber times out (not inside an uninterruptible region, so the timeout interrupts the wait directly). `StartupStepTimeoutError extends Error` with `name = "StartupStepTimeoutError"` set in the constructor and message `"<step> exceeded <ms> ms"` (so the desktop dialog's error formatting shows both the class and the step name) → desktop shows it in the existing `Startup Failed` dialog; CLI/ACP/tests paths unchanged. Step **errors keep their identity** (v4 `runPromise` rejects with the raw failure). Downgrading any step to best-effort is a **policy change, out of scope** (audit of the six implementations: extensionMetadata/telemetry/experiments are local fs, <50 ms; policy has its own 10 s fetch timeout; workspaceService bounds its sync internally; only `taskService.initialize` — config scan + `editConfig` + recovery `sendMessage`s — is potentially unbounded). The three `start()`s stay sync (`Effect.sync`), the two fire-and-forget sweeps stay outside the effect. `stepDurationsMs` is preserved. **Abandon-and-quit safety:** an abandoned `taskService.initialize` may be mid-`editConfig` when the root exits. Parity requirement for PR 3: after a rejected `initialize()`, every root runs the bounded `dispose()` before exiting. Verified: desktop already does — `services` is assigned before the await (`main.ts:653–656`), the catch calls `app.quit()`, and the `before-quit` listener (`:1271–1305`, guard `if (isDisposing || !services) return`) races `services.dispose()` against 5 s; ACP does (`serverConnection.ts:205–216`); **`cli/server.ts` does not** (`:133–136` awaited at top level, `main().catch` at `:282` only logs) → PR 3 adds a bounded `dispose()` there (≤ 10 LoC, same 5 s budget). **D7 — streamBridge: thread the runtime context, not a runner.** `subscriptionIterable` gains `context?: Context.Context<never>` → `Stream.toAsyncIterableWith(context)`; `routerSubscriptions` passes the handler's `"effect/context"`. Production behavior identical; heartbeat sleeps on the runtime `Clock`; tests can run the 3 clock-bound waits on `TestClock`. Honest scope: the 8 readiness polls stay. ## 3. PRs (ordered by value ÷ risk; each independently mergeable) ### PR 1 — StreamManager engine core becomes the first `AppFiberScope` occupant **Value:** high (the only remaining shutdown data-integrity gap; the reason `AppFiberScope` exists). **Risk:** medium → low with D1/D2. **Net product LoC ≈ +55** (`superviseEngine` ~25, ctor param/field ~5, `engineFiber` field ~2, D3 guards ~12, `core.ts` wiring ~2, doc updates in `appRuntime.ts`/`appFiberScope.ts` "occupant" text ~10). Files: `src/node/services/streamManager.ts`, `src/node/services/di/layers/core.ts`, `src/node/services/di/appRuntime.ts` + `appFiberScope.ts` (docs), tests below. Pre-work (before writing product code; each yields a note in the PR body): 1. Confirm `processStreamWithCleanup` never rejects (try/catch/finally shape `:3331–4089`); else the supervisor must fold rejections (it already `catchDefect`s). 2. Confirm `Effect.promise` interruption + `onInterrupt` await ordering under `Scope.close` in a 20-line probe test (pattern of `appFiberScope.test.ts:27–47`). 3. Enumerate abort observers that run _after_ the finalizer resolves (AIService `stream-abort` listener → `commitPartial`; agentSession completion continuations) and confirm the durable order (`writePartial` → commit → `deletePartial`) makes a mid-flight `process.exit` recoverable on next load (it is: partial survives until commit completes). 4. Measure: `[shutdown] AppFiberScope closed { ms }` with a live stream in the sandbox (D4). 5. Pin (same probe test): a fiber forked with `startImmediately: true` into an already-closed scope still runs its `onInterrupt` finalizer (reviewer-verified in rc.112 internals; the test guards RC bumps). Acceptance (behavioral tests only): - `streamManager.test.ts` (new cases; existing cases untouched): with `engineScope = Scope.makeUnsafe("parallel")` and a fake `createStreamResult` whose `fullStream` yields one `text-delta` then blocks until its `AbortSignal` fires — `closeScopeBounded(engineScope)` resolves; `writePartial` was called with the streamed text; exactly one `stream-abort` (`abortReason: "system"`) and zero `stream-end`; `completion` settles `{status:"aborted"}`; `workspaceStreams` is empty. - Wedged provider (fullStream never yields, ignores abort): `closeScopeBounded` resolves within the bound, never rejects, warns once (assert the returned promise resolves and no throw; do **not** assert log text). - No-scope construction: identical event sequence to today (guards existing suites; no new assertions needed beyond the unchanged suites passing). - D3(a): cancel issued after the loop exits but before `COMPLETED` → history has exactly one final message, `partial.json` absent, event order `stream-end` only. - D3(b): `stopStream` + `closeScopeBounded` racing on one stream → exactly one `stream-abort`, one settle. - Fiber residue: after 50 completed streams, `closeScopeBounded(engineScope)` emits zero `stream-abort` and completes in the same tick class as an empty scope (assert no aborts and `workspaceStreams.size === 0`). - `streamManager.chaos.test.ts` — existing cases byte-identical; **one new fuzz variant** constructs with an engine scope and closes it at a random iteration: every stream settles **exactly once** (count terminal events per `messageId` ≤ 1, all `completion` promises settle). - `serviceContainer.test.ts`: "dispose() aborts and awaits an in-flight stream before `desktopBridgeServer.stop()`" (extend the ordering harness at `:295–323`); `coreServicesRoot.test.ts`: `xum run` cleanup list does the same via `appFiberScope.close`. Gate suites: `streamManager.test.ts`, `streamManager.chaos.test.ts`, `streamManager.modelOnlyNotifications.test.ts`, `aiService.test.ts`, `agentSession.disposeRace.test.ts`, `agentSession.sinceReplayContract.test.ts`, `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `taskService.test.ts`, `workspaceService.test.ts`, `turnRequestBuilder.test.ts`; `make static-check`. House pre-review audits: interruption posture (supervisor's only suspension is the promise; finalizer uninterruptible end-to-end incl. `cancelStreamSafely` → `cleanupAbortedStream`); no defect escapes (`catchDefect` on the supervisor; `Effect.promise` thunks `async`); spy-seam check (`processStreamWithCleanup`, `createStreamResult`, `createStreamAtomically`, `startStream` signatures unchanged; constructor arity unchanged, trailing optional); sync-start (`processingPromise` assigned before fork; `runSync(forkIn)` completes synchronously); no constructor side-effects added; zero-suspension check on D3(b) latch (`cancelPromise` checked-and-assigned synchronously at `cancelStreamSafely` entry, before the first `await` at `:1789`; review the diff for any inserted `await`/lookup ahead of the assignment). Rollback: revert the `core.ts` wiring line → `engineScope` undefined → today's behavior; D3 guards can stay (independent bug fixes). ### PR 2 — Turn-settlement invariant + false-settle regression harness (gated on #3949) **Value:** high (7× production race). **Risk:** low. **Net product LoC ≈ +40** (`WorkspaceTurnSettlementCause` union + `cause` on `settleWorkspaceTurn` params + assert ~10; 11 call sites × 1–3 lines). Relationship to open PRs — explicit: - **#3949 is the fix and a hard prerequisite.** PR 2 rebases on it, changes none of its logic (`interruptWorkspaceTurnFromUncorrelatedStreamEnd`, `isWorkspaceTurnAnchorForRecord`, `isManualChildWorkspaceInput`), and adds the invariant + proof on top. If #3949 has not merged when PRs 1/3 are done: **do not fork a competing fix**; report to the coordinator, offer the regression test file to #3949's author as a review artifact, and hold PR 2 (it is not on any other PR's critical path). - **#3915 is a soft prerequisite.** Its diff (`:3800–4064` incl. `settleStaleWorkspaceTurn`, a `settleWorkspaceTurn` caller) overlaps PR 2's one-line-per-caller change. Prefer landing after it; if PR 2 must go first, the conflict is a one-line `cause:` addition per caller. PR 2 never edits liveness/reservation code. - Wave 4 does not otherwise touch `workspaceTurnManager.ts`. Design: `type WorkspaceTurnSettlementCause` enumerated from the 11 callers (audited at `main` @ `b87f62729`): `:1373` creation validation failure; `:1476` pre-stream interrupt during launch; `:1500`/`:1518` pre-stream send failure; `:3834`/`:3863` stale-liveness recovery / restart timeout (`settleStaleWorkspaceTurn` — #3915's region); `:4301` **uncorrelated-stream-end manual supersession** (the only uncorrelated settle in the codebase; the path #3949 rewrites); `:4529` correlated terminal; `:4571` stream-abort; `:4675` deferred stream error; `:4736` terminal stream error. `settleWorkspaceTurn` asserts `params.cause` is a member and, for `manual-supersession`, that the superseding input's `messageId` is supplied — turning D5 into an exhaustive `Record<Cause, …>` check rather than prose, so a future "settle on uncorrelated end" cannot be added without naming (and justifying) a cause. Acceptance: - New `workspaceTurnManager.uncorrelatedStreamEnd.test.ts` (real `WorkspaceTurnManager` + `TaskHandleStore` + fake `aiService` emitter, following the existing suite's harness): (1) create turn → correlated `stream-start` → **synthetic wake stream** on the same child ends uncorrelated after the anchor → handle stays `running`, waiter unresolved, no disposable cleanup, no terminal attention → correlated `stream-end` → `completed`. (2) same with `finishReason:"tool-calls"` continuation in between. (3) manual child input between anchor and end → `interrupted` with `cause: manual-supersession`. (4) explicit `interruptWorkspaceTurn` → `interrupted`, `cause: explicit-interrupt`. Case (1) is the **scripted reproduction**: it must **fail on the pre-#3949 merge-base** (run the file from a sibling worktree at `git merge-base origin/main <#3949 head>`; record the failing assertion in the PR body) and pass after. - Existing 113 `workspaceTurnManager.test.ts` cases and `taskService.test.ts` turn cases unchanged. Gate suites: `workspaceTurnManager.test.ts`, `taskService.test.ts`, `taskHandleStore.test.ts`, `tools/task*.test.ts`; `make static-check`. Audits: spy-seam (`getWorkspaceTurn`, `listAllWorkspaceTurns`, `enqueueTerminalAttention`, `deliverPersistentChildWorkspaceTurnResult` untouched); settlement lock held across the assert; no new suspension inside `withLock`. Rollback: revert; the test file stays valid against #3949 alone (drop the `cause` assertions). ### PR 3 — `ServiceContainer.initialize()` as a runtime-run startup effect with per-step timeouts **Value:** medium (a hung `taskService.initialize()` currently pins the splash screen forever; deterministic TestClock tests of startup). **Risk:** low–medium. **Net product LoC ≈ +80** (step table ~20, timed-step helper ~15, `StartupStepTimeoutError` ~8, constant ~3, facade ~10, root dispose-on-failure parity ≤ 10, doc update ~10). Files: `serviceContainer.ts`, `src/constants/terminationTimeouts.ts` (keep with the termination constants so the budget doc stays in one place), `cli/server.ts` (dispose in the startup catch if missing), `di/appRuntime.ts` doc ("Deliberately not done" → remove the initialize() line; add startup contract). Design (D6): `initialize(): Promise<void>` → `this.runtime.managed.runPromise(this.startupEffect())`. `startupEffect = Effect.gen` over an ordered `readonly steps: ReadonlyArray<{ name, run: () => Promise<void> }>` (assert names unique); each step: `recordStep` timing kept, `Effect.tryPromise({ try: async () => run(), catch: identity }).pipe(Effect.timeoutOrElse({ duration: STARTUP_STEP_TIMEOUT_MS, orElse: () => Effect.fail(new StartupStepTimeoutError(name, STARTUP_STEP_TIMEOUT_MS)) }))`. Then `Effect.sync` for the three `start()`s; the sweeps remain after `runPromise`. Constant `STARTUP_STEP_TIMEOUT_MS` — pre-work measures `[startup] <step> { ms }` across sandbox cold starts and picks ≥ 10× the slowest observed (propose 60 s; must be generous — a false timeout turns a slow-but-fine start into a crash). Roots dispose after a rejected `initialize()` (D6 abandon-and-quit safety). Acceptance (all in `serviceContainer.test.ts`, TestClock via the existing `AppLive` spy at `:355–395`): - A step that never resolves → `initialize()` rejects with `StartupStepTimeoutError` naming the step after exactly `TestClock.adjust(STARTUP_STEP_TIMEOUT_MS)`; later steps did not run. - A rejecting step → `initialize()` rejects with **the same error object** (identity), later steps did not run (parity with today). - Happy path → `stepDurationsMs` has all six keys; `start()`s called once each; second `initialize()` call behavior unchanged from today (verify whether re-entry is guarded today; preserve). - `tests/ipc` harness and ACP entry still pass unchanged. Gate suites: `serviceContainer.test.ts`, `coreServicesRoot.test.ts`, `di/*.test.ts`, `src/node/acp/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (smoke subset); `make static-check`. Audits: I1 untouched (no layer body changes); I2 (only the composition root touches the runtime); error identity preserved (no wrapping); abandoned-step safety — every root runs bounded `dispose()` after a rejected `initialize()` (D6; `cli/server.ts` gains it in this PR); no sweep moved into the effect; the six-step order and `[startup] <step>` names unchanged. Rollback: revert; constant removal. ### PR 4 (optional — cut if budget is exhausted) — `streamBridge` on the runtime context **Value:** low (closes the last documented "global runtime" exception; enables TestClock for the heartbeat). **Risk:** low. **Net product LoC ≈ +30** (`context?` option + `toAsyncIterableWith` ~8; 19 call sites × 1 line via one shared helper in `routerSubscriptions.ts` ~3). Acceptance: the 3 clock-bound waits (`:207, :241, :255`) run on `TestClock`; the heartbeat test asserts N heartbeats after `TestClock.adjust(N × interval)` with zero real time; existing behavioral assertions unchanged; `tests/ipc` subscription tests pass. Do not rewrite the 8 readiness polls. Gate suites: `streamBridge.test.ts`, `routerSubscriptions*.test.ts`, `orpc/*.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc` (subscription subset); `make static-check`. Audits: `Stream.toAsyncIterableWith` preserves double-close safety (pin with the existing test); `Cause.Done` typing unchanged; no `Scope`/`MemoMap`/`Scheduler` captured (pass the oRPC `effect/context`, which the DI layer already strips per `EffectRunnerLive`); `context` stays optional so direct callers/tests without a runtime keep today's global-runtime path. Rollback: revert; the optional `context` default (`Context.empty()`) is exactly today's `toAsyncIterable`, so a partial revert of call sites is also safe. ### Execution order and size Net product LoC for the wave ≈ **+205** (PR 1 ≈ +55, PR 2 ≈ +40, PR 3 ≈ +80, PR 4 ≈ +30); tests ≈ +600–800. PR 1 starts immediately. PR 2 starts the moment #3949 merges (parallel with PR 1/3 — disjoint files). PR 3 after PR 1 merges (both touch `appRuntime.ts` docs; PR 3 also touches `serviceContainer.ts`). PR 4 last, only if PRs 1–3 landed and no OFF-RAMP fired. Each PR: Codex dual review, `Codex Comments` minimization, merge queue; commit WIP early (`/tmp` wipes). ## 4. STOP criterion (measurable) and OFF-RAMPs Wave 4 is **done** — and the Effect migration line **stops** without a new RFC — when all hold: 1. **dispose() awaits in-flight streams:** `serviceContainer.test.ts` ordering test + `coreServicesRoot.test.ts` pass on main; a sandbox `script -f` transcript of `xum server` receiving SIGTERM mid-stream shows `stream-abort` → `[shutdown] AppFiberScope closed { ms }` → `[shutdown] desktopBridgeServer.stop`, and immediately after exit `partial.json` is absent while `chat.jsonl` contains the interrupted assistant message (baseline on `main`: `partial.json` present, message absent until next load). `{ ms }` < 2000 in the flowing-stream case. 2. **False-settle class eliminated:** the scripted reproduction fails on the pre-#3949 merge-base and passes on main after PR 2; `settleWorkspaceTurn` rejects any settlement without an enumerated cause; the coordinator's own Mux sessions show zero "superseded by an uncorrelated workspace stream-end" in the two weeks after PR 2 (soft signal, logged in the wave summary). 3. **Startup:** timeout and error-identity tests pass under TestClock; `[startup]` per-step lines unchanged in the sandbox transcript; a throwaway build with the constant set to 1 ms shows `Startup failed: StartupStepTimeoutError: <step> exceeded 1 ms` and a clean exit. 4. **No new lifecycle flakes:** 0 failures attributable to the touched suites across **N = 20** consecutive _completed_ `Test / Unit` runs on `main` after the last Wave 4 merge — query `gh run list --workflow pr.yml --branch main --limit 60 --json databaseId,status,conclusion,event,headSha` (note: `gh run list --json` serializes these fields in **lowercase**, e.g. `{"status":"completed","conclusion":"success"}`, unlike `statusCheckRollup`), keep `status === "completed"` (pending runs have an empty `conclusion`, not null), take the newest 20, and for any run with `conclusion !== "success"` (case-insensitive normalization acceptable) inspect the failing job's log for the touched suite names (job `timeout`/`cancelled` from the 15-min budget is not a flake); plus green merge-queue runs for each PR. Any attributable flake → fix or revert before declaring done. **OFF-RAMP (PR 1):** fires if pre-work 1–3 shows (a) routing shutdown through `cancelStreamSafely` cannot preserve crash-recovery semantics without changing `cleanupAbortedStream`'s contract beyond D3, (b) the chaos variant exposes a double-settle not closable by D3(b), or (c) the finalizer cannot fit the 2 s bound for flowing streams. Then: stop PR 1, keep `AppFiberScope` unoccupied, update `appRuntime.ts` "Deliberately not done" with the concrete blocker and the measured evidence, land D3 alone as a bug-fix PR. PRs 2–4 are independent and proceed. **OFF-RAMP (PR 3):** if error identity or the `tests/ipc`/ACP paths cannot be preserved, keep `initialize()` as is and record why. **OFF-RAMP (PR 2):** #3949 not merged → hold (see PR 2). ## 5. Risk register | Risk | Likelihood | Mitigation | | ---------------------------------------------------------------------------------------------------- | --------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Crash-recovery regression: double commit / partial resurrection when shutdown-abort races completion | medium (window widens with dispose()) | D3(a) guard + test; `commitPartial`'s `historySequence` update-or-append is idempotent (`historyService.ts:2036–2041`) | | Provider abort emits an `error` chunk → error path instead of abort path | low | identical to today's user-stop path (parity); chaos variant covers hostile streams | | Wedged provider pins the 2 s bound → warning every shutdown | low | `boundedTeardown` already bounds; transcript measures; no budget change possible (5 s outer) | | AIService `stream-abort` listener (`commitPartial`) still in flight when `process.exit` runs | low | durable order writePartial → commit → deletePartial; next-load recovery; PR 1 transcript checks `partial.json` is already gone when `cli/server.ts` logs its final cleanup line before `process.exit(0)` (`:252–266`) | | Chaos-test seams (`createStreamResult`, `tokenTracker`) | none if scope-less construction stays default | new variant added, old cases untouched | | Collision with #3915/#3949 | medium | PR 2 gated; no edits to their regions; one-line `cause:` conflicts only | | RC churn (rc.113+ renames `forkIn`/`onInterrupt`/`toAsyncIterableWith`) | low | all Effect imports already in `streamManager.ts`/`streamBridge.ts`; pins fixed; GA upgrade is a separate lockstep PR (§6) | | Startup false timeout on slow hosts | medium if constant too small | measure first; ≥ 10× slowest observed; generous default (60 s) | | Sync-start assumptions in tests that `Reflect.set(processStreamWithCleanup)` | low | promise assigned before fork; forkIn `runSync` synchronous | | `shutdown()` (desktop second `before-quit` listener) still does not await streams | accepted | contract says `shutdown()` never touches the runtime; desktop's dispose race is the covered path | | Streams starting during shutdown | covered | `forkIn` on closed scope interrupts immediately → `system` abort (`startImmediately` semantics verified in rc.112; pinned by test) | | `system` abort triggers an in-session RetryManager retry during shutdown | **unreachable** (verified) | `"aborted"` ∈ `NON_RETRYABLE_STREAM_ERRORS` → `retryManager.ts:99–104` abandons; no fiber scheduled | | PR 3: abandoned `taskService.initialize` mid-`editConfig` when the root exits after a timeout | low | desktop/ACP already dispose on startup failure; PR 3 adds the missing `cli/server.ts` dispose; config writes are lock/journal-protected | | `startImmediately`/`onInterrupt` semantics differ in a later RC | low | pinned by the probe test in `appFiberScope.test.ts` style; RC bumps are a separate lockstep PR | ## 6. Standing item — effect v4 GA + `@orpc/experimental-effect` lockstep (analysis only) v4 is **not GA** (rc.112 is current; v3 `3.x` remains the stable line). No PR this wave. When GA ships: one lockstep PR bumping `effect` + all `@orpc/*` (`1.14.11` today; check the GA-compatible `@orpc/experimental-effect`), canary gates = `di/*.test.ts`, `streamBridge.test.ts`, `streamManager.test.ts`, `serviceContainer.test.ts`, `TEST_INTEGRATION=1 bun x jest tests/ipc`, `make static-check`. The `Context → ServiceMap` rename risk is firewalled: `Context.Service` tags, `Context.omit/get`, `Layer`, `ManagedRuntime`, `TestClock` live only under `di/` + `orpc/effectContext.ts`; `streamManager.ts`/`streamBridge.ts` use `Effect`/`Scope`/`Fiber`/`Exit`/`Stream`/`Queue`/`Cause` only. PR 4 adds one `Context.Context<never>` type reference to `streamBridge.ts` — keep it as a type-only import so a rename is a one-line fix. ## 7. Dogfooding (per PR; evidence attached to the PR with `gh … --attach`) Common setup: `make dev-server-sandbox DEV_SERVER_SANDBOX_ARGS="--clean-projects"` (or `xum server` on a temp `XUM_ROOT`) with `XUM_LOG_LEVEL=debug`, run under `script -f ~/wave4-scratch/<pr>-<scenario>.log`; scratch under `$HOME/wave4-scratch/` (never `/tmp`). Drive the UI with `agent-browser` (`open` → `snapshot -i` → click the explicit "Send message" ref; re-snapshot after typing). Screenshots are primary evidence; record WebM and finalize with `ffmpeg -c copy`. - **PR 1:** (1) start a long stream (prompt that streams ~30 s), wait 3–5 s, `kill -TERM <server pid>`; transcript must show the order in STOP #1 and `[shutdown] AppFiberScope closed { ms }`; (2) `ls <XUM_ROOT>/sessions/<ws>/partial.json` (absent) + `tail -n 1 chat.jsonl` (interrupted assistant message); (3) restart, open the workspace in agent-browser, screenshot the persisted interrupted message; (4) same scenario on `main` for the baseline diff; (5) `xum run` Ctrl-C mid-stream transcript (CLI root parity); (6) quality gate between phases: gate suites green before the sandbox run, sandbox evidence before requesting review. - **PR 2:** primary evidence is the regression file run on both the merge-base worktree (failing output) and the branch (passing). Secondary: sandbox parent workspace delegates via `task` kind=workspace to a child that arms a background bash monitor firing within ~10 s and keeps working ~60 s; screenshot the parent's task result (baseline `main`: `interrupted … uncorrelated workspace stream-end`; after: `completed`) and the child's log lines. - **PR 3:** `xum server` cold start transcript with `[startup] <step> { ms }` for the six steps (parity); throwaway worktree build with `STARTUP_STEP_TIMEOUT_MS = 1` → transcript of `Startup failed: StartupStepTimeoutError …` and exit code (do not ship); desktop dialog cannot be shown headless — cite the unchanged `desktop/main.ts:1249–1265` catch. - **PR 4:** agent-browser session left idle 60 s with the connection indicator visible (heartbeats keep it green) + screenshot; `streamBridge.test.ts` on TestClock. ## PR 2 implementation addendum — Sep 5, after #3949 merged This addendum supersedes stale factual assumptions in D5 / PR 2, not their minimal-change goal. Verified base: #3949 merged as `2f99c6a5b027cd09c7574f7af3eeba1cb804ec6a`; its pre-merge main parent is `3937abd4e96ab7475f5f702bd40118016f83e5c4`. #3915 remains open and is only a soft prerequisite. Proceed with PR 2 without changing its liveness algorithm. - **Preserve the actual #3949 fallback.** It still conservatively interrupts when history cannot be read or the stream-end/turn anchor cannot be found. Such a transition must have a distinct `uncorrelated-conservative-fallback` cause with a reason (history read failed, missing stream end, or missing turn anchor), never a fabricated manual-input ID. Proven `manual-supersession` requires the actual superseding input's message ID. Change only evidence plumbing through the existing helper; preserve read count, classification, stale-event behavior and outcomes. - **There are two settlement paths.** `settleWorkspaceTurn` is the central helper, but explicit `interruptWorkspaceTurn` also persists terminal state inside its own lock while establishing stop epoch/latch and mirror ordering. Use one small synchronous cause validator inside both existing lock boundaries. Do not route explicit interrupt through the central helper, nest locks, move stop-latch work, or add async work. Enumerate actual current call sites (including newer creation/admission/recovery failures), not the old count of 11. - **Internal contract only.** Causes and their evidence are internal parameters/assertions, not persisted task-handle fields, schemas, wire payloads or migrations. Use exhaustive typing plus runtime validation for missing/invalid causes; add behavioral negative coverage. Keep real HistoryService/TaskHandleStore and established test/spy seams. - **Honest invariant:** an uncorrelated synthetic end is never authoritative completion. With readable, anchored history and no proven manual supersession it remains nonterminal; explicit conservative fallback may interrupt when evidence cannot be established. Keep failure/abort/recovery and explicit-stop semantics unchanged. A concrete missing final-continuation correlation belongs at its dispatch source, not in arbitrary wake-output completion (the fresh #3949 review examined this and accepted the production correlation trace). - **Reproduction base:** the old `merge-base origin/main <PR head>` recipe is invalid after the prerequisite's additive main sync. Use the exact pre-#3949 main parent above. Run the behavior-only reproduction there (strip only PR2-specific cause assertions if needed); it must compile and fail on the intended premature terminal settlement. The complete harness must pass on the PR2 branch. Add fallback-reason tests and preserve explicit-interrupt side effects/order. - **Dogfooding:** retain §7 PR2's scripted regression evidence plus a freshly built isolated parent/child monitor-wake scenario, with screenshots/video. Do not assume this coordinator's currently running backend has upgraded just because main merged. Report the two-week live-session observation as a soft signal with version/exposure caveats, not as proof supplied by unit tests. Acceptance remains regression proof + explicit, validated settlement causes without lifecycle-policy changes. Record this reconciliation in the PR description. Advisor reviewed this approach on Sep 5 and recommended preserving the explicit-interrupt path and conservative fallback as separate named causes rather than forcing the old plan's inaccurate single-chokepoint claim. ## 8. Non-goals (restated; out of this wave) Typed-error propagation sweep / removing the ~113 facades; converting services to `yield*`-based Effect services; PubSub for the internal EventEmitter bus; Schema at persistence boundaries; Effect observability; converting sync read paths, AI-SDK per-request callbacks, cross-process lock interiors, or deterministic try-lock funnels; replacing `AbortController` as the SDK cancellation transport; converting the `fullStream` loop to `Stream`; fiberizing turn-handle waiters; downgrading startup steps to best-effort; changing outer quit budgets; supervising the pre-registration stream-start window; `shutdown()` semantics; the effect GA bump (standing analysis only). </details> --- _Generated with [`mux`](https://github.com/coder/mux) • Model: `coder:openai/gpt-6-astra` • Thinking: `high` • Cost: `$20.37`_ <!-- mux-attribution: model=coder:openai/gpt-6-astra thinking=high costs=20.37 -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final phase of the progressive Effect migration roadmap: makes
handlerGenthe documented default for future oRPC procedures, converts the two remaining router procedures whose backing service is already Effect-native (muxGateway.getAccountStatus,muxGatewayOauth.startDesktopFlow), and delivers the streamManager placement decision plus a migration completion audit (below). Deliberately progressive, not wholesale: procedures backed by Promise services are left untouched — the convention is to convert the service surface first, never to wrap Promises in Effect at the router.Background
Phases 0–3 landed in #4022 (spike/effectBridge), #4025 (memory), #4027 (retryManager + gateway OAuth internals), #4028 (providerService), #4030 (providerModelFactory), #4031 (heartbeat/idle workers on Schedule+Scope).
router.tshas ~315 handler sites; 11 already rodehandlerGen(7memory.*, 4providers.*mutations). This PR audits the remaining 300+, converts exactly the ones whose backing pipelines already exist as Effect, and encodes the go-forward convention as a module doc inrouter.ts.Implementation
muxGatewayOauthService: the Effect pipelines from 🤖 refactor: convert retryManager and muxGatewayOauthService internals to Effect #4027 were private behindEffect.runPromisefacades. They are now exposed as wire-shaped public Effect methods (matching theproviderService.setConfigEffecthouse pattern):getAccountStatusEffect()— left interruptible: the balance fetch is a pure read, and the session-expired credential clear is a single best-effort promise that runs to completion even if the fiber is interrupted while awaiting it (JS promises are not cancelled by fiber interruption).startDesktopFlowEffect()— wrapped inEffect.uninterruptible(mirrorsasAtomicMutationin providerService): a client abort between loopback-server acquisition anddesktopFlows.registerwould otherwise leak the server with nothing left to close it.runPromisewrappers) so the existing service tests stay byte-identical; the router no longer calls them.router.ts: the two procedures ridehandlerGen; a module doc codifies the convention (handlerGen default for new unary procedures; plain handlers only for Promise-backed services pending conversion, event-iterator subscriptions, and trivial sync reads; audit abort-atomicity before converting mutations). No lint rule: no cheap existing rule expresses "async handler in this one file is suspect" without flagging the ~280 legitimately deferred sites, and building lint infrastructure is out of scope.Conversion audit
Converted here (2):
muxGateway.getAccountStatusmuxGatewayOauthService(Effect since #4027)muxGatewayOauth.startDesktopFlowmuxGatewayOauthServiceEffect.uninterruptible— prevents loopback-server leak on client abortAlready on handlerGen (11):
memory.list/read/save/delete/setPinned/consolidationStatus/consolidate(#4025),providers.addCustomProvider/removeCustomProvider/setProviderConfig/setModels(#4028). Total after this PR: 13.Audited and deferred (with reasons):
muxGatewayOauth.waitForDesktopFlow/cancelDesktopFlowOAuthFlowManager(promise-native deferred registry)providers.list/getConfigproviderServicesync readsproviders.updateRoutePreferencesConfig(Promise)subscribe*,onChange,onConfigChanged, terminal/chat streams)handlerGencannot produce event iterators; blocked on an Effect Stream bridge (existing backlog item from #4025)config.*(19 sites)Config(Promise)projects.idleCompaction.get/set,workspace.heartbeat.setprojectService/workspaceServicesettings storescodexOauth/copilotOauth/coderOauth/muxGovernorOauth(~18 sites)workspace.*(~47),projects.*(~23),mcp*(~26),terminal,analytics,backup,update, remaining (~200 total)streamManager placement decision
Recommendation: defer wholesale conversion; migrate by seams, starting with the two lifecycle seams below. (Analysis of the 5,281-line file, informed by the #4031 lesson that
runSync(Scope.close(...))only composes when fibers suspend on clock timers.)Why wholesale conversion is wrong right now:
AbortControllermismatch. streamManager sits on AI SDK v5streamText, cancelled via WebAbortSignal(per-stream controllers allocated instartStream, polled everyfullStreamiteration). Interrupting a fiber does not cancel the SDK network stream; every one of the ~30 abort touchpoints would need dual-cancellation glue (Scope finalizer →abort()and signal → interrupt).fullStreamconsumption loop (processStreamWithCleanup, ~750 lines) suspends on network I/O and tool execution — exactly the case where 🤖 refactor: convert periodic-worker scheduling to Effect Schedule + Scope #4031 showed synchronousScope.closecannot work. Teardown must berunPromise-based with stopped-flag latching, otherwise late chunks race new streams in the same workspace slot and can corruptpartial.json.WorkspaceStreamInfocarries 30+ interconnected fields (parts accumulation, step tracker, usage accumulators, fallback chains, pending tool buffers, throttle timers) mutated across four phases; a single-pass rewrite would touch hundreds of transitions at once.TurnEngineEventSinkpushes toAIService/AgentSession/IPC; bridging to EffectStream/Hubforces cross-layer churn in three consumers.Proposed seam map for incremental follow-up (in order):
createTempDirForStream/cleanupStreamTempDir)Effect.acquireReleasein a per-stream ScopeschedulePartialWrite/flushPartialWrite)setTimeout+ promise chainingEffect.sleep+ interrupt), same template as idleDispatcher (#4031)partialWriteTimernot pinnedcategorizeError,isResponseIdLost)SetSchema.TaggedErrorclassification pipelines;Reffor the registryisResponseIdLostasserted directly; one test pinscreateStreamResultvia castrecordSessionUsage,resolveTotalUsageForStreamEnd)Effect.genpipelinestokenTrackerfield (re-type as marker per #4031 lesson if swapped)Seams 1–2 are the #4031 patterns verbatim and are safe first steps; the outer stream engine (reader loop, retry/fallback chains, event sink) should convert last, if ever, and only after seams shrink it.
Migration completion state
Effect-native today:
memoryOperations/memoryMeta(#4025),retryManager+muxGatewayOauthService(#4027, public Effect surface as of this PR),providerServicemutations (#4028),providerModelFactory(#4030),heartbeatService/idleCompactionService/idleDispatcher(#4031). Router: 13/~315 sites on handlerGen; every remaining site is either a subscription (Stream bridge backlog) or backed by a Promise service.Suggested future order (value ÷ risk):
waitForDesktopFlow/cancelDesktopFlowplus the four sibling OAuth services (~20 router sites) as mechanical batches.memory.onChangebacklog item.Validation
make static-checkgreen;bun test src/node/services/muxGatewayOauthService.test.ts src/node/orpc/effectBridge.test.ts src/node/orpc/router.test.ts— 23 pass, 0 fail, tests unchanged.runPromiseover the same Effects the router now yields), covering the session-expired credential-clear path and desktop-flow start/callback/exchange.Risks
Low. Wire contracts, schemas, and service behavior are unchanged; the two converted procedures execute the same Effect pipelines as before, now directly on the oRPC fiber instead of behind
runPromise. The one intentional semantic change: client aborts can now interruptgetAccountStatusmid-fetch (previously it always ran to completion) — safe for a read; the credential-clear write is single-promise atomic.startDesktopFlowis explicitly uninterruptible, so its abort behavior is identical to before.Generated with
xum• Model:anthropic:claude-fable-5• Thinking:xhigh• Cost:$0.00