From 25109790f3f6880b6d1e83fed255dd4e86bfc73c Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 1 Sep 2026 13:34:16 +0000 Subject: [PATCH 1/2] refactor: convert Config service mutation surface and stores to Effect, config router sites to handlerGen --- src/node/config/index.ts | 182 +++++++++++++++--------- src/node/config/providersConfigStore.ts | 87 +++++++---- src/node/config/secretsStore.ts | 173 ++++++++++++++-------- src/node/orpc/router.ts | 181 +++++++++++++++++------ 4 files changed, 430 insertions(+), 193 deletions(-) diff --git a/src/node/config/index.ts b/src/node/config/index.ts index c44bb04cb8..fb96d40973 100644 --- a/src/node/config/index.ts +++ b/src/node/config/index.ts @@ -3,6 +3,7 @@ import * as fs from "fs"; import * as crypto from "crypto"; import { EventEmitter } from "events"; import writeFileAtomic from "write-file-atomic"; +import { Effect, Semaphore } from "effect"; import { resolveXumEnvironmentValue } from "@/common/compat/legacyMux"; import { log } from "@/node/services/log"; import { ProvidersConfigStore } from "./providersConfigStore"; @@ -906,8 +907,14 @@ export class Config { */ private readonly legacyTaskVariantGroups = new Map(); private readonly legacyTaskVariantMetadataOnlyIds = new Set(); - /** Serializes editConfig calls; see editConfig for why. */ - private editConfigQueue: Promise = Promise.resolve(); + /** + * Serializes editConfig calls; see editConfig for why. An Effect Semaphore (FIFO + * permits) replaces the old promise-chain queue 1:1: each edit holds the single + * permit for its whole read-modify-write cycle, and a failed edit releases its + * permit on the way out, which preserves the old queue-keep-alive behavior (one + * edit's failure never wedges later edits). + */ + private readonly editSemaphore = Semaphore.makeUnsafe(1); /** One-shot guard for the queued load-time migration persist; see loadConfigOrDefault. */ private migrationPersist: Promise | null = null; @@ -1806,11 +1813,16 @@ export class Config { * removeWorkspace() and written after it resurrected the removed workspace entry * as a permanent sidebar ghost. All mutations must go through editConfig so each * write is derived from a fresh serialized read. + * + * Never fails: the whole pipeline folds every failure and defect into the same + * log-and-swallow the old try/catch applied (total catch discipline). */ - private async saveConfig(config: ProjectsConfig): Promise { - try { - if (!fs.existsSync(this.rootDir)) { - ensurePrivateDirSync(this.rootDir); + private saveConfig(config: ProjectsConfig): Effect.Effect { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` + const self = this; + return Effect.gen(function* () { + if (!fs.existsSync(self.rootDir)) { + ensurePrivateDirSync(self.rootDir); } const data: Partial> & { @@ -1824,7 +1836,7 @@ export class Config { }; for (const workspace of persistedProjectConfig.workspaces) { const workspaceId = parseOptionalNonEmptyString(workspace.id); - const legacy = workspaceId ? this.legacyTaskVariantGroups.get(workspaceId) : undefined; + const legacy = workspaceId ? self.legacyTaskVariantGroups.get(workspaceId) : undefined; if (legacy && workspace.bestOf == null) { // Keep downgrade-only metadata on disk without exposing it to current runtime types, // UI grouping, or provider-facing task schemas. @@ -2084,22 +2096,29 @@ export class Config { } } } - await writeFileAtomic(this.configFile, JSON.stringify(data, null, 2), "utf-8"); - for (const workspaceId of this.legacyTaskVariantGroups.keys()) { + yield* Effect.tryPromise({ + try: async () => writeFileAtomic(self.configFile, JSON.stringify(data, null, 2), "utf-8"), + catch: (error) => error, + }); + for (const workspaceId of self.legacyTaskVariantGroups.keys()) { if (!persistedWorkspaceIds.has(workspaceId)) { // A load-time settings migration can save before getAllWorkspaceMetadata's queued // identity migration adds this legacy workspace ID. Keep metadata-only entries alive // until a later save can attach them to the migrated config record. - if (!this.legacyTaskVariantMetadataOnlyIds.has(workspaceId)) { - this.legacyTaskVariantGroups.delete(workspaceId); + if (!self.legacyTaskVariantMetadataOnlyIds.has(workspaceId)) { + self.legacyTaskVariantGroups.delete(workspaceId); } } else { - this.legacyTaskVariantMetadataOnlyIds.delete(workspaceId); + self.legacyTaskVariantMetadataOnlyIds.delete(workspaceId); } } - } catch (error) { - log.error("Error saving config:", error); - } + }).pipe( + // Mirror the old whole-pipeline try/catch: fold both the typed write failure and + // any defect thrown by the synchronous serialization above into the same + // log-and-swallow, so this pipeline never fails. + Effect.catch((error) => Effect.sync(() => log.error("Error saving config:", error))), + Effect.catchDefect((error) => Effect.sync(() => log.error("Error saving config:", error))) + ); } /** @@ -2418,57 +2437,90 @@ export class Config { * detail rather than a caller-initiated mutation. */ private enqueueConfigEdit(fn: (config: ProjectsConfig) => ProjectsConfig): Promise { - const run = this.editConfigQueue.then(async () => { - const config = this.loadConfigOrDefault(); - const newConfig = fn(config); - // If that load failed, writing would replace the corrupt file with defaults. Only - // proceed when the bytes on disk right now are the ones with a confirmed sidecar: - // no confirmed backup, a concurrent replacement since the load, or an unreadable - // file all reject the edit so callers do not treat the mutation as durable (unlike - // saveConfig's log-and-swallow of unexpected I/O errors, this skip is deliberate). - // A missing file is safe to overwrite. This cannot fully close the cross-process - // race (that needs file locking, which editConfig has never had); it binds the - // approval to the current bytes and shrinks the window to the atomic write itself. - const failureState = configLoadFailureStates.get(this.configFile); - if (failureState) { - const rejectEdit = (reason: string): never => { - const message = `Skipping config write to ${this.configFile}: ${reason}`; - log.error(message); - throw new Error(message); - }; - if (failureState.backupSignature === null) { - rejectEdit( - "the existing corrupt config has no confirmed backup yet. Fix the reported backup failure or move the corrupt file aside, then retry." - ); - } - let currentSignature: string | null = null; - try { - const currentBytes = fs.readFileSync(this.configFile); - currentSignature = crypto.createHash("sha256").update(currentBytes).digest("hex"); - } catch (readError) { - if ((readError as NodeJS.ErrnoException).code !== "ENOENT") { - rejectEdit( - `the file could not be re-read before writing (${readError instanceof Error ? readError.message : String(readError)}). Retry the settings change.` - ); - } - } - if (currentSignature !== null && currentSignature !== failureState.backupSignature) { - rejectEdit( - "the file changed after this edit loaded it and the new content has no confirmed backup. Retry the settings change." - ); - } - } - await this.saveConfig(newConfig); - // Backend-initiated config edits (for example gateway auth changes) use this signal - // so frontend subscribers can refresh derived state without polling. - this.notifyConfigChanged(); - }); - // Keep the queue alive when an edit fails; the failure still propagates to this caller. - this.editConfigQueue = run.then( - () => undefined, - () => undefined + // Defer the fiber start to a microtask: Effect.runPromise executes fibers + // synchronously on the caller's stack until the first async boundary, but the old + // promise-chain queue always ran edit bodies on a later microtask. + // loadConfigOrDefault's one-shot migrationPersist guard depends on that ordering: + // the edit body's own loadConfigOrDefault must observe the `this.migrationPersist` + // assignment, or a load-time migration would schedule its persist twice. + // Effect failures reject this promise with the raw error (v4 runPromise does not + // wrap causes), so callers observe the same rejections as before. + return Promise.resolve().then(() => Effect.runPromise(this.enqueueConfigEditEffect(fn))); + } + + /** + * Semaphore(1)-serialized edit pipeline: FIFO permits guarantee each edit's read + * happens only after the previous edit's write has landed, replacing the old + * promise-chain queue 1:1. The body is uninterruptible so an interruption can never + * separate the corrupt-file gate from the write it approves, or drop the change + * notification after a write landed; waiting for the permit stays interruptible (a + * fiber cancelled while waiting never runs its edit). + */ + private enqueueConfigEditEffect( + fn: (config: ProjectsConfig) => ProjectsConfig + ): Effect.Effect { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` + const self = this; + return this.editSemaphore.withPermits(1)( + Effect.uninterruptible( + Effect.gen(function* () { + // Effect.try keeps the pre-Effect contract: a throwing transform or a rejected + // corrupt-file gate reaches the caller as the raw original error. + const newConfig = yield* Effect.try({ + try: () => { + const config = self.loadConfigOrDefault(); + const newConfig = fn(config); + // If that load failed, writing would replace the corrupt file with defaults. Only + // proceed when the bytes on disk right now are the ones with a confirmed sidecar: + // no confirmed backup, a concurrent replacement since the load, or an unreadable + // file all reject the edit so callers do not treat the mutation as durable (unlike + // saveConfig's log-and-swallow of unexpected I/O errors, this skip is deliberate). + // A missing file is safe to overwrite. This cannot fully close the cross-process + // race (that needs file locking, which editConfig has never had); it binds the + // approval to the current bytes and shrinks the window to the atomic write itself. + const failureState = configLoadFailureStates.get(self.configFile); + if (failureState) { + const rejectEdit = (reason: string): never => { + const message = `Skipping config write to ${self.configFile}: ${reason}`; + log.error(message); + throw new Error(message); + }; + if (failureState.backupSignature === null) { + rejectEdit( + "the existing corrupt config has no confirmed backup yet. Fix the reported backup failure or move the corrupt file aside, then retry." + ); + } + let currentSignature: string | null = null; + try { + const currentBytes = fs.readFileSync(self.configFile); + currentSignature = crypto.createHash("sha256").update(currentBytes).digest("hex"); + } catch (readError) { + if ((readError as NodeJS.ErrnoException).code !== "ENOENT") { + rejectEdit( + `the file could not be re-read before writing (${readError instanceof Error ? readError.message : String(readError)}). Retry the settings change.` + ); + } + } + if ( + currentSignature !== null && + currentSignature !== failureState.backupSignature + ) { + rejectEdit( + "the file changed after this edit loaded it and the new content has no confirmed backup. Retry the settings change." + ); + } + } + return newConfig; + }, + catch: (error) => error, + }); + yield* self.saveConfig(newConfig); + // Backend-initiated config edits (for example gateway auth changes) use this signal + // so frontend subscribers can refresh derived state without polling. + self.notifyConfigChanged(); + }) + ) ); - return run; } getUpdateChannel(): UpdateChannel { diff --git a/src/node/config/providersConfigStore.ts b/src/node/config/providersConfigStore.ts index 5361a77bbc..e8612c8d8e 100644 --- a/src/node/config/providersConfigStore.ts +++ b/src/node/config/providersConfigStore.ts @@ -2,6 +2,7 @@ import * as crypto from "crypto"; import * as fs from "fs"; import * as path from "path"; import * as jsonc from "jsonc-parser"; +import { Effect } from "effect"; import writeFileAtomic from "write-file-atomic"; import { getXumHome } from "@/common/constants/paths"; import type { @@ -23,16 +24,31 @@ export class ProvidersConfigStore { } loadProvidersConfig(): ProvidersConfig | null { - try { - if (fs.existsSync(this.providersFile)) { - const data = fs.readFileSync(this.providersFile, "utf-8"); - return jsonc.parse(data) as ProvidersConfig; - } - } catch (error) { - log.error("Error loading providers config:", error); - } + return Effect.runSync(this.loadProvidersConfigEffect()); + } - return null; + /** + * Total pre-Effect catch discipline: any read/parse failure folds to `null` + * (logged), matching the old whole-body try/catch. + */ + private loadProvidersConfigEffect(): Effect.Effect { + return Effect.try({ + try: (): ProvidersConfig | null => { + if (fs.existsSync(this.providersFile)) { + const data = fs.readFileSync(this.providersFile, "utf-8"); + return jsonc.parse(data) as ProvidersConfig; + } + return null; + }, + catch: (error) => error, + }).pipe( + Effect.catch((error) => + Effect.sync(() => { + log.error("Error loading providers config:", error); + return null; + }) + ) + ); } /** @@ -50,12 +66,15 @@ export class ProvidersConfigStore { * write signal. */ getProvidersFileFingerprint(): string | null { - try { + return Effect.runSync(this.getProvidersFileFingerprintEffect()); + } + + /** Total pre-Effect catch discipline: any read failure folds silently to `null`. */ + private getProvidersFileFingerprintEffect(): Effect.Effect { + return Effect.try((): string | null => { const contents = fs.readFileSync(this.providersFile); return crypto.createHash("sha256").update(contents).digest("hex"); - } catch { - return null; - } + }).pipe(Effect.catch(() => Effect.succeed(null))); } /** @@ -144,14 +163,25 @@ export class ProvidersConfigStore { } saveProvidersConfig(config: ProvidersConfig): void { - try { - if (!fs.existsSync(this.rootDir)) { - ensurePrivateDirSync(this.rootDir); - } + // runSync rethrows the raw typed failure, so callers observe the same throw as + // before the Effect conversion. + Effect.runSync(this.saveProvidersConfigEffect(config)); + } + + /** + * Log-then-rethrow pre-Effect catch discipline: failures are logged and pass + * through raw to the caller in the typed failure channel. + */ + private saveProvidersConfigEffect(config: ProvidersConfig): Effect.Effect { + return Effect.try({ + try: () => { + if (!fs.existsSync(this.rootDir)) { + ensurePrivateDirSync(this.rootDir); + } - const jsonString = JSON.stringify(config, null, 2); + const jsonString = JSON.stringify(config, null, 2); - const contentWithComments = `// Providers configuration for xum + const contentWithComments = `// Providers configuration for xum // Configure your AI providers here // Example: // { @@ -170,13 +200,16 @@ export class ProvidersConfigStore { // } ${jsonString}`; - writeFileAtomic.sync(this.providersFile, contentWithComments, { - encoding: "utf-8", - mode: 0o600, - }); - } catch (error) { - log.error("Error saving providers config:", error); - throw error; // Re-throw to let caller handle - } + writeFileAtomic.sync(this.providersFile, contentWithComments, { + encoding: "utf-8", + mode: 0o600, + }); + }, + catch: (error) => error, + }).pipe( + Effect.tapError((error) => + Effect.sync(() => log.error("Error saving providers config:", error)) + ) + ); } } diff --git a/src/node/config/secretsStore.ts b/src/node/config/secretsStore.ts index c2b95ca48e..08c24ddd04 100644 --- a/src/node/config/secretsStore.ts +++ b/src/node/config/secretsStore.ts @@ -1,5 +1,6 @@ import * as fs from "fs"; import * as path from "path"; +import { Effect } from "effect"; import writeFileAtomic from "write-file-atomic"; import { getXumHome } from "@/common/constants/paths"; import { isSecretReferenceValue, type Secret, type SecretsConfig } from "@/common/types/secrets"; @@ -149,17 +150,32 @@ export class SecretsStore { } loadSecretsConfig(): SecretsConfig { - try { - if (fs.existsSync(this.secretsFile)) { - const data = fs.readFileSync(this.secretsFile, "utf-8"); - const parsed = JSON.parse(data) as unknown; - return SecretsStore.normalizeSecretsConfig(parsed); - } - } catch (error) { - log.error("Error loading secrets config:", error); - } + return Effect.runSync(this.loadSecretsConfigEffect()); + } - return {}; + /** + * Total pre-Effect catch discipline: any read/parse failure folds to `{}` (logged), + * matching the old whole-body try/catch. + */ + private loadSecretsConfigEffect(): Effect.Effect { + return Effect.try({ + try: (): SecretsConfig => { + if (fs.existsSync(this.secretsFile)) { + const data = fs.readFileSync(this.secretsFile, "utf-8"); + const parsed = JSON.parse(data) as unknown; + return SecretsStore.normalizeSecretsConfig(parsed); + } + return {}; + }, + catch: (error) => error, + }).pipe( + Effect.catch((error) => + Effect.sync(() => { + log.error("Error loading secrets config:", error); + return {}; + }) + ) + ); } /** @@ -167,19 +183,27 @@ export class SecretsStore { * paths so unsupported legacy entries survive round-trips to disk instead of * being silently deleted when an unrelated secret is saved. */ - private loadRawSecretsConfig(): Record { - try { - if (fs.existsSync(this.secretsFile)) { - const parsed = JSON.parse(fs.readFileSync(this.secretsFile, "utf-8")) as unknown; - if (parsed !== null && typeof parsed === "object" && !Array.isArray(parsed)) { - return { ...(parsed as Record) }; + private loadRawSecretsConfigEffect(): Effect.Effect> { + return Effect.try({ + try: (): Record => { + if (fs.existsSync(this.secretsFile)) { + const parsed = JSON.parse(fs.readFileSync(this.secretsFile, "utf-8")) as unknown; + if (parsed !== null && typeof parsed === "object" && !Array.isArray(parsed)) { + return { ...(parsed as Record) }; + } } - } - } catch (error) { - log.error("Error loading secrets config:", error); - } - - return {}; + return {}; + }, + catch: (error) => error, + }).pipe( + // Total pre-Effect catch discipline: any read/parse failure folds to `{}` (logged). + Effect.catch((error) => + Effect.sync(() => { + log.error("Error loading secrets config:", error); + return {}; + }) + ) + ); } /** @@ -187,47 +211,72 @@ export class SecretsStore { * project path) while leaving every other bucket byte-for-byte intact and * preserving unsupported legacy entries within the target bucket. */ - private async updateSecretsBucket(bucketKey: string, secrets: Secret[]): Promise { - const raw = this.loadRawSecretsConfig(); - - // Project paths may be persisted with trailing slashes; fold every raw key - // that maps to this bucket so preserved entries aren't left in a shadowed - // duplicate bucket. - const rawBucketEntries: unknown[] = []; - for (const [rawKey, rawValue] of Object.entries(raw)) { - const mappedKey = - rawKey === SecretsStore.GLOBAL_SECRETS_KEY - ? rawKey - : SecretsStore.normalizeSecretsProjectPath(rawKey) || rawKey; - if (mappedKey !== bucketKey) { - continue; - } + private updateSecretsBucketEffect( + bucketKey: string, + secrets: Secret[] + ): Effect.Effect { + // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` + const self = this; + return Effect.gen(function* () { + const raw = yield* self.loadRawSecretsConfigEffect(); + + // Project paths may be persisted with trailing slashes; fold every raw key + // that maps to this bucket so preserved entries aren't left in a shadowed + // duplicate bucket. + const rawBucketEntries: unknown[] = []; + for (const [rawKey, rawValue] of Object.entries(raw)) { + const mappedKey = + rawKey === SecretsStore.GLOBAL_SECRETS_KEY + ? rawKey + : SecretsStore.normalizeSecretsProjectPath(rawKey) || rawKey; + if (mappedKey !== bucketKey) { + continue; + } - if (Array.isArray(rawValue)) { - // Array.isArray narrows unknown to any[]; retype to unknown[] for safe handling. - rawBucketEntries.push(...(rawValue as unknown[])); + if (Array.isArray(rawValue)) { + // Array.isArray narrows unknown to any[]; retype to unknown[] for safe handling. + rawBucketEntries.push(...(rawValue as unknown[])); + } + delete raw[rawKey]; } - delete raw[rawKey]; - } - raw[bucketKey] = SecretsStore.mergeSecretsPreservingUnsupported(rawBucketEntries, secrets); - await this.saveSecretsConfig(raw); + raw[bucketKey] = SecretsStore.mergeSecretsPreservingUnsupported(rawBucketEntries, secrets); + yield* self.saveSecretsConfigEffect(raw); + }); } - async saveSecretsConfig(config: SecretsConfig | Record): Promise { - try { - if (!fs.existsSync(this.rootDir)) { - ensurePrivateDirSync(this.rootDir); - } + saveSecretsConfig(config: SecretsConfig | Record): Promise { + // runPromise rejects with the raw typed failure, so callers observe the same + // rejection as before the Effect conversion. + return Effect.runPromise(this.saveSecretsConfigEffect(config)); + } - await writeFileAtomic(this.secretsFile, JSON.stringify(config, null, 2), { - encoding: "utf-8", - mode: 0o600, - }); - } catch (error) { - log.error("Error saving secrets config:", error); - throw error; - } + /** + * Log-then-rethrow pre-Effect catch discipline: failures are logged and pass + * through raw to the caller in the typed failure channel. The thunk is async so a + * synchronous throw (e.g. from ensurePrivateDirSync) follows the same rejection + * path the old `await`-based body produced. + */ + private saveSecretsConfigEffect( + config: SecretsConfig | Record + ): Effect.Effect { + return Effect.tryPromise({ + try: async () => { + if (!fs.existsSync(this.rootDir)) { + ensurePrivateDirSync(this.rootDir); + } + + await writeFileAtomic(this.secretsFile, JSON.stringify(config, null, 2), { + encoding: "utf-8", + mode: 0o600, + }); + }, + catch: (error) => error, + }).pipe( + Effect.tapError((error) => + Effect.sync(() => log.error("Error saving secrets config:", error)) + ) + ); } /** @@ -241,8 +290,10 @@ export class SecretsStore { } /** Update global secrets (not project-scoped). */ - async updateGlobalSecrets(secrets: Secret[]): Promise { - await this.updateSecretsBucket(SecretsStore.GLOBAL_SECRETS_KEY, secrets); + updateGlobalSecrets(secrets: Secret[]): Promise { + return Effect.runPromise( + this.updateSecretsBucketEffect(SecretsStore.GLOBAL_SECRETS_KEY, secrets) + ); } /** @@ -394,9 +445,9 @@ export class SecretsStore { return config[normalizedProjectPath] ?? []; } - async updateProjectSecrets(projectPath: string, secrets: Secret[]): Promise { + updateProjectSecrets(projectPath: string, secrets: Secret[]): Promise { const normalizedProjectPath = SecretsStore.normalizeSecretsProjectPath(projectPath) || projectPath; - await this.updateSecretsBucket(normalizedProjectPath, secrets); + return Effect.runPromise(this.updateSecretsBucketEffect(normalizedProjectPath, secrets)); } } diff --git a/src/node/orpc/router.ts b/src/node/orpc/router.ts index 01a18115e5..6faeaee27f 100644 --- a/src/node/orpc/router.ts +++ b/src/node/orpc/router.ts @@ -28,6 +28,7 @@ import { setWorkspaceMcpOverrides, } from "@/node/services/agentPlugins/workspacePluginOperations"; import { handlerGen } from "@orpc/experimental-effect"; +import { Effect } from "effect"; import { assertMemoryEnabled, consolidateMemoryEffect, @@ -189,18 +190,36 @@ export const router = (authToken?: string) => { .output(schemas.tokenizer.calculateStats.output) .handler(({ context, input }) => context.tokenizerService.calculateWorkspaceStats(input)), }, + // Config-backed procedures ride handlerGen. Interruption posture (also applies to + // the `config` and `uiLayouts` namespaces below): reads are single Effect.sync + // steps (interruption is a don't-care); mutations wrap the whole pre-Effect + // handler body in one Effect.promise thunk, so they are uninterruptible by + // construction — a client abort interrupts the handler fiber, never the in-flight + // Semaphore(1)-serialized config edit, and multi-step bodies (mutate + notify) + // cannot be torn apart. Rejections become defects, surfacing as the same internal + // error the old async handlers produced. splashScreens: { getViewedSplashScreens: t .input(schemas.splashScreens.getViewedSplashScreens.input) .output(schemas.splashScreens.getViewedSplashScreens.output) - .handler(({ context }) => { - const config = context.config.loadConfigOrDefault(); - return config.viewedSplashScreens ?? []; - }), + .handler( + handlerGen(function* ({ context }) { + return yield* Effect.sync(() => { + const config = context.config.loadConfigOrDefault(); + return config.viewedSplashScreens ?? []; + }); + }) + ), markSplashScreenViewed: t .input(schemas.splashScreens.markSplashScreenViewed.input) .output(schemas.splashScreens.markSplashScreenViewed.output) - .handler(({ context, input }) => context.config.markSplashScreenViewed(input.splashId)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.markSplashScreenViewed(input.splashId) + ); + }) + ), }, server: { getLaunchProject: t @@ -253,7 +272,13 @@ export const router = (authToken?: string) => { getConfig: t .input(schemas.config.getConfig.input) .output(schemas.config.getConfig.output) - .handler(({ context }) => context.config.getClientConfig()), + .handler( + handlerGen(function* ({ context }) { + return yield* Effect.sync(() => context.config.getClientConfig()); + }) + ), + // Event-iterator subscription: stays on the plain handler until the Effect + // Stream bridge phase converts event subscriptions wholesale. onConfigChanged: t .input(schemas.config.onConfigChanged.input) .output(schemas.config.onConfigChanged.output) @@ -261,89 +286,155 @@ export const router = (authToken?: string) => { updateAgentAiDefaults: t .input(schemas.config.updateAgentAiDefaults.input) .output(schemas.config.updateAgentAiDefaults.output) - .handler(({ context, input }) => - context.config.updateAgentAiDefaults(input.agentAiDefaults) + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateAgentAiDefaults(input.agentAiDefaults) + ); + }) ), updateMuxGatewayPrefs: t .input(schemas.config.updateMuxGatewayPrefs.input) .output(schemas.config.updateMuxGatewayPrefs.output) - .handler(async ({ context, input }) => { - await context.config.updateMuxGatewayPrefs(input); - context.providerService.notifyConfigChanged(); - }), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => { + await context.config.updateMuxGatewayPrefs(input); + context.providerService.notifyConfigChanged(); + }); + }) + ), updateRoutePreferences: t .input(schemas.config.updateRoutePreferences.input) .output(schemas.config.updateRoutePreferences.output) - .handler(({ context, input }) => context.providerService.updateRoutePreferences(input)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.providerService.updateRoutePreferences(input) + ); + }) + ), updateMinThinkingLevels: t .input(schemas.config.updateMinThinkingLevels.input) .output(schemas.config.updateMinThinkingLevels.output) - .handler(({ context, input }) => - context.config.updateMinThinkingLevels(input.minThinkingLevelByModel) + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateMinThinkingLevels(input.minThinkingLevelByModel) + ); + }) ), updateModelFallbacks: t .input(schemas.config.updateModelFallbacks.input) .output(schemas.config.updateModelFallbacks.output) - .handler(({ context, input }) => context.config.updateModelFallbacks(input.modelFallbacks)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateModelFallbacks(input.modelFallbacks) + ); + }) + ), updateModelPreferences: t .input(schemas.config.updateModelPreferences.input) .output(schemas.config.updateModelPreferences.output) - .handler(({ context, input }) => context.config.updateModelPreferences(input)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => context.config.updateModelPreferences(input)); + }) + ), updateCoderPrefs: t .input(schemas.config.updateCoderPrefs.input) .output(schemas.config.updateCoderPrefs.output) - .handler(({ context, input }) => context.config.updateCoderPrefs(input)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => context.config.updateCoderPrefs(input)); + }) + ), updateRuntimeEnablement: t .input(schemas.config.updateRuntimeEnablement.input) .output(schemas.config.updateRuntimeEnablement.output) - .handler(({ context, input }) => context.config.updateRuntimeEnablement(input)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => context.config.updateRuntimeEnablement(input)); + }) + ), saveConfig: t .input(schemas.config.saveConfig.input) .output(schemas.config.saveConfig.output) - .handler(async ({ context, input }) => { - await context.config.saveUserConfig(input); - await context.taskService.maybeStartQueuedTasks(); - }), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => { + await context.config.saveUserConfig(input); + await context.taskService.maybeStartQueuedTasks(); + }); + }) + ), updateChatTranscriptFullWidth: t .input(schemas.config.updateChatTranscriptFullWidth.input) .output(schemas.config.updateChatTranscriptFullWidth.output) - .handler(({ context, input }) => - context.config.updateChatTranscriptFullWidth(input.enabled) + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateChatTranscriptFullWidth(input.enabled) + ); + }) ), updateLlmDebugLogs: t .input(schemas.config.updateLlmDebugLogs.input) .output(schemas.config.updateLlmDebugLogs.output) - .handler(({ context, input }) => context.config.updateLlmDebugLogs(input.enabled)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => context.config.updateLlmDebugLogs(input.enabled)); + }) + ), updateHeartbeatDefaultPrompt: t .input(schemas.config.updateHeartbeatDefaultPrompt.input) .output(schemas.config.updateHeartbeatDefaultPrompt.output) - .handler(({ context, input }) => - context.config.updateHeartbeatDefaultPrompt(input.defaultPrompt) + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateHeartbeatDefaultPrompt(input.defaultPrompt) + ); + }) ), updateHeartbeatDefaultIntervalMs: t .input(schemas.config.updateHeartbeatDefaultIntervalMs.input) .output(schemas.config.updateHeartbeatDefaultIntervalMs.output) - .handler(({ context, input }) => - context.config.updateHeartbeatDefaultIntervalMs(input.intervalMs) + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateHeartbeatDefaultIntervalMs(input.intervalMs) + ); + }) ), updateGoalDefaults: t .input(schemas.config.updateGoalDefaults.input) .output(schemas.config.updateGoalDefaults.output) - .handler(({ context, input }) => context.config.updateGoalDefaults(input.goalDefaults)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.updateGoalDefaults(input.goalDefaults) + ); + }) + ), unenrollMuxGovernor: t .input(schemas.config.unenrollMuxGovernor.input) .output(schemas.config.unenrollMuxGovernor.output) - .handler(async ({ context }) => { - await context.config.unenrollMuxGovernor(); - await context.policyService.refreshNow(); - }), + .handler( + handlerGen(function* ({ context }) { + yield* Effect.promise(async () => { + await context.config.unenrollMuxGovernor(); + await context.policyService.refreshNow(); + }); + }) + ), }, devtools: { getRuns: t @@ -404,14 +495,24 @@ export const router = (authToken?: string) => { getAll: t .input(schemas.uiLayouts.getAll.input) .output(schemas.uiLayouts.getAll.output) - .handler(({ context }) => { - const config = context.config.loadConfigOrDefault(); - return config.layoutPresets ?? DEFAULT_LAYOUT_PRESETS_CONFIG; - }), + .handler( + handlerGen(function* ({ context }) { + return yield* Effect.sync(() => { + const config = context.config.loadConfigOrDefault(); + return config.layoutPresets ?? DEFAULT_LAYOUT_PRESETS_CONFIG; + }); + }) + ), saveAll: t .input(schemas.uiLayouts.saveAll.input) .output(schemas.uiLayouts.saveAll.output) - .handler(({ context, input }) => context.config.saveLayoutPresets(input.layoutPresets)), + .handler( + handlerGen(function* ({ context }, input) { + yield* Effect.promise(async () => + context.config.saveLayoutPresets(input.layoutPresets) + ); + }) + ), }, agents: { list: t From 75e3602c4ffd1a521bd1ee3a416724d011822717 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 1 Sep 2026 13:54:06 +0000 Subject: [PATCH 2/2] fix: keep saveConfig as spy-compatible Promise facade over saveConfigEffect --- src/node/config/index.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/node/config/index.ts b/src/node/config/index.ts index fb96d40973..d49ba817b0 100644 --- a/src/node/config/index.ts +++ b/src/node/config/index.ts @@ -1814,10 +1814,18 @@ export class Config { * as a permanent sidebar ghost. All mutations must go through editConfig so each * write is derived from a fresh serialized read. * + * Kept as a Promise facade (tests spy on it with Promise mocks to simulate + * swallowed writes); saveConfigEffect below holds the actual pipeline. + */ + private saveConfig(config: ProjectsConfig): Promise { + return Effect.runPromise(this.saveConfigEffect(config)); + } + + /** * Never fails: the whole pipeline folds every failure and defect into the same * log-and-swallow the old try/catch applied (total catch discipline). */ - private saveConfig(config: ProjectsConfig): Effect.Effect { + private saveConfigEffect(config: ProjectsConfig): Effect.Effect { // eslint-disable-next-line @typescript-eslint/no-this-alias -- Effect.gen generator bodies do not inherit `this` const self = this; return Effect.gen(function* () { @@ -2514,7 +2522,9 @@ export class Config { }, catch: (error) => error, }); - yield* self.saveConfig(newConfig); + // Route through the saveConfig Promise facade (not saveConfigEffect) so test + // spies on saveConfig keep intercepting the serialized write. + yield* Effect.promise(async () => self.saveConfig(newConfig)); // Backend-initiated config edits (for example gateway auth changes) use this signal // so frontend subscribers can refresh derived state without polling. self.notifyConfigChanged();