🤖 refactor: convert memoryConsolidationService and workspaceStatusGenerator internals to Effect; make in-flight run-lock reservation deterministic

[ThomasK33]

Summary

Phase 8 of the progressive Effect migration (after #4033/#4034/#4035/#4036): converts memoryConsolidationService and workspaceStatusGenerator internals to Effect-native pipelines behind unchanged Promise facades, and fixes the "rejects a second trigger while a run is still in flight" 5s-timeout flake that evicted #4035 from the merge queue — by making the in-flight run-lock reservation deterministic.

Background

Wave 1 (#4031) established the worker templates (Schedule-driven fibers, runFork sleep fibers); Wave 2 converted the OAuth services and Config. This phase covers the two remaining Wave 2 services:

• memoryConsolidationService (~1100 ln): trigger-driven orchestration around the dream/harvest runners, with a sidecar MutexMap and per-workspace in-flight promise maps.
• workspaceStatusGenerator (~277 ln): the sidebar-status candidate retry loop with a bounded usage read (previously Promise.race + setTimeout — converted to the timeout/sleep-fiber shape from the idleDispatcher template).

Flake root cause (chartered fix)

maybeRun awaited the durable workspace-removal tombstone (fsPromises.access on the libuv threadpool) before the synchronous in-flight check-and-reserve. Two near-simultaneous triggers both suspended on that probe, and threadpool completion order — not call order — decided which caller reserved the run lock. Mutual exclusion always held (the check-and-reserve itself is one microtask), but the winner was nondeterministic.

In the test, when the second trigger won the reservation, the first returned the "in flight" refusal and the test then awaited the second run — which was gated on a model-creation promise the test only releases after asserting the second call failed. Deadlock → 5s bun timeout. Same hazard existed in maybeHarvestThenSweep (boundary-key coalescing) and in the sibling "queues an archive trigger" test.

Fix (code, not test): the funnels now check only the synchronous in-process teardown mark (removalCancelled) before the check-and-reserve, so reservation order is decided purely by call order in one synchronous frame; the durable cross-process tombstone probe moved behind the reservation, to the top of the locked pipelines (runLockedEffect / harvestThenSweepLockedEffect), preserving the same refusal messages and r60/r61 teardown coverage. No awaits were added between any liveness check and its write. The previously-flaky tests now pass deterministically (stressed 25×; timeouts unchanged).

Implementation

memoryConsolidationService — Effect.gen pipelines with thin Effect.runPromise facades; wire Result<_, string> skip/record unions stay in the success channel:

• Wrap-around-locks (doctrine from 🤖 refactor: convert coderOauthService internals to Effect and adopt handlerGen for coder OAuth procedures #4035/🤖 refactor: convert Config service mutation surface to Effect Semaphore pipeline and config router sites to handlerGen #4036): the sidecar MutexMap critical sections remain callback-owned Promise seams with byte-identical interiors; saveRecordEffect/saveHarvestRecordEffect are Effect.uninterruptible mutations wrapping those seams. The inFlight/harvestInFlight reservation maps remain synchronous try-lock seams in the funnels (documented in the module header and maybeRun doc) — the funnels stay plain async methods because the check-and-reserve atomicity is load-bearing.
• runLockedEffect, harvestThenSweepLockedEffect + runHarvestAttemptEffect (the old try-block), recoverRetryableHarvestsEffect, runLaunchSweepEffect (composes metaService.effects.getEntries() directly), loadEffect/getRecordEffect/getStatusEffect.
• cancelInFlightConsolidationEffect: teardown is Effect.uninterruptible end-to-end (r61 mark → abort loop → residual handoff), with the bounded drain explicitly Effect.interruptible + Effect.timeout (it is a wait; the old Promise.race + setTimeout timer is gone).
• triggerInBackground/triggerHarvestThenSweepInBackground: detached Effect.runFork fibers (idleDispatcher template). runFork executes synchronously up to the first suspension, so trigger-call ordering of reservations is preserved from the old void-promise chains.

workspaceStatusGenerator — generateWorkspaceStatus keeps its exact Promise export (agentStatusService tests spy this module symbol with mockResolvedValue; the spy seam pins the facade). Per-candidate attempts are one attemptCandidate pipeline: the old whole-attempt try/catch/finally becomes Effect.catch + Effect.catchDefect folds (any failure tries the next candidate — no defect escapes the facade where the old code caught) + Effect.ensuring for runLanguageModelCleanup. The 2s usage-read race is now Effect.timeout.

Interruption posture: mutations (saveRecordEffect, saveHarvestRecordEffect) uninterruptible; teardown (cancelInFlightConsolidationEffect) uninterruptible with an explicitly interruptible bounded wait; waits (usage read, drain) interruptible so timeouts work; reads (load/getRecord/getStatus) don't-care. Nothing externally interrupts these fibers today (all entry points are runPromise/runFork facades); the posture is for composition safety.

Catch-discipline audit: paths uncaught pre-Effect still reject through facades via Effect.promise defects (v4 rethrows raw errors); paths inside old try/catch blocks are folded (attemptCandidate, runHarvestAttemptEffect incl. the completed-record save whose rejection previously fell into the same catch, loadEffect self-healing with parsing inside the caught thunk, per-iteration .catch folds in recovery/launch-sweep).

Router sites: none exist for these services (memoryConsolidationService reaches oRPC only via the statusChange EventEmitter subscription in routerSubscriptions.ts, unchanged; the Effect Stream bridge for subscriptions is Phase 9).

Validation

• make static-check green; memoryConsolidationService (40), workspaceStatusGenerator (5), agentStatusService (36) and all memory* suites (164) pass unchanged — zero test-file edits.
• Stress: 25 consecutive runs of the two in-flight-race tests + harvest coalescing test, all green.

Risks

• Highest-risk area is trigger funnel ordering (compaction/archive/manual/launch races) and removal teardown (r60/r61). Mitigations: reservation semantics are strictly tighter (no suspension before reserve), refusal strings unchanged, lock interiors byte-identical, and the full behavioral suite passes unchanged.
• Tombstoned-workspace triggers now briefly reserve the run lock before refusing inside the locked pipeline (previously refused before reserving). The refusal settles immediately; removal drains observe a promptly-settling promise, and memory mutations remain gated by the durable tombstone at commit points.

Lessons for Phase 9 (Effect Stream bridge, ~24 subscriptions incl. memory.onChange)

• memoryConsolidationService emits statusChange + analyticsIngest via EventEmitter; routerSubscriptions.ts consumes statusChange through the on/off + asyncIterableFromSubscription push seam. The emit sites are now inside Effect pipelines (saveRecordEffect emits after the lock releases), so bridging to Stream can hook those seams directly — but note emits are synchronous post-write; a Stream bridge must preserve emit-after-durable-write ordering.
• Fire-and-forget recordUsage callbacks (emit("analyticsIngest") from inside provider-stream callbacks) fire from non-Effect contexts (AI SDK callbacks); the bridge needs a queue that accepts synchronous emissions from foreign callsites, not just fiber-context offers.
• Spy-seam rule extends to module-level function exports, not just methods: agentStatusService.test.ts spies generateWorkspaceStatus via spyOn(module, "name") — module functions consumed through namespace imports must keep Promise signatures.
• Deterministic-winner lesson generalizes: any funnel whose "who wins" matters must do check-and-reserve with zero suspensions; converting such funnels to Effect facades is possible (v4 runs fibers synchronously to the first suspension) but keeping them as documented synchronous seams is the honest shape and reviews better.

---

Generated with xum • Model: anthropic:claude-fable-5 • Thinking: xhigh

[ThomasK33]

@codex review

[ThomasK33]

@codex security review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: b6db26eee7

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