🤖 refactor: convert periodic-worker scheduling to Effect Schedule + Scope

[ThomasK33]

Summary

Phase 3 of the progressive Effect migration: the periodic-worker internals of heartbeatService, idleCompactionService, and idleDispatcher now run on Effect fibers — Schedule-driven loops for the two interval services and forked sleep fibers for per-workspace debounce — with service start/stop lifecycles owned by an Effect Scope (the first real Scope use in the migration). Public APIs are unchanged and all pre-existing tests pass unchanged.

Background

Follows #4022 (spike), #4025 (memory ops), #4027 (retryManager + gateway OAuth), #4028 (providerService), #4030 (providerModelFactory). #4027 established that Effect Schedule fits runtime-owned loops (the repeated effect lives inside the Effect world) rather than externally-driven state machines — these three services are exactly the runtime-owned case: hand-rolled setTimeout startup delays chained into setInterval ticks, plus per-workspace debounce timers.

Implementation

heartbeatService — the startupTimeout + checkInterval timer pair becomes one scheduler fiber: Effect.sleep(STARTUP_DELAY_MS) → first tick → Effect.repeat(Schedule.fixed(CHECK_INTERVAL_MS)). Schedule.fixed is the setInterval analogue (wall-clock anchored, no burst catch-up; probe-verified). start() acquires the idle-consumer registration and workspace event listeners via Effect.acquireRelease and forks the scheduler with Effect.forkIn, all inside a Scope.makeUnsafe() lifecycle scope; stop() closes the scope, which releases everything in reverse acquisition order — fiber interrupt (synchronously clearing the pending timer), listeners off, consumer dispose — the exact order the hand-rolled stop() used. The legacy startupTimeout/checkInterval field pair is kept (now holding the fiber) because the null/non-null phase progression is the observable lifecycle contract that tests pin.

idleCompactionService — same shape: sleep(INITIAL_CHECK_DELAY_MS) → immediate first check → Schedule.fixed(CHECK_INTERVAL_MS), forked into a lifecycle scope; checks stay fire-and-forget so a slow sweep never delays a cadence slot (matching setInterval).

idleDispatcher — each per-workspace debounce setTimeout becomes a forked fiber (Effect.sleep(debounceMs) → mark ready). Effect.runFork executes synchronously up to the sleep, so timer registration ordering relative to requestDispatch is unchanged, and a zero-duration sleep still defers to a timer tick rather than firing inline (probe-verified).

Behavioral improvement (one new test) — under the hand-rolled version, a throw partway through heartbeatService.start() leaked earlier acquisitions (idle-consumer registration) and left the service permanently wedged (stopped=false, so both retry-start() and the duplicate-consumer assert would fire). Scope finalizers now guarantee release on partial startup failure and the rollback restores the stopped state, so a retry succeeds. This is the only new test; no other test files changed beyond that addition.

Runtime semantics probe (verified against effect 4.0.0-rc.112 before implementation)

• Scope.makeUnsafe + Effect.acquireRelease + Effect.forkIn under Effect.runSync execute fully synchronously; the forked fiber runs to its first sleep before fork returns.
• Effect.runSync(Scope.close(...)) completes synchronously while the fiber is suspended on its clock timer, runs finalizers in reverse order, and no late ticks fire afterward.
• Effect.repeat runs the first execution immediately; Schedule.fixed re-anchors after a slow body without bursting ([0, 30, {50ms body}, immediate, 120, 150, 180]).
• Forked Effect.sleep(0) defers like setTimeout(0) rather than firing inline.

Explicitly out of scope (fit assessment)

• memoryConsolidationService: not converted here. Its loop is a fit for the same Schedule/Scope pattern if it is a plain delay+interval worker, but it also participates in memory-file locking; conversion should be planned together with its lock-ordering constraints rather than ride along in a scheduling PR.
• workspaceStatusGenerator: not converted. Its regeneration is event/debounce-driven off workspace activity rather than a fixed-cadence loop; the idleDispatcher debounce-fiber pattern from this PR is the right template when it is converted.
• Proactive OAuth token-refresh worker (suggested by 🤖 refactor: convert providerModelFactory model-creation internals to Effect #4030): new functionality, not migration — remains backlog.

Lessons for Phase 4 (core router progressive conversion + streamManager placement)

1. Scope + acquireRelease + forkIn composes cleanly with synchronous start/stop facades: everything runs under runSync because acquisitions are Effect.sync and fibers only suspend on clock timers. Phase 4's streamManager owns fibers that suspend on I/O, where Scope.close cannot complete synchronously — plan async close (or runFork + stopped-flag latching) for those lifecycles before converting.
2. Schedule.fixed vs Schedule.spaced: fixed is the honest setInterval replacement (wall-clock anchor, no burst); spaced drifts by body duration. Pick per legacy timer type, and probe — repeat's first execution is immediate, which conveniently matches the "fire once when the startup timer lands, then every interval" idiom.
3. Fields that tests pin (startupTimeout/checkInterval) can survive a mechanism swap by re-typing them as fiber/phase markers rather than editing tests; document that the null/non-null progression is the contract. The core router has many more internals-pinning tests — budget for this.
4. TS6133 trap: a phase-marker field written only from inside the fiber counts as "never read" — give it a genuine read (e.g. a shutdown debug log) instead of suppressing.
5. type Fiber import: fibers held only as fields trip consistent-type-imports; import the namespace as type.

Validation

• Probe scripts against effect 4.0.0-rc.112 validated all timing/interruption assumptions before implementation (results above).
• bun test on the three service suites: 100 pass / 0 fail (99 pre-existing unchanged + 1 new). Consumer suites (workspaceGoalService, agentSession.goalAutoPause, agentSession.waitForIdle, workspaceService.heartbeatSettings, tools/heartbeat, timelineMapper, tools): all green. The single workspaceService.test.ts failure ("bash monitor wakes > accepted history suppresses redelivery…") reproduces identically on unmodified main in this environment (baseline, unrelated).
• make static-check green.

Risks

Low-to-moderate: heartbeat and idle-compaction scheduling drive background automation for every workspace, so a cadence regression would be user-visible but not data-destructive. The queue/eligibility business logic is untouched — only the timer skeletons moved. The main semantic risk (synchronous stop ordering) is pinned by existing lifecycle tests and was probe-verified; interruption of a sleeping fiber clears its timer synchronously, matching clearTimeout/clearInterval.

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

[ThomasK33]

@codex review

[ThomasK33]

@codex security review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f683a0592c

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

[ThomasK33]

@codex review

[ThomasK33]

@codex security review