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/docs/conductor-frontends-design.md b/docs/conductor-frontends-design.md index af991fd8..c3443018 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). @@ -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. 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..3ef9b841 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. */ @@ -1184,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" }, @@ -1220,6 +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: 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, @@ -1261,6 +1278,15 @@ 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; + /** Skip the operator-facing startup warnings — for previews that run on + * every settings write, where repeating them is noise. */ + quiet?: boolean; } /** @@ -1282,8 +1308,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); @@ -1394,6 +1420,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/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..aa1f6a21 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,63 @@ 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; + // 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 ${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 * 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"); +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", displayName: "Claude (Anthropic)", @@ -189,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)", @@ -212,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", @@ -241,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", @@ -271,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", @@ -301,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", @@ -352,5 +403,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/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 e649557d..cf407dbe 100644 --- a/src/daemon/session-manager.ts +++ b/src/daemon/session-manager.ts @@ -15,13 +15,14 @@ import { Session, type AttachedClient, type WindowScope } from "./session.js"; import { type CatalogEntry, isPlaceholderModel, type SessionProvider } from "./providers/interface.js"; import { createDefaultProviderRegistry, + 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 { BackendLoginBroker, BackendLoginError, redact } from "./auth/backend-login.js"; import { RateLimiter } from "./rate-limit.js"; import type { TranscriptMeta, TranscriptStore } from "./transcript.js"; @@ -107,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, mutateConfigFile } 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 { @@ -127,6 +128,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"; @@ -297,12 +299,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 { @@ -664,7 +660,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++; @@ -705,7 +704,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: meta.providerId, + providerId, forkedFrom: meta.forkedFrom, worktree: meta.worktree, // A collaboration is durable state, not turn state: the goal and @@ -845,6 +844,8 @@ mcpHub: this.#mcpHub, #resumeRoleChild( meta: TranscriptMeta, goalConfigs: ReadonlyMap, + /** The backend the child actually resumes on (`#resumeProviderId`). */ + providerId: string, ): | { orphaned: boolean; @@ -882,7 +883,7 @@ mcpHub: this.#mcpHub, { roleName: role.roleName, ordinal: role.ordinal, - providerId: meta.providerId ?? "claude", + providerId, shape: role.write ? "ship" : "scout", write: role.write, }, @@ -897,8 +898,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. @@ -1551,7 +1556,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 +1580,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 }; } @@ -1663,6 +1670,29 @@ mcpHub: this.#mcpHub, }; } try { + // 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( + 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, @@ -1688,6 +1718,123 @@ mcpHub: this.#mcpHub, } } + /** + * 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; + } + + /** + * Why the config these patches leave behind would stop the next boot, or + * 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 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. + * + * 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 { + if (patches.length === 0) return undefined; + const after = previewPatches(patches); + 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: 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; + 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); + if (!problem) return undefined; + let brokenAlready: boolean; + try { + const before = previewPatches([]); + 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; + } + if (brokenAlready) return undefined; + 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. */ + #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) { + // 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; + try { + createDefaultProviderRegistry(config, next.env); + return undefined; + } catch (err) { + if (err instanceof DefaultProviderError) return { kind: "provider", message: err.message }; + throw err; + } + } + #settingsForbidden(requestId: string): DaemonMessage { return { type: "response.error", @@ -2405,7 +2552,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 +2597,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 +3458,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 +4709,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; @@ -4572,7 +4721,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 cb895b3a..1eaa92f6 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, …). 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", + }), 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/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/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/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/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..40f23929 --- /dev/null +++ b/src/tests/default-provider.test.ts @@ -0,0 +1,392 @@ +/** + * `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 { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +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"; +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; +// 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. +// 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(() => { + 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. + 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"); + chmodSync(join(bin, "pi"), 0o755); + process.env.PATH = `${bin}:${savedEnv.PATH ?? ""}`; +}); + +afterEach(async () => { + 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 {} +}); + +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"); + }); + + 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", () => { + 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"); + }); + + 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; + } + }); + + // 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("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("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. + 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=next-boot" }); + }); + + 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..ae7abff0 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,71 @@ 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(/"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", () => { + // 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();