🤖 refactor: convert streamManager resource seams to Effect (temp-dir acquireRelease + scope-tied debounce fiber) by ThomasK33 · Pull Request #4040 · coder/xum

ThomasK33 · 2026-09-01T18:39:10Z

Summary

Phase 10 (final chartered phase of Wave 2 of the progressive Effect migration) converts the four incremental resource seams inside StreamManager that Phase 4 (#4032) mapped and #4039 refined — temp-dir lifecycle, partial-write debounce, error-categorization state, and usage accounting — without touching the stream engine core, StreamManager's public API, or any crash-recovery semantics.

Background

Phase 4 deliberately deferred wholesale streamManager.ts conversion (5,281 lines; fibers suspend on I/O; AbortController-centric; 30+ field WorkspaceStreamInfo) but identified four seams that convert cleanly. #4039 established the implementation templates this PR applies: per-resource Effect.acquireRelease, scope-tied fibers via Effect.forkIn, the plain-mutables rule for AI-SDK-callback state, and the zero-suspension foreign-callsite emit pattern.

Implementation

Seam 1 — Temp-dir lifecycle → per-resource `Effect.acquireRelease` (converted)

Each stream now creates a Scope (resourceScope) in startStream. The temp dir is acquired through Effect.acquireRelease: the release finalizer (the existing fire-and-forget cleanupStreamTempDir) registers only after acquisition succeeds. Scope.close replaces the previous two hand-coordinated cleanup sites (startStream's !streamRegistered finally and processStreamWithCleanup's finally); close is idempotent, so ownership transfer at registration can never double-release or leak. The falsy guard for whitebox providedRuntimeTempDir: "" fixtures is preserved inside the finalizer.

Seam 2 — Partial-write debounce → scope-tied fiber (converted)

partialWriteTimer (setTimeout handle + three manual clearTimeout sites) becomes partialWriteFiber, forked into the stream's resourceScope (#4039's scope-tied ticker template, adapted to debounce semantics). Re-arm and explicit-flush cancellation go through one interruptPartialWriteFiber helper; the stream-teardown cancellation is now implicit — closing the scope interrupts a pending flush, so a debounced write can never fire after the stream ends and resurrect partial.json for a dead stream. Effect.runSync(Effect.forkIn(...))/runFork execute synchronously up to the sleep, preserving the previous setTimeout registration ordering; Effect's clock registers a plain setTimeout under the hood, so scheduling semantics are unchanged.

Seam 3 — Error categorization / `lostResponseIds` (stayed plain, documented)

Applying #4039's plain-mutables rule honestly: lostResponseIds is mutated exclusively from non-Effect contexts (AI-SDK error paths inside processStreamWithCleanup) and read synchronously by isResponseIdLost (a spy-pinned public seam used by TurnRequestBuilder). No fiber reads or mutates it, so a Ref would add ceremony without adding safety. categorizeError is pure synchronous classification — nothing to convert. Both stay plain; the rationale is now documented at the declaration site. This seam is smaller than the #4032 map suggested, by design.

Seam 4 — Usage accounting (stayed plain, documented; zero-suspension invariant pinned in comments)

cumulativeUsage / lastStepUsage / cumulativeProviderMetadata are mutated only by the AI-SDK fullStream loop (finish-step) and plain retry/reset methods — foreign callsites, never fibers — so they stay plain mutables per the rule. The SubscriptionEmit-transfer property already holds structurally and is now documented: the usage-delta emit runs with zero suspension after the mutation (emitTurnEvent invokes the sink synchronously), so downstream Effect queue bridges (#4039's SubscriptionEmit.push → Queue.offerUnsafe) observe usage events in mutation order.

Interruption-posture audit

The debounce fiber is interruptible by design (cancelling a pending checkpoint write is exactly what clearTimeout did). If interrupted mid-flush, the underlying flushPartialWrite promise still runs to completion — no torn write; writes stay serialized via partialWritePromise.
Temp-dir release and fiber interruption are synchronous finalizers on a root fiber (Effect.runFork(Scope.close(...))); nothing can interrupt teardown mid-way.
No awaits were added between liveness checks and writes; every abort-signal checkpoint in startStream keeps its position.
Verified empirically against effect@4.0.0-rc.112: forkIn into a closed scope never throws and the fiber never runs; double Scope.close releases exactly once; closing a scope with a sleeping fiber interrupts it and still runs later finalizers.

What stayed out (per charter)

The stream engine core (fullStream loop, AbortController seams, WorkspaceStreamInfo restructure), StreamManager's public API, and all spy-pinned seams (createTempDirForStream mockResolvedValue spies, cleanupStreamTempDir Reflect-extraction, chaos-test Reflect.set internals) are untouched.

Validation

streamManager.test.ts (119 existing + 2 new), streamManager.chaos.test.ts, streamManager.modelOnlyNotifications.test.ts, streamSimulation, replayBufferedStreamMessageRelay, agentSession.preStreamError, agentSession.resumeStreamEmptyHistory, aiService, hooks, turnRequestBuilder — all pass unchanged.
New tests cover the two genuinely new invariants: temp-dir release runs exactly once across the ownership transfer, and a pending debounced partial write is interrupted at stream end (deterministically armed via stream-state gating, not sleeps).
make static-check green.
Standalone probe validated the Effect v4 scope/fiber edge semantics the design relies on (closed-scope fork, double-close idempotence, sleeping-fiber interruption).

Risks

streamManager.ts is the most crash-sensitive file in the repo. The regression surface here is partial-write scheduling and temp-dir cleanup:

Partial-write loss window: unchanged — a pending debounce cancelled at teardown was also dropped by the old clearTimeout; terminal paths still perform their own awaited flush/commit.
Partial resurrection after stream end: strictly improved and now test-pinned (scope close interrupts the pending flush; previously relied on clearTimeout placement).
Temp-dir leak/double-free: strictly improved (idempotent single-owner release vs. two coordinated call sites + flag).
Crash-recovery semantics (repairable incomplete state, malformed-history tolerance) are untouched: no persistence formats, no request-building paths, no commit predicates changed.

Wave 2 completion state

Wave 2 (#4033 OAuth flow scopes, #4034 OAuth services + router sites, #4035 coderOauthService, #4036 Config semaphore, #4038 consolidation/status generator, #4039 oRPC subscription Stream bridge, this PR) is complete. What remains Promise-based repo-wide, in suggested Wave 3 order:

StreamManager engine core — the fullStream consumption loop, AbortController lifecycle, and WorkspaceStreamInfo restructure; deliberately deferred (I/O-suspended fiber teardown needs async Scope.close, which the current sync teardown contract can't host). Revisit after a ManagedRuntime/Layer DI RFC.
workspace/project/task services — lock-heavy CRUD services; natural next targets for the wrap-around-locks + Effect.gen-behind-facade house pattern.
WorkflowService internal event seam (deferred from 🤖 refactor: bridge oRPC subscription procedures through Effect Stream #4039) and createTickIterable (no resource to manage; convert opportunistically).
ManagedRuntime/Layer dependency injection — an RFC-scale change replacing constructor wiring; unlocks TestClock for the timing-sensitive suites and app-lifetime scopes.
Schema at persistence boundaries — Schema.decodeUnknown for config/history/session artifacts.
OAuth token-refresh worker (optional; needs product sign-off) and effect v4 GA + oRPC lockstep upgrades.

Generated with xum • Model: anthropic:claude-fable-5 • Thinking: xhigh • Cost: $n/a

…acquireRelease + scope-tied debounce fiber)

ThomasK33 · 2026-09-01T18:39:30Z

@codex review

ThomasK33 · 2026-09-01T18:39:32Z

@codex security review

chatgpt-codex-connector · 2026-09-01T18:43:14Z

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 9db1edabb5

ℹ️ 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".

…e + Stores/MemoryMeta layers + runtime-backed effect/context) (coder#4049) ## Summary Effect migration **Wave 3 / Phase 11, PR 1 of 6 (skeleton)**: introduces the app-lifetime Effect `ManagedRuntime` ("AppRuntime") built from a Layer graph, with the first two layers (`StoresLive` exposing the `ConfigStores`, `MemoryMetaLive` constructing `MemoryMetaService`), and wires it into `ServiceContainer`: the runtime is built eagerly and synchronously before the constructor-wired services, its built `Context` becomes the oRPC `"effect/context"`, and `disposeAppRuntime` (bounded, never rejects, idempotent) is the last `dispose()` step. Every service class, constructor signature, facade, and test seam is unchanged; existing tests are untouched and green. ## Background Two migration waves (#4022→#4040) converted service internals to Effect behind Promise facades. #4040's completion notes name a DI/runtime skeleton as the prerequisite for the streamManager engine-core conversion (needs an app-owned async `Scope.close`), `TestClock` for timing suites, and app-lifetime scopes. The approved Phase 11 plan (embedded below) phases that into six PRs; this is the smallest possible first step that proves the pattern end-to-end with tests unchanged. ## Implementation - `src/node/services/di/tags.ts` — `Context.Service` tags (`ConfigTag`, `SessionLocatorTag`, `ProvidersConfigStoreTag`, `SecretsStoreTag`, `FileLeaseManagerTag`, `MemoryMeta` moved here from `orpc/effectContext.ts`, which re-exports it) + the `AppTags` union. Type-only imports of service classes → no import cycles. - `di/layers/stores.ts` (`StoresLive`), `di/layers/core.ts` (`MemoryMetaLive = Layer.effect(MemoryMeta, Effect.map(ConfigTag, c => new MemoryMetaService(c.rootDir)))`), `di/layers/app.ts` (`AppLive(stores) = MemoryMetaLive.pipe(Layer.provideMerge(StoresLive(stores)))`). - `di/appRuntime.ts` — `makeAppRuntime(layer)`: `ManagedRuntime.make` + eager `runSync(Effect.context())` + `assert(cachedContext)`; a layer body that suspends or throws fails **at construction**, exactly where a throwing service constructor fails today (so every entry point's existing startup catch path applies). `disposeAppRuntime(runtime, timeoutMs)`: uninterruptible shell, `disposeEffect` forked detached, interruptible bounded `Fiber.join` + `Effect.timeout`, `TimeoutError`/defects folded to `log.warn`. The module doc comment carries the DI contract (sync layer bodies as a Phase 11 compatibility rule; only the composition root holds the runtime; no layer finalizers yet). - `ServiceContainer`: `public readonly runtime: AppRuntime<AppTags>` built first; the layer-built `MemoryMetaService` is handed to `createCoreServices` via a new optional `memoryMetaService?` (same precedent as `workspaceMcpOverridesService?`); `toORPCContext()["effect/context"] = runtime.context`; `dispose()` ends with `disposeAppRuntime` behind a `runtimeDisposed` latch. - `orpc/effectContext.ts`: `OrpcEffectServices = AppTags`; `buildOrpcEffectContext` stays as the narrow test helper it already is (only caller: `effectBridge.test.ts`). - `headlessEnvironment.dispose` now calls `services.dispose()` (the bench harness previously leaked the container; runtime ownership starts here). - `APP_RUNTIME_DISPOSE_TIMEOUT_MS = 2 s` in `src/constants/terminationTimeouts.ts` (inside the 5 s quit budgets of `desktop/main.ts` / `cli/server.ts`). ## Validation New tests: `di/appRuntime.test.ts` (sync build caches context; async layer body → synchronous throw; throwing body → synchronous throw; `runFork` after eager build starts synchronously; finalizers run in reverse acquisition order; dispose idempotent; hung finalizer → returns at timeout with a warn, no rejection) and three `serviceContainer.test.ts` cases (field ↔ `effect/context` identity for `MemoryMeta`; runtime still alive at the last explicit dispose step and gone after; a throwing layer surfaces as a synchronous `new ServiceContainer()` throw). `effectBridge.test.ts` / `memoryMeta*.test.ts` unchanged and green. Pre-review audits (plan §3): interruption posture — one detached fiber (`disposeEffect`), whose join is the only interruptible wait; teardown shell `Effect.uninterruptible`; `makeAppRuntime` is the single place allowed to throw and `disposeAppRuntime` folds `TimeoutError` + defects; spy seams — no `spyOn` targets `MemoryMetaService`, constructor arity unchanged (`(xumHome)`), tests typecheck; sync-start pinned by test; `MemoryMetaService` constructor only computes a path (no collaborator side effects). ### Dogfooding (dev-server sandbox on a headless Coder host; `XUM_LOG_LEVEL=debug DEV_SERVER_SANDBOX_ARGS="--clean-projects"`) - **Startup ordering** (branch): `[startup] AppRuntime built { ms: 3 }` → `[startup] ServiceContainer.initialize starting` → six step durations → `[startup] ServiceContainer.initialize completed { totalMs: 251 }`. Baseline `origin/main` standalone `xum server`: `initialize completed { totalMs: 271 }` (workspaceService 82 / taskService 104 ms). The added constructor work is the 3 ms build. - **Memory pin/unpin through the UI** (Settings → Experiments → Agent Memory on; Settings → Memory; seeded `<XUM_ROOT>/memory/global/pr1-dogfood.md`): Pin → `memory-meta.json` shows `"pinned": true, "accessCount": 1`; Unpin → `"pinned": false`. These `memory.*` procedures ride `handlerGen`; they use the transitional `context.memoryMetaService.effects…` style, so this proves the **layer-built instance** serves the handler path (tag resolution through `effect/context` itself is proven by `effectBridge.test.ts` + the identity test). Screenshots (`03-memory-experiment-enabled`, `04-memory-panel`, `05-memory-pinned`, `06-memory-unpinned`) and a WebM of the full pin/unpin cycle were captured with agent-browser and are attached in the Mux chat transcript — GitHub image upload is unavailable from this host (SSO upload-token failure). - **Graceful quit** (standalone `node dist/cli/index.js server` under `script -q`, `kill -TERM`): `Shutting down server...` → `AgentStatusService stopped` → `terminateAll` → `[shutdown] AppRuntime disposed { ms: 2 }` → `COMMAND_EXIT_CODE="0"`, no "Cleanup timed out". Repeated after the probe below was reverted (clean rebuild): same result. - **Startup-never-crash parity probe** (local edit, not committed): a `Layer.sync(MemoryMeta, () => { throw … })` in `AppLive` → `Failed to initialize server: Error: PR1 dogfood probe: layer body threw during startup` via the existing `main().catch` → exit code 1, stack points at the layer body, **no** unhandled-rejection trace, no `AppRuntime built`/`initialize` lines (failed at construction, as designed). The sandbox's nodemon showed `app crashed - waiting for file changes`, identical to a throwing constructor. - **Electron path**: not exercised here (no display); the desktop quit path shares `ServiceContainer.dispose()` with `cli/server.ts`, which was exercised above, and `tests/e2e` covers it in CI. ### Test lanes `make static-check` green. `bun test src/node/services/di src/node/services/serviceContainer.test.ts src/node/orpc src/node/services/memoryMeta*` green. Full `bun test src` under host load wedged in `workflow_run.test.ts` (unrelated tool test); every unexpected `(fail)` in that lane either passes in isolation (`workflow_run` 25/25, `workspaceGoalService` 189/189, `WorkspaceFooterBar` 15/15) or fails identically on an `origin/main` worktree (`workspaceTurnManager` 2, `productIdentity` 1, `agent_skill_delete` 1 — pre-existing environment failures, alongside the known `taskService`/`workspaceService` baselines). jest lane (`TEST_INTEGRATION=1 bun x jest tests`, tests/ipc + tests/ui): see the CI checks / the run summary in the notes below. ## Risks Low. Behavior change is confined to: (1) `MemoryMetaService` is constructed by a layer instead of `createCoreServices` (same arguments, same single instance — asserted by identity tests), (2) `"effect/context"` carries six tags instead of one (only `effectBridge` probes yield tags today), (3) `dispose()` gains a final bounded runtime close (no layer finalizers exist yet, so it reorders nothing), (4) the headless bench harness now disposes its container. The remaining risk is the sync-layer contract itself; it is enforced at construction and covered by tests. ## PR 1 notes - **Echo-probe overhead** (`effectBridge.test.ts` informational bench, 2000 sequential calls, three runs each): branch — effect 24.3 / 21.9 / 22.0 µs/call, overhead vs plain async 8.3 / 8.3 / 7.9 µs/call; `origin/main` — effect 18.5 / 26.9 / 23.0 µs/call, overhead −1.5 / 4.0 / 10.0 µs/call. Noise-level identical; note the probe uses the test's narrow `buildOrpcEffectContext` context on both sides, and the production context has six entries in PR 1. - **Deviations from the plan** (all within the plan's stated contract): 1. `disposeAppRuntime` bounds the wait by forking `disposeEffect` detached and timing out the interruptible `Fiber.join`, rather than `Effect.timeout` directly on `disposeEffect`: scope finalizers run uninterruptibly, so interrupting the close itself would still wait on a hung finalizer. The contract (bounded, never rejects, idempotent) is what the tests pin. 2. `AppRuntime<R>` is a small interface `{ managed, context, get }` and `ServiceContainer.runtime` is that wrapper; `"effect/context"` is `runtime.context` (the plan's `serviceContext`). Keeps `Context.get` inside `di/`. 3. The latch guards only the runtime-dispose step, not all of `dispose()`, so existing dispose semantics are byte-identical. 4. The `[startup] AppRuntime built` debug line lives in `makeAppRuntime` (shared by the future CLI root in PR 3) instead of `ServiceContainer`. - **Lessons for PR 2 (EffectRunner + AppFiberScope + TestClock on idleCompaction/heartbeat/retryManager)**: - rc.112 API notes: `Layer.succeed` is curried-only (`Layer.succeed(Tag)(value)`); `ManagedRuntime<R, ER>` is contravariant in `R`, so helpers accepting any runtime must take `ManagedRuntime<never, never>`; `Effect.timeout` fails with `Cause.TimeoutError` (`_tag: "TimeoutError"`); `Effect.runSync` really does throw on an `Effect.promise` inside a layer body, so the eager-build assert is a belt-and-braces check. - Bounded teardown shape that actually bounds: `Effect.forkDetach(target)` + `Effect.interruptible(Fiber.join(fiber).pipe(Effect.timeout(ms)))` inside `Effect.uninterruptible`. Reuse for `closeScopeBounded(appFiberScope)`. - Bun `spyOn(namespaceImport, "AppLive")` intercepts `ServiceContainer`'s named import (live binding) — a cheap way to inject test layers/probes without new production seams; PR 2's `EffectRunner` tests can use the same trick for `AppLive`/`EffectRunnerLive`. - Dev-server sandbox is fragile across a crash cycle (its build watcher died after the probe; nodemon stayed in "app crashed"); for startup/shutdown probes prefer a standalone `node dist/cli/index.js server` under `script -q` with a temp `XUM_ROOT` — it also yields the exit code. - Full `bun test src` can wedge under host load (a `workflow_run` duplicate-guard test hung without a timeout); classify unexpected failures by re-running in isolation and against an `origin/main` worktree with a symlinked `node_modules` (~15 s per file) rather than waiting on the lane. --- <details> <summary>📋 Implementation Plan</summary> # Effect migration — Wave 3 / Phase 11: ManagedRuntime + Layer dependency injection ## 0. Summary Replace the two hand-written composition roots (`createCoreServices` + the `ServiceContainer` constructor) with an **Effect `Layer` graph** built once per process by a **`ManagedRuntime`** ("AppRuntime"), while keeping every service class, constructor signature, Promise facade, private method, and test seam compatible. The runtime becomes (a) the owner of the app-lifetime `Scope`, (b) the provider of `"effect/context"` for oRPC Effect-native handlers, and (c) the source of two runtime seams: an **`EffectRunner`** (context-bound, *unsupervised* runner that lets clock-driven workers run on a `TestClock`) and an **`AppFiberScope`** (a runtime-owned, *supervised* scope whose close is awaited by `dispose()` — the slot the streamManager engine core will occupy later). Six stacked, independently mergeable PRs. Product PRs keep existing tests unchanged; only the final test-modernization PR edits tests. Net product LoC ≈ **+420** (per-PR estimates below). Service classes are *not* rewritten — Layers are thin adapters around existing constructors; cycle-breaking setter wiring moves into explicit "wiring layers" that replay today's order. Unlocks (not done here): streamManager ENGINE CORE conversion, `TestClock` for timing suites, app-lifetime scopes. ## 1. Verified current state (evidence) - **Roots.** `src/node/services/coreServices.ts:103-389` (`createCoreServices`: 25 constructions, 12 `turnRequestBuilderBindings` writes, ~14 setters) and `src/node/services/serviceContainer.ts:161-575` (45 more constructions; `aiService.on(...)`/`workspaceService.on(...)` analytics wiring at 474-574; global registrations `setGlobalCoderService/setSshPromptService` at 469-471). `new ServiceContainer(stores)` is called by `headlessEnvironment.ts:111`, `tests/ipc/setup.ts`, `src/cli/server.ts:132`, `src/node/acp/serverConnection.ts:155`, `src/desktop/main.ts:653`; `src/cli/run.ts:661` and `src/cli/workflow.ts:376` call `createCoreServices` directly. ⇒ two graph roots (App vs Core), five process entry points, all constructing **synchronously**. - **Startup.** `ServiceContainer.initialize()` (577-642) awaits six `initialize()`s (no try/catch; failure propagates to `main.ts:1255-1265` "Startup Failed" dialog + quit; `server.ts`/ACP log and exit), then sync `start()`s idleCompaction/heartbeat/agentStatus, then two fire-and-forget sweeps. All constructors are synchronous; two have side effects on **declared constructor dependencies** only (`AIService` → `streamManager.setEventSink`, `WorkspaceService` → `backgroundProcessManager.on/aiService.on`). - **Teardown.** `dispose()` (746-779) is explicit and hand-ordered (`backgroundProcessManager.beginShutdown()` MUST be first — it is a latch protecting persisted monitor records; bridges stop before sessions close; `terminateAll` late; `timelineService.flush()` last). `shutdown()` (718-732) is a *second* sequence fired concurrently by a second `before-quit` listener (`main.ts:1321`). `main.ts:1296-1304` races `dispose()` against 5 s then `app.quit()`; `cli/server.ts:227-268` has a 5 s `process.exit(1)` force timer; `tests/ipc` cleanup calls `dispose()` then `shutdown()`; `headlessEnvironment.dispose` never calls `services.dispose()`. - **Existing Effect surface.** 25 files import `effect`. Only `Context.Service` tag: `MemoryMeta` (`src/node/orpc/effectContext.ts:21`). `handlerGen` (`@orpc/experimental-effect`) runs `Effect.runPromiseExit` per request and `Effect.provide`s `opts.context["effect/context"]`. `streamBridge.ts` runs streams on the global runtime. Scope-owning workers: `heartbeatService.ts:134-243`, `idleCompactionService.ts:86-122` (`Scope.makeUnsafe` + `Effect.runSync(Scope.close(..))`, valid only because their fibers suspend solely on the clock), `oauthFlowManager.ts:164`, `streamManager.ts:4767/4054` (already `Effect.runFork(Scope.close(..))` — the async-close precedent). `memoryConsolidationService.ts:667-703, 837-860`: check-and-reserve funnels with zero suspensions before `inFlight.set`/`harvestInFlight.set`. - **effect@4.0.0-rc.112 API (verified in `node_modules/effect/dist`).** `Context.Service<Self, Shape>()("id")` (module `Context`, not `ServiceMap`); `Layer.{succeed,sync,effect,effectContext,effectDiscard,provide,provideMerge,mergeAll,build,buildWithScope}` (no `Layer.scoped`; `Layer.effect` strips `Scope` from R); `ManagedRuntime.make(layer)` → `{ runSync, runSyncExit, runFork, runPromise, runPromiseExit, contextEffect, cachedContext, scope, dispose(), disposeEffect }`; `Effect.{runSyncWith,runForkWith,runPromiseWith,runPromiseExitWith}(context)`; `Effect.context<R>()`; `Effect.serviceOption`; `Scope.{fork,forkUnsafe,close,provide}`; `TestClock` from `effect/testing` (`layer, adjust, setTime, withLive`); `Clock.Clock` is a `Context.Reference` (defaulted; `TestClock.layer()` overrides it). - **ManagedRuntime internals the design relies on** (`ManagedRuntime.js`): `make` creates `scope = Scope.makeUnsafe("parallel")` and `layerScope = Scope.forkUnsafe(scope, "sequential")`; the first `runX` forks a build fiber over `Layer.buildWithMemoMap` — a **fully synchronous layer graph builds synchronously**, so `runtime.runSync(Effect.context())` succeeds and sets `cachedContext`; afterwards every `runX` is `Effect.run…With(cachedContext)` (no extra async boundary). Fibers started through `runtime.runX` are registered in `scope` (`onFiberStart: Fiber.runIn(scope)`). `dispose()` = `Scope.close(scope)` (interrupt registered fibers in parallel → layer finalizers sequentially in reverse), after which any `runtime.runX` dies with `"ManagedRuntime disposed"`. - **Layer composition semantics.** `Layer.mergeAll(A, B)` is *not* a dependency resolver: B's requirements are not satisfied by A's outputs; requirements bubble up. Dependencies are satisfied only via `Layer.provide`/`provideMerge` chains. Siblings in `mergeAll` may build concurrently. - **Test seams that pin signatures** (Explore report): private-method spies (`Config.saveConfig`, `WorkspaceService.retireKernelWorkflowRunReferences/startStartupRecovery/createSession/updateAgentStatus`, `MCPServerManager.startServers`, `AgentPluginInstallService.reconcileJournals`, …); module-level export spies (`agentStatusService.generateWorkspaceStatus`, `sshConnectionPool.verifyHostKeyAgainstPolicyEffect`, …); direct construction in tests (`Config` 44 files, `HistoryService` 22, `MemoryMetaService` 11, `WorkspaceService` 7, `IdleDispatcher` 6, `StreamManager` 4, `ServiceContainer` 3); partial-mock casts (`InitStateManager` 193, `AIService` 158, `TaskService` 149, `ORPCContext` 62). `effectBridge.test.ts:24-30` builds a partial `ORPCContext` via `buildOrpcEffectContext` + `as unknown as ORPCContext`. - **Timing probes** (TestClock candidates): `heartbeatService.test.ts` 6 real sleeps, `idleCompactionService.test.ts` 2, `retryManager.test.ts` 3 `setSystemTime`, `streamManager.test.ts` 7 (partial-write debounce), `streamBridge.test.ts` 11 (heartbeat ticker), OAuth device-flow suites 14 (non-goal). ## 2. Target architecture ### 2.1 Building blocks (all under `src/node/services/di/`; the *only* directory allowed to import `Layer`/`Context`/`ManagedRuntime`/`TestClock`) | Module | Contents | |---|---| | `tags.ts` | One `Context.Service` tag per service class provided by the graph. Type-only imports of service classes ⇒ no runtime import cycles. Ids `"xum/<Name>"`. Naming: class name minus trailing `Service` (`MemoryMeta`, `Workspace`, `History`); classes without that suffix or colliding with an exported name get a `Tag` suffix (`ConfigTag`, `StreamManagerTag`, `IdleDispatcherTag`). Exports the unions `CoreTags` and `AppTags`. | | `effectRunner.ts` | `interface EffectRunner { runSync<A,E>(e: Effect<A,E,never>): A; runSyncExit; runFork; runPromise; runPromiseExit }` — a **context-bound, unsupervised** runner whose methods accept only effects with **no service requirements** (`R = never`; defaulted references like `Clock` do not appear in `R`). That makes "not a service locator" type-enforced: a fiber that needs services must take them as explicit constructor dependencies and, if it must be awaited on shutdown, fork into `AppFiberScope`. `defaultEffectRunner` = the global `Effect.runX` (today's exact behavior). `effectRunnerFromContext(ctx)` = `Effect.run…With(ctx)`. `EffectRunnerTag` + `EffectRunnerLive = Layer.effect(EffectRunnerTag, Effect.map(Effect.context<never>(), effectRunnerFromContext))`, placed at the **base** of the graph so the captured context contains only refs (`Clock`, later `Logger`/`Random`) plus stores. Fibers forked through it are owned by the worker's own `Scope` (explicit `start/stop`), **not** by the ManagedRuntime; `runtime.dispose()` does not interrupt them. Services import only this file from `di/`. | | `appFiberScope.ts` | `AppFiberScopeTag: Scope.Closeable`. `AppFiberScopeLive = Layer.effect(AppFiberScopeTag, Effect.gen(function*(){ const parent = yield* Effect.scope; return yield* Scope.fork(parent, "parallel"); }))` — a child of the runtime's layer scope. Fibers forked into it via `Effect.forkIn(_, appFiberScope)` are interrupted **and awaited** when the scope closes. This is the **supervised** seam for I/O-suspended fibers (engine core, later). `ServiceContainer.dispose()` closes it explicitly and early (§5) so interrupted fibers can still use their dependencies during finalization; `runtime.dispose()` later re-closes it idempotently as a backstop. No production occupant in Phase 11; the seam exists with tests. | | `appRuntime.ts` | `makeAppRuntime(layer)`: `ManagedRuntime.make(layer)` + **eager synchronous build** (`runtime.runSync(Effect.context<R>())`; `assert(runtime.cachedContext !== undefined)`); a layer body that suspends is a programming error and throws here — exactly where a throwing constructor throws today, so every entry point's existing catch/dialog/log path is preserved. `disposeAppRuntime(runtime, timeoutMs)` and `closeScopeBounded(scope, timeoutMs)` share one shape: `Effect.uninterruptible` teardown shell around `Effect.interruptible(target.pipe(Effect.timeout(timeoutMs)))` where `target` is `runtime.disposeEffect` resp. `Scope.close(scope, Exit.void)` (never a non-cancellable JS Promise wrapper); `Effect.catchTag("TimeoutError", …)` + `Effect.catchDefect` → `log.warn`; run via `Effect.runPromise`; **never rejects**; idempotent (`Scope.close` is idempotent; `disposeEffect` is guarded by a latch). Verify the exact rc `Effect.timeout` error type at implementation time (rc.112: fails with `Cause.TimeoutError`, `_tag: "TimeoutError"`). Module doc comment = the DI contract (§2.3, §5). | | `layers/stores.ts` | `StoresLive(stores: ConfigStores)` = `Layer.mergeAll` of `Layer.succeed` for `ConfigTag`, `SessionLocatorTag`, `ProvidersConfigStoreTag`, `SecretsStoreTag`, `FileLeaseManagerTag` (true siblings — no inter-dependencies). `StoresFromCoreOptionsLive` reproduces the `opts.x ?? new X(config.rootDir)` defaults of `coreServices.ts:106-112` for the CLI root. | | `layers/core.ts` | `CoreOptionsTag` (today's `CoreServicesOptions` minus stores — carries the *optional* cross-cutting services exactly as today). **PR 3:** `CoreProjectionLive = Layer.effectContext(...)` wrapping the existing `createCoreServices` body and returning a `Context<CoreTags>` (coarse projection, zero behavior change). **PR 4:** peel into per-service `Layer.effect(Tag, Effect.gen(...))` layers composed in **explicit dependency stages** (`Layer.provideMerge` between stages; `Layer.mergeAll` only for true siblings within a stage — every sibling claim below was checked against the constructor argument lists in `coreServices.ts` and must be re-checked in the PR): S1 History · InitState · Provider · BackgroundProcess · ExtensionMetadata · MemoryMeta · TerminalAttention · IdleDispatcher · WorkspaceMcpOverrides(default) · `TurnRequestBuilderBindingsTag` (`Layer.succeed(_, {})`) → S2a SessionUsage · Goal · Memory → S2b StreamManager (needs SessionUsage) → S3 AIService → S4 Consolidation · MCPConfig → S5 MCPServerManager → S6 Workspace → S7 Task → S8 TurnManager → `CoreWiringLive` (`Layer.effectDiscard`, **`Effect.sync` only — no `acquireRelease`**, replays `coreServices.ts:137-166, 209-210, 258-270, 288-325, 349-352, 360-367` in order). | | `layers/desktop.ts` | `CrossCuttingLive` (policy, telemetry, experiments, backup, sessionTiming, analytics, devTools, workspaceMcpOverrides, browserBridgeTokenManager), `CoreOptionsFromDesktopLive` (derives `CoreOptionsTag` from those tags + `extensionMetadataPath`), then **group layers** (`Layer.effectContext` returning a `Context` of several tags, constructed in today's order): `BrowserLive`, `DesktopBridgeLive`, `OauthLive`, `WorkersLive` (idleCompaction, heartbeat, agentStatus, timeline, refine), `TerminalEditorLive`, `MiscDesktopLive`; staged with `provideMerge` where one group needs another. `DesktopWiringLive` (`Effect.sync` only) = setters + `aiService.on/workspaceService.on/memoryConsolidationService.on` wiring + global registrations. | | `layers/app.ts` | `AppLive(stores) = DesktopLive ▹ CoreLive ▹ CoreOptionsFromDesktopLive ▹ CrossCuttingLive ▹ AppFiberScopeLive ▹ EffectRunnerLive ▹ StoresLive(stores)` — read `X ▹ Y` as "X is *provided with* Y, and both stay exposed", i.e. **`X.pipe(Layer.provideMerge(Y))`** (rc.112 signature: `provideMerge(that: provider)(self: consumer)`; the *right-hand* operand is the dependency). Every `▹` keeps all tags visible in the final `Context<AppTags>`. | | `testEffectRunner.ts` (test helper, sibling of `testHistoryService.ts`) | `makeTestEffectRunner()` → `{ runner, adjust(duration), setTime(ms), dispose }` over one memoised `ManagedRuntime.make(EffectRunnerLive.pipe(Layer.provideMerge(TestClock.layer())))` (the TestClock is the *provider*; the runner captures it), so the worker under test and `TestClock.adjust` share one `TestClock`. | ### 2.2 Composition roots after Phase 11 ```mermaid flowchart TB Stores["StoresLive(stores) Config · SessionLocator · ProvidersConfigStore · SecretsStore · FileLeaseManager"] Runner["EffectRunnerLive (unsupervised, ref-bound) + AppFiberScopeLive (supervised, closed on dispose)"] Cross["CrossCuttingLive (desktop only) Policy · Telemetry · Experiments · Analytics · SessionTiming · DevTools · WorkspaceMcpOverrides · Backup"] Opts["CoreOptionsTag desktop: derived from CrossCutting · CLI: Layer.succeed(opts)"] Core["CoreLive PR 3: coarse CoreProjectionLive → PR 4: stages S1…S8 + CoreWiringLive"] Desk["DesktopLive — group Layers Browser · DesktopBridge · OAuth · Workers · TerminalEditor · Misc → DesktopWiringLive"] RT["AppRuntime = ManagedRuntime.make(AppLive) eager sync build · Context<AppTags> = oRPC effect/context · dispose() last"] Stores --> Runner --> Cross --> Opts --> Core --> Desk --> RT CLI["CLI root (xum run / xum workflow) createCoreServices(opts) = makeAppRuntime(CoreLive ▹ StoresFromCoreOptionsLive ▹ AppFiberScopeLive ▹ EffectRunnerLive ▹ succeed(CoreOptionsTag, opts))"] Core -.same Layer definitions.-> CLI ``` `ServiceContainer` keeps its public fields and the synchronous `new ServiceContainer(stores)`: the constructor calls `makeAppRuntime(AppLive(stores))`, stores `this.serviceContext = runtime.runSync(Effect.context<AppTags>())`, and assigns fields via `Context.get(this.serviceContext, Tag)`. `toORPCContext()` returns the same plain fields plus `"effect/context": this.serviceContext`. `initialize()` is untouched. `dispose()` follows §5. `createCoreServices(opts)` keeps its signature and return shape plus `runtime` and `appFiberScope` fields; `cli/run.ts:1574-1580` and `cli/workflow.ts:275-320` cleanup lists gain `closeScopeBounded(appFiberScope)` before `session.dispose()` and `disposeAppRuntime(runtime)` as the final step (PR 3). **Staged composition skeleton (PR 4 shape; direction matters):** ```ts // Each stage depends only on stages defined above it. `provideMerge` keeps both sides exposed. const S1 = Layer.mergeAll(HistoryLive, InitStateLive, ProviderLive, /* … true siblings only */); const S2a = Layer.mergeAll(SessionUsageLive, GoalLive, MemoryLive).pipe(Layer.provideMerge(S1)); const S2b = StreamManagerLive.pipe(Layer.provideMerge(S2a)); // StreamManager needs SessionUsage const S3 = AIServiceLive.pipe(Layer.provideMerge(S2b)); // … S4 … S8 likewise … export const CoreLive = CoreWiringLive.pipe(Layer.provideMerge(S8)); // wiring runs after every service exists ``` **oRPC typing.** `OrpcEffectServices` (in `effectContext.ts`) becomes `AppTags`, so `ORPCContext["effect/context"]: Context<AppTags>` is satisfied by the runtime context in production. `buildOrpcEffectContext` stays as the narrow test helper it already is (its only caller, `effectBridge.test.ts:24-30`, deliberately builds a partial context and casts it via `unknown`); no production caller remains after PR 1. ### 2.3 Invariants (the "DI contract"; enforced by tests and the `appRuntime.ts` doc comment) | # | Invariant | Constraint served | |---|---|---| | I1 | **Phase 11 compatibility contract, not permanent law:** layer bodies are synchronous (`Layer.succeed`/`Layer.sync`/`Layer.effect` over sync effects; `acquireRelease` with a sync acquire is fine). `makeAppRuntime` asserts the eager build completed. Future async resource acquisition belongs in `initialize()`/startup effects or an explicit async factory root (`ServiceContainer.create()`), never silently inside a layer. | #2 sync-start, #5 startup parity | | I2 | Services never hold the `ManagedRuntime`. Workers hold an `EffectRunner` (default `defaultEffectRunner`); `EffectRunner.runX` ≡ `Effect.run…With(ctx)` — same sync-start semantics as `Effect.runX`, and still valid after `runtime.dispose()`, so late callbacks cannot hit "ManagedRuntime disposed". Supervision, when needed, is explicit via `AppFiberScope`. | #2, #3 | | I3 | Per-call pipelines (`Effect.runPromise(this.effects…)` facades) and the `memoryConsolidationService` funnels are untouched. **Audit item:** no DI lookup, runner call, or `await` may be inserted before `inFlight.set` / `harvestInFlight.set`. Only lifecycle forks in workers move to `this.runner.runX`. | #1, #2 | | I4 | Constructors, facades, private methods, module exports unchanged; new constructor parameters are optional, trailing, defaulting to `defaultEffectRunner`. | #1, #6 | | I5 | Teardown order stays explicit in `dispose()`/`shutdown()`. Layer bodies and wiring layers register **no finalizers** in Phase 11 (`Effect.sync` only), so `runtime.dispose()` reorders nothing. The one supervised resource (`AppFiberScope`) is closed explicitly at a fixed position in `dispose()` (§5). | #3 | | I6 | Wiring layers replay today's setter/listener order; a constructor may touch only its *declared* dependencies (built earlier by staging). Per-PR audit: grep each moved constructor for calls on setter-provided collaborators → forbidden. Dependency order is expressed only with `provide`/`provideMerge` stages; never rely on `mergeAll` sibling order. | #6 | | I7 | No persisted-data changes; DI is in-process only. | #4 | | I8 | Every process root builds from the same Layer definitions (`CoreLive` shared by App and CLI). Unit harnesses (`createTestHistoryService`, `createTestToolConfig`, `createAgentSessionHarness`, …) intentionally bypass Layers. | #7 | ### 2.4 Decisions and alternatives (product-LoC deltas) <details> <summary>D1 — Granularity: coarse core first (PR 3), per-service core stages behind a decision gate (PR 4), group layers for the desktop tail (PR 5)</summary> Honest framing: the three unlocks (engine-core async scope, TestClock, app-lifetime scope) are delivered by `AppRuntime` + `EffectRunner` + `AppFiberScope` and **do not require per-service layers**. Per-service core layers are *migration leverage*: typed requirement sets for the engine-core work, per-service swap in integration tests, explicit dependency stages instead of implicit ordering. - **(A) Per-service everywhere** (~70 layers): +~900/−~700. Desktop tail has hand-tuned teardown that must not become finalizers, so per-service there buys uniformity only. Rejected. - **(B) Recommended:** PR 3 coarse `CoreProjectionLive` (+~120/−~10) delivers the shared root and runtime ownership; PR 4 peels the core into staged per-service layers (+~330/−~290) **only if** PR 3's typecheck/startup budgets hold (gate in §3); desktop tail as ~6 group layers (+~170/−~150). Tags for all services either way (~3 LoC each). - **(C) Coarse only:** stop after PR 3 + desktop projection (~+200 total). Cheapest; the engine-core phase would then redo dependency declarations. Remains the fallback if PR 4's gate fails. </details> <details> <summary>D2 — Async init stays an explicit `initialize()`; Layers construct only</summary> Folding `initialize()` into layer construction would make the build asynchronous (breaks I1), change failure semantics (today: fail-fast → dialog/log), and move the six-step order into memoised builds. Deferred; a later phase can turn `initialize()` into `runtime.runPromise(startupEffect)` with per-step `Effect.timeout`. </details> <details> <summary>D3 — Optional cross-cutting services stay optional via `CoreOptionsTag`, not `Effect.serviceOption`</summary> Core layer bodies read `opts.policyService` etc. exactly as today, so CLI (absent) vs desktop (present) behavior is unchanged and no service gains a new `undefined` branch. </details> <details> <summary>D4 — Two seams instead of one: `EffectRunner` (unsupervised, clock-bound) + `AppFiberScope` (supervised)</summary> A single "runtime handle" conflates two needs. Workers need *which clock* (TestClock) and must keep sync `stop()`; the engine core needs *who awaits me on shutdown*. Explicit `Clock` injection per worker was rejected (a `provideService(Clock.Clock, …)` at every fork site, and it does not extend to other refs). </details> <details> <summary>D5 — oRPC: `effect/context` = the runtime's `Context`; `handlerGen` unchanged</summary> `handlerGen` already `Effect.provide`s the context per request; providing ~70 entries instead of one is one Map merge per request. The existing `echoAsync`/`echoEffect` probes record the delta as a **diagnostic** in the PR body (no stable benchmark harness exists to make it a hard gate). `effect/wrap` not needed. </details> ## 3. Phasing — six stacked PRs Every PR: `make static-check`; gate suites below; existing tests unchanged (PR 6 is the only PR that edits tests, and only to replace real-timer probes). Before `@codex review`, run the **house pre-review audits**: 1. **Interruption posture** — list every new/moved fiber fork; state what interrupts it and when (unsupervised via `EffectRunner` + worker scope, or supervised via `AppFiberScope`). 2. **Uninterruptible teardown** — teardown effects are `Effect.uninterruptible` end-to-end; bounded waits inside use `Effect.interruptible(Effect.timeout(...))` (house shape from #4038). 3. **No defect escapes** — `disposeAppRuntime`/`closeScopeBounded` and every Promise facade fold defects; `makeAppRuntime` is the one place allowed to throw (constructor semantics). 4. **Spy-seam check** — `rg 'spyOn\(' src/node/services/<touched>.test.ts tests/` per touched class; constructor arity and private-method Promise signatures unchanged (typecheck of tests proves it). 5. **Sync-start check** — a fork through `EffectRunner` runs to its first `sleep` before `runFork` returns (mirrors `heartbeatService.ts:199-202`). 6. **Constructor side-effect audit (I6)** for every constructor moved into a Layer in that PR. 7. **Zero-suspension audit (I3)** whenever `memoryConsolidationService` is in the diff. ### PR 1 — Skeleton: AppRuntime + Stores/MemoryMeta layers + runtime-backed `effect/context` + dispose hook (+~150 LoC) **Scope** - `di/tags.ts` (`ConfigTag`, `SessionLocatorTag`, `ProvidersConfigStoreTag`, `SecretsStoreTag`, `FileLeaseManagerTag`, `MemoryMeta` moved from `orpc/effectContext.ts`, which re-exports it; `AppTags` union). - `di/layers/stores.ts` (`StoresLive`), `di/layers/core.ts` with `MemoryMetaLive = Layer.effect(MemoryMeta, Effect.map(ConfigTag, c => new MemoryMetaService(c.rootDir)))`, `di/layers/app.ts` (`AppLive(stores) = MemoryMetaLive ▹ StoresLive`). - `di/appRuntime.ts` (`makeAppRuntime`, `disposeAppRuntime`); `APP_RUNTIME_DISPOSE_TIMEOUT_MS` in `src/constants/`. - `coreServices.ts`: `CoreServicesOptions.memoryMetaService?` (precedent: `workspaceMcpOverridesService?`). - `serviceContainer.ts`: build runtime first, pass `Context.get(ctx, MemoryMeta)` to `createCoreServices`, `public readonly runtime`, `toORPCContext()["effect/context"] = this.serviceContext`, `dispose()` appends `disposeAppRuntime` behind a `disposed` latch; new `log.debug("[startup] AppRuntime built", { ms })`. - `orpc/effectContext.ts`: `OrpcEffectServices = AppTags`; `buildOrpcEffectContext` retyped/test-helper doc. - `headlessEnvironment.dispose` calls `await services.dispose()` before removing the temp dir (the bench harness currently leaks the container; runtime ownership starts here). **Acceptance** - `di/appRuntime.test.ts`: (a) sync build sets `cachedContext`; (b) a layer with an async body makes `makeAppRuntime` **throw synchronously** (I1 enforced); (c) probe layers' finalizers run in reverse order on dispose; (d) dispose is idempotent and bounded (hung finalizer → `warn`, resolves at the timeout); (e) `runtime.runFork` after the eager build starts synchronously. - `serviceContainer.test.ts`: `Context.get(toORPCContext()["effect/context"], MemoryMeta) === services.memoryMetaService`; `dispose()` closes the runtime; `dispose(); shutdown()` (tests/ipc order) is clean; a throwing layer surfaces as a synchronous throw from `new ServiceContainer(stores)` (same shape as today's constructor throw → existing entry-point catch paths). - `effectBridge.test.ts`, `memoryMeta*.test.ts` unchanged and green; echo-probe overhead recorded in the PR body. - Gate: `bun test src/node/services/di src/node/services/serviceContainer.test.ts src/node/orpc src/node/services/memoryMeta*` · `make test-integration` · `make static-check`. **Rollback:** `git revert`; classes untouched. ### PR 2 — Runtime seams: `EffectRunner` + `AppFiberScope`; TestClock on idleCompaction/heartbeat/retryManager (+~140 LoC) **Scope** - `di/effectRunner.ts`, `di/appFiberScope.ts`; `AppLive` gains `AppFiberScopeLive ▹ EffectRunnerLive` at the base; `ServiceContainer` exposes `appFiberScope` (used only by `dispose()` in Phase 11) and closes it per §5. - `IdleCompactionService`, `HeartbeatService`, `RetryManager`: trailing optional `runner: EffectRunner = defaultEffectRunner`; every lifecycle `Effect.runSync/runFork` in `start/stop/schedule/cancel` becomes `this.runner.runX`. Deadline math (`Date.now()`/injected `now`) unchanged. `ServiceContainer` passes `Context.get(ctx, EffectRunnerTag)` to the two workers; `RetryManager` keeps the default until PR 5 (so `streamManager.ts` is untouched here). - `di/testEffectRunner.ts` helper. **Acceptance** - New TestClock tests (existing real-timer tests untouched — they exercise the `defaultEffectRunner` path, which is production behavior wherever no runner is injected): heartbeat `STARTUP_DELAY_MS` → first tick after `adjust`, one tick per `CHECK_INTERVAL_MS`, no ticks after `stop()`; idleCompaction initial delay + cadence; retryManager fires exactly at `delayMs`, `cancel()` before `adjust` never fires. - Pin runtime facts: `runner.runSync(Scope.close(scope, Exit.void))` completes synchronously for a fiber suspended on a TestClock sleep; `runFork` through the runner reaches its first sleep synchronously; `Effect.context<never>()` inside `EffectRunnerLive` sees the upstream `TestClock` (else the helper provides `Clock.Clock` explicitly — same seam, one line). - `AppFiberScope` contract tests: (i) an **I/O-suspended** fiber (interruptible `Effect.async` that never resolves, with a cancel path) forked with `Effect.forkIn(_, appFiberScope)` is interrupted **and awaited** by `closeScopeBounded(appFiberScope)` — and this happens *before* the explicit teardown steps in `dispose()` (assert ordering against a spy on `desktopBridgeServer.stop`); (ii) a fiber forked via `EffectRunner` is *not* interrupted by either close (documents the asymmetry); (iii) `disposeAppRuntime` afterwards idempotently re-closes the already-closed child scope (no error, no second finalizer run). - If `TestClock.adjust` leaves continuations pending, the helper adds `Effect.yieldNow`/`Fiber.await` — decided by tests. - Gate: `heartbeatService.test.ts`, `idleCompactionService.test.ts`, `retryManager.test.ts`, `serviceContainer.test.ts`, `di/*`, tests/ipc. **Rollback:** revert restores defaults; no call site depends on the new params. ### PR 3 — Shared core root: coarse `CoreProjectionLive` + `createCoreServices` facade + CLI runtime disposal (+~120 / −~10) **Scope** - Tags for the remaining 19 core services; `CoreOptionsTag`; `StoresFromCoreOptionsLive`. - `CoreProjectionLive = Layer.effectContext(Effect.gen(function*(){ const opts = yield* CoreOptionsTag; const stores = yield* …; const core = buildCoreGraph({ ...opts, ...stores }); return Context.make(History, core.historyService).pipe(Context.add(...)) }))` where `buildCoreGraph` is today's `createCoreServices` body, unchanged, renamed. - `createCoreServices(opts)` = `makeAppRuntime(CoreProjectionLive ▹ StoresFromCoreOptionsLive ▹ AppFiberScopeLive ▹ EffectRunnerLive ▹ Layer.succeed(CoreOptionsTag, opts))`, returns today's `CoreServices` object read from the context plus `runtime` and `appFiberScope`. `cli/run.ts` and `cli/workflow.ts` cleanup lists append `closeScopeBounded(appFiberScope)` **before** `session.dispose()` and `disposeAppRuntime(runtime)` **after** `backgroundProcessManager.terminateAll()`. - `ServiceContainer` stops calling `createCoreServices`; `AppLive = CoreProjectionLive ▹ CoreOptionsFromDesktopLive ▹ CrossCuttingLive ▹ …` (cross-cutting services move into `CrossCuttingLive` now because core options derive from them). Desktop constructions otherwise stay in the constructor. **Acceptance** - Identity test: every `CoreServices` field `===` `Context.get(ctx, Tag)`; `serviceContainer.test.ts` unchanged and green. - **Decision gate for PR 4** recorded in the PR body: `make typecheck` wall time, `[startup] AppRuntime built` ms and `initialize` totals vs `origin/main` baseline from the sandbox (§7). Proceed to PR 4 only if typecheck regresses < 10 % and startup within noise; otherwise stop at (C). - Gate: `bun test src/node/services`, `src/cli/*.test.ts` (run/workflow/server/cli), tests/ipc, `make static-check`. **Rollback:** revert restores the imperative call; PR 1/2 unaffected. ### PR 4 — Peel the core into staged per-service Layers + `CoreWiringLive` (+~330 / −~290 ⇒ net ≈ +40; split 4a/4b if > ~600 diff lines) **Scope** - Stages S1, S2a, S2b, S3…S8 (§2.1 + skeleton in §2.2) as `Layer.effect` adapters with today's argument lists; `CoreWiringLive` (`Effect.sync` only) replays the wiring lines in order; `CoreLive = CoreWiringLive.pipe(Layer.provideMerge(S8))` replaces `CoreProjectionLive`; `buildCoreGraph` deleted. - Before writing any stage: re-derive the DAG from the constructor argument lists (the plan's stage table was checked once; `StreamManager → SessionUsage` is the kind of edge that turns "siblings" into a stage split) and record it in the PR body. - 4a (S1–S3: leaves through `AIService`) / 4b (S4–S8 + wiring) if needed — 4a alone is mergeable because the remaining services are built by a shrunken projection layer that reads S1–S3 from the context. **Acceptance** - Wiring assertions that are behavioral (a missing wiring line fails them): `turnRequestBuilderBindings` fully populated; goal continuation consumer registered on `idleDispatcher`; `streamManager` MCP manager set; registration probe installed on `extensionMetadata`. - I6 audit table for all 19 constructors in the PR body; missing-provider = compile error (R must be `never` at `makeAppRuntime`) demonstrated by a type-level test (`// @ts-expect-error`). - Gate: as PR 3 plus `streamManager*.test.ts`, `aiService.test.ts`, `workspaceService*.test.ts`. **Rollback:** revert to PR 3's projection. ### PR 5 — `DesktopLive` group layers + `DesktopWiringLive`; thin `ServiceContainer`; `StreamManager` runner param (+~170 / −~150 ⇒ net ≈ +20) **Scope** - Tags for the 45 desktop services; six group layers (`Layer.effectContext`, today's construction order inside each; `provideMerge` between groups that depend on each other); `DesktopWiringLive` (`Effect.sync` only) = `serviceContainer.ts:209, 263-265, 271, 288-290, 334-340, 348, 365, 375, 381-382, 434, 438-471, 474-574` in order. - `ServiceContainer` constructor = `makeAppRuntime(AppLive(stores))` + field assignment from the context. `toORPCContext()` unchanged in shape. - `StreamManager`: optional trailing `runner: EffectRunner`; `schedulePartialWrite` fork (`streamManager.ts:1141`) and `RetryManager` construction use it; `Scope.close` stays `Effect.runFork` (existing async-close precedent). `WorkersLive` receives `EffectRunnerTag`. **Acceptance** - All four existing `serviceContainer.test.ts` assertions unchanged; new identity test over `toORPCContext()` fields vs tags; `dispose()`/`shutdown()` call order asserted via spies on the *public* methods already spied today. - I6 audit for the 45 constructors. - Gate: tests/ipc + tests/ui (`make test-integration`), `src/cli/server.test.ts`, `src/cli/cli.test.ts`, `streamManager*.test.ts`, `aiService.test.ts`. ### PR 6 — TestClock adoption sweep + shutdown hardening + contract docs (+~20 LoC product; tests edited) **Scope** - Replace real-sleep cadence probes with `makeTestEffectRunner()` in `heartbeatService.test.ts`, `idleCompactionService.test.ts`, `retryManager.test.ts`, and the partial-write debounce cases of `streamManager.test.ts`; keep **one real-timer smoke test per worker** (guards the `defaultEffectRunner` path). - `cli/server.ts`: `[shutdown]` log lines per step incl. `AppRuntime disposed {ms}`; confirm the whole `dispose()` fits the existing 5 s force-exit budget. - Finalize the contract doc comment in `di/appRuntime.ts` (I1–I8, §5). **Acceptance:** converted suites have zero `setTimeout`-based cadence waits (grep in PR body), same assertions; `make test-integration` green; sandbox startup/shutdown evidence (§7). ## 4. TestClock story - **Mechanism.** `Effect.sleep`, `Schedule.fixed`, `Effect.timeout`, `Clock.currentTimeMillis` read the `Clock` reference from the running fiber's context. Workers that fork through an `EffectRunner` built under `TestClock.layer()` run on the test clock; `await testRunner.adjust("2 minutes")` advances it. `Date.now()`, `setTimeout`, `setInterval` are unaffected — heartbeat deadline math via injected `now`, `AgentStatusService`'s ref'd `setInterval`, and `backgroundProcessManager` stay on real timers/injected timestamps. - **Benefit now:** `heartbeatService.test.ts` (6), `idleCompactionService.test.ts` (2), `retryManager.test.ts` (3 `setSystemTime` → `adjust`; `Date.now`-based `retryAt` may move to `Clock.currentTimeMillis` only if a test needs both clocks aligned), `streamManager.test.ts` debounce cases (7). - **Deferred:** `streamBridge.test.ts` ticker (11) — needs a context/runner parameter on `subscriptionIterable`; OAuth device-flow polling and `oauthFlowManager.test.ts` (25) — non-goal. - **Stays real:** child-process/PTY/WASM/fs-lock waits (`backgroundProcessManager` 72, `quickjsRuntime` 26, lock sleeps in `workspaceService`/`taskService`), end-to-end suites (tests/ipc, e2e). - **Pinned in PR 2, not assumed:** `adjust` runs due sleeps and their synchronous continuations before resolving (or the helper yields until they do); `Schedule.fixed` anchoring under `TestClock` matches the wall-clock expectations in `heartbeatService.ts:149-155`; sync `Scope.close` of a TestClock-suspended fiber completes synchronously. ## 5. Shutdown protocol 1. **Trigger points unchanged:** `main.ts` `before-quit` (preventDefault → `dispose()` raced with 5 s → `app.quit()`; update-install path fire-and-forget), the second `before-quit` listener's `shutdown()` (unchanged, concurrent), `cli/server.ts` SIGINT/SIGTERM (5 s force exit), ACP `close()`, tests/ipc (`dispose()` then `shutdown()`), headless bench (`dispose()` from PR 1). 2. **`ServiceContainer.dispose()` order:** 1. `backgroundProcessManager.beginShutdown()` — unchanged, first (latch protecting persisted monitor records). 2. **`closeScopeBounded(appFiberScope, APP_FIBER_SCOPE_CLOSE_TIMEOUT_MS)`** — interrupts and awaits supervised fibers *while every dependency they might touch during finalization is still alive*. No occupants in Phase 11; the position is fixed now so the engine-core phase does not have to re-derive it. 3. The existing explicit sequence verbatim (`desktopBridgeServer.stop()` … `terminateAll()` … `timelineService.flush()`). 4. **`disposeAppRuntime(runtime, APP_RUNTIME_DISPOSE_TIMEOUT_MS)`** — closes the runtime scope (interrupts any fiber started via `runtime.runX` — none long-lived in Phase 11; runs layer finalizers — none in Phase 11 by I5). Hung → `warn` at the timeout; never rejects. Budget: 2 s + 2 s inner bounds inside the callers' 5 s outer budgets; the outer race in `main.ts` remains the last line of defense. **Rule for future occupants:** anything forked into `AppFiberScope` must tolerate interruption at any suspension point and must not depend on resources torn down in step 1; anything that needs a Layer finalizer must first prove reverse-construction order is compatible with steps 2–3 (I5). 3. **Latches:** `disposed` makes `dispose()` idempotent (two `before-quit` listeners, tests/ipc dispose+shutdown). `shutdown()` never touches the runtime or `AppFiberScope`. 4. **Late callers:** `EffectRunner` handles keep working after runtime dispose (I2), so a stray `tick()`/`scheduleRetry()` after quit cannot defect. The `ManagedRuntime` is referenced only by `ServiceContainer` and the `createCoreServices` return value. 5. **Worker `stop()` stays synchronous** (`runner.runSync(Scope.close)`) because their fibers suspend only on the clock. The engine core will fork into `AppFiberScope` (step 2.2 awaits it) — the reason both seams exist now. 6. **Crash paths:** unchanged — `uncaughtException`/SIGKILL run no finalizers. Finalizers are best-effort; durable state must remain crash-safe without them (AGENTS.md self-healing rule). Nothing in Phase 11 makes a finalizer the sole guardian of durable state. ## 6. Risk register | # | Risk | L/I | Mitigation | |---|---|---|---| | R1 | A layer body suspends → `runSync` throws at startup | M/H | I1 assert + PR 1 test (b); doc comment; review checklist; entry-point catch paths verified in PR 1 | | R2 | Construction-order side effects differ under staged builds | L/H | I6 audit per moved constructor; explicit `provideMerge` stages; wiring layers replay today's order; tests/ipc as behavioral gate | | R3 | Double teardown (`shutdown()` ∥ `dispose()`; dispose+shutdown in tests) | M/M | `disposed` latch; runtime/AppFiberScope closed only in `dispose()`; PR 1 test | | R4 | Late `runtime.runX` after dispose → defect | M/M | I2: services hold `EffectRunner`, never the ManagedRuntime | | R5 | TestClock semantics differ from assumptions | M/L | PR 2 pins them before any suite converts; per-suite fallback to real timers | | R6 | effect v4 RC churn (`Context`→`ServiceMap`, Layer renames) | M/M | All `Layer/Context/ManagedRuntime/TestClock` imports confined to `di/`; exact pin | | R7 | Startup latency regression (splash) | L/M | `AppRuntime built` ms + `initialize` totals vs baseline in sandbox; PR 3 gate | | R8 | Typecheck slowdown from large requirement unions | L/L | PR 3 gate records `make typecheck` wall time; fallback (C) | | R9 | Per-request `Effect.provide` of a ~70-entry Context | L/L | echo-probe diagnostic in PR 1/5 bodies | | R10 | Spy seams / direct-construction tests break | L/H | I4; optional trailing params; audit 4; typecheck of tests | | R11 | CLI roots forget to dispose runtime/scope | M/L | PR 3 wires both cleanups; `src/cli/*.test.ts` assert the cleanup steps exist | | R12 | Someone forks long-lived I/O work via `EffectRunner` expecting dispose to await it | M/M | Doc on `EffectRunner` ("unsupervised"); PR 2 asymmetry test; review audit 1 | **Rollback:** PRs are stacked; revert in reverse order (6→1). Service classes are never modified except for optional trailing params, so any revert restores the previous composition root wholesale with no data or API implications. ## 7. Dogfooding (per PR; evidence attached to the PR body) **Environment (headless Coder host, no `DISPLAY`):** ```bash XUM_LOG_LEVEL=debug DEV_SERVER_SANDBOX_ARGS="--clean-projects" make dev-server-sandbox # background bash task; prints URL + XUM_ROOT ``` - **Startup correctness:** `<XUM_ROOT>/logs/*.log` shows, in order: `Loading services...`, `[startup] AppRuntime built {ms}`, `[startup] ServiceContainer.initialize starting`, six step durations, `[startup] ServiceContainer.initialize completed {totalMs, stepDurationsMs}`. Paste baseline (`origin/main`) vs branch numbers. - **Startup-never-crash parity (once, locally, not committed):** inject a throwing scratch layer → `xum server` exits non-zero with the existing logged error and **no** unhandled-rejection trace; for desktop, confirm by code path (`loadServices()` rejects → `main.ts:1255` dialog) and via `src/cli/server.test.ts`/ACP tests. - **UI smoke (agent-browser):** `open <url>` → `snapshot -i` → add a scratch git repo as a project → create a workspace → send one message → `screenshot` the loaded app and the response; `attach_file` both. **Video:** start `agent-browser record` before the flow and stop it with a hard timeout (`timeout 30 agent-browser record stop`); if stopping hangs (known), attach the truncated WebM plus the screenshots and say so. - **oRPC Effect path:** pin/unpin a memory entry (rides `handlerGen` + runtime `effect/context`); screenshot before/after; grep logs for `ManagedRuntime disposed`/defect lines (expect none). - **Graceful quit:** record the terminal with `script -q /tmp/<workspace>-shutdown.log` (or `agent-tty` if present), `kill -TERM <pid>` → expect `[shutdown]` lines, `AppRuntime disposed {ms}`, exit 0, no force-exit message; attach the typescript. Exercise the timeout branch once with a scratch hung finalizer → `warn` + timely exit. - **Electron (best effort):** with `Xvfb`, `make dev` + agent-browser via CDP (electron skill): screenshot splash → main window, quit via menu, confirm exit < 5 s; otherwise state that the Electron path is covered by `tests/e2e` in CI and the shared `dispose()` path exercised by `server.ts`. **Gate suites per PR** (plus `make static-check` always): | PR | Must pass | |---|---| | 1 | `src/node/services/di/*`, `serviceContainer.test.ts`, `src/node/orpc/*`, `memoryMeta*`, `make test-integration` | | 2 | + `heartbeatService.test.ts`, `idleCompactionService.test.ts`, `retryManager.test.ts` | | 3 | + `bun test src/node/services`, `src/cli/*.test.ts`; record PR 4 gate numbers | | 4 | + `streamManager*.test.ts`, `aiService.test.ts`, `workspaceService*.test.ts` | | 5 | + tests/ui via `make test-integration`, `src/cli/server.test.ts`, `src/cli/cli.test.ts` | | 6 | converted suites + full `make test-integration` + sandbox startup/shutdown evidence | ## 8. Non-goals (explicit) - streamManager ENGINE CORE conversion (first `AppFiberScope` occupant; separate phase). - `Schema` at persistence boundaries; OAuth refresh/device-flow workers; `AgentStatusService` `setInterval` → Effect. - `initialize()` as a Layer/startup effect (D2); per-service optional tags (D3); `streamBridge` on the runtime; layer finalizers for existing `dispose()` steps. - Any change to persisted data, IPC wire shapes, or oRPC handler bodies beyond the `effect/context` source. ## 9. Assumptions stated - `Effect.context<never>()` inside `EffectRunnerLive` returns the enclosing build context including an upstream `TestClock` entry (PR 2 test; fallback: provide `Clock.Clock` explicitly in the helper). - `Scope.fork(parent)` inside a `Layer.effect` body yields a child closed by the runtime's layer scope on `dispose()` (PR 2 `AppFiberScope` test). - Layer bodies never need to observe sibling construction order; all ordering that matters is expressed as `provide`/`provideMerge` stages or wiring-layer statement order. - `EffectRunner`'s `R = never` constraint is sufficient for every lifecycle fork in the three Phase 11 workers and `StreamManager.schedulePartialWrite` (they only use `Effect.sleep`/`Schedule`/`Effect.sync`/`Effect.tryPromise` — no service tags). Verified by typecheck in PR 2/5. - The desktop tail's teardown remains explicit unless a later RFC proves reverse-construction order compatible; this plan does not attempt it. </details> --- _Generated with `xum` • Model: `anthropic:claude-fable-5-1` • Thinking: `xhigh` • Cost: `$45.57`_

🤖 refactor: convert streamManager resource seams to Effect (temp-dir …

9db1eda

…acquireRelease + scope-tied debounce fiber)

This comment has been minimized.

Sign in to view

ThomasK33 added this pull request to the merge queue Sep 1, 2026

Merged via the queue into main with commit 7404cb1 Sep 1, 2026
36 of 38 checks passed

ThomasK33 deleted the effect-phase10-streammanager-seams branch September 1, 2026 19:06

ThomasK33 mentioned this pull request Sep 2, 2026

🤖 refactor: Effect Phase 11 PR 1 — ManagedRuntime skeleton (AppRuntime + Stores/MemoryMeta layers + runtime-backed effect/context) #4049

Merged

Navigation Menu

Uh oh!

🤖 refactor: convert streamManager resource seams to Effect (temp-dir acquireRelease + scope-tied debounce fiber) - #4040

🤖 refactor: convert streamManager resource seams to Effect (temp-dir acquireRelease + scope-tied debounce fiber)#4040
ThomasK33 merged 1 commit into
mainfrom
effect-phase10-streammanager-seams

ThomasK33 commented Sep 1, 2026

Uh oh!

This comment has been minimized.

ThomasK33 commented Sep 1, 2026

Uh oh!

ThomasK33 commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

ThomasK33 commented Sep 1, 2026

Summary

Background

Implementation

Seam 1 — Temp-dir lifecycle → per-resource Effect.acquireRelease (converted)

Seam 2 — Partial-write debounce → scope-tied fiber (converted)

Seam 3 — Error categorization / lostResponseIds (stayed plain, documented)

Seam 4 — Usage accounting (stayed plain, documented; zero-suspension invariant pinned in comments)

Interruption-posture audit

What stayed out (per charter)

Validation

Risks

Wave 2 completion state

Uh oh!

This comment has been minimized.

ThomasK33 commented Sep 1, 2026

Uh oh!

ThomasK33 commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Seam 1 — Temp-dir lifecycle → per-resource `Effect.acquireRelease` (converted)

Seam 3 — Error categorization / `lostResponseIds` (stayed plain, documented)