🤖 refactor: convert Config service mutation surface to Effect Semaphore pipeline and config router sites to handlerGen

[ThomasK33]

Summary

Phase 7 of the progressive Effect migration (Wave 2): converts the Config service mutation surface to an Effect Semaphore(1)-serialized pipeline, converts the fallible read/write pipelines in ProvidersConfigStore/SecretsStore to Effect per their pre-Effect catch discipline, and moves all 20 config-backed unary router procedures (splashScreens, config, uiLayouts) to handlerGen. All public Promise/sync APIs are preserved by thin runPromise/runSync facades; existing tests pass unchanged.

Background

Follows #4033 (OAuthFlowManager per-flow Scope), #4034 (codex/governor/copilot OAuth + 15 router sites), #4035 (coderOauthService). The Config class serializes all mutations through a private promise-chain queue feeding a private saveConfig — the module docs record the lost-update history (stale-snapshot writes resurrecting removed workspaces). This phase re-expresses that serialization in Effect while preserving those protections exactly.

Implementation

Mutation-surface conversion (src/node/config/index.ts)

• editConfigQueue promise chain → Semaphore.makeUnsafe(1) (editSemaphore). FIFO permits map the old chain 1:1: each edit's read happens only after the previous edit's write landed, and a failed edit releases its permit on the way out (the old "keep the queue alive on failure" behavior).
• enqueueConfigEdit stays the only Promise entry point (public editConfig is widely spied/overridden in tests, so named mutators keep routing through it unchanged). It defers the fiber start by one microtask: Effect v4 runPromise executes fibers synchronously until the first async boundary, but the old chain always ran edit bodies on a later microtask, and loadConfigOrDefault's one-shot migrationPersist guard depends on the body observing the guard assignment (otherwise load-time migrations would double-schedule their write-back).
• The corrupt-file gate (backup-signature CAS) runs inside a single Effect.try step immediately followed by the saveConfig yield — no new await points between the check and the write it approves.
• saveConfig stays private and keeps its Promise signature (tests spy on it with Promise mocks to simulate swallowed writes); its body moves to saveConfigEffect, an Effect.Effect<void> with a whole-pipeline Effect.catch + Effect.catchDefect fold mirroring the old total try/catch (log-and-swallow; never fails). The serialized pipeline routes the write through the facade so spies keep intercepting it. Load-time-migration write-back semantics (identity transform re-run under the serialized pipeline) are untouched.
• Raw error identity is preserved end-to-end: v4 runPromise/runSync reject/throw with the original error for both typed failures and defects (verified empirically), so editConfig callers and tests observe byte-identical rejections.

Catch-discipline classification (stores)

Method	Pre-Effect discipline	Effect shape
Config.saveConfig	total try/catch, log-and-swallow	whole-pipeline catch + catchDefect fold → Effect<void> (never fails)
Config.enqueueConfigEdit	throw-through (gate + transform)	Effect.try with catch: (e) => e; raw error to caller via facade
ProvidersConfigStore.loadProvidersConfig	total, fold → null (logged)	Effect.try + catch fold → Effect<ProvidersConfig | null>
ProvidersConfigStore.getProvidersFileFingerprint	total, fold → null (silent)	Effect.try + catch fold → Effect<string | null>
ProvidersConfigStore.saveProvidersConfig	log-then-rethrow	Effect.try + tapError log; raw failure via runSync facade
ProvidersConfigStore.watchProvidersFile	callback/watcher lifecycle	not converted (fs.watch callback seam; no Effect value)
SecretsStore.loadSecretsConfig / loadRawSecretsConfig	total, fold → {} (logged)	Effect.try + catch fold
SecretsStore.saveSecretsConfig	log-then-rethrow	async Effect.tryPromise thunk + tapError; raw rejection via runPromise facade
SecretsStore.updateSecretsBucket (+ updateGlobalSecrets/updateProjectSecrets)	throw-through composition	Effect.gen composing load → sync bucket fold → save
SecretsStore.getEffectiveSecrets / getGlobalSecrets etc.	pure/sync, no catch	unchanged (nothing fallible to convert)

Legacy-data passthrough (unsupported secret entries, legacy bestOf metadata, unknown fields) is untouched — the conversion moves control flow only.

fileLeaseManager

Not converted (deliberate). Its lock/lease lifecycle is cross-process (lock directories on disk, PID liveness, stale-breaking, TTLs), not an in-process resource: per the wrap-around-locks doctrine from #4035, Effect wraps around such cross-process critical sections at the caller, never through them. Its Promise seams stay byte-identical.

Router interruption posture (per-procedure)

Reads are single Effect.sync steps (interruption is a don't-care: no partial state possible). Mutations wrap the whole pre-Effect handler body in one Effect.promise thunk, making them uninterruptible by construction: a client abort interrupts the handler fiber but never the in-flight config edit, and multi-step bodies cannot be torn between steps. Rejections become defects → the same internal error the old async handlers produced.

Procedure	Kind	Posture
splashScreens.getViewedSplashScreens	read	don't-care (Effect.sync)
splashScreens.markSplashScreenViewed	mutation	uninterruptible (single thunk)
config.getConfig	read	don't-care (Effect.sync)
config.onConfigChanged	subscription	not converted — event-iterator seam waits for the Effect Stream bridge phase
config.updateAgentAiDefaults	mutation	uninterruptible (single thunk)
config.updateMuxGatewayPrefs	mutation ×2 steps	uninterruptible (mutate + providerService.notifyConfigChanged in one thunk)
config.updateRoutePreferences	mutation (via providerService)	uninterruptible (single thunk)
config.updateMinThinkingLevels	mutation	uninterruptible (single thunk)
config.updateModelFallbacks	mutation	uninterruptible (single thunk)
config.updateModelPreferences	mutation	uninterruptible (single thunk)
config.updateCoderPrefs	mutation	uninterruptible (single thunk)
config.updateRuntimeEnablement	mutation	uninterruptible (single thunk)
config.saveConfig	mutation ×2 steps	uninterruptible (saveUserConfig + maybeStartQueuedTasks in one thunk)
config.updateChatTranscriptFullWidth	mutation	uninterruptible (single thunk)
config.updateLlmDebugLogs	mutation	uninterruptible (single thunk)
config.updateHeartbeatDefaultPrompt	mutation	uninterruptible (single thunk)
config.updateHeartbeatDefaultIntervalMs	mutation	uninterruptible (single thunk)
config.updateGoalDefaults	mutation	uninterruptible (single thunk)
config.unenrollMuxGovernor	mutation ×2 steps	uninterruptible (unenroll + policyService.refreshNow in one thunk)
uiLayouts.getAll	read	don't-care (Effect.sync)
uiLayouts.saveAll	mutation	uninterruptible (single thunk)

Additionally, enqueueConfigEditEffect itself is Effect.uninterruptible (defense in depth: the corrupt-file gate, write, and change notification form one unit) while waiting for the permit stays interruptible.

Validation

• Empirical Effect v4 probes (rc.112): runPromise/runSync raw error identity for failures and defects; Semaphore FIFO + serialization; sync fiber start (motivates the microtask defer).
• src/node/config.test.ts 113/113; src/node/config/ 34/34 (secretsStore, providersConfigStore, fileLeaseManager); workspaceService.configResurrection.test.ts 4/4 (the lost-update contract); router + effectBridge suites; providerService/backup + workspaceService heartbeat/tags/goalDefaults + projectService + updateService + worktreeArchiveSnapshotService consumer suites.
• tests/ipc/config jest: 7/9 suites green; mcpConfig.test.ts and modelNotFound.test.ts failures reproduce identically on the unmodified base (live-AI bridge environment; see Risks note) — verified via stash/run/pop.
• Pre-existing env baselines unrelated to this change: taskService (2), workspaceService (bash-monitor-wake), BackupRepoCache 200-commit timeout.

Risks

Severity: medium (config serialization is the app's central persistence chokepoint). The conversion is control-flow-preserving by construction: same read→gate→write→notify order under the same mutual exclusion, same error contracts (verified empirically for identity), same on-disk serialization (untouched). The main behavioral surface is scheduling: the microtask defer preserves the old chain's assignment-before-body ordering that the migration-persist one-shot guard depends on; the config suite pins this (migration write-back tests).

Lessons for Phase 8 (memoryConsolidationService Schedule conversion + workspaceStatusGenerator dispatcher)

1. Effect v4 fibers start synchronously on runPromise. Any facade replacing a promise-chain/queue must check whether callers depend on deferred body execution (one-shot guards, listener registration windows) and add an explicit microtask defer if so. This will matter for workspaceStatusGenerator's dispatcher loop.
2. Test-spy seams pin conversion boundaries. editConfig is spied/overridden across many suites, so the Effect pipeline had to live behind the existing method rather than replacing named mutators with Effect surfaces. Check spyOn/property-override usage before choosing the Effect-native surface for memoryConsolidationService.
3. Wrap-around-locks confirmed again: fileLeaseManager (cross-process dir locks) stayed Promise-native; memoryConsolidationService's file-lock ordering should keep lock interiors byte-identical and put Effect around them.
4. Merge-queue flake watch: MemoryConsolidationService "rejects a second trigger" 5s-timeout flake evicted 🤖 refactor: convert coderOauthService internals to Effect and adopt handlerGen for coder OAuth procedures #4035 from the queue once; expect it again (verify locally, re-enqueue). Phase 8 touches that very service — consider fixing the flake as part of the phase.
5. Semaphore.makeUnsafe(1) + withPermits(1) maps AsyncMutex/promise-queue semantics 1:1 (FIFO, release-on-failure), no acquireUseRelease needed.

---

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

[ThomasK33]

@codex review

[ThomasK33]

@codex security review

[ThomasK33]

@codex review

[ThomasK33]

@codex security review