Skip to content

πŸ€– refactor: Effect Phase 11 PR 6 β€” TestClock sweep, per-step [shutdown] timing, DI contract docs - #4062

Merged
ThomasK33 merged 5 commits into
mainfrom
effect-phase11-pr6-testclock-hardening
Sep 2, 2026
Merged

πŸ€– refactor: Effect Phase 11 PR 6 β€” TestClock sweep, per-step [shutdown] timing, DI contract docs#4062
ThomasK33 merged 5 commits into
mainfrom
effect-phase11-pr6-testclock-hardening

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

Effect migration Phase 11, PR 6 of 6 β€” the phase's closing PR, and the only one allowed to edit existing tests. (1) TestClock sweep: the real-timer probes that actually waited on a worker's clock now run on a TestClock through the workers' injected EffectRunner (makeTestEffectRunner()), with exactly one default-runner smoke per worker guarding the production (real-clock) path. (2) Shutdown hardening: every step of ServiceContainer.dispose(), the xum server signal handler and the CLI roots' cleanup lists now writes a [shutdown] <step> {ms} debug line (new shutdownStep helper, no suspension added between synchronous steps), which made the long-standing "SIGTERM ~6 s after startup exits ~11 s later" gap measurable β€” and root-caused it to a spot outside dispose() (below). (3) DI contract: the di/appRuntime.ts module comment is now the durable contract β€” invariants I1–I8, the two-seam asymmetry, the Β§5 shutdown order, the rule for future AppFiberScope occupants, the recorded Layer-machinery costs, and the R6 firewall.

Stacked on PR 1 #4049, PR 2 #4050, PR 3 #4051, PR 4a #4054, PR 4b #4057, PR 5 #4061 (all on main). Plan: <details> at the bottom (Β§3 "PR 6", Β§4 TestClock story, Β§5 shutdown protocol, Β§2.3 invariants, Β§7 dogfooding).

Implementation

  • retryManager.test.ts β€” the hand-rolled setTimeout/clearTimeout spy harness (runNextTimer(), scheduledTimers) is gone; every timing case drives the backoff with clock.adjust(...) on a makeTestEffectRunner() passed as the 4th ctor arg. "Timer pending / no timer pending" assertions became isRetryPending plus a negative adjust far past any backoff (a still-armed retry would fire). PR 2's separate retryManager.testClock.test.ts is folded in (its three cases are now the main suite's "exactly at the backoff delay", "cancel clears pending retry timer" and "reschedules…" cases) and deleted. One default-runner smoke remains: it intercepts the real setTimeout registration once to prove the default runner's sleep lands on Effect's default clock with the backoff delay and fires onRetry β€” without a 2 s wall-clock wait. setSystemTime stays: it only pins Date.now() for the scheduledAt equality and never drove timing.
  • streamManager.test.ts β€” "interrupts a pending debounced partial write when the stream ends" (the one real cadence wait: sleep(throttleMs + 200) β‰ˆ 720 ms) now injects a TestClock runner (5th ctor arg, PR 5's template) and proves the negative with adjust(2 Γ— throttle) β€” 47 ms. A new default-runner smoke ("a debounced partial write arms a real setTimeout through the default runner", β‰ˆ 7 ms; intercepts the real timer registration like the RetryManager smoke, so there is no wall-clock window to overrun) replaces the real-clock coverage it took away.
  • idleCompactionService.test.ts β€” gains the missing default-runner smoke (start() β†’ nothing sweeps within 20 ms of a 60 s initial delay β†’ synchronous stop()); its testClock sibling's doc comment claimed the real-timer suite covered the default runner, but that suite never called start(). heartbeatService.test.ts β€” unchanged except a comment marking "startup does not fire heartbeats immediately" as the retained smoke (see "What the plan's counts meant").
  • shutdownStep.ts (new) β€” shutdownStep(name, run): writes [shutdown] <name> starting before run(), times it, and writes [shutdown] <name> {ms} on completion, all at debug level β€” so a hung step (an awaited disposer that never settles or a blocking synchronous call) is named by the last line before silence. Overloads: a thenable-returning step (thenable check, not instanceof Promise, so a cross-realm promise is still awaited) is awaited and logged via .finally; a synchronous step is logged before returning with no Promise created, so wrapping one adds no suspension point and adjacent synchronous teardown statements still run on the same tick (audit 2). Errors propagate unchanged. The Promise overload is declared first because Promise<void> is assignable to void; @typescript-eslint/no-misused-promises guards the other direction. shutdownStep.test.ts pins the sync-no-Promise / thenable-awaited / error-propagation contract.
  • serviceContainer.ts β€” disposeOnce() wraps each of its 21 explicit steps; closeScopeBounded/disposeAppRuntime keep their own lines; [shutdown] ServiceContainer.dispose starting/completed {totalMs} bracket the sequence. Order byte-identical (asserted by the PR 5 order test). cli/server.ts β€” terminalService.closeAllSessions and serverService.stopServer are timed and a final [shutdown] exiting {totalMs} is the last JS-side line before process.exit(0). cli/runCleanup.ts β€” the loop times each step; cli/workflow.ts β€” disposeWorkflowResources now builds the same kind of step list and runs it through runBestEffortCleanup (same containment as its former eight try/catch blocks; warn wording is now xum workflow: cleanup step failed: <step>).
  • di/appRuntime.ts β€” module doc comment rewritten as the contract (details in "PR 6 notes").

PR 6 notes

Suite wall time, before β†’ after (bun test, 3 runs each, same loaded host: 96 cores, load β‰ˆ 145, CPU PSI some avg60 β‰ˆ 33 %)

suite before (wall, min–max) after tests note
streamManager.test.ts 2.47–2.59 s 1.69–1.78 s 122 β†’ 123 the 720 ms real wait ("interrupts a pending debounced partial write…") is now 47 ms on virtual time; +7 ms default-runner smoke
retryManager.test.ts (+ deleted retryManager.testClock.test.ts) 0.31–0.43 s (+ β‰ˆ0.3 s for the separate file) 0.35–0.42 s 14 + 3 β†’ 15 already fake-timer; the value is one harness (TestClock) instead of two, and one file instead of two
idleCompactionService.test.ts 0.69–0.73 s 0.72–0.78 s 19 β†’ 20 +20 ms default-runner smoke that did not exist
heartbeatService.test.ts 1.78–1.93 s 1.76–1.77 s 74 untouched (comment only)

Grep in the converted files (acceptance): streamManager.test.ts no longer waits setTimeout(…, throttleMs + …); retryManager.test.ts has no spyOn(globalThis, "setTimeout") outside the single smoke and no runNextTimer.

What the plan's probe counts meant on inspection (deviations)

The plan's Β§1 counts (heartbeat 6 Β· idleCompaction 2 Β· retryManager 3 Β· streamManager 7) came from a setTimeout/setSystemTime/fixture grep. Reading each site:

  • heartbeat "6" β€” one is the clock-cadence probe ("startup does not fire heartbeats immediately", 100 ms) and is exactly the default-runner smoke to keep; the other five are waitForCondition polls and 20 ms settles on Promise chains (tick()β†’resyncFromConfigβ†’queue) plus one 300 ms mock dispatch delay whose assertion is Date.now() deadline math β€” plan Β§4 "stays real: injected timestamps". None waits on the scheduler fiber's clock, so a TestClock changes nothing there. Left as is.
  • idleCompaction "2" β€” both are Promise-settlement waits; the real-timer suite never called start(), so there was no default-runner smoke to keep β€” added one.
  • retryManager "3" β€” the setSystemTime calls pin Date.now() for scheduledAt; the actual timer surrogate was the global setTimeout spy harness, which is what the TestClock replaces. setSystemTime stays (no test needs the two clocks aligned, so scheduledAt stays on Date.now()).
  • streamManager "7" β€” seven fixtures set lastPartialWriteTime inside the throttle window, but they call attachWorkflowRunToToolCall/appendPartAndEmit(…, false), which flush immediately and never wait on the debounce. The single real cadence wait was the 720 ms scope-interrupt probe β€” converted.
  • agentSession* harness tests (optional in the plan): their sleeps wait on waitForStartupAutoRetryRerunWindow (plain setTimeout inside agentSession.ts) and Promise settlement, not on RetryManager's clock β€” not convertible through a stream-manager double; skipped.
  • Red check on the converted scope-interrupt probe: with the forkIn(resourceScope) branch replaced by a plain runFork, both the original 720 ms version and the TestClock version still pass β€” the stream-end path also calls interruptPartialWriteFiber directly, so the scope close is a second guard. Same discriminating power before and after (recorded, not changed: the test's purpose is "no late write", which it does prove).

The "SIGTERM ~6 s after startup exits ~11 s later" gap β€” root cause and disposition

Per-step lines make it unambiguous. xum server (node dist/cli/index.js server, temp XUM_ROOT, XUM_LOG_LEVEL=debug, under script -q -e -f), 5 runs each:

SIGTERM sent JS teardown (Shutting down server... β†’ [shutdown] exiting) SIGTERM β†’ process gone exit code
6 s after initialize completed (β‰ˆ 8.4 s after spawn) 64–67 ms (dispose 61–64 ms) 10.57 / 10.89 / 10.83 / 10.64 / 10.75 s 0 Γ—5
30 s after initialize completed 79–85 ms (dispose 76–82 ms) 171 / 161 / 183 / 150 / 168 ms 0 Γ—5

In every 6 s run the last JS-side line ([shutdown] exiting { totalMs: 67 }) is printed β‰ˆ 70 ms after SIGTERM; the process then lingers β‰ˆ 10.7 s after process.exit(0). Nothing inside dispose() (every step 0–16 ms; AppRuntime disposed 6–16 ms), nothing in serverService.stopServer() (1 ms β€” PR 2's guess was wrong; PR 5's strace was right).

Cause: workerPool.ts creates the tokenizer Worker at import time (β‰ˆ 1.9 s after spawn in xum server); the worker evaluates ai-tokenizer/encoding (31 MB of encodings), which takes β‰ˆ 18 s on this host (require("ai-tokenizer/encoding"): 17.9 / 18.1 / 18.1 s standalone). process.exit() terminates worker threads via V8 TerminateExecution, which cannot interrupt a parse/compile in progress, so the main thread joins the worker until its current module finishes. A controlled repro (new Worker(tokenizer.worker.js) + process.exit at t; unref'd, no other work):

process.exit at process gone at wait
1 s 3.1 s 2.1 s
4 s 5.0 s 1.0 s
7 s 17.3 s 10.3 s
10 s 17.5 s 7.4 s
13 s 17.6 s 4.5 s
16 s 17.1 s 1.0 s

i.e. one β‰ˆ 10 s uninterruptible window from β‰ˆ 7 s to β‰ˆ 17 s into the worker's load (a single huge encoding module). SIGTERM 6 s after init lands β‰ˆ 6.5 s into that load β†’ β‰ˆ 10.7 s wait; at 30 s the worker is long done β†’ 150–180 ms. Disposition: not a leak inside dispose()'s scope (the plan's fix criterion), pre-existing on main (PR 2/5 saw the same numbers), and not DI-related β†’ recorded as a follow-up, not fixed here. Follow-up options: create the tokenizer worker lazily on first run() (an idle xum server/ACP never pays), or split the worker's encoding import per model (encoding[model.encoding] is already selected per call) so the uninterruptible window is one encoding, not all of them. The desktop is unaffected in practice (it imports the tokenizer first and shuts down long after startup).

Two smaller observations from the transcripts (both pre-existing, both left alone): (a) a one-time 25–45 ms gap right after [shutdown] AppFiberScope closed is source-map-support (registered by cli/server.ts) mapping Effect-internal frames the first time the log helper captures a stack inside a fiber β€” verified standalone: first in-fiber new Error().stack 47.8 ms with source maps vs 0.3 ms after / 0.3 ms without; not teardown work. (b) In the CLI roots' lists appFiberScope.close/appRuntime.dispose are timed by runBestEffortCleanup and log their own … closed/… disposed line (boundedTeardown); kept uniform rather than special-casing two steps β€” the outer line adds the Promise-settle time.

Whole-dispose() latch and both bounded teardowns: unchanged and re-asserted (serviceContainer.test.ts "shares one teardown across concurrent dispose() calls", appRuntime.test.ts timeout/never-rejects cases, the PR 5 order test).

Pre-review audits (plan Β§3 preamble)

  1. Interruption posture β€” unchanged: no new fibers or forks in product code; shutdownStep creates none. The TestClock suites fork the same effects through a TestClock-bound runner.
  2. Uninterruptible teardown β€” boundedTeardown untouched. In disposeOnce() synchronous steps are timed without a Promise (no new suspension point); async steps get one .finally microtask after an await that already existed. Order asserted unchanged.
  3. No defect escapes β€” shutdownStep rethrows after logging (containment unchanged: disposeOnce propagates as before, runBestEffortCleanup contains as before); log.debug cannot throw (safePipeLog catches). Both bounded teardowns still never reject.
  4. Spy-seam check β€” rg 'spyOn\(' src/node/services/serviceContainer.test.ts tests/: the same public methods are spied (desktopBridgeServer.stop, desktopSessionManager.closeAll, browserBridgeServer.stop, analyticsService.dispose, timelineService.flush, telemetryService.shutdown) and shutdownStep calls them on the instance, so every spy intercepts (56/56 in the container + CLI suites). No constructor arity changed; the converted tests use the existing optional trailing runner params.
  5. Sync-start β€” still pinned by the converted suites (isRetryPending true and partialWriteFiber defined synchronously after the scheduling call, before any adjust) and directly by di/effectRunner.test.ts.
  6. / 7. N/A (no constructor moved; memoryConsolidationService not in the diff).

Phase 11 completion state

  • The DI graph now owns: every service in the process (5 stores β†’ EffectRunner/AppFiberScope β†’ MemoryMeta β†’ 8 cross-cutting β†’ 19 core layers in 8 stages + CoreWiringLive β†’ 6 desktop group layers + DesktopWiringLive), built once per process by one ManagedRuntime (AppLive for desktop/xum server/ACP/tests-ipc; CoreRootLive for xum run/xum workflow); the oRPC effect/context; the two runtime seams (EffectRunner in the three clock-driven workers and StreamManager/RetryManager; AppFiberScope with its fixed dispose slot and bounded close); startup/shutdown observability ([startup]/[shutdown] lines). Product LoC for the whole phase (pre-PR 1 β†’ this branch, non-test): di/ +2327 (tags 389, layers 1571, runtime/seams/helper β‰ˆ 370), composition roots +360/βˆ’851, workers/stream/CLI/misc +237/βˆ’89 β€” net β‰ˆ +1.98k, well above the plan's β‰ˆ +420 estimate (the per-service tags and the layer adapters that restate every constructor call are the bulk; each PR body recorded its actual diff). Service classes untouched except optional trailing runner params.
  • Still imperative (explicit non-goals, D2/Β§8): ServiceContainer.initialize() (six awaited initialize()s + three start()s β€” a future runtime.runPromise(startupEffect) with per-step Effect.timeout); streamBridge.ts streams on the global runtime (needs a runner/context parameter on subscriptionIterable, which would also let streamBridge.test.ts's 11 ticker waits move to a TestClock); the hand-ordered dispose()/shutdown() steps (layer finalizers would require proving reverse-construction order compatible β€” I5); AgentStatusService's ref'd setInterval; OAuth device-flow polling.
  • First AppFiberScope occupant (next phase): the streamManager engine core β€” fork the per-stream engine fiber into AppFiberScope so dispose() step 2 interrupts and awaits in-flight streams while historyService/sessionUsage are still alive; the position, bound (APP_FIBER_SCOPE_CLOSE_TIMEOUT_MS), asymmetry tests and occupant rules are in place, so that phase does not have to re-derive shutdown.

Validation

  • make static-check green (typecheck both projects, prettier, eslint, docs).
  • Converted/touched suites: retryManager 15/15, streamManager 123/123, idleCompactionService 20/20 + testClock 2/2, heartbeatService 74/74 + testClock 2/2, serviceContainer + runCleanup + workflow + server + cli 56/56, di/*.
  • Full local gate on this host: bun test src 13 900 pass / 12 fail / 8 skip (834 files, 1045 s) β€” the 12 are exactly the known host baselines (taskGitPatchEngine Γ—2, gitNoHooksEnv Γ—3, WorkspaceTurnManager Γ—2, agent_skill_delete, BackupRepoCache, WorkspaceFooterBar load flake Γ—3), none in files this PR touches. TEST_INTEGRATION=1 bun x jest tests 677 pass / 75 fail / 49 skip (122 suites, 1298 s) β€” the same environment baseline as PR 5 (669/77/49): every failing suite is provider-backed (403 Forbidden from the AI bridge / no xAI key: tests/ipc/streaming/*, providers/*, workspace/fork|init, acp.integration, run/smoke, nameGeneration Γ—2, runtime/*), one of the four src/**/__tests__ bun:test files jest picks up, or the known terminal.test.ts (1) / sendModeDropdown.test.ts (1) rows; CI Test / Integration (real keys) is green on this head. CI round 1: Test / Integration failed only on tests/ipc/providers/anthropicCacheStrategy.test.ts ("Expected cache creation but got 0 tokens" β€” a live-provider cache-token assertion unrelated to this diff).
  • Dogfooding (headless Coder host): the two xum server SIGTERM matrices above (exit 0 Γ—10, every [shutdown] line present in every transcript); xum workflow echo run from the branch β€” AppRuntime built β†’ ok from pr6 β†’ [shutdown] backgroundProcessManager.beginShutdown β†’ AppFiberScope closed β†’ session.dispose β†’ … β†’ terminateAll β†’ AppRuntime disposed (the order workflow.test.ts pins), exit 0; dev-server sandbox (XUM_LOG_LEVEL=debug DEV_SERVER_SANDBOX_ARGS=--clean-projects make dev-server-sandbox): AppRuntime built { ms: 21 } β†’ initialize completed; via agent-browser loaded the app (v0.28.3-nightly.148-29-gac4fc78c2), added a scratch git repo as a project, created a worktree workspace, sent "Reply with exactly the single word: pong" β†’ pong, Stats tab populated (screenshot below); then SIGTERM to the sandbox backend with the live workspace β†’ full [shutdown] sequence incl. [analytics-worker] Shutting down, closing DuckDB, ServiceContainer.dispose completed { totalMs: 82 }, process gone in 173 ms. Not exercisable here: Electron quit (no DISPLAY; covered by tests/e2e in CI and the shared dispose() path above), provider-backed integration suites (AI bridge 403s β€” CI lane).

Risks

Low. Product behavior changes are limited to debug-level log lines and the xum workflow cleanup list going through the same best-effort runner as xum run (same containment; warn wording generalized). The teardown order is unchanged and asserted; the timing helper adds no suspension between synchronous steps. Test changes replace timer surrogates with the runner seam that PRs 2/5 already made production behavior, and each worker keeps one real-clock smoke.

Sandbox smoke on this branch: pong reply in a worktree workspace, Stats tab populated


πŸ“‹ Implementation Plan

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.provides 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

flowchart TB
  Stores["StoresLive(stores)<br/>Config Β· SessionLocator Β· ProvidersConfigStore Β· SecretsStore Β· FileLeaseManager"]
  Runner["EffectRunnerLive (unsupervised, ref-bound)<br/>+ AppFiberScopeLive (supervised, closed on dispose)"]
  Cross["CrossCuttingLive (desktop only)<br/>Policy Β· Telemetry Β· Experiments Β· Analytics Β· SessionTiming Β· DevTools Β· WorkspaceMcpOverrides Β· Backup"]
  Opts["CoreOptionsTag<br/>desktop: derived from CrossCutting Β· CLI: Layer.succeed(opts)"]
  Core["CoreLive<br/>PR 3: coarse CoreProjectionLive β†’ PR 4: stages S1…S8 + CoreWiringLive"]
  Desk["DesktopLive β€” group Layers<br/>Browser Β· DesktopBridge Β· OAuth Β· Workers Β· TerminalEditor Β· Misc β†’ DesktopWiringLive"]
  RT["AppRuntime = ManagedRuntime.make(AppLive)<br/>eager sync build Β· Context<AppTags> = oRPC effect/context Β· dispose() last"]
  Stores --> Runner --> Cross --> Opts --> Core --> Desk --> RT
  CLI["CLI root (xum run / xum workflow)<br/>createCoreServices(opts) = makeAppRuntime(CoreLive β–Ή StoresFromCoreOptionsLive β–Ή AppFiberScopeLive β–Ή EffectRunnerLive β–Ή succeed(CoreOptionsTag, opts))"]
  Core -.same Layer definitions.-> CLI
Loading

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):

// 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)

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)

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.
D2 β€” Async init stays an explicit `initialize()`; Layers construct only

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.

D3 β€” Optional cross-cutting services stay optional via `CoreOptionsTag`, not `Effect.serviceOption`

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.

D4 β€” Two seams instead of one: `EffectRunner` (unsupervised, clock-bound) + `AppFiberScope` (supervised)

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).

D5 β€” oRPC: `effect/context` = the runtime's `Context`; `handlerGen` unchanged

handlerGen already Effect.provides 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.

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 πŸ€– refactor: convert memoryConsolidationService and workspaceStatusGenerator internals to Effect; make in-flight run-lock reservation deterministicΒ #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):

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.

Generated with xum β€’ Model: anthropic:claude-fable-5-1 β€’ Thinking: xhigh

…reamManager debounce interrupt probe on virtual time, one default-runner smoke per worker)
…tainer.dispose(), xum server cleanup, CLI cleanup lists
… shutdown order, layer cost); fix lint in idleCompaction smoke
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac4fc78c22

ℹ️ 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 thread src/node/services/shutdownStep.ts Outdated
Comment thread src/node/services/di/appRuntime.ts Outdated
Comment thread src/node/services/streamManager.test.ts Outdated
…tract doc names timeout constants; deterministic default-runner debounce smoke
@ThomasK33

Copy link
Copy Markdown
Member Author

@codex review

@ThomasK33

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. πŸš€

Reviewed commit: 0966ef56dc

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

@chatgpt-codex-connector

This comment has been minimized.

@ThomasK33
ThomasK33 added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 1e86979 Sep 2, 2026
36 of 38 checks passed
@ThomasK33
ThomasK33 deleted the effect-phase11-pr6-testclock-hardening branch September 2, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant