[task-service] 🤖 fix: correlate workspace-turn liveness - #3915
Open
coadler wants to merge 5 commits into
Open
Conversation
Collaborator
Author
|
@codex review Please review the workspace-turn lifecycle changes. |
This comment has been minimized.
This comment has been minimized.
Collaborator
Author
|
@codex review Please review the implementation. The prior response only contained the completed security review. |
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fbe142a01
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Addressed both Codex findings in
Added regression tests for waiter preservation, handoff order, enqueue failure, and explicit cancellation. |
Collaborator
Author
|
@codex review Both P1 findings are addressed in |
This comment has been minimized.
This comment has been minimized.
Collaborator
Author
|
@codex review The security review completed, but no code-review approval appeared. Please run the code review for |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5674e42de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
@codex review The CI unit test now matches the queued terminal continuation behavior in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07eba9212c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
Collaborator
Author
|
Addressed The terminal attention record is now persisted before the background continuation starts. The Added a regression test that invokes |
Collaborator
Author
|
@codex review The final attention-ordering finding is addressed in |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3da714e8cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Addressed the current Codex findings in
The fix also preserves delivery callbacks after correlation removal. Stable child generations now use generation-specific terminal queue keys. Local validation passed:
|
Collaborator
Author
|
@codex review Please review commit |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5f36592d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Addressed
Added a regression for a workspace-turn entry with no callbacks. Local validation passed:
|
Collaborator
Author
|
@codex review Please review commit |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This comment has been minimized.
This comment has been minimized.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 21, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 21, 2026
Collaborator
Author
|
@codex review The branch was rebuilt from current main. The previous broad diff is replaced by a focused liveness and capacity fix. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Collaborator
Author
|
The branch was rebuilt from current main. These three findings refer to removed taskService code and are no longer present in the diff:
The replacement diff only changes correlation-aware liveness and stale-handle capacity accounting. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10023aa1ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
added 3 commits
September 1, 2026 15:46
Use exact stream and queued-turn correlation before slot accounting.
Keep busy-workspace admission tests aligned with exact handle liveness.
coadler
force-pushed
the
fix/workspace-turn-false-interruptions
branch
from
September 1, 2026 20:46
10023aa to
3c1d37b
Compare
Collaborator
Author
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c1d37bb14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Addressed all four findings in 3f22e12:
|
Collaborator
Author
|
@codex review All current findings are addressed. The full workspace-turn manager suite and make static-check pass. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f22e1209f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Correlate compaction, retry, wake, queue, and send-preflight activity. Carry workspace ownership evidence through stale settlement. --- _Generated with `xum` • Model: `openai:gpt-5.6-sol` • Thinking: `xhigh` • Cost: `$0.19`_ <!-- mux-attribution: model=openai:gpt-5.6-sol thinking=xhigh costs=0.19 -->
Collaborator
Author
|
Addressed all four findings in
|
Collaborator
Author
|
@codex review Please review commit |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f190cc897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+3888
to
+3891
| const hasUncorrelatedCreationReservation = | ||
| creationReservations != null && | ||
| creationReservations.size > 0 && | ||
| !hasCorrelatedCreationReservation; |
There was a problem hiding this comment.
When handle A is live only through its active registration—for example, after a deferred stream-end while descendant work remains—and follow-up B enters send preflight, B's reservation makes hasUncorrelatedCreationReservation true for A. A concurrent task-list or capacity scan therefore terminally interrupts A before B reaches queue admission; if B's send subsequently fails, no replacement turn ever existed and A's waiter/result has already been lost. Creation reservations should keep their own handle live without becoming supersession evidence for other handles until the new send is admitted.
AGENTS.md reference: AGENTS.md:L150-L150
Useful? React with 👍 / 👎.
Comment on lines
+3902
to
+3903
| const hasUncorrelatedRetryOrWake = | ||
| !hasActiveRegistration && (hasPendingAutoRetry || hasPendingBashMonitorWake); |
There was a problem hiding this comment.
During startup recovery of a non-task workspace, WorkspaceService.initialize() can schedule the correlated auto-retry before any process-local active registration is restored. While that retry is pending, this condition classifies it as uncorrelated solely because hasActiveRegistration is false, so a task-list or capacity scan settles the durable handle as interrupted and rejects its waiter even though the same turn is about to resume. Pending restart retries need correlation from their preserved message metadata, or an equivalent registration established before liveness checks.
AGENTS.md reference: AGENTS.md:L112-L112
Useful? React with 👍 / 👎.
This was referenced Sep 3, 2026
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
Make workspace-turn liveness depend on exact turn correlation.
Background
A stale workspace-turn handle could remain running when unrelated work started in the same target workspace. The stale handle could then consume a
maxParallelAgentTasksslot.A second race could interrupt a newly persisted queued handle while
sendMessagecompleted its preflight work.This branch previously contained a broad pre-refactor handoff patch. The new branch starts from current main and contains only the liveness and capacity fix.
Implementation
sendMessageexposes queue or stream correlation.Risks
Moderate. This changes workspace-turn recovery and task admission. The tests cover streams, compaction, queued continuations, preflight races, retry paths, monitor wakes, ownership transfer, and unrelated activity.
Generated with
xum• Model:openai:gpt-5.6-sol• Thinking:xhigh• Cost:$0.19