From 389e8afc77d42617e0efb7227235d0abd9e7ecf6 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 26 Sep 2026 19:09:13 +0800 Subject: [PATCH 1/6] feat: make the default backend configurable with session.defaultProvider The registry default was the literal "claude", so an operator running only pi, codex or qwen had to pass --provider on every session. It now comes from session.defaultProvider (env CODEOID_DEFAULT_PROVIDER). - Fails fast: a misspelled, disabled or uninstalled default stops daemon startup with one actionable line, instead of resolve()'s warn-and-fall-back (which exists for resuming sessions from a newer codeoid). - settings.set checks the value against the live registry with the same rule, so the settings UI can't save a default the next boot refuses. - "No provider" now means the registry default everywhere it meant claude: models.list, session.create model validation, pack-role model validation, and dispatch model canonicalisation. DEFAULT_PROVIDER_ID, which always meant claude, is gone. - A resumed meta with no providerId predates multi-backend support and stays on claude, rather than moving to the new default and losing its backing conversation. The conductor keeps conductor.provider. - Telegram /model lists the attached session's catalog instead of the default backend's. Closes #339 Co-Authored-By: Claude Opus 5.5 (1M context) --- README.md | 3 + docs/CONFIGURATION.md | 8 ++ src/cli.ts | 56 +++++---- src/config.ts | 15 ++- src/daemon/models.ts | 12 +- src/daemon/providers/registry.ts | 42 ++++++- src/daemon/session-manager.ts | 52 +++++--- src/daemon/settings/manifest.ts | 4 + src/frontends/telegram/index.ts | 9 +- src/tests/config.test.ts | 17 +++ src/tests/default-provider.test.ts | 187 ++++++++++++++++++++++++++++ src/tests/models.test.ts | 25 +++- src/tests/provider-registry.test.ts | 42 +++++++ src/tests/telegram-flows.test.ts | 20 ++- 14 files changed, 435 insertions(+), 57 deletions(-) create mode 100644 src/tests/default-provider.test.ts diff --git a/README.md b/README.md index 014226bc..4b8a90a2 100644 --- a/README.md +++ b/README.md @@ -262,6 +262,9 @@ Rule of thumb: reach for Omnigent when you need OS-level isolation and the broad - **In-daemon API backends** (`openai`, `gemini`) register only when their **API key** is set in `~/.codeoid/.env`. These bill against the key, not a subscription. Pick a backend per session with `codeoid new --provider `, or switch a live session with `/provider `. +To make another backend the default for new sessions, set `session.defaultProvider` in `config.json` (or `CODEOID_DEFAULT_PROVIDER`), e.g. `{"session": {"defaultProvider": "pi"}}`. +The daemon refuses to start if that backend is misspelled, disabled, or not installed, rather than silently falling back to Claude. +Existing sessions keep the backend they were created on, and the conductor stays on `conductor.provider`. Set keys from the Settings screen (⚙ / `/settings`) or by editing `~/.codeoid/.env` — see [Configuration](docs/CONFIGURATION.md) for every variable. > **Gemini needs an API key (or Vertex) — a consumer Google (AI Pro/Ultra) subscription can't be used with Codeoid.** diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 3025e1cc..f323c0ab 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -85,6 +85,11 @@ CODEOID_RESUME_MAX_SESSIONS=200 # sessions restored from disk at startu # Resume is also time-boxed, so raising this costs # startup time; the remainder stays on disk and loads # on a later restart. +CODEOID_DEFAULT_PROVIDER=claude # backend for sessions created without --provider + # (claude | codex | pi | qwen | gemini-cli | openai | gemini). + # An unknown, disabled or uninstalled backend stops the + # daemon at startup. Resumed sessions and the conductor + # keep their own backend. # Memory CODEOID_MEMORY=1 # default: on; set to 0 to disable @@ -147,6 +152,9 @@ Optional `~/.codeoid/config.json` (env vars take precedence): "enabled": true, "dbPath": "~/.codeoid/memory.db", "model": "Xenova/bge-small-en-v1.5" + }, + "session": { + "defaultProvider": "claude" } } ``` diff --git a/src/cli.ts b/src/cli.ts index 04f9b2b9..6a4e55d2 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -21,6 +21,7 @@ import { program } from "commander"; // string to drift). release-smoke asserts these two stay equal. import pkg from "../package.json" with { type: "json" }; import { DaemonServer } from "./daemon/server.js"; +import { DefaultProviderError } from "./daemon/providers/registry.js"; import { parseRoleSpec } from "./daemon/collaboration.js"; import { type ModelBinding, roleBindingsFromSpecs } from "./daemon/pipeline/binding.js"; import { @@ -99,29 +100,38 @@ program }; } - const daemon = new DaemonServer({ - port: bindPort, - host: bindHost, - dbPath: config.dbPath, - transcriptDir: config.transcriptDir, - auth: config.auth, - localMode, - // ZeroID-dependent subsystems are simply not passed in local mode. (The - // daemon guards these too — belt and suspenders — so an embedder that - // builds its own DaemonConfig gets the same offline guarantee.) - oauth: localMode ? undefined : config.oauth, - agentIdentity: localMode ? undefined : config.agentIdentity, - memory: config.memory?.enabled - ? { - dbPath: config.memory.dbPath, - model: config.memory.model, - modelCacheDir: config.memory.modelCacheDir, - } - : undefined, - // Forward the full config so session-level features (compress, etc.) - // get the parsed shape rather than re-reading env/file. - fullConfig: config, - }); + let daemon: DaemonServer; + try { + daemon = new DaemonServer({ + port: bindPort, + host: bindHost, + dbPath: config.dbPath, + transcriptDir: config.transcriptDir, + auth: config.auth, + localMode, + // ZeroID-dependent subsystems are simply not passed in local mode. (The + // daemon guards these too — belt and suspenders — so an embedder that + // builds its own DaemonConfig gets the same offline guarantee.) + oauth: localMode ? undefined : config.oauth, + agentIdentity: localMode ? undefined : config.agentIdentity, + memory: config.memory?.enabled + ? { + dbPath: config.memory.dbPath, + model: config.memory.model, + modelCacheDir: config.memory.modelCacheDir, + } + : undefined, + // Forward the full config so session-level features (compress, etc.) + // get the parsed shape rather than re-reading env/file. + fullConfig: config, + }); + } catch (err) { + // A misconfigured default backend is operator error: say what to fix + // and stop. Anything else is a bug and keeps its stack trace. + if (!(err instanceof DefaultProviderError)) throw err; + console.error(`\n[codeoid] ${err.message}\n`); + process.exit(1); + } // ── Register frontends ──────────────────────────────────────── diff --git a/src/config.ts b/src/config.ts index edc34a71..14ca40e2 100644 --- a/src/config.ts +++ b/src/config.ts @@ -308,7 +308,9 @@ const TelemetrySchema = z * sane defaults; users can tune via config or env. */ /** - * Per-session model defaults. `defaultModel` is used on session creation; + * Per-session defaults. `defaultProvider` picks the backend a new session runs + * on when the caller names none (unset = "claude"); `defaultModel` is used on + * session creation; * `fallbackModel` is handed to the SDK's `fallbackModel` option so a 429 * or 529 transparently retries with a cheaper/less-loaded model instead of * failing the turn. Both accept aliases (`opus`/`sonnet`/`haiku`) or full @@ -316,6 +318,14 @@ const TelemetrySchema = z */ const SessionSchema = z .object({ + /** + * Backend for a session created without a provider. Must name a backend + * this daemon registers — a typo, a disabled backend, or one whose binary + * is missing fails daemon startup rather than silently reverting to + * "claude". Resumed sessions keep the backend they were created on; the + * conductor keeps `conductor.provider`. + */ + defaultProvider: z.string().trim().min(1).optional(), defaultModel: z.string().optional(), fallbackModel: z.string().optional(), /** @@ -1013,6 +1023,8 @@ export interface CodeoidConfig { }; /** Model selection defaults applied when a session is created. */ session: { + /** Backend for a session created without a provider (unset = "claude"). Validated at startup. */ + defaultProvider?: string; defaultModel?: string; fallbackModel?: string; /** Stall watchdog: ms of event-stream silence while the model should be generating before a turn is force-recovered (0 = off; paused during tool execution and pending approvals). Defaults to 300000 when omitted. */ @@ -1220,6 +1232,7 @@ const ENV_OVERRIDES: readonly EnvOverride[] = [ { env: "CODEOID_AUTO_ROTATE_PCT", path: "autoRotate.rotatePct", kind: "float" }, { env: "CODEOID_AUTO_ROTATE_HARD_PCT", path: "autoRotate.hardRotatePct", kind: "float" }, { env: "CODEOID_AUTO_ROTATE_MIN_TURNS", path: "autoRotate.minTurnsBeforeRotate", kind: "int" }, + { env: "CODEOID_DEFAULT_PROVIDER", path: "session.defaultProvider", kind: "string" }, { env: "CODEOID_DEFAULT_MODEL", path: "session.defaultModel", kind: "string" }, // Dispatch kill switch — disable send-class fleet dispatch per-invocation // without touching config.json. Other dispatch knobs are file-config only, diff --git a/src/daemon/models.ts b/src/daemon/models.ts index b419394a..d0e47a3b 100644 --- a/src/daemon/models.ts +++ b/src/daemon/models.ts @@ -117,11 +117,13 @@ export function resolveModelId(identifier: string): string | null { export const DEFAULT_MODEL_ALIAS = "opus"; /** - * The provider id whose model space this catalog describes. Must match the - * default id the provider registry is built with - * (`createDefaultProviderRegistry` → `new ProviderRegistry("claude")`); - * `DEFAULT_PROVIDER_ID` in session-manager re-exports this so there is one - * source of truth. + * The provider id whose model space this catalog (and `DEFAULT_MODEL_ALIAS`) + * describes. It is also the registry's default when `session.defaultProvider` + * is unset — but only that: the configured default can be any backend, so + * code asking "which backend will this session run on?" reads + * `ProviderRegistry.defaultId`, and code asking "is this Claude?" compares + * against this constant. Conflating the two is how a non-Claude default ends + * up validated against Claude's catalog. */ export const CLAUDE_PROVIDER_ID = "claude"; diff --git a/src/daemon/providers/registry.ts b/src/daemon/providers/registry.ts index 436eb18a..b3c1bcf2 100644 --- a/src/daemon/providers/registry.ts +++ b/src/daemon/providers/registry.ts @@ -22,6 +22,7 @@ import type { McpHub } from "../mcp/hub.js"; import type { CompressionRegistry } from "../compress/index.js"; import type { CodeoidConfig } from "../../config.js"; import type { SessionProvider, CatalogEntry } from "./interface.js"; +import { CLAUDE_PROVIDER_ID } from "../models.js"; import { ClaudeProvider } from "./claude/index.js"; import { GeminiProvider } from "./gemini/index.js"; import { OpenAIProvider } from "./openai/index.js"; @@ -88,7 +89,7 @@ export class ProviderRegistry { /** Id used when a session doesn't carry a provider selection. */ readonly defaultId: string; - constructor(defaultId = "claude") { + constructor(defaultId = CLAUDE_PROVIDER_ID) { this.defaultId = defaultId; } @@ -156,13 +157,48 @@ export class ProviderRegistry { } } +/** + * A `session.defaultProvider` the daemon cannot honour. Its own class so the + * CLI can print it as the operator's config mistake it is — one line, exit 1 + * — without also swallowing the stack trace of a genuine startup bug. + */ +export class DefaultProviderError extends Error { + override name = "DefaultProviderError"; +} + +/** + * Why `id` cannot be this daemon's default backend, or undefined when it can. + * + * Shared by startup (`createDefaultProviderRegistry`) and the settings write + * path, so a value the settings UI accepts is exactly a value the next boot + * accepts. A backend codeoid supports but could not activate (binary missing, + * no API key) gets its actionable hint; anything else is a typo or a backend + * disabled under `providers..enabled`. + */ +export function defaultProviderProblem(registry: ProviderRegistry, id: string): string | undefined { + if (registry.has(id)) return undefined; + const hint = registry.unavailableHint(id); + if (hint) return `session.defaultProvider "${id}" is not available on this daemon: ${hint}`; + return ( + `session.defaultProvider "${id}" is not a registered backend ` + + `(registered: ${registry.ids().join(", ")}). ` + + `Check the spelling, or whether providers.${id}.enabled is false.` + ); +} + /** * The built-in backends. Daemon startup builds exactly one of these. * `config` gates optional backends (pi can be disabled) and carries their * settings (binary path); absent = every backend with defaults. + * + * `config.session.defaultProvider` picks the default backend and is checked + * once every backend has registered. It THROWS rather than warns: unlike + * `resolve()`'s fallback — which exists so resume survives a session written + * by a newer codeoid — a misspelled default is operator error, and falling + * back would silently put every new session on a backend they opted out of. */ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderRegistry { - const registry = new ProviderRegistry("claude"); + const registry = new ProviderRegistry(config?.session?.defaultProvider ?? CLAUDE_PROVIDER_ID); registry.register({ id: "claude", displayName: "Claude (Anthropic)", @@ -352,5 +388,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR }), }); } + const problem = defaultProviderProblem(registry, registry.defaultId); + if (problem) throw new DefaultProviderError(problem); return registry; } diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index e649557d..b2b08592 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -15,6 +15,7 @@ import { Session, type AttachedClient, type WindowScope } from "./session.js"; import { type CatalogEntry, isPlaceholderModel, type SessionProvider } from "./providers/interface.js"; import { createDefaultProviderRegistry, + defaultProviderProblem, type ProviderRegistry, } from "./providers/registry.js"; import type { HookBus } from "./hooks/bus.js"; @@ -297,12 +298,6 @@ const RESUME_DEADLINE_MS = 20_000; * arrival, so cap the read slightly above the scrollback byte cap. */ const RESUME_TRANSCRIPT_MAX_BYTES = 24 * 1024 * 1024; -/** Provider assumed when a client doesn't say which catalog it wants. - * Re-exported from models.ts so the id that gates Claude-only alias - * expansion (`resolveModelIdForProvider`) and the id used for catalog - * defaults can never drift apart. */ -export const DEFAULT_PROVIDER_ID = CLAUDE_PROVIDER_ID; - /** Sort key for resume ordering: most-recently-active first. Falls back to * createdAt, then 0, so a malformed timestamp never throws. */ function resumeSortKey(m: { lastActivityAt?: string; createdAt?: string }): number { @@ -705,7 +700,11 @@ mcpHub: this.#mcpHub, // The conductor self-persists (design R2): its role, provider // selection, and fleet tools all come back across a restart. role: meta.role, - providerId: meta.providerId, + // Absent = a meta written before providers existed, which means + // claude. NOT the registry default: with session.defaultProvider + // set, an old session would otherwise resume on another backend and + // lose its backing conversation. + providerId: meta.providerId ?? CLAUDE_PROVIDER_ID, forkedFrom: meta.forkedFrom, worktree: meta.worktree, // A collaboration is durable state, not turn state: the goal and @@ -882,7 +881,7 @@ mcpHub: this.#mcpHub, { roleName: role.roleName, ordinal: role.ordinal, - providerId: meta.providerId ?? "claude", + providerId: meta.providerId ?? CLAUDE_PROVIDER_ID, shape: role.write ? "ship" : "scout", write: role.write, }, @@ -1551,7 +1550,8 @@ mcpHub: this.#mcpHub, const persisted = this.#persistedModels(providerId); if (persisted) return { models: persisted, live: false }; return { - models: providerId === DEFAULT_PROVIDER_ID ? fallbackModelInfos() : [], + // The built-in fallback is Claude's catalog, whatever the default is. + models: providerId === CLAUDE_PROVIDER_ID ? fallbackModelInfos() : [], live: false, }; } @@ -1574,7 +1574,8 @@ mcpHub: this.#mcpHub, #modelsList( msg: Extract, ): DaemonMessage { - const provider = msg.provider ?? DEFAULT_PROVIDER_ID; + // No provider = the catalog of the backend a new session would land on. + const provider = msg.provider ?? this.#providers.defaultId; const { models, live } = this.#currentModels(provider); return { type: "models.list.result", requestId: msg.id, models, live, provider }; } @@ -1662,6 +1663,25 @@ mcpHub: this.#mcpHub, code: "forbidden", }; } + // The default backend is only checked at startup, and a bad one refuses + // to boot — so a typo saved here would take the daemon down on its next + // restart. Check it against the live registry now, with the same rule the + // boot uses, and reject the batch before anything is written. + const providerErrors = msg.patches.flatMap((p) => { + if (p.key !== "session.defaultProvider" || typeof p.value !== "string") return []; + const problem = p.value.trim() ? defaultProviderProblem(this.#providers, p.value.trim()) : undefined; + return problem ? [{ key: p.key, message: problem }] : []; + }); + if (providerErrors.length > 0) { + return { + type: "settings.set.result", + requestId: msg.id, + ok: false, + snapshot: { ...getSnapshot(), mcpServers: this.#mcpServerStatuses() }, + errors: providerErrors, + restartRequired: false, + }; + } try { const result = applyPatches(msg.patches); this.#store.audit( @@ -2405,7 +2425,7 @@ mcpHub: this.#mcpHub, code: "invalid_request", }; } - const forProvider = providerId ?? DEFAULT_PROVIDER_ID; + const forProvider = providerId ?? this.#providers.defaultId; const resolved = resolveModelIdForProvider(msg.model, forProvider); if (!resolved) { return { @@ -2450,7 +2470,7 @@ mcpHub: this.#mcpHub, code: "invalid_request", }; } - const targetProvider = resolved.provider ?? providerId ?? DEFAULT_PROVIDER_ID; + const targetProvider = resolved.provider ?? providerId ?? this.#providers.defaultId; // A chain-resolved model goes through the SAME provider-aware // validation as an explicit --model — but SKIPS with a warning rather // than hard-failing the create: the operator never typed this id, so a @@ -3311,8 +3331,10 @@ mcpHub: this.#mcpHub, mkdirSync(workdir, { recursive: true }); const conductorConfig = this.#config?.conductor; - const providerId = conductorConfig?.provider ?? DEFAULT_PROVIDER_ID; - if (providerId !== "claude") { + // Deliberately NOT session.defaultProvider: the conductor needs the fleet + // MCP tools, which only the claude provider mounts today. + const providerId = conductorConfig?.provider ?? CLAUDE_PROVIDER_ID; + if (providerId !== CLAUDE_PROVIDER_ID) { console.warn( `[codeoid] conductor provider is "${providerId}" — MCP fleet tools are only surfaced by the claude provider today; the conductor will chat but cannot see the fleet`, ); @@ -4560,7 +4582,7 @@ mcpHub: this.#mcpHub, // IdForProvider(...)`, which read as strict validation but could never // reject anything — the fallback's last branch returns the input // unchanged. The dead branch is gone; only the real rule remains. - const providerId = provider ?? DEFAULT_PROVIDER_ID; + const providerId = provider ?? this.#providers.defaultId; const { models } = this.#currentModels(providerId); const canonical = models.length > 0 ? resolveAgainstList(model, models) : null; diff --git a/src/daemon/settings/manifest.ts b/src/daemon/settings/manifest.ts index cb895b3a..b2757582 100644 --- a/src/daemon/settings/manifest.ts +++ b/src/daemon/settings/manifest.ts @@ -51,6 +51,10 @@ const general: SettingsTab = { title: "Session defaults", description: "Applied when a new session is created.", fields: [ + cfg("session.defaultProvider", "Default backend", "Backend for new sessions that don't pick one (claude, codex, pi, qwen, …). Must be a backend this daemon has available — an unknown or unavailable one stops the daemon at startup. Existing sessions and the conductor keep their own.", { + envVar: "CODEOID_DEFAULT_PROVIDER", + placeholder: "claude", + }), cfg("session.defaultModel", "Default model", "Model for new sessions — an alias (opus / sonnet / haiku) or a full model id.", { envVar: "CODEOID_DEFAULT_MODEL", placeholder: "opus", diff --git a/src/frontends/telegram/index.ts b/src/frontends/telegram/index.ts index 8a16ae71..cddf20e2 100644 --- a/src/frontends/telegram/index.ts +++ b/src/frontends/telegram/index.ts @@ -806,9 +806,14 @@ export class TelegramFrontend implements Frontend { } const arg = ctx.message?.text?.split(/\s+/)[1]; if (!arg) { - // No argument → list the live model catalog. + // No argument → list the attached session's catalog. Omitting the + // provider would list the daemon DEFAULT backend's models, which are + // the wrong ones for a session on any other backend. + const provider = state.attachedSessionName + ? this.#manager.findByName(state.attachedSessionName, state.auth!)?.providerId + : undefined; const resp = await this.#manager.handle( - { type: "models.list", id: randomUUID() }, + { type: "models.list", id: randomUUID(), ...(provider ? { provider } : {}) }, state.auth!, this.#makeClient(state, ctx), ); diff --git a/src/tests/config.test.ts b/src/tests/config.test.ts index 88b78530..e4e361a8 100644 --- a/src/tests/config.test.ts +++ b/src/tests/config.test.ts @@ -173,6 +173,23 @@ describe("loadConfig — env precedence", () => { expect(c.compress.excludeCommands).toEqual(["curl", "wget", "playwright"]); }); + it("session.defaultProvider: unset by default, set from file, overridden by CODEOID_DEFAULT_PROVIDER", () => { + writeConfig({}); + expect(loadConfig({ configPath, env: {} }).session.defaultProvider).toBeUndefined(); + + writeConfig({ session: { defaultProvider: " pi " } }); + // Trimmed, so a stray space can't become an unregistered id at startup. + expect(loadConfig({ configPath, env: {} }).session.defaultProvider).toBe("pi"); + expect(loadConfig({ configPath, env: { CODEOID_DEFAULT_PROVIDER: "codex" } }).session.defaultProvider).toBe("codex"); + // Empty env = no override — the file value wins. + expect(loadConfig({ configPath, env: { CODEOID_DEFAULT_PROVIDER: "" } }).session.defaultProvider).toBe("pi"); + }); + + it("session.defaultProvider rejects a blank value rather than treating it as a backend id", () => { + writeConfig({ session: { defaultProvider: " " } }); + expect(() => loadConfig({ configPath, env: {} })).toThrow(/defaultProvider/); + }); + it("boolean env accepts 1/true, ignores empty", () => { writeConfig({ memory: { enabled: true } }); const c1 = loadConfig({ configPath, env: { CODEOID_MEMORY: "1" } }); diff --git a/src/tests/default-provider.test.ts b/src/tests/default-provider.test.ts new file mode 100644 index 00000000..2e2ac30e --- /dev/null +++ b/src/tests/default-provider.test.ts @@ -0,0 +1,187 @@ +/** + * `session.defaultProvider` (#339) at the SessionManager layer. + * + * The registry-level rules (the configured id becomes `defaultId`; a typo, + * a disabled backend or an uninstalled one fails startup) live in + * provider-registry.test.ts. These tests pin what the daemon DOES with a + * non-Claude default — the places that used to assume "no provider" meant + * claude even though the session would be built on the registry default: + * + * - a provider-less create lands on the default and validates its model + * against THAT backend, not Claude's catalog; + * - a legacy session meta with no provider resumes on claude, never on the + * new default (it would lose its backing conversation); + * - the settings write path refuses a default the next boot would refuse. + * + * The registry is real (not `_testProviderFactory`) so the provider a session + * reports is the one the registry resolved, which is the thing under test. + */ + +import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Store } from "../daemon/store.js"; +import { TranscriptStore } from "../daemon/transcript.js"; +import { SessionManager } from "../daemon/session-manager.js"; +import { ProviderRegistry, type ProviderFactory } from "../daemon/providers/registry.js"; +import { MockSessionProvider } from "../daemon/providers/mock/session-provider.js"; +import type { AttachedClient } from "../daemon/session.js"; +import type { AuthContext, SettingsSetResultMsg } from "../protocol/types.js"; +import { ALL_SCOPES } from "../protocol/scopes.js"; +import { configFilePaths } from "../config.js"; + +const OWNER: AuthContext = { + sub: "user:default-provider", + scopes: [...ALL_SCOPES] as AuthContext["scopes"], + delegationDepth: 0, + accountId: "acc-dp", + projectId: "proj-dp", +}; +const client: AttachedClient = { id: "client-dp", auth: OWNER, send: () => {} }; + +function mockFactory(id: string): ProviderFactory { + return { id, displayName: `Mock ${id}`, create: () => new MockSessionProvider(id) }; +} + +/** A daemon configured with `session.defaultProvider: "pi"`. */ +function piDefaultRegistry(): ProviderRegistry { + const registry = new ProviderRegistry("pi"); + registry.register(mockFactory("claude")); + registry.register(mockFactory("pi")); + return registry; +} + +let tmp: string; +let store: Store; +let transcript: TranscriptStore; +let prevXdg: string | undefined; + +beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), "codeoid-default-provider-")); + store = new Store(join(tmp, "codeoid.db")); + transcript = new TranscriptStore(join(tmp, "transcripts")); + // settings.set writes config.json — keep it off the real ~/.codeoid. + prevXdg = process.env.XDG_CONFIG_HOME; + process.env.XDG_CONFIG_HOME = tmp; +}); + +afterEach(async () => { + if (prevXdg === undefined) delete process.env.XDG_CONFIG_HOME; + else process.env.XDG_CONFIG_HOME = prevXdg; + try { await transcript.flush(); } catch {} + try { store.close(); } catch {} + try { rmSync(tmp, { recursive: true, force: true }); } catch {} +}); + +const manager = () => + new SessionManager(store, transcript, undefined, undefined, undefined, { providers: piDefaultRegistry() }); + +describe("a non-claude default backend", () => { + it("is advertised first, so clients preselect it", () => { + expect(manager().providerIds()[0]).toBe("pi"); + }); + + it("is what a provider-less session.create lands on", async () => { + const resp = await manager().handle( + { type: "session.create", id: "c1", name: "plain", workdir: tmp }, + OWNER, + client, + ); + expect(resp.type).toBe("response.ok"); + if (resp.type !== "response.ok") return; + expect((resp.data as { providerId?: string }).providerId).toBe("pi"); + }); + + it("validates a provider-less create's model against itself, not Claude's catalog", async () => { + // Before #339 this resolved "opus" as a Claude alias and built a pi + // session carrying claude-opus-*, which pi would reject on the first turn. + const resp = await manager().handle( + { type: "session.create", id: "c2", name: "opus-on-pi", workdir: tmp, model: "opus" }, + OWNER, + client, + ); + expect(resp).toMatchObject({ type: "response.error", code: "invalid_request" }); + if (resp.type === "response.error") { + expect(resp.error).toMatch(/Model "opus" is not valid for provider "pi"/); + } + }); + + it("still takes an explicit claude session and its alias", async () => { + const resp = await manager().handle( + { type: "session.create", id: "c3", name: "claude", workdir: tmp, providerId: "claude", model: "opus" }, + OWNER, + client, + ); + expect(resp.type).toBe("response.ok"); + if (resp.type !== "response.ok") return; + const info = resp.data as { providerId?: string; model?: string }; + expect(info.providerId).toBe("claude"); + expect(info.model).toMatch(/^claude-opus-/); + }); +}); + +describe("resume under a non-claude default", () => { + const legacyMeta = (sessionId: string, providerId?: string) => ({ + sessionId, + sessionName: sessionId, + workdir: tmp, + createdBy: OWNER.sub, + createdAt: new Date().toISOString(), + lastStatus: "idle" as const, + lastActivityAt: new Date().toISOString(), + accountId: OWNER.accountId!, + projectId: OWNER.projectId!, + ...(providerId ? { providerId } : {}), + }); + + it("keeps a pre-provider meta on claude instead of moving it to the default", async () => { + // A meta with no providerId predates multi-backend support: it IS a + // claude session. Resolving it through the registry default would resume + // it on pi and orphan its Claude backing conversation. + await transcript.saveMeta(legacyMeta("legacy")); + await transcript.saveMeta(legacyMeta("modern", "pi")); + await transcript.flush(); + + const m = manager(); + expect(await m.resumeSessions()).toBe(2); + expect(m.findByName("legacy", OWNER)?.providerId).toBe("claude"); + expect(m.findByName("modern", OWNER)?.providerId).toBe("pi"); + }); +}); + +describe("settings.set session.defaultProvider", () => { + const set = (value: string | null) => + manager().handle( + { type: "settings.set", id: "s1", patches: [{ key: "session.defaultProvider", value }] }, + OWNER, + client, + ) as Promise; + + it("refuses a backend the next boot would refuse, and writes nothing", async () => { + const res = await set("claud"); + expect(res.type).toBe("settings.set.result"); + expect(res.ok).toBe(false); + expect(res.restartRequired).toBe(false); + expect(res.errors).toEqual([ + { key: "session.defaultProvider", message: expect.stringMatching(/"claud" is not a registered backend/) }, + ]); + expect(existsSync(configFilePaths().configPath)).toBe(false); + }); + + it("accepts a registered backend and persists it for the next boot", async () => { + const res = await set("claude"); + expect(res.ok).toBe(true); + expect(res.restartRequired).toBe(true); + const onDisk = JSON.parse(readFileSync(configFilePaths().configPath, "utf8")); + expect(onDisk.session.defaultProvider).toBe("claude"); + }); + + it("lets the value be cleared back to the built-in default", async () => { + await set("claude"); + const res = await set(null); + expect(res.ok).toBe(true); + const onDisk = JSON.parse(readFileSync(configFilePaths().configPath, "utf8")); + expect(onDisk.session?.defaultProvider).toBeUndefined(); + }); +}); diff --git a/src/tests/models.test.ts b/src/tests/models.test.ts index d6817132..990cd716 100644 --- a/src/tests/models.test.ts +++ b/src/tests/models.test.ts @@ -21,7 +21,9 @@ import { } from "../daemon/models.js"; import { contextWindowForModel } from "../daemon/context-windows.js"; import { Store } from "../daemon/store.js"; -import { SessionManager, DEFAULT_PROVIDER_ID } from "../daemon/session-manager.js"; +import { SessionManager } from "../daemon/session-manager.js"; +import { createDefaultProviderRegistry } from "../daemon/providers/registry.js"; +import type { CodeoidConfig } from "../config.js"; import type { WindowScope } from "../daemon/session.js"; import { TranscriptStore } from "../daemon/transcript.js"; @@ -235,10 +237,6 @@ describe("resolveModelIdForProvider (per-child backend resolution)", () => { expect(resolveModelIdForProvider(" ", "gemini")).toBeNull(); expect(resolveModelIdForProvider("", CLAUDE_PROVIDER_ID)).toBeNull(); }); - - it("keeps CLAUDE_PROVIDER_ID in lockstep with DEFAULT_PROVIDER_ID", () => { - expect(DEFAULT_PROVIDER_ID).toBe(CLAUDE_PROVIDER_ID); - }); }); describe("Store session.model persistence", () => { @@ -408,12 +406,27 @@ describe("models.list serves live → persisted → baked-in fallback, per provi const manager = new SessionManager(store, new TranscriptStore(join(tmp, "t"))); const res = await listModels(manager); expect(res.live).toBe(false); - expect(res.provider).toBe(DEFAULT_PROVIDER_ID); + expect(res.provider).toBe(CLAUDE_PROVIDER_ID); expect(res.models.map((m) => m.value)).toEqual( fallbackModelInfos().map((m) => m.value), ); }); + it("a configured default backend answers a provider-less request, not claude's fallback", async () => { + // #339: "no provider" means the backend a new session would land on. Serving + // Claude's catalog here would populate the picker with models the default + // backend rejects. + const config = { session: { defaultProvider: "qwen" } } as unknown as CodeoidConfig; + const manager = new SessionManager(store, new TranscriptStore(join(tmp, "t")), undefined, undefined, undefined, { + providers: createDefaultProviderRegistry(config), + }); + const res = await listModels(manager); + expect(res.provider).toBe("qwen"); + expect(res.models).toEqual([]); + // Claude's catalog is still Claude's when asked for by name. + expect((await listModels(manager, "claude")).models.length).toBeGreaterThan(0); + }); + it("non-default provider with no reports yet: empty list, not the claude fallback", async () => { const manager = new SessionManager(store, new TranscriptStore(join(tmp, "t"))); const res = await listModels(manager, "gemini"); diff --git a/src/tests/provider-registry.test.ts b/src/tests/provider-registry.test.ts index 8d3d8850..7260eccd 100644 --- a/src/tests/provider-registry.test.ts +++ b/src/tests/provider-registry.test.ts @@ -10,6 +10,8 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { createDefaultProviderRegistry, + DefaultProviderError, + defaultProviderProblem, ProviderRegistry, type ProviderFactory, type ProviderSessionInit, @@ -106,6 +108,46 @@ describe("ProviderRegistry", () => { expect(registry.defaultId).toBe("claude"); }); + // session.defaultProvider (#339). A config literal is enough — the registry + // reads only `session.defaultProvider` and `providers.*`. + const withDefault = (defaultProvider: string, extra: Record = {}) => + ({ session: { defaultProvider }, ...extra }) as unknown as Parameters[0]; + + it("session.defaultProvider makes another backend the default", () => { + const registry = createDefaultProviderRegistry(withDefault("qwen")); + expect(registry.defaultId).toBe("qwen"); + // A session carrying no provider selection lands on it. + expect(registry.resolve(undefined, "test").id).toBe("qwen"); + }); + + it("a misspelled defaultProvider fails startup instead of reverting to claude", () => { + // Its own class, so the CLI prints it as a config mistake, not a crash. + expect(() => createDefaultProviderRegistry(withDefault("claud"))).toThrow(DefaultProviderError); + expect(() => createDefaultProviderRegistry(withDefault("claud"))).toThrow( + /session\.defaultProvider "claud" is not a registered backend \(registered: claude,/, + ); + }); + + it("a disabled defaultProvider fails startup and names the switch", () => { + expect(() => + createDefaultProviderRegistry(withDefault("pi", { providers: { pi: { enabled: false, command: "pi" } } })), + ).toThrow(/providers\.pi\.enabled is false/); + }); + + it("an unavailable defaultProvider fails startup with that backend's own hint", () => { + // Keys deleted by beforeEach, so openai is supported but not activatable. + expect(() => createDefaultProviderRegistry(withDefault("openai"))).toThrow( + /"openai" is not available on this daemon: .*OPENAI_API_KEY/, + ); + }); + + it("defaultProviderProblem accepts exactly the registered backends", () => { + const registry = createDefaultProviderRegistry(); + expect(defaultProviderProblem(registry, "claude")).toBeUndefined(); + expect(defaultProviderProblem(registry, "qwen")).toBeUndefined(); + expect(defaultProviderProblem(registry, "nope")).toMatch(/not a registered backend/); + }); + it("config can disable the pi backend", () => { withApiKeys(); const registry = createDefaultProviderRegistry({ diff --git a/src/tests/telegram-flows.test.ts b/src/tests/telegram-flows.test.ts index e75308fe..f18a4810 100644 --- a/src/tests/telegram-flows.test.ts +++ b/src/tests/telegram-flows.test.ts @@ -121,9 +121,9 @@ afterEach(async () => { /** Recording fake SessionManager: two known sessions, capture attach clients. */ function makeFakeManager() { - const sessions: Record = { - alpha: { id: "sess-a", name: "alpha", workdir: "/repos/alpha" }, - beta: { id: "sess-b", name: "beta", workdir: "/repos/beta" }, + const sessions: Record = { + alpha: { id: "sess-a", name: "alpha", workdir: "/repos/alpha", providerId: "claude" }, + beta: { id: "sess-b", name: "beta", workdir: "/repos/beta", providerId: "codex" }, }; const handled: any[] = []; const disconnected: string[] = []; @@ -1056,6 +1056,20 @@ describe("Telegram flows — inline-code spans escape only ` and \\", () => { expect(msg.payload.text).toContain("Claude Opus 4\\.8"); }); + it("/model lists the attached session's backend, not the daemon default's", async () => { + // A provider-less models.list answers with the DEFAULT backend's catalog + // (#339), which is the wrong list for a session on any other backend. + const { drive, texts, manager } = await boot(); + + await drive("/attach beta"); + await until(() => texts().some((t) => t.startsWith("Attached to"))); + await drive("/model"); + await until(() => texts().some((t) => t.includes("Models"))); + + const req = manager.handled.find((m) => m.type === "models.list"); + expect(req?.provider).toBe("codex"); + }); + it("/who renders identity values in clean code spans", async () => { const { drive, sent, texts } = await boot(); From bf46ecb143a771f538024662b78021db98074f11 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 26 Sep 2026 19:20:51 +0800 Subject: [PATCH 2/6] fix: check a settings write against the registry the next boot builds Audit round 1 on #339. - settings.set validated session.defaultProvider against the LIVE registry, so "default pi + disable pi" in one batch, or disabling or clearing the key of an existing default later, was saved and the next boot refused to start. It now dry-runs createDefaultProviderRegistry on the post-batch config and env (loadConfig takes an in-memory object; the registry takes its env as a parameter) for any batch touching the default, providers.*, or an env key. Enabling a backend and making it the default in one batch is accepted, where the live check refused it. Refused writes are audited. - A resumed session whose backend is no longer available resumes on claude, not on the configured default. - A provider-less fleet spawn whose model was checked against the default is pinned to that backend, so a later default change can't put its model on another vendor. - The "disabled" hint names the real switch (providers.geminiCli.enabled) and is only offered for backends that have one; the id is JSON-quoted. - Startup logs which backend is the default; the settings help notes that backends' approval gates differ. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/conductor-frontends-design.md | 2 +- src/config.ts | 10 ++- src/daemon/providers/registry.ts | 39 ++++++++---- src/daemon/server.ts | 5 +- src/daemon/session-manager.ts | 99 +++++++++++++++++++++++------ src/daemon/settings/manifest.ts | 2 +- src/daemon/settings/store.ts | 35 ++++++++++ src/tests/default-provider.test.ts | 87 +++++++++++++++++++++++++ src/tests/provider-registry.test.ts | 27 +++++++- 9 files changed, 269 insertions(+), 37 deletions(-) diff --git a/docs/conductor-frontends-design.md b/docs/conductor-frontends-design.md index af991fd8..32820999 100644 --- a/docs/conductor-frontends-design.md +++ b/docs/conductor-frontends-design.md @@ -131,7 +131,7 @@ It is a real turn (§3.C), not a hidden side channel. If you are watching that session in another pane or another client, you will see the conductor-issued turn arrive there. **Caveat (honest scope).** -Fleet tools are surfaced only by the Claude provider today, and spawned workers currently default to Claude. +Fleet tools are surfaced only by the Claude provider today, and spawned workers default to the daemon's default backend (`session.defaultProvider`, Claude unless configured). So `send`-to-an-existing-session (the Spark case, where the target already runs its own backend) works now. True cross-backend *spawn* is spec-not-shipped; the UI must not over-promise it (§7, §13). diff --git a/src/config.ts b/src/config.ts index 14ca40e2..887ae4e4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1274,6 +1274,12 @@ export interface LoadOptions { configPath?: string; /** Env source (default process.env). Tests inject a controlled object. */ env?: Record; + /** + * An in-memory config.json object to load INSTEAD of reading the file — + * used to preview the config a settings write would leave for the next + * boot, through exactly the path that boot takes. + */ + raw?: unknown; } /** @@ -1295,8 +1301,8 @@ export function loadConfig(opts: LoadOptions = {}): CodeoidConfig { const env = opts.env ?? process.env; // 1. File defaults. - let fileConfig: unknown = {}; - if (existsSync(configPath)) { + let fileConfig: unknown = opts.raw ?? {}; + if (opts.raw === undefined && existsSync(configPath)) { try { const raw = readFileSync(configPath, "utf8"); fileConfig = JSON.parse(raw); diff --git a/src/daemon/providers/registry.ts b/src/daemon/providers/registry.ts index b3c1bcf2..aa1f6a21 100644 --- a/src/daemon/providers/registry.ts +++ b/src/daemon/providers/registry.ts @@ -177,15 +177,24 @@ export class DefaultProviderError extends Error { */ export function defaultProviderProblem(registry: ProviderRegistry, id: string): string | undefined { if (registry.has(id)) return undefined; + // JSON-quoted: the value is caller-supplied on the settings path, and a + // quoted string can't smuggle a newline into a log line. + const quoted = JSON.stringify(id); const hint = registry.unavailableHint(id); - if (hint) return `session.defaultProvider "${id}" is not available on this daemon: ${hint}`; - return ( - `session.defaultProvider "${id}" is not a registered backend ` + - `(registered: ${registry.ids().join(", ")}). ` + - `Check the spelling, or whether providers.${id}.enabled is false.` - ); + if (hint) return `session.defaultProvider ${quoted} is not available on this daemon: ${hint}`; + const toggle = ENABLE_KEY[id]; + const fix = toggle ? `It is disabled — set providers.${toggle}.enabled to true.` : "Check the spelling."; + return `session.defaultProvider ${quoted} is not a registered backend (registered: ${registry.ids().join(", ")}). ${fix}`; } +/** Backend id → its `providers..enabled` switch, for the ones that have one. */ +const ENABLE_KEY: Record = { + pi: "pi", + codex: "codex", + "gemini-cli": "geminiCli", + qwen: "qwen", +}; + /** * The built-in backends. Daemon startup builds exactly one of these. * `config` gates optional backends (pi can be disabled) and carries their @@ -197,7 +206,13 @@ export function defaultProviderProblem(registry: ProviderRegistry, id: string): * by a newer codeoid — a misspelled default is operator error, and falling * back would silently put every new session on a backend they opted out of. */ -export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderRegistry { +export function createDefaultProviderRegistry( + config?: CodeoidConfig, + /** Where backend keys and binary PATH lookups are read. A parameter so the + * settings path can dry-run the NEXT boot's registry without touching the + * live process env. */ + env: Record = process.env, +): ProviderRegistry { const registry = new ProviderRegistry(config?.session?.defaultProvider ?? CLAUDE_PROVIDER_ID); registry.register({ id: "claude", @@ -225,7 +240,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR // binary checks below. Without this, forking onto e.g. openai created a // session that failed cryptically ("401 Incorrect API key: missing") // instead of the option simply not appearing. - if (process.env.GOOGLE_API_KEY) { + if (env.GOOGLE_API_KEY) { registry.register({ id: "gemini", displayName: "Gemini (Google)", @@ -248,7 +263,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR "GOOGLE_API_KEY is not set — add it to ~/.codeoid/.env to use the Gemini backend", ); } - if (process.env.OPENAI_API_KEY) { + if (env.OPENAI_API_KEY) { registry.register({ id: "openai", displayName: "OpenAI", @@ -277,7 +292,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR // resolution means picking pi can't fail on a missing binary; no // resolution means the catalog says "not installed" with the fix. const configured = config?.providers?.pi?.command; - const resolution = resolvePiCommand(configured === "pi" ? undefined : configured); + const resolution = resolvePiCommand(configured === "pi" ? undefined : configured, env); if (resolution) { registry.register({ id: "pi", @@ -307,7 +322,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR } if (config?.providers?.codex?.enabled !== false) { const configured = config?.providers?.codex?.command; - const resolution = resolveCodexCommand(configured === "codex" ? undefined : configured); + const resolution = resolveCodexCommand(configured === "codex" ? undefined : configured, env); if (resolution) { registry.register({ id: "codex", @@ -337,7 +352,7 @@ export function createDefaultProviderRegistry(config?: CodeoidConfig): ProviderR } if (config?.providers?.geminiCli?.enabled !== false) { const configured = config?.providers?.geminiCli?.command; - const resolution = resolveGeminiCliCommand(configured === "gemini" ? undefined : configured); + const resolution = resolveGeminiCliCommand(configured === "gemini" ? undefined : configured, env); if (resolution) { registry.register({ id: "gemini-cli", diff --git a/src/daemon/server.ts b/src/daemon/server.ts index eeca58ce..98601211 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -318,7 +318,10 @@ export class DaemonServer { }, ); - console.log(`[codeoid] providers: ${this.#manager.providerIds().join(", ")}`); + // Default first and labelled: with session.defaultProvider set, which + // backend unqualified sessions land on is worth seeing at boot. + const [defaultProvider, ...otherProviders] = this.#manager.providerIds(); + console.log(`[codeoid] providers: ${[`${defaultProvider} (default)`, ...otherProviders].join(", ")}`); for (const { id, hint } of this.#manager.unavailableProviders()) { console.warn(`[codeoid] provider ${id} unavailable: ${hint}`); } diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index b2b08592..70348d00 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -15,14 +15,15 @@ import { Session, type AttachedClient, type WindowScope } from "./session.js"; import { type CatalogEntry, isPlaceholderModel, type SessionProvider } from "./providers/interface.js"; import { createDefaultProviderRegistry, - defaultProviderProblem, + DefaultProviderError, type ProviderRegistry, } from "./providers/registry.js"; import type { HookBus } from "./hooks/bus.js"; import type { Store } from "./store.js"; import { createPushTransport, PushService } from "./push/index.js"; import { hasScope, SCOPES } from "../protocol/scopes.js"; -import { applyPatches, getManifest, getSnapshot } from "./settings/store.js"; +import { applyPatches, getManifest, getSnapshot, previewPatches } from "./settings/store.js"; +import { fieldByKey } from "./settings/manifest.js"; import { BackendLoginBroker, BackendLoginError, redact } from "./auth/backend-login.js"; import { RateLimiter } from "./rate-limit.js"; import type { TranscriptMeta, TranscriptStore } from "./transcript.js"; @@ -108,7 +109,7 @@ import type { DispatchEventRow, DispatchTaskRow } from "./store.js"; import { type MemoryEngine, type MemoryMcpMount, workspaceIdFromPath } from "./memory/index.js"; import type { McpRegistry } from "./mcp/registry.js"; import type { McpHub } from "./mcp/hub.js"; -import { type CodeoidConfig, mutateConfigFile } from "../config.js"; +import { type CodeoidConfig, loadConfig, mutateConfigFile } from "../config.js"; import type { CompressionRegistry } from "./compress/index.js"; import { ORCHESTRATOR_ROLE } from "../protocol/types.js"; import type { @@ -128,6 +129,7 @@ import type { SessionInfo, SessionMode, SessionWorktree, + SettingPatch, } from "../protocol/types.js"; import type { Scope } from "../protocol/scopes.js"; import type { PipelineState } from "./pipeline/interface.js"; @@ -700,11 +702,7 @@ mcpHub: this.#mcpHub, // The conductor self-persists (design R2): its role, provider // selection, and fleet tools all come back across a restart. role: meta.role, - // Absent = a meta written before providers existed, which means - // claude. NOT the registry default: with session.defaultProvider - // set, an old session would otherwise resume on another backend and - // lose its backing conversation. - providerId: meta.providerId ?? CLAUDE_PROVIDER_ID, + providerId: this.#resumeProviderId(meta.providerId, meta.sessionId), forkedFrom: meta.forkedFrom, worktree: meta.worktree, // A collaboration is durable state, not turn state: the goal and @@ -1664,21 +1662,27 @@ mcpHub: this.#mcpHub, }; } // The default backend is only checked at startup, and a bad one refuses - // to boot — so a typo saved here would take the daemon down on its next - // restart. Check it against the live registry now, with the same rule the - // boot uses, and reject the batch before anything is written. - const providerErrors = msg.patches.flatMap((p) => { - if (p.key !== "session.defaultProvider" || typeof p.value !== "string") return []; - const problem = p.value.trim() ? defaultProviderProblem(this.#providers, p.value.trim()) : undefined; - return problem ? [{ key: p.key, message: problem }] : []; - }); - if (providerErrors.length > 0) { + // to boot — with the web UI down, recovery then needs a shell. So check + // every write that could change the answer against the registry the NEXT + // boot would build: the default itself, a backend's own switch or binary + // (`providers.*`), or an env key (a backend's API key). Checking the live + // registry instead would pass "default pi + disable pi" in one batch and + // refuse "enable codex + default codex". + const bootProblem = this.#nextBootDefaultProviderProblem(msg.patches); + if (bootProblem) { + const errors = [{ key: bootProblem.key, message: bootProblem.message }]; + this.#store.audit( + auth.sub, + "settings.set", + "", + `keys=${msg.patches.map((p) => p.key).join(",")} ok=false reason=defaultProvider`, + ); return { type: "settings.set.result", requestId: msg.id, ok: false, snapshot: { ...getSnapshot(), mcpServers: this.#mcpServerStatuses() }, - errors: providerErrors, + errors, restartRequired: false, }; } @@ -1708,6 +1712,60 @@ mcpHub: this.#mcpHub, } } + /** + * Why the config these patches leave behind would stop the next boot on its + * default backend, or undefined. Skips batches that can't affect it, and a + * claude default (always registered). A batch that fails to preview or + * parse is left to `applyPatches`, which reports it with its own errors. + */ + /** + * The backend a resumed session comes back on. Never the registry default: + * with session.defaultProvider set, that would silently move an existing + * session onto a backend it never ran on (losing its backing conversation, + * and possibly landing on a weaker approval gate). + * + * - absent → claude: the meta predates providers, so it IS a claude session. + * - no longer registered (key removed, backend disabled, a newer codeoid's + * id) → claude, loudly — the same backend it fell back to before the + * default was configurable. + */ + #resumeProviderId(persisted: string | undefined, sessionId: string): string { + if (persisted === undefined) return CLAUDE_PROVIDER_ID; + if (this.#providers.has(persisted)) return persisted; + console.warn( + `[codeoid/resume] session ${sessionId} ran on "${persisted}", which is not available now — resuming it on ${CLAUDE_PROVIDER_ID}`, + ); + return CLAUDE_PROVIDER_ID; + } + + #nextBootDefaultProviderProblem(patches: SettingPatch[]): { key: string; message: string } | undefined { + const relevant = patches.filter((p) => { + if (p.key === "session.defaultProvider" || p.key.startsWith("providers.")) return true; + return fieldByKey(p.key)?.backing === "env"; + }); + if (relevant.length === 0) return undefined; + const next = previewPatches(patches); + if (!next) return undefined; + let config: CodeoidConfig; + try { + config = loadConfig({ raw: next.raw, env: next.env }); + } catch { + return undefined; + } + const id = config.session.defaultProvider; + if (!id || id === CLAUDE_PROVIDER_ID) return undefined; + try { + createDefaultProviderRegistry(config, next.env); + return undefined; + } catch (err) { + if (!(err instanceof DefaultProviderError)) throw err; + // Attribute it to the default when the batch set it, else to the patch + // that broke an existing default (disabling it, clearing its key). + const key = relevant.find((p) => p.key === "session.defaultProvider")?.key ?? relevant[0]!.key; + return { key, message: err.message }; + } + } + #settingsForbidden(requestId: string): DaemonMessage { return { type: "response.error", @@ -4594,7 +4652,10 @@ mcpHub: this.#mcpHub, error: `Model "${model}" is not valid for provider "${providerId}". Omit \`model\` to use the provider's default.`, }; } - return { ok: true, provider, model: resolved }; + // Pin the backend the model was validated against. Left unset, the + // task would spawn on whatever the default is at CLAIM time — after a + // default change and restart, a codex model on a claude worker. + return { ok: true, provider: providerId, model: resolved }; }, enqueuePanel: (input) => this.#dispatcher.enqueueGroup({ diff --git a/src/daemon/settings/manifest.ts b/src/daemon/settings/manifest.ts index b2757582..1eaa92f6 100644 --- a/src/daemon/settings/manifest.ts +++ b/src/daemon/settings/manifest.ts @@ -51,7 +51,7 @@ const general: SettingsTab = { title: "Session defaults", description: "Applied when a new session is created.", fields: [ - cfg("session.defaultProvider", "Default backend", "Backend for new sessions that don't pick one (claude, codex, pi, qwen, …). Must be a backend this daemon has available — an unknown or unavailable one stops the daemon at startup. Existing sessions and the conductor keep their own.", { + cfg("session.defaultProvider", "Default backend", "Backend for new sessions that don't pick one (claude, codex, pi, qwen, …). Backends differ in how strictly they gate tool use (qwen runs read-only shell commands without asking), so this changes the default posture for every unqualified session. Must be a backend this daemon can start — saving one it can't is refused. Existing sessions and the conductor keep their own.", { envVar: "CODEOID_DEFAULT_PROVIDER", placeholder: "claude", }), diff --git a/src/daemon/settings/store.ts b/src/daemon/settings/store.ts index f6eeb71b..d3950bd3 100644 --- a/src/daemon/settings/store.ts +++ b/src/daemon/settings/store.ts @@ -177,6 +177,41 @@ export function applyPatches(patches: SettingPatch[]): ApplyResult { return { ok: true, errors: [], restartRequired, snapshot: getSnapshot() }; } +/** + * The config.json object and environment the NEXT boot would see if these + * patches were applied — computed, never written. Lets the caller check a + * write against rules that only run at startup (a default backend the next + * boot would refuse) before committing it. + * + * Returns undefined when the batch can't be previewed (an unknown key, an + * unreadable config.json): `applyPatches` rejects those with its own errors. + */ +export function previewPatches( + patches: SettingPatch[], +): { raw: Record; env: Record } | undefined { + let raw: Record; + try { + raw = readRawConfig(); + } catch { + return undefined; + } + const env: Record = { ...process.env }; + for (const p of patches) { + const f = fieldByKey(p.key); + if (!f) return undefined; + if (f.backing === "config") { + const coerced = coerceForConfig(p.value, f.kind); + if (coerced === undefined) deleteByPath(raw, f.path!); + else setByPath(raw, f.path!, coerced); + } else { + const formatted = formatForEnv(p.value, f.kind); + if (formatted === null) delete env[f.envVar!]; + else env[f.envVar!] = formatted; + } + } + return { raw, env }; +} + // ── config.json IO ────────────────────────────────────────────────────────── function readRawConfig(): Record { diff --git a/src/tests/default-provider.test.ts b/src/tests/default-provider.test.ts index 2e2ac30e..14db69f7 100644 --- a/src/tests/default-provider.test.ts +++ b/src/tests/default-provider.test.ts @@ -148,6 +148,28 @@ describe("resume under a non-claude default", () => { expect(m.findByName("legacy", OWNER)?.providerId).toBe("claude"); expect(m.findByName("modern", OWNER)?.providerId).toBe("pi"); }); + + it("resumes a session whose backend is gone on claude, not on the default", async () => { + // e.g. its API key was removed. Falling back to the configured default + // would move it onto a backend it never ran on. + await transcript.saveMeta(legacyMeta("orphan", "gemini")); + await transcript.flush(); + + const m = manager(); + expect(await m.resumeSessions()).toBe(1); + expect(m.findByName("orphan", OWNER)?.providerId).toBe("claude"); + }); +}); + +describe("dispatch under a non-claude default", () => { + it("validates a provider-less spawn's model against the default, and pins it", () => { + const deps = manager()._fleetDispatchDeps(OWNER.accountId!, OWNER.projectId!); + // Pinned: a task left provider-less would spawn on whatever the default is + // at claim time, which may have changed since this model was checked. + expect(deps.resolveBackend(undefined, "gpt-5-codex")).toEqual({ ok: true, provider: "pi", model: "gpt-5-codex" }); + // A Claude alias is not valid for the backend the worker would run on. + expect(deps.resolveBackend(undefined, "opus").ok).toBe(false); + }); }); describe("settings.set session.defaultProvider", () => { @@ -177,6 +199,71 @@ describe("settings.set session.defaultProvider", () => { expect(onDisk.session.defaultProvider).toBe("claude"); }); + const setBatch = (patches: Array<{ key: string; value: string | boolean | null }>) => + manager().handle({ type: "settings.set", id: "sb", patches }, OWNER, client) as Promise; + const onDisk = () => + existsSync(configFilePaths().configPath) ? JSON.parse(readFileSync(configFilePaths().configPath, "utf8")) : {}; + + // The check is against the registry the NEXT boot builds, not the live one: + // these are the writes a live-registry check gets wrong in both directions. + it("refuses a batch that makes a backend the default and disables it", async () => { + const res = await setBatch([ + { key: "session.defaultProvider", value: "pi" }, + { key: "providers.pi.enabled", value: false }, + ]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "session.defaultProvider" }); + expect(res.errors[0]!.message).toMatch(/set providers\.pi\.enabled to true/); + expect(existsSync(configFilePaths().configPath)).toBe(false); + }); + + it("refuses disabling the backend that is already the default, blaming that patch", async () => { + expect((await setBatch([{ key: "session.defaultProvider", value: "pi" }])).ok).toBe(true); + const res = await setBatch([{ key: "providers.pi.enabled", value: false }]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "providers.pi.enabled" }); + expect(onDisk().providers?.pi?.enabled).toBeUndefined(); + }); + + it("accepts enabling a backend and making it the default in one batch", async () => { + expect((await setBatch([{ key: "providers.pi.enabled", value: false }])).ok).toBe(true); + const res = await setBatch([ + { key: "providers.pi.enabled", value: true }, + { key: "session.defaultProvider", value: "pi" }, + ]); + expect(res.errors).toEqual([]); + expect(res.ok).toBe(true); + expect(onDisk().session.defaultProvider).toBe("pi"); + }); + + it("refuses clearing the API key the default backend needs", async () => { + const saved = process.env.OPENAI_API_KEY; + process.env.OPENAI_API_KEY = "sk-test"; + try { + expect((await setBatch([{ key: "session.defaultProvider", value: "openai" }])).ok).toBe(true); + const res = await setBatch([{ key: "OPENAI_API_KEY", value: null }]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "OPENAI_API_KEY" }); + expect(res.errors[0]!.message).toMatch(/"openai" is not available on this daemon: .*OPENAI_API_KEY/); + expect(process.env.OPENAI_API_KEY).toBe("sk-test"); + } finally { + if (saved === undefined) delete process.env.OPENAI_API_KEY; + else process.env.OPENAI_API_KEY = saved; + } + }); + + it("records a refused write in the audit log", async () => { + await set("claud"); + // Store has no audit-read API on purpose; read the table directly. + const { Database } = await import("bun:sqlite"); + const db = new Database(join(tmp, "codeoid.db"), { readonly: true }); + const row = db + .prepare("SELECT subject, detail FROM audit_log WHERE action = 'settings.set' ORDER BY id DESC LIMIT 1") + .get() as { subject: string; detail: string } | undefined; + db.close(); + expect(row).toEqual({ subject: OWNER.sub, detail: "keys=session.defaultProvider ok=false reason=defaultProvider" }); + }); + it("lets the value be cleared back to the built-in default", async () => { await set("claude"); const res = await set(null); diff --git a/src/tests/provider-registry.test.ts b/src/tests/provider-registry.test.ts index 7260eccd..ae7abff0 100644 --- a/src/tests/provider-registry.test.ts +++ b/src/tests/provider-registry.test.ts @@ -131,7 +131,32 @@ describe("ProviderRegistry", () => { it("a disabled defaultProvider fails startup and names the switch", () => { expect(() => createDefaultProviderRegistry(withDefault("pi", { providers: { pi: { enabled: false, command: "pi" } } })), - ).toThrow(/providers\.pi\.enabled is false/); + ).toThrow(/"pi" is not a registered backend .*It is disabled — set providers\.pi\.enabled to true\./); + }); + + it("names the real switch for a backend whose config key differs from its id", () => { + expect(() => + createDefaultProviderRegistry( + withDefault("gemini-cli", { providers: { geminiCli: { enabled: false, command: "gemini" } } }), + ), + ).toThrow(/set providers\.geminiCli\.enabled to true/); + }); + + it("does not suggest a switch for an id that is simply unknown", () => { + let message = ""; + try { + createDefaultProviderRegistry(withDefault("claud")); + } catch (e) { + message = (e as Error).message; + } + expect(message).toMatch(/Check the spelling\.$/); + expect(message).not.toContain("providers.claud"); + }); + + it("reads backend keys from the env it is given, not process.env", () => { + // Keys deleted from process.env by beforeEach; the dry-run env has one. + const registry = createDefaultProviderRegistry(withDefault("openai"), { ...process.env, OPENAI_API_KEY: "sk-test" }); + expect(registry.defaultId).toBe("openai"); }); it("an unavailable defaultProvider fails startup with that backend's own hint", () => { From ad36de531e50d7f7438cc46bbeb165fd143e4251 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 26 Sep 2026 19:32:42 +0800 Subject: [PATCH 3/6] fix: refuse only the settings saves that break the next boot, not every one Audit round 2 on #339. - The next-boot check refused ANY save touching an env key or providers.* once the default was already broken (its binary gone after an upgrade), and blamed the unrelated key, turning Settings into a wall exactly when it was needed. It now refuses only what the batch causes: a batch that sets the default is always checked, any other only if the config boots now and wouldn't after it. - An env override that makes the config fail to load is refused the same way; schema failures are still applyPatches' to report, per field. - The check runs inside the handler's error wrapper, and its preview load is quiet (no startup warnings repeated on every save). - Resume resolves a session's backend once and uses it for the role rebuild too: a planned collaboration child whose backend is gone resumes on claude without carrying the other vendor's model. - Settings tests no longer depend on the shell's CODEOID_DEFAULT_PROVIDER or the bundled pi resolving. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/config.ts | 4 + src/daemon/session-manager.ts | 133 ++++++++++++++++++----------- src/tests/collaboration.test.ts | 43 ++++++++++ src/tests/default-provider.test.ts | 57 +++++++++++-- 4 files changed, 178 insertions(+), 59 deletions(-) diff --git a/src/config.ts b/src/config.ts index 887ae4e4..828f1075 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1280,6 +1280,9 @@ export interface LoadOptions { * boot, through exactly the path that boot takes. */ raw?: unknown; + /** Skip the operator-facing startup warnings — for previews that run on + * every settings write, where repeating them is noise. */ + quiet?: boolean; } /** @@ -1413,6 +1416,7 @@ export function loadConfig(opts: LoadOptions = {}): CodeoidConfig { // daemon's local identity store would be keyed personal/dev while the minted // identities live in the badge's actual tenant — a silent split. Surface it. if ( + !opts.quiet && parsed.agentIdentity.registrarKey !== undefined && parsed.agentIdentity.accountId === "personal" && parsed.agentIdentity.projectId === "dev" diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 70348d00..35aee619 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -109,7 +109,7 @@ import type { DispatchEventRow, DispatchTaskRow } from "./store.js"; import { type MemoryEngine, type MemoryMcpMount, workspaceIdFromPath } from "./memory/index.js"; import type { McpRegistry } from "./mcp/registry.js"; import type { McpHub } from "./mcp/hub.js"; -import { type CodeoidConfig, loadConfig, mutateConfigFile } from "../config.js"; +import { type CodeoidConfig, loadConfig, mutateConfigFile, validateConfigObject } from "../config.js"; import type { CompressionRegistry } from "./compress/index.js"; import { ORCHESTRATOR_ROLE } from "../protocol/types.js"; import type { @@ -661,7 +661,10 @@ export class SessionManager { // Role-children need their restrictions rebuilt BEFORE construction — // worker shape and capability role are constructor inputs, not things // that can be attached afterwards. - const child = this.#resumeRoleChild(meta, goalConfigs); + // Resolved once: the role posture and the session must agree on the + // backend, including when the persisted one is no longer available. + const providerId = this.#resumeProviderId(meta.providerId, meta.sessionId); + const child = this.#resumeRoleChild(meta, goalConfigs, providerId); if (child) { if (child.orphaned) orphanedChildren++; else resumedChildren++; @@ -702,7 +705,7 @@ mcpHub: this.#mcpHub, // The conductor self-persists (design R2): its role, provider // selection, and fleet tools all come back across a restart. role: meta.role, - providerId: this.#resumeProviderId(meta.providerId, meta.sessionId), + providerId, forkedFrom: meta.forkedFrom, worktree: meta.worktree, // A collaboration is durable state, not turn state: the goal and @@ -842,6 +845,8 @@ mcpHub: this.#mcpHub, #resumeRoleChild( meta: TranscriptMeta, goalConfigs: ReadonlyMap, + /** The backend the child actually resumes on (`#resumeProviderId`). */ + providerId: string, ): | { orphaned: boolean; @@ -879,7 +884,7 @@ mcpHub: this.#mcpHub, { roleName: role.roleName, ordinal: role.ordinal, - providerId: meta.providerId ?? CLAUDE_PROVIDER_ID, + providerId, shape: role.write ? "ship" : "scout", write: role.write, }, @@ -894,8 +899,12 @@ mcpHub: this.#mcpHub, options: { ...roleChildPosture(planned, role.parentSessionId, childBrief(collaboration, planned)), // The roster's resolved model comes back with the child — same source - // (`plannedChildFor`) as the spawn path, so they can't drift. - ...(planned.model !== undefined ? { defaultModel: planned.model } : {}), + // (`plannedChildFor`) as the spawn path, so they can't drift. Only on + // the planned backend: if that is gone and the child resumes on claude, + // the planned model belongs to another vendor. + ...(planned.model !== undefined && planned.providerId === providerId + ? { defaultModel: planned.model } + : {}), // Re-armed per boot, not persisted: the budget is a per-stretch-of-work // allowance, and carrying a spent one across a restart would resume a // child with zero turns left. @@ -1661,32 +1670,29 @@ mcpHub: this.#mcpHub, code: "forbidden", }; } - // The default backend is only checked at startup, and a bad one refuses - // to boot — with the web UI down, recovery then needs a shell. So check - // every write that could change the answer against the registry the NEXT - // boot would build: the default itself, a backend's own switch or binary - // (`providers.*`), or an env key (a backend's API key). Checking the live - // registry instead would pass "default pi + disable pi" in one batch and - // refuse "enable codex + default codex". - const bootProblem = this.#nextBootDefaultProviderProblem(msg.patches); - if (bootProblem) { - const errors = [{ key: bootProblem.key, message: bootProblem.message }]; - this.#store.audit( - auth.sub, - "settings.set", - "", - `keys=${msg.patches.map((p) => p.key).join(",")} ok=false reason=defaultProvider`, - ); - return { - type: "settings.set.result", - requestId: msg.id, - ok: false, - snapshot: { ...getSnapshot(), mcpServers: this.#mcpServerStatuses() }, - errors, - restartRequired: false, - }; - } try { + // The default backend is only checked at startup, and a bad one refuses + // to boot — with the web UI down, recovery then needs a shell. So check + // the write against the config the NEXT boot would load, not the live + // registry (which would pass "default pi + disable pi" in one batch and + // refuse "enable codex + default codex"). + const bootProblem = this.#nextBootProblem(msg.patches); + if (bootProblem) { + this.#store.audit( + auth.sub, + "settings.set", + "", + `keys=${msg.patches.map((p) => p.key).join(",")} ok=false reason=next-boot`, + ); + return { + type: "settings.set.result", + requestId: msg.id, + ok: false, + snapshot: { ...getSnapshot(), mcpServers: this.#mcpServerStatuses() }, + errors: [bootProblem], + restartRequired: false, + }; + } const result = applyPatches(msg.patches); this.#store.audit( auth.sub, @@ -1712,12 +1718,6 @@ mcpHub: this.#mcpHub, } } - /** - * Why the config these patches leave behind would stop the next boot on its - * default backend, or undefined. Skips batches that can't affect it, and a - * claude default (always registered). A batch that fails to preview or - * parse is left to `applyPatches`, which reports it with its own errors. - */ /** * The backend a resumed session comes back on. Never the registry default: * with session.defaultProvider set, that would silently move an existing @@ -1738,19 +1738,51 @@ mcpHub: this.#mcpHub, return CLAUDE_PROVIDER_ID; } - #nextBootDefaultProviderProblem(patches: SettingPatch[]): { key: string; message: string } | undefined { - const relevant = patches.filter((p) => { - if (p.key === "session.defaultProvider" || p.key.startsWith("providers.")) return true; - return fieldByKey(p.key)?.backing === "env"; - }); + /** + * Why the config these patches leave behind would stop the next boot, or + * undefined. Two ways it can: the config no longer loads, or its default + * backend can't be built (typo, disabled, binary or API key gone). + * + * Refuses only what the batch is responsible for. A batch that SETS the + * default is always checked — the operator is choosing it, so it must work. + * Any other batch is refused only if the config boots now and wouldn't after + * it: when the default is already broken (its binary vanished after an + * upgrade, say), an unrelated save must still go through, or the one screen + * that could repair things refuses every edit and blames the wrong key. + * + * Only batches that can change the answer are checked: the default itself, + * `providers.*`, and env-backed keys (API keys, and env overrides feeding + * loadConfig). A batch that can't be previewed (unknown key, unreadable + * config.json) is left to `applyPatches`, which rejects it with its own error. + */ + #nextBootProblem(patches: SettingPatch[]): { key: string; message: string } | undefined { + const relevant = patches.filter( + (p) => p.key === "session.defaultProvider" || p.key.startsWith("providers.") || fieldByKey(p.key)?.backing === "env", + ); if (relevant.length === 0) return undefined; - const next = previewPatches(patches); - if (!next) return undefined; + const after = previewPatches(patches); + // A config.json the schema rejects is applyPatches' to report — it does so + // per field. What only a load catches is an env override that fails the + // re-validation, and that is what #bootProblem is left to find. + if (!after || !validateConfigObject(after.raw).ok) return undefined; + const problem = this.#bootProblem(after); + if (!problem) return undefined; + const setsDefault = relevant.find((p) => p.key === "session.defaultProvider"); + if (!setsDefault) { + const before = previewPatches([]); + if (!before || this.#bootProblem(before)) return undefined; + } + // Blame the default when the batch set it, else the patch that broke it. + return { key: (setsDefault ?? relevant[0]!).key, message: problem }; + } + + /** Why a previewed config.json + env would fail the boot, or undefined. */ + #bootProblem(next: { raw: Record; env: Record }): string | undefined { let config: CodeoidConfig; try { - config = loadConfig({ raw: next.raw, env: next.env }); - } catch { - return undefined; + config = loadConfig({ raw: next.raw, env: next.env, quiet: true }); + } catch (err) { + return err instanceof Error ? err.message : String(err); } const id = config.session.defaultProvider; if (!id || id === CLAUDE_PROVIDER_ID) return undefined; @@ -1758,11 +1790,8 @@ mcpHub: this.#mcpHub, createDefaultProviderRegistry(config, next.env); return undefined; } catch (err) { - if (!(err instanceof DefaultProviderError)) throw err; - // Attribute it to the default when the batch set it, else to the patch - // that broke an existing default (disabling it, clearing its key). - const key = relevant.find((p) => p.key === "session.defaultProvider")?.key ?? relevant[0]!.key; - return { key, message: err.message }; + if (err instanceof DefaultProviderError) return err.message; + throw err; } } diff --git a/src/tests/collaboration.test.ts b/src/tests/collaboration.test.ts index 59855b8b..47deca01 100644 --- a/src/tests/collaboration.test.ts +++ b/src/tests/collaboration.test.ts @@ -2169,6 +2169,49 @@ describe("collaboration survives a daemon restart", () => { ).toEqual(["reasoning#1", "review#1", "review#2"]); }); + test("a child whose backend is gone resumes on claude without the other vendor's model", async () => { + // The planned model belongs to the planned backend. If that backend is not + // available after the restart, the child resumes on claude — and must not + // carry a gemini model id into a Claude session. + manager.setBlackboardUrl(BLACKBOARD_URL); + const resp = await run({ + type: "session.create", + id: "rsg", + name: "rsg", + workdir, + collaboration: { + goal: "backend disappears", + roles: [ + { name: "orchestrator", providerId: "claude" }, + { name: "review", providerId: "gemini", model: "gemini-2.5-pro" }, + ], + }, + }); + if (resp.type !== "response.ok") throw new Error(`create failed: ${JSON.stringify(resp)}`); + const parent = resp.data as SessionInfo; + const [before] = childrenOf(await allSessions(), parent.id); + expect(before!.providerId).toBe("gemini"); + expect(before!.model).toBe("gemini-2.5-pro"); + + await manager.drain(3_000); + await Bun.sleep(150); + const claudeOnly = new ProviderRegistry("claude"); + claudeOnly.register({ id: "claude", displayName: "claude", create: () => new MockSessionProvider("claude", []) }); + const next = new SessionManager( + new Store(join(tmp, "codeoid.db")), + new TranscriptStore(join(tmp, "transcripts")), + undefined, undefined, undefined, + { config: mkConfig(), providers: claudeOnly }, + ); + next.setBlackboardUrl(BLACKBOARD_URL); + await next.resumeSessions(); + manager = next; + + const [after] = childrenOf(await listFrom(next), parent.id); + expect(after!.providerId).toBe("claude"); + expect(after!.model).not.toBe("gemini-2.5-pro"); + }); + test("write authority is restored per role, not uniformly", async () => { const parent = await createGoal("rs2"); const kids = childrenOf(await listFrom(await restart()), parent.id); diff --git a/src/tests/default-provider.test.ts b/src/tests/default-provider.test.ts index 14db69f7..a8602efb 100644 --- a/src/tests/default-provider.test.ts +++ b/src/tests/default-provider.test.ts @@ -18,9 +18,9 @@ */ import { describe, it, expect, beforeEach, afterEach } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { Store } from "../daemon/store.js"; import { TranscriptStore } from "../daemon/transcript.js"; import { SessionManager } from "../daemon/session-manager.js"; @@ -55,20 +55,31 @@ function piDefaultRegistry(): ProviderRegistry { let tmp: string; let store: Store; let transcript: TranscriptStore; -let prevXdg: string | undefined; +// The next-boot check reads process.env: keep the shell's out of it, and give +// it a `pi` on PATH so the pi cases don't hinge on the bundled optional dep. +const ENV_KEYS = ["XDG_CONFIG_HOME", "CODEOID_DEFAULT_PROVIDER", "PATH"] as const; +let savedEnv: Record = {}; beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), "codeoid-default-provider-")); store = new Store(join(tmp, "codeoid.db")); transcript = new TranscriptStore(join(tmp, "transcripts")); + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); // settings.set writes config.json — keep it off the real ~/.codeoid. - prevXdg = process.env.XDG_CONFIG_HOME; process.env.XDG_CONFIG_HOME = tmp; + delete process.env.CODEOID_DEFAULT_PROVIDER; + const bin = join(tmp, "bin"); + mkdirSync(bin); + writeFileSync(join(bin, "pi"), "#!/bin/sh\n"); + chmodSync(join(bin, "pi"), 0o755); + process.env.PATH = `${bin}:${savedEnv.PATH ?? ""}`; }); afterEach(async () => { - if (prevXdg === undefined) delete process.env.XDG_CONFIG_HOME; - else process.env.XDG_CONFIG_HOME = prevXdg; + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } try { await transcript.flush(); } catch {} try { store.close(); } catch {} try { rmSync(tmp, { recursive: true, force: true }); } catch {} @@ -252,6 +263,38 @@ describe("settings.set session.defaultProvider", () => { } }); + // A default that broke AFTER boot (its binary vanished in an upgrade) must + // not turn Settings into a wall: only a batch that causes the breakage, or + // that picks the default itself, is refused. + const brokenDefault = () => { + const { configPath } = configFilePaths(); + mkdirSync(dirname(configPath), { recursive: true }); + writeFileSync( + configPath, + JSON.stringify({ session: { defaultProvider: "pi" }, providers: { pi: { command: "/nonexistent/pi" } } }), + ); + }; + + it("still accepts unrelated saves while the default is already broken", async () => { + brokenDefault(); + const res = await setBatch([{ key: "TELEGRAM_ALLOWED_USER_IDS", value: "123" }]); + expect(res.errors).toEqual([]); + expect(res.ok).toBe(true); + expect((await setBatch([{ key: "providers.codex.enabled", value: false }])).ok).toBe(true); + }); + + it("accepts the save that repairs a broken default", async () => { + brokenDefault(); + expect((await setBatch([{ key: "providers.pi.command", value: "pi" }])).ok).toBe(true); + }); + + it("still checks a default the batch picks, even when the current one is broken", async () => { + brokenDefault(); + const res = await setBatch([{ key: "session.defaultProvider", value: "claud" }]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "session.defaultProvider" }); + }); + it("records a refused write in the audit log", async () => { await set("claud"); // Store has no audit-read API on purpose; read the table directly. @@ -261,7 +304,7 @@ describe("settings.set session.defaultProvider", () => { .prepare("SELECT subject, detail FROM audit_log WHERE action = 'settings.set' ORDER BY id DESC LIMIT 1") .get() as { subject: string; detail: string } | undefined; db.close(); - expect(row).toEqual({ subject: OWNER.sub, detail: "keys=session.defaultProvider ok=false reason=defaultProvider" }); + expect(row).toEqual({ subject: OWNER.sub, detail: "keys=session.defaultProvider ok=false reason=next-boot" }); }); it("lets the value be cleared back to the built-in default", async () => { From e3778c12be39a9170ec404b457d9dbe2cd0fe331 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 26 Sep 2026 19:42:57 +0800 Subject: [PATCH 4/6] fix: check every settings save against the next boot, and a set default on its own Audit round 3 on #339. - A config value can be valid alone and fail against an env override (mcpToolTimeoutMs saved against a CODEOID_TURN_STALL_TIMEOUT_MS in .env), which the next boot rejects. Every batch now gets the next-boot load check, not only provider-shaped ones; the before/after rule still lets unrelated saves through while the config is already broken, and an error while checking the current config counts as "already broken". - A default the batch sets is also checked with CODEOID_DEFAULT_PROVIDER taken out, so an env override can't mask a typo that config.json would carry into a later boot. - "Always check a batch that sets the default" now covers backend problems only; a pre-existing load problem follows the before/after rule and is no longer blamed on the default. - Test hygiene: env keys a test saves are restored. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/config.ts | 6 ++- src/daemon/session-manager.ts | 74 ++++++++++++++++++------------ src/tests/default-provider.test.ts | 43 ++++++++++++++++- 3 files changed, 92 insertions(+), 31 deletions(-) diff --git a/src/config.ts b/src/config.ts index 828f1075..3ef9b841 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1196,6 +1196,10 @@ interface EnvOverride { kind: OverrideKind; } +/** Env override for `session.defaultProvider` — named so the settings check + * can look past it at the value config.json would carry on its own. */ +export const DEFAULT_PROVIDER_ENV = "CODEOID_DEFAULT_PROVIDER"; + const ENV_OVERRIDES: readonly EnvOverride[] = [ { env: "CODEOID_DAEMON_URL", path: "daemonUrl", kind: "string" }, { env: "CODEOID_DB_PATH", path: "dbPath", kind: "string" }, @@ -1232,7 +1236,7 @@ const ENV_OVERRIDES: readonly EnvOverride[] = [ { env: "CODEOID_AUTO_ROTATE_PCT", path: "autoRotate.rotatePct", kind: "float" }, { env: "CODEOID_AUTO_ROTATE_HARD_PCT", path: "autoRotate.hardRotatePct", kind: "float" }, { env: "CODEOID_AUTO_ROTATE_MIN_TURNS", path: "autoRotate.minTurnsBeforeRotate", kind: "int" }, - { env: "CODEOID_DEFAULT_PROVIDER", path: "session.defaultProvider", kind: "string" }, + { env: DEFAULT_PROVIDER_ENV, path: "session.defaultProvider", kind: "string" }, { env: "CODEOID_DEFAULT_MODEL", path: "session.defaultModel", kind: "string" }, // Dispatch kill switch — disable send-class fleet dispatch per-invocation // without touching config.json. Other dispatch knobs are file-config only, diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 35aee619..825f7c38 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -23,7 +23,6 @@ import type { Store } from "./store.js"; import { createPushTransport, PushService } from "./push/index.js"; import { hasScope, SCOPES } from "../protocol/scopes.js"; import { applyPatches, getManifest, getSnapshot, previewPatches } from "./settings/store.js"; -import { fieldByKey } from "./settings/manifest.js"; import { BackendLoginBroker, BackendLoginError, redact } from "./auth/backend-login.js"; import { RateLimiter } from "./rate-limit.js"; import type { TranscriptMeta, TranscriptStore } from "./transcript.js"; @@ -109,7 +108,7 @@ import type { DispatchEventRow, DispatchTaskRow } from "./store.js"; import { type MemoryEngine, type MemoryMcpMount, workspaceIdFromPath } from "./memory/index.js"; import type { McpRegistry } from "./mcp/registry.js"; import type { McpHub } from "./mcp/hub.js"; -import { type CodeoidConfig, loadConfig, mutateConfigFile, validateConfigObject } from "../config.js"; +import { type CodeoidConfig, DEFAULT_PROVIDER_ENV, loadConfig, mutateConfigFile, validateConfigObject } from "../config.js"; import type { CompressionRegistry } from "./compress/index.js"; import { ORCHESTRATOR_ROLE } from "../protocol/types.js"; import type { @@ -1740,49 +1739,66 @@ mcpHub: this.#mcpHub, /** * Why the config these patches leave behind would stop the next boot, or - * undefined. Two ways it can: the config no longer loads, or its default - * backend can't be built (typo, disabled, binary or API key gone). + * undefined. Two ways it can: the config no longer loads (every batch is + * checked — a value can be valid alone and fail against an env override, + * like a timeout saved here against a stall timeout set in .env), or its + * default backend can't be built (typo, disabled, binary or API key gone). * - * Refuses only what the batch is responsible for. A batch that SETS the - * default is always checked — the operator is choosing it, so it must work. - * Any other batch is refused only if the config boots now and wouldn't after - * it: when the default is already broken (its binary vanished after an - * upgrade, say), an unrelated save must still go through, or the one screen - * that could repair things refuses every edit and blames the wrong key. + * Refuses only what the batch is responsible for. A default the batch SETS + * is always checked, on its own as well as in the merged config — the + * operator is choosing it, and an env override masking a typo today would + * stop a later boot once the override goes. Anything else is refused only + * if the config boots now and wouldn't after the batch: when it is already + * broken (the default's binary vanished after an upgrade, say), an + * unrelated save must still go through, or the one screen that could + * repair things refuses every edit and blames the wrong key. * - * Only batches that can change the answer are checked: the default itself, - * `providers.*`, and env-backed keys (API keys, and env overrides feeding - * loadConfig). A batch that can't be previewed (unknown key, unreadable - * config.json) is left to `applyPatches`, which rejects it with its own error. + * A batch that can't be previewed (unknown key, unreadable config.json) or + * whose config.json the schema rejects is left to `applyPatches`, which + * rejects it with its own, per-field errors. */ #nextBootProblem(patches: SettingPatch[]): { key: string; message: string } | undefined { - const relevant = patches.filter( - (p) => p.key === "session.defaultProvider" || p.key.startsWith("providers.") || fieldByKey(p.key)?.backing === "env", - ); - if (relevant.length === 0) return undefined; + if (patches.length === 0) return undefined; const after = previewPatches(patches); - // A config.json the schema rejects is applyPatches' to report — it does so - // per field. What only a load catches is an env override that fails the - // re-validation, and that is what #bootProblem is left to find. if (!after || !validateConfigObject(after.raw).ok) return undefined; + + const setsDefault = patches.find( + (p) => p.key === "session.defaultProvider" && typeof p.value === "string" && p.value.trim() !== "", + ); + if (setsDefault) { + // The chosen value itself, with no env override in front of it. + const { [DEFAULT_PROVIDER_ENV]: _masked, ...unmasked } = after.env; + for (const env of [after.env, unmasked]) { + const problem = this.#bootProblem({ raw: after.raw, env }); + if (problem?.kind === "provider") return { key: setsDefault.key, message: problem.message }; + } + } + const problem = this.#bootProblem(after); if (!problem) return undefined; - const setsDefault = relevant.find((p) => p.key === "session.defaultProvider"); - if (!setsDefault) { + let brokenAlready: boolean; + try { const before = previewPatches([]); - if (!before || this.#bootProblem(before)) return undefined; + brokenAlready = !before || this.#bootProblem(before) !== undefined; + } catch { + // Can't tell whether the current config boots: don't let that block + // every save — treat it as already broken and let the batch through. + brokenAlready = true; } - // Blame the default when the batch set it, else the patch that broke it. - return { key: (setsDefault ?? relevant[0]!).key, message: problem }; + if (brokenAlready) return undefined; + return { key: patches[0]!.key, message: problem.message }; } /** Why a previewed config.json + env would fail the boot, or undefined. */ - #bootProblem(next: { raw: Record; env: Record }): string | undefined { + #bootProblem(next: { + raw: Record; + env: Record; + }): { kind: "load" | "provider"; message: string } | undefined { let config: CodeoidConfig; try { config = loadConfig({ raw: next.raw, env: next.env, quiet: true }); } catch (err) { - return err instanceof Error ? err.message : String(err); + return { kind: "load", message: err instanceof Error ? err.message : String(err) }; } const id = config.session.defaultProvider; if (!id || id === CLAUDE_PROVIDER_ID) return undefined; @@ -1790,7 +1806,7 @@ mcpHub: this.#mcpHub, createDefaultProviderRegistry(config, next.env); return undefined; } catch (err) { - if (err instanceof DefaultProviderError) return err.message; + if (err instanceof DefaultProviderError) return { kind: "provider", message: err.message }; throw err; } } diff --git a/src/tests/default-provider.test.ts b/src/tests/default-provider.test.ts index a8602efb..42147dc3 100644 --- a/src/tests/default-provider.test.ts +++ b/src/tests/default-provider.test.ts @@ -57,7 +57,16 @@ let store: Store; let transcript: TranscriptStore; // The next-boot check reads process.env: keep the shell's out of it, and give // it a `pi` on PATH so the pi cases don't hinge on the bundled optional dep. -const ENV_KEYS = ["XDG_CONFIG_HOME", "CODEOID_DEFAULT_PROVIDER", "PATH"] as const; +// TELEGRAM_ALLOWED_USER_IDS: applyPatches writes accepted env keys into +// process.env, so a test that saves one must not leak it to later files. +const ENV_KEYS = [ + "XDG_CONFIG_HOME", + "CODEOID_DEFAULT_PROVIDER", + "CODEOID_TURN_STALL_TIMEOUT_MS", + "CODEOID_AUTO_ROTATE_PCT", + "TELEGRAM_ALLOWED_USER_IDS", + "PATH", +] as const; let savedEnv: Record = {}; beforeEach(() => { @@ -68,6 +77,8 @@ beforeEach(() => { // settings.set writes config.json — keep it off the real ~/.codeoid. process.env.XDG_CONFIG_HOME = tmp; delete process.env.CODEOID_DEFAULT_PROVIDER; + delete process.env.CODEOID_TURN_STALL_TIMEOUT_MS; + delete process.env.CODEOID_AUTO_ROTATE_PCT; const bin = join(tmp, "bin"); mkdirSync(bin); writeFileSync(join(bin, "pi"), "#!/bin/sh\n"); @@ -295,6 +306,36 @@ describe("settings.set session.defaultProvider", () => { expect(res.errors[0]).toMatchObject({ key: "session.defaultProvider" }); }); + it("refuses a config value that only breaks the boot in combination with an env override", async () => { + // Valid on its own (the default stall timeout is 300000), so the schema + // check passes — but the next boot applies the env's 150000 and rejects it. + process.env.CODEOID_TURN_STALL_TIMEOUT_MS = "150000"; + const res = await setBatch([{ key: "session.mcpToolTimeoutMs", value: 200000 as unknown as string }]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "session.mcpToolTimeoutMs" }); + expect(res.errors[0]!.message).toMatch(/mcpToolTimeoutMs/); + expect(existsSync(configFilePaths().configPath)).toBe(false); + }); + + it("checks a default the batch sets even when an env override hides it", async () => { + // The env wins at boot, so the merged config is fine — but config.json + // would carry the typo, and the boot after the override goes would fail. + process.env.CODEOID_DEFAULT_PROVIDER = "pi"; + const res = await setBatch([{ key: "session.defaultProvider", value: "claud" }]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "session.defaultProvider" }); + expect(res.errors[0]!.message).toMatch(/"claud" is not a registered backend/); + }); + + it("does not blame the default for a load problem that was already there", async () => { + // An unrelated env override already breaks loading; setting a working + // default neither causes nor fixes that, so it isn't refused for it. + process.env.CODEOID_AUTO_ROTATE_PCT = "abc"; + const res = await setBatch([{ key: "session.defaultProvider", value: "claude" }]); + expect(res.errors).toEqual([]); + expect(res.ok).toBe(true); + }); + it("records a refused write in the audit log", async () => { await set("claud"); // Store has no audit-read API on purpose; read the table directly. From 863596b62189b29847705dcdcf25c78062dd1d83 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 26 Sep 2026 19:49:50 +0800 Subject: [PATCH 5/6] fix: file a refused settings save under the field that caused it Audit round 4 on #339. - A refused multi-key batch was filed under its first key. The web drawer sends every tab's edits in one batch and shows an error only beside the matching field, so the error could land on another tab and the save looked like a no-op. The culprit is now the patch whose removal makes the next boot work, else "" (the drawer's save bar). - The full-env check on a batch that sets the default was redundant, and its only remaining effect was blaming the default for a broken CODEOID_DEFAULT_PROVIDER the batch didn't cause. Dropped; that case follows the before/after rule. - Load-error messages returned to the caller go through redact(). Co-Authored-By: Claude Opus 5.5 (1M context) --- src/daemon/session-manager.ts | 48 ++++++++++++++++++++++-------- src/tests/default-provider.test.ts | 34 +++++++++++++++++++++ 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/daemon/session-manager.ts b/src/daemon/session-manager.ts index 825f7c38..cf407dbe 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -1670,11 +1670,12 @@ mcpHub: this.#mcpHub, }; } try { - // The default backend is only checked at startup, and a bad one refuses - // to boot — with the web UI down, recovery then needs a shell. So check - // the write against the config the NEXT boot would load, not the live - // registry (which would pass "default pi + disable pi" in one batch and - // refuse "enable codex + default codex"). + // A config the next boot rejects — one that no longer loads, or whose + // default backend can't be built — takes the daemon down, and with the + // web UI down recovery needs a shell. So every write is checked against + // the config the NEXT boot would load (not the live registry, which + // would pass "default pi + disable pi" and refuse "enable codex + + // default codex"). See #nextBootProblem. const bootProblem = this.#nextBootProblem(msg.patches); if (bootProblem) { this.#store.audit( @@ -1766,12 +1767,13 @@ mcpHub: this.#mcpHub, (p) => p.key === "session.defaultProvider" && typeof p.value === "string" && p.value.trim() !== "", ); if (setsDefault) { - // The chosen value itself, with no env override in front of it. + // The chosen value itself, with no env override in front of it: that + // covers a bad value and a clash within the batch. A problem only the + // override has (its backend gone) isn't this batch's doing, so it goes + // through the before/after rule below like anything else. const { [DEFAULT_PROVIDER_ENV]: _masked, ...unmasked } = after.env; - for (const env of [after.env, unmasked]) { - const problem = this.#bootProblem({ raw: after.raw, env }); - if (problem?.kind === "provider") return { key: setsDefault.key, message: problem.message }; - } + const problem = this.#bootProblem({ raw: after.raw, env: unmasked }); + if (problem?.kind === "provider") return { key: setsDefault.key, message: problem.message }; } const problem = this.#bootProblem(after); @@ -1786,7 +1788,27 @@ mcpHub: this.#mcpHub, brokenAlready = true; } if (brokenAlready) return undefined; - return { key: patches[0]!.key, message: problem.message }; + return { key: this.#culpritKey(patches), message: problem.message }; + } + + /** + * Which key a refused batch is shown under. The web drawer renders an error + * only beside the field whose key matches — and sends edits from every tab + * in one batch — so blaming the wrong field hides the error entirely. The + * culprit is the patch whose removal makes the next boot work; with none + * (or several needed together), `""`, which the drawer shows in its save bar. + */ + #culpritKey(patches: SettingPatch[]): string { + if (patches.length === 1) return patches[0]!.key; + for (const p of patches) { + const without = previewPatches(patches.filter((q) => q !== p)); + try { + if (without && !this.#bootProblem(without)) return p.key; + } catch { + // Unknown for this patch — try the next. + } + } + return ""; } /** Why a previewed config.json + env would fail the boot, or undefined. */ @@ -1798,7 +1820,9 @@ mcpHub: this.#mcpHub, try { config = loadConfig({ raw: next.raw, env: next.env, quiet: true }); } catch (err) { - return { kind: "load", message: err instanceof Error ? err.message : String(err) }; + // Redacted: it's returned to the caller, and a secret pasted into the + // wrong numeric env var would otherwise come back in "got \"…\"". + return { kind: "load", message: redact(err instanceof Error ? err.message : String(err)) }; } const id = config.session.defaultProvider; if (!id || id === CLAUDE_PROVIDER_ID) return undefined; diff --git a/src/tests/default-provider.test.ts b/src/tests/default-provider.test.ts index 42147dc3..40f23929 100644 --- a/src/tests/default-provider.test.ts +++ b/src/tests/default-provider.test.ts @@ -336,6 +336,40 @@ describe("settings.set session.defaultProvider", () => { expect(res.ok).toBe(true); }); + it("files a multi-key refusal under the key that caused it", async () => { + // The drawer sends every tab's edits in one batch and shows an error only + // beside the matching field — blaming the first key would hide it. + expect((await setBatch([{ key: "session.defaultProvider", value: "pi" }])).ok).toBe(true); + const res = await setBatch([ + { key: "session.defaultModel", value: "sonnet" }, + { key: "providers.pi.enabled", value: false }, + ]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "providers.pi.enabled" }); + }); + + it("files it under no field when no single key is to blame", async () => { + // Each change alone breaks pi, so dropping either one still leaves the + // boot broken — no single field is the fix, so the save bar gets it. + expect((await setBatch([{ key: "session.defaultProvider", value: "pi" }])).ok).toBe(true); + const res = await setBatch([ + { key: "providers.pi.command", value: "/nonexistent/pi" }, + { key: "providers.pi.enabled", value: false }, + ]); + expect(res.ok).toBe(false); + expect(res.errors[0]).toMatchObject({ key: "" }); + }); + + it("does not refuse a valid default because the env override's backend broke", async () => { + // CODEOID_DEFAULT_PROVIDER names pi, whose binary is gone: the next boot + // fails either way, and saving claude to config.json didn't cause that. + brokenDefault(); + process.env.CODEOID_DEFAULT_PROVIDER = "pi"; + const res = await setBatch([{ key: "session.defaultProvider", value: "claude" }]); + expect(res.errors).toEqual([]); + expect(res.ok).toBe(true); + }); + it("records a refused write in the audit log", async () => { await set("claud"); // Store has no audit-read API on purpose; read the table directly. From c98a7172bb29c27ede1c66c2e3a1c07b5cae2da1 Mon Sep 17 00:00:00 2001 From: Yash Datta Date: Sat, 26 Sep 2026 20:27:19 +0800 Subject: [PATCH 6/6] docs: say spawned workers default to session.defaultProvider The open-questions line still said spawns default to Claude, contradicting the line #339 updated earlier in the same doc. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/conductor-frontends-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conductor-frontends-design.md b/docs/conductor-frontends-design.md index 32820999..c3443018 100644 --- a/docs/conductor-frontends-design.md +++ b/docs/conductor-frontends-design.md @@ -377,7 +377,7 @@ Primary view is the state-grouped list; tree/graph is the co-primary map. Resolution is visible and correctable before dispatch; dispatched instructions land in the target session's own transcript. **Open.** -Cross-backend `spawn`: today spawns default to Claude and fleet tools are Claude-only — sequencing the daemon work to let the conductor spawn a codex/gemini/pi worker (via the anyagent adapter) is out of P5 scope but gates the full §7 story; when does it land? +Cross-backend `spawn`: today spawns default to `session.defaultProvider` (Claude unless configured) and fleet tools are Claude-only — sequencing the daemon work to let the conductor spawn a codex/gemini/pi worker (via the anyagent adapter) is out of P5 scope but gates the full §7 story; when does it land? Default home: does a user with an active conductor default to the Conductor home or the Sessions home? Review-queue merge: how much of the sequenced-merge / conflict-pre-detection lands in P5.4 vs a later slice? Normalized "conductor credit": exact conversion model across token / credit / quota / GPU-second wallets.