diff --git a/docs-site/src/content/docs/fr/reference/configuration.md b/docs-site/src/content/docs/fr/reference/configuration.md index 36ed8b6a383..dbc24b3d8ce 100644 --- a/docs-site/src/content/docs/fr/reference/configuration.md +++ b/docs-site/src/content/docs/fr/reference/configuration.md @@ -27,6 +27,12 @@ Si le fichier ne peut pas être analysé, opencodex le sauvegarde en tant que `config.json.invalid-`, affiche un avertissement dans la console et démarre avec les valeurs par défaut. Si le fichier est absent, la valeur par défaut d'une nouvelle installation est également utilisée : un fournisseur `openai` en mode transfert. +Les commandes locales `ocx config set`, `unset`, `import`, la commande hors ligne `ocx effort`, +les opérations add/remove sur les modèles personnalisés et add/remove/set-default sur les fournisseurs +refusent un fichier `config.json` existant invalide ou illisible sans en modifier les octets. +Exécutez `ocx config validate` et réparez le fichier avant de réessayer. Si une configuration existe déjà, +import exige également que le fichier cible soit lisible et valide. + ## Priorité et valeurs par défaut Les valeurs valides de `config.json` remplacent les valeurs par défaut intégrées. Les champs facultatifs absents utilisent les valeurs par défaut diff --git a/docs-site/src/content/docs/ja/reference/configuration.md b/docs-site/src/content/docs/ja/reference/configuration.md index df2527ef866..6ccfab9b3e5 100644 --- a/docs-site/src/content/docs/ja/reference/configuration.md +++ b/docs-site/src/content/docs/ja/reference/configuration.md @@ -19,6 +19,11 @@ opencodex は、永続的な設定を `$OPENCODEX_HOME/config.json` (通常は ` ファイルを解析できない場合、opencodex はファイルを `config.json.invalid-` としてバックアップし、コンソールに警告を表示し、デフォルトで起動します。欠落しているファイルでも、新規インストールのデフォルトである 1 つの `openai` フォワード プロバイダーが使用されます。 +ローカルの `ocx config set`、`unset`、`import`、オフラインの `ocx effort`、カスタムモデルの add/remove、 +プロバイダーの add/remove/set-default は、既存の `config.json` が無効または読み取り不能な場合、 +元のバイト列を変更せずに拒否します。`ocx config validate` を実行し、ファイルを修復してから再試行してください。 +既存の設定がある場合、import にも読み取り可能で有効な保存先ファイルが必要です。 + ## 優先順位とデフォルト `config.json` の有効な値は、組み込みのデフォルトをオーバーライドします。省略可能なフィールドが欠落している場合は、ドメイン ページに記載されているデフォルトが使用されます。 `OPENCODEX_HOME` は、デフォルトの構成ディレクトリよりも優先されます。 `apiKey: "${PROVIDER_API_KEY}"` などの環境参照を受け入れるフィールドは、リクエスト時にその変数を解決します。送信プロキシの場合、すでに設定されている `HTTP_PROXY` または `HTTPS_PROXY` が最上位の `proxy` フィールドよりも優先されます。 diff --git a/docs-site/src/content/docs/ko/reference/configuration.md b/docs-site/src/content/docs/ko/reference/configuration.md index 4f80b5136a9..3d7a67be175 100644 --- a/docs-site/src/content/docs/ko/reference/configuration.md +++ b/docs-site/src/content/docs/ko/reference/configuration.md @@ -27,6 +27,11 @@ opencodex는 지속 설정을 `$OPENCODEX_HOME/config.json`에 저장합니다. 남긴 뒤 기본값으로 시작합니다. 파일이 없어도 새로 설치한 경우의 기본값을 그대로 사용하며, 그것은 단일 `openai` forward 프로바이더입니다. +로컬 `ocx config set`, `unset`, `import`, 오프라인 `ocx effort`, 커스텀 모델 추가·삭제, +프로바이더 추가·삭제·기본값 변경은 기존 `config.json`이 유효하지 않거나 읽을 수 없으면 +원본 바이트를 바꾸지 않고 거부합니다. `ocx config validate`로 확인하고 파일을 고친 뒤 +다시 실행하세요. import도 기존 설정 파일이 있다면 읽을 수 있고 유효한 대상 파일을 요구합니다. + ## 우선순위와 기본값 `config.json`의 유효한 값은 내장 기본값보다 우선합니다. 선택 사항으로 비어 있는 필드는 각 도메인 페이지에 diff --git a/docs-site/src/content/docs/reference/configuration.md b/docs-site/src/content/docs/reference/configuration.md index baba267dadf..f422a166b9d 100644 --- a/docs-site/src/content/docs/reference/configuration.md +++ b/docs-site/src/content/docs/reference/configuration.md @@ -27,6 +27,11 @@ If the file cannot be parsed, opencodex backs it up as `config.json.invalid-`, warns on the console, and starts with defaults. A missing file also uses the fresh-install default: one `openai` forward provider. +Local `ocx config set`, `unset`, `import`, offline `ocx effort`, custom-model add/remove, and +provider add/remove/set-default refuse an invalid or unreadable existing `config.json` without +changing its bytes. Run `ocx config validate` and repair the file before retrying. Import also +requires a readable, valid target when a config already exists. + ## Precedence and defaults ### Provider and model aliases diff --git a/docs-site/src/content/docs/ru/reference/configuration.md b/docs-site/src/content/docs/ru/reference/configuration.md index 25f0894ede2..2d6ce09c329 100644 --- a/docs-site/src/content/docs/ru/reference/configuration.md +++ b/docs-site/src/content/docs/ru/reference/configuration.md @@ -29,6 +29,12 @@ Dashboard, management API и mutating-команды CLI записывают в `config.json.invalid-`, пишет предупреждение в консоль и стартует с настройками по умолчанию. Если файла нет, используется тот же свежий дефолт: один forward-провайдер `openai`. +Локальные команды `ocx config set`, `unset`, `import`, офлайн-команда `ocx effort`, +операции add/remove для пользовательских моделей и add/remove/set-default для провайдеров +отказываются изменять существующий некорректный или нечитаемый `config.json`, сохраняя исходные байты. +Выполните `ocx config validate` и исправьте файл перед повторной попыткой. Если конфигурация уже существует, +для import целевой файл также должен быть читаемым и корректным. + ## Приоритет и значения по умолчанию Корректные значения из `config.json` перекрывают встроенные дефолты. Для отсутствующих diff --git a/docs-site/src/content/docs/tr/reference/configuration.md b/docs-site/src/content/docs/tr/reference/configuration.md index 89b9848878f..18b87c1b1e3 100644 --- a/docs-site/src/content/docs/tr/reference/configuration.md +++ b/docs-site/src/content/docs/tr/reference/configuration.md @@ -32,6 +32,12 @@ Dosya ayrıştırılamazsa opencodex dosyayı `config.json.invalid-`，在控制台警告，并以默认值启动。文件缺失时也会使用新安装默认值： 一个 `openai` forward provider。 +本地 `ocx config set`、`unset`、`import`、离线 `ocx effort`、自定义模型的 add/remove +以及 provider 的 add/remove/set-default 操作，会拒绝修改已有的无效或不可读的 `config.json`， +并保留原始字节。请运行 `ocx config validate` 并修复文件后再重试。已有配置时， +import 同样要求目标文件可读且有效。 + ## 优先级与默认值 `config.json` 中的有效值会覆盖内置默认值。缺失的可选字段使用各 domain 页面文档中说明的默认值。 diff --git a/docs-site/src/content/docs/zh-tw/reference/configuration.md b/docs-site/src/content/docs/zh-tw/reference/configuration.md index 19d4db12cbe..5da4fe7093a 100644 --- a/docs-site/src/content/docs/zh-tw/reference/configuration.md +++ b/docs-site/src/content/docs/zh-tw/reference/configuration.md @@ -26,6 +26,11 @@ opencodex 把持久化設定存放在 `$OPENCODEX_HOME/config.json`，通常是 如果檔案無法解析，opencodex 會把它備份為 `config.json.invalid-`、在 console 警告，並以預設值啟動。檔案缺失時也使用全新安裝的預設：一個 `openai` forward provider。 +本機的 `ocx config set`、`unset`、`import`、離線 `ocx effort`、自訂模型的 add/remove +與 provider 的 add/remove/set-default 操作，會拒絕修改既有但無效或無法讀取的 `config.json`， +並保留原始位元組。請執行 `ocx config validate` 並修復檔案後再重試。已有設定時， +import 也要求目標檔案可讀且有效。 + ## 優先順序與預設值 `config.json` 中的有效值會覆寫內建預設。缺失的選用欄位使用各領域頁面記載的預設值。 diff --git a/scripts/test-layout/layout-clients.json b/scripts/test-layout/layout-clients.json index c7dc4e9131f..fb8a72fe2bf 100644 --- a/scripts/test-layout/layout-clients.json +++ b/scripts/test-layout/layout-clients.json @@ -1,4 +1,5 @@ { + "windows-config-root-async-writers.test.ts": "windows", "t3code-client-lifecycle.test.ts": "clients", "fix-jsonc-cr-comments.test.ts": "lib", "fix-jsonc-empty-comma.test.ts": "lib", diff --git a/src/cli/config-command.ts b/src/cli/config-command.ts index 5c75a19e5a6..b0f8d26b75d 100644 --- a/src/cli/config-command.ts +++ b/src/cli/config-command.ts @@ -1,6 +1,8 @@ import { readFileSync, writeFileSync } from "node:fs"; import { clearCodexAccountPin } from "../codex/account-priority"; import { getConfigPath, mutatePersistedConfig, readConfigDiagnostics, sanitizeModelCostsForDisplay, saveConfig, validateConfigCandidate } from "../config"; +import { getConfigDir, withConfigDirHardeningSettled } from "../config/paths"; +import { readCliConfigForMutation } from "./config-mutation"; import { VISION_REASONING_EFFORTS, isVisionReasoningEffort } from "../reasoning-effort"; import type { OcxConfig } from "../types"; import { normalizeVisionReasoningForModel } from "../vision/reasoning"; @@ -260,7 +262,7 @@ export async function handleConfigCommand(argv: string[]): Promise { // `mutatePersistedConfig` reruns this callback against the latest validated disk // state, so the operation is applied to what is actually there at commit time. let savedValue: unknown = null; - const outcome = mutatePersistedConfig(fresh => { + const outcome = await withConfigDirHardeningSettled(getConfigDir(), () => mutatePersistedConfig(fresh => { // Snapshot BEFORE mutating: comparing after the write compares a value with // itself and would report every no-op as a change, bumping the generation. const before = JSON.stringify(fresh); @@ -285,9 +287,9 @@ export async function handleConfigCommand(argv: string[]): Promise { } Object.assign(fresh, config); return { changed: JSON.stringify(fresh) !== before, value: undefined }; - }); + })); if (outcome.status === "unavailable") { - throw new Error(outcome.reason === "conflict" + throw new CliUsageError(outcome.reason === "conflict" ? "config changed while applying this update; retry" : `config is ${outcome.reason}`); } @@ -327,7 +329,10 @@ export async function handleConfigCommand(argv: string[]): Promise { if (!path) throw new CliUsageError("import path is required", USAGE); if (!yes) throw new CliUsageError("import requires --yes", USAGE); rejectArgs(args, USAGE); - saveConfig(validate(loadInput(path))); + await withConfigDirHardeningSettled(getConfigDir(), () => { + readCliConfigForMutation(); + saveConfig(validate(loadInput(path))); + }); printData({ ok: true, source: path }, wantsJson, [`Imported config from ${path}. Restart or run ocx sync if needed.`]); return; } diff --git a/src/cli/config-mutation.ts b/src/cli/config-mutation.ts new file mode 100644 index 00000000000..935d28da551 --- /dev/null +++ b/src/cli/config-mutation.ts @@ -0,0 +1,13 @@ +import { readConfigDiagnostics } from "../config/diagnostics"; +import type { OcxConfig } from "../types"; +import { CliUsageError } from "./runtime-api"; + +/** Inside admission unless this is a passive preliminary selection read. */ +export function readCliConfigForMutation(options: { passive?: boolean } = {}): OcxConfig { + const diagnostics = readConfigDiagnostics(options); + if (diagnostics.source === "fallback" || diagnostics.error) { + throw new CliUsageError("config.json is invalid or unreadable; run ocx config validate and repair it before retrying. Nothing was saved."); + } + // Fresh installs retain the full-config commands' existing default initialization. + return diagnostics.config; +} diff --git a/src/cli/dispatch.ts b/src/cli/dispatch.ts index fbf3addf8c1..6f9809c9916 100644 --- a/src/cli/dispatch.ts +++ b/src/cli/dispatch.ts @@ -10,6 +10,7 @@ import type { ProxyRestartStartOutcome } from "./tray-proxy"; * never needs to import the entry module back (no cycle). */ import { CLI_COMMANDS } from "./registry"; +import { runCliAction } from "./runtime-api"; import { uninstallArgsError, type CliHead } from "./root"; import type { ReadyArgs } from "./ready"; import type { LivenessIo, LiveProxy } from "../server/proxy-liveness"; @@ -870,12 +871,12 @@ const commandRunners: Record = { // Reset first, like the service runner below: reading process.exitCode only // reports THIS command's outcome if nothing earlier in the process set it. process.exitCode = 0; - await handleProviderCommand(deps.args.slice(1)); + const code = await runCliAction(() => handleProviderCommand(deps.args.slice(1))); // handleProviderCommand reports failure through process.exitCode, which it sets // from handleProviderRuntimeCommand. Returning a literal 0 here made index.ts // call process.exit(0) and erase it, so `ocx provider quota` against a stopped // proxy printed an error and still exited 0 (#2697). - return Number(process.exitCode ?? 0); + return code || Number(process.exitCode ?? 0); }, account: async deps => { const { cmdAccount } = await import("./account"); @@ -884,10 +885,10 @@ const commandRunners: Record = { models: async deps => { const { handleModels } = await import("./models"); process.exitCode = 0; - await handleModels(deps.args.slice(1)); + const code = await runCliAction(() => handleModels(deps.args.slice(1))); // Same as the provider runner above: handleModels sets process.exitCode from // handleModelsRuntimeCommand, and a literal 0 discarded it (#2697). - return Number(process.exitCode ?? 0); + return code || Number(process.exitCode ?? 0); }, alias: async deps => { const { handleAliasCommand } = await import("./alias"); diff --git a/src/cli/effort.ts b/src/cli/effort.ts index 709e3e47647..ee0bbc0afc6 100644 --- a/src/cli/effort.ts +++ b/src/cli/effort.ts @@ -1,4 +1,6 @@ import { loadConfig, saveConfig } from "../config"; +import { readCliConfigForMutation } from "./config-mutation"; +import { getConfigDir, withConfigDirHardeningSettled } from "../config/paths"; import { CODEX_REASONING_LEVELS, configuredReasoningEfforts, @@ -232,20 +234,23 @@ async function setEffort( } // Offline persistence path: only reached when no live proxy was found before mutation - const config = loadConfig(); - if (validatedMain !== undefined) { - if (validatedMain === null) delete config.effortCap; - else config.effortCap = validatedMain; - } - if (validatedSubagent !== undefined) { - if (validatedSubagent === null) delete config.subagentEffortCap; - else config.subagentEffortCap = validatedSubagent; - } - if (validatedInjection !== undefined) { - if (validatedInjection === null) delete config.injectionEffort; - else config.injectionEffort = validatedInjection; - } - saveConfig(config); + const config = await withConfigDirHardeningSettled(getConfigDir(), () => { + const config = readCliConfigForMutation(); + if (validatedMain !== undefined) { + if (validatedMain === null) delete config.effortCap; + else config.effortCap = validatedMain; + } + if (validatedSubagent !== undefined) { + if (validatedSubagent === null) delete config.subagentEffortCap; + else config.subagentEffortCap = validatedSubagent; + } + if (validatedInjection !== undefined) { + if (validatedInjection === null) delete config.injectionEffort; + else config.injectionEffort = validatedInjection; + } + saveConfig(config); + return config; + }); const result = { ok: true, diff --git a/src/cli/models.ts b/src/cli/models.ts index 00102a2be42..28d83536b49 100644 --- a/src/cli/models.ts +++ b/src/cli/models.ts @@ -1,12 +1,14 @@ /** * `ocx models` subcommand — list configured models and manage custom models. */ +import { getConfigDir, withConfigDirHardeningSettled } from "../config/paths"; import { resolveMatchedPrice, type MatchedPrice } from "../usage/cost"; import { randomUUID } from "node:crypto"; import { createInterface } from "node:readline/promises"; import { syncModelsToCodex } from "../codex/sync"; import { configuredContextWindow } from "../codex/catalog/provider-fetch"; import { hasOwnProvider, isValidProviderName, loadConfig, saveConfig } from "../config"; +import { readCliConfigForMutation } from "./config-mutation"; import { configuredReasoningEfforts, modelRecordValue, @@ -174,58 +176,61 @@ async function handleCustomAdd(args: string[], deps: ModelsCommandDeps): Promise if (!provider || !modelId) fail("provider and modelId are required", ADD_USAGE); if (!isValidProviderName(provider)) fail(`invalid provider name "${provider}"`); - const config = loadConfig(); - if (!hasOwnProvider(config.providers, provider)) { - fail(`provider "${provider}" is not configured. See: ocx provider list`); - } + const { config, entry } = await withConfigDirHardeningSettled(getConfigDir(), () => { + const config = readCliConfigForMutation(); + if (!hasOwnProvider(config.providers, provider)) { + fail(`provider "${provider}" is not configured. See: ocx provider list`); + } - const displayName = displayNameValue?.trim() || undefined; - if (displayName?.includes("/")) fail("displayName must not contain /"); + const displayName = displayNameValue?.trim() || undefined; + if (displayName?.includes("/")) fail("displayName must not contain /"); - let contextWindow: number | undefined; - if (contextWindowValue !== undefined) { - contextWindow = Number(contextWindowValue); - if (!Number.isInteger(contextWindow) || contextWindow <= 0) { - fail("context window must be a positive integer"); + let contextWindow: number | undefined; + if (contextWindowValue !== undefined) { + contextWindow = Number(contextWindowValue); + if (!Number.isInteger(contextWindow) || contextWindow <= 0) { + fail("context window must be a positive integer"); + } } - } - let inputModalities: string[] | undefined; - if (modalitiesValue !== undefined) { - inputModalities = modalitiesValue.split(",").map(value => value.trim()); - const invalid = inputModalities.filter(value => !ALLOWED_MODALITIES.has(value)); - if (inputModalities.length === 0 || invalid.length > 0) { - fail("modalities must be comma-separated values from text|image|audio"); + let inputModalities: string[] | undefined; + if (modalitiesValue !== undefined) { + inputModalities = modalitiesValue.split(",").map(value => value.trim()); + const invalid = inputModalities.filter(value => !ALLOWED_MODALITIES.has(value)); + if (inputModalities.length === 0 || invalid.length > 0) { + fail("modalities must be comma-separated values from text|image|audio"); + } + inputModalities = [...new Set(inputModalities)]; } - inputModalities = [...new Set(inputModalities)]; - } - const parsed = parseReasoningArgs(reasoningEffortsValue, defaultEffortValue); - if (parsed.error) fail(parsed.error); + const parsed = parseReasoningArgs(reasoningEffortsValue, defaultEffortValue); + if (parsed.error) fail(parsed.error); - const existing = config.customModels ?? []; - const slug = routedSlug(provider, modelId); - if (existing.some(model => routedSlug(model.provider, model.modelId) === slug)) { - fail(`custom model "${slug}" already exists`); - } - const known = knownModelIdsForProvider(provider, config.providers[provider], config); - if (encodedModelIdCollides(modelId, known)) { - fail(`custom model "${slug}" is ambiguous; it encodes to an existing model id`); - } + const existing = config.customModels ?? []; + const slug = routedSlug(provider, modelId); + if (existing.some(model => routedSlug(model.provider, model.modelId) === slug)) { + fail(`custom model "${slug}" already exists`); + } + const known = knownModelIdsForProvider(provider, config.providers[provider], config); + if (encodedModelIdCollides(modelId, known)) { + fail(`custom model "${slug}" is ambiguous; it encodes to an existing model id`); + } - const entry: OcxCustomModel = { - id: randomUUID(), - provider, - modelId, - ...(displayName ? { displayName } : {}), - ...(contextWindow ? { contextWindow } : {}), - ...(inputModalities ? { inputModalities } : {}), - ...(parsed.reasoningEfforts ? { reasoningEfforts: parsed.reasoningEfforts } : {}), - ...(parsed.defaultReasoningEffort ? { defaultReasoningEffort: parsed.defaultReasoningEffort } : {}), - addedAt: new Date().toISOString(), - }; - config.customModels = [...existing, entry]; - saveConfig(config); + const entry: OcxCustomModel = { + id: randomUUID(), + provider, + modelId, + ...(displayName ? { displayName } : {}), + ...(contextWindow ? { contextWindow } : {}), + ...(inputModalities ? { inputModalities } : {}), + ...(parsed.reasoningEfforts ? { reasoningEfforts: parsed.reasoningEfforts } : {}), + ...(parsed.defaultReasoningEffort ? { defaultReasoningEffort: parsed.defaultReasoningEffort } : {}), + addedAt: new Date().toISOString(), + }; + config.customModels = [...existing, entry]; + saveConfig(config); + return { config, entry }; + }); const sync = await syncCustomModelsIfLive(config, deps); printCustomMutation("added", entry, sync, wantsJson); } @@ -256,7 +261,7 @@ async function handleCustomRemove(args: string[], deps: ModelsCommandDeps): Prom return; } - const config = loadConfig(); + const config = readCliConfigForMutation({ passive: true }); const existing = config.customModels ?? []; // Slug matching goes through the shared resolver so this command sees the same collision // class catalog filtering and persisted sync see (#2491). `slugEquals` compares the raw and @@ -298,10 +303,18 @@ async function handleCustomRemove(args: string[], deps: ModelsCommandDeps): Prom return; } - const next = existing.filter((_, modelIndex) => modelIndex !== index); - config.customModels = next.length > 0 ? next : undefined; - saveConfig(config); - const sync = await syncCustomModelsIfLive(config, deps); + const savedConfig = await withConfigDirHardeningSettled(getConfigDir(), () => { + const fresh = readCliConfigForMutation(); + const current = fresh.customModels ?? []; + if (JSON.stringify(current.find(row => row.id === model.id)) !== JSON.stringify(model)) { + fail("custom model changed before removal; inspect it before retrying"); + } + const next = current.filter(row => row.id !== model.id); + fresh.customModels = next.length > 0 ? next : undefined; + saveConfig(fresh); + return fresh; + }); + const sync = await syncCustomModelsIfLive(savedConfig, deps); printCustomMutation("removed", model, sync, wantsJson); } diff --git a/src/cli/provider.ts b/src/cli/provider.ts index 06d7b7c8a74..e2bd461240f 100644 --- a/src/cli/provider.ts +++ b/src/cli/provider.ts @@ -9,6 +9,8 @@ * set-default Change the default provider */ import { hasOwnProvider, isValidProviderName, loadConfig, readConfigAdmissionSnapshot, sanitizeModelCostsForDisplay, saveConfig, validateConfigCandidate, withConfigMutationLockSync } from "../config"; +import { readCliConfigForMutation } from "./config-mutation"; +import { getConfigDir, withConfigDirHardeningSettled } from "../config/paths"; import { apiKeyTransportConfigError, modelCapabilitiesConfigError, mergeModelCapabilities } from "../config/provider-validation"; import { hasHelpFlag, printSubcommandUsage } from "./help"; import { getProviderRegistryEntry, PROVIDER_REGISTRY } from "../providers/registry"; @@ -248,165 +250,170 @@ async function handleAdd(args: string[], deps: ProviderCommandDeps): Promise { + const config = readCliConfigForMutation(); - const namespaceCollision = codexAccountNamespaceProviderCollisionError(config.codexAccountNamespaces, name); - if (namespaceCollision) { - console.error(`Error: ${namespaceCollision}.`); - process.exit(1); - } + const namespaceCollision = codexAccountNamespaceProviderCollisionError(config.codexAccountNamespaces, name); + if (namespaceCollision) { + console.error(`Error: ${namespaceCollision}.`); + process.exit(1); + } - if (hasOwnProvider(config.providers, name) && !force) { - console.error(`Provider "${name}" already exists. Use --force to overwrite.`); - process.exit(1); - } + if (hasOwnProvider(config.providers, name) && !force) { + console.error(`Provider "${name}" already exists. Use --force to overwrite.`); + process.exit(1); + } - let provConfig: OcxProviderConfig; - const registryEntry = getProviderRegistryEntry(name); + let provConfig: OcxProviderConfig; + const registryEntry = getProviderRegistryEntry(name); - if (registryEntry) { - if (registryEntry.deprecated) console.warn(`Warning: provider "${name}" is deprecated: ${registryEntry.deprecated}`); - provConfig = providerConfigSeed(registryEntry); - if (apiKey) { - const candidateError = providerCredentialInputError({ apiKey }); - if (candidateError) { - console.error(`Error: ${candidateError}`); - process.exitCode = 2; - return; + if (registryEntry) { + if (registryEntry.deprecated) console.warn(`Warning: provider "${name}" is deprecated: ${registryEntry.deprecated}`); + provConfig = providerConfigSeed(registryEntry); + if (apiKey) { + const candidateError = providerCredentialInputError({ apiKey }); + if (candidateError) { + console.error(`Error: ${candidateError}`); + process.exitCode = 2; + return; + } + if ((authMode ?? registryEntry.authKind) === "forward") { + console.warn(`Warning: provider "${name}" uses ChatGPT login (forward auth); --api-key is ignored.`); + } else if ((authMode ?? registryEntry.authKind) === "oauth") { + console.warn(`Warning: provider "${name}" uses OAuth auth; --api-key is ignored. Run: ocx login ${name}`); + } else { + provConfig.apiKey = apiKey; + } } - if ((authMode ?? registryEntry.authKind) === "forward") { - console.warn(`Warning: provider "${name}" uses ChatGPT login (forward auth); --api-key is ignored.`); - } else if ((authMode ?? registryEntry.authKind) === "oauth") { - console.warn(`Warning: provider "${name}" uses OAuth auth; --api-key is ignored. Run: ocx login ${name}`); - } else { - provConfig.apiKey = apiKey; + if (defaultModel) provConfig.defaultModel = defaultModel; + if (adapter) provConfig.adapter = adapter; + if (baseUrl) provConfig.baseUrl = baseUrl; + } else { + if (!adapter || !baseUrl) { + console.error(`Provider "${name}" is not in the registry. --adapter and --base-url are required.`); + console.error("Usage: ocx provider add --adapter --base-url [--api-key ]"); + process.exit(1); } + provConfig = { + adapter, + baseUrl, + ...(apiKey ? { apiKey } : {}), + ...(defaultModel ? { defaultModel } : {}), + }; } - if (defaultModel) provConfig.defaultModel = defaultModel; - if (adapter) provConfig.adapter = adapter; - if (baseUrl) provConfig.baseUrl = baseUrl; - } else { - if (!adapter || !baseUrl) { - console.error(`Provider "${name}" is not in the registry. --adapter and --base-url are required.`); - console.error("Usage: ocx provider add --adapter --base-url [--api-key ]"); - process.exit(1); - } - provConfig = { - adapter, - baseUrl, - ...(apiKey ? { apiKey } : {}), - ...(defaultModel ? { defaultModel } : {}), - }; - } - if (responsesPath !== undefined) provConfig.responsesPath = responsesPath; - const credentialError = providerCredentialInputError(provConfig); - if (credentialError) { - console.error(`Error: ${credentialError}`); - process.exitCode = 2; - return; - } - if (authMode !== undefined) provConfig.authMode = authMode as OcxProviderConfig["authMode"]; - if (name === "anthropic2") { - if (provConfig.adapter !== "anthropic" || provConfig.authMode !== "oauth") delete provConfig.anthropicOAuthInstance; - if (provConfig.anthropicOAuthInstance) { - try { assertAnthropicInstanceLoginConfig(config, name); } - catch { - console.error("Error: cannot add Pool 2 over an existing custom or unreadable provider configuration; resolve it first."); - process.exitCode = 2; - return; + if (responsesPath !== undefined) provConfig.responsesPath = responsesPath; + const credentialError = providerCredentialInputError(provConfig); + if (credentialError) { + console.error(`Error: ${credentialError}`); + process.exitCode = 2; + return; + } + if (authMode !== undefined) provConfig.authMode = authMode as OcxProviderConfig["authMode"]; + if (name === "anthropic2") { + if (provConfig.adapter !== "anthropic" || provConfig.authMode !== "oauth") delete provConfig.anthropicOAuthInstance; + if (provConfig.anthropicOAuthInstance) { + try { assertAnthropicInstanceLoginConfig(config, name); } + catch { + console.error("Error: cannot add Pool 2 over an existing custom or unreadable provider configuration; resolve it first."); + process.exitCode = 2; + return; + } } } - } - if (name === "openai" && (authMode !== undefined || responsesPath !== undefined) - && !isCanonicalOpenAiForwardProvider(provConfig)) { - console.error("Error: Canonical OpenAI must keep its built-in forward destination and authentication. Use a separate provider name for a custom endpoint."); - process.exitCode = 2; - return; - } + if (name === "openai" && (authMode !== undefined || responsesPath !== undefined) + && !isCanonicalOpenAiForwardProvider(provConfig)) { + console.error("Error: Canonical OpenAI must keep its built-in forward destination and authentication. Use a separate provider name for a custom endpoint."); + process.exitCode = 2; + return; + } - if (apiKeyTransport !== undefined) { - if (apiKeyTransport !== "x-api-key" && apiKeyTransport !== "bearer") { - console.error('Error: --api-key-transport must be "x-api-key" or "bearer".'); - process.exit(1); + if (apiKeyTransport !== undefined) { + if (apiKeyTransport !== "x-api-key" && apiKeyTransport !== "bearer") { + console.error('Error: --api-key-transport must be "x-api-key" or "bearer".'); + process.exit(1); + } + const transportError = apiKeyTransportConfigError({ ...provConfig, apiKeyTransport }); + if (transportError) { + console.error(`Error: ${transportError}.`); + process.exit(1); + } + provConfig.apiKeyTransport = apiKeyTransport; } - const transportError = apiKeyTransportConfigError({ ...provConfig, apiKeyTransport }); - if (transportError) { - console.error(`Error: ${transportError}.`); - process.exit(1); + if (googleToolSchemaPolicy !== undefined) { + if (googleToolSchemaPolicy !== "compatible" && googleToolSchemaPolicy !== "reject-lossy") { + console.error('Error: --google-tool-schema-policy must be "compatible" or "reject-lossy".'); + process.exit(1); + } + if (provConfig.adapter !== "google") { + console.error("Error: --google-tool-schema-policy requires the google adapter."); + process.exit(1); + } + provConfig.googleToolSchemaPolicy = googleToolSchemaPolicy; } - provConfig.apiKeyTransport = apiKeyTransport; - } - if (googleToolSchemaPolicy !== undefined) { - if (googleToolSchemaPolicy !== "compatible" && googleToolSchemaPolicy !== "reject-lossy") { - console.error('Error: --google-tool-schema-policy must be "compatible" or "reject-lossy".'); - process.exit(1); + + const existingProvider = config.providers[name]; + if (existingProvider?.modelCapabilities !== undefined && provConfig.modelCapabilities === undefined) { + provConfig.modelCapabilities = structuredClone(existingProvider.modelCapabilities); } - if (provConfig.adapter !== "google") { - console.error("Error: --google-tool-schema-policy requires the google adapter."); - process.exit(1); + if (existingProvider?.modelContextTiers !== undefined && provConfig.modelContextTiers === undefined) { + provConfig.modelContextTiers = structuredClone(existingProvider.modelContextTiers); } - provConfig.googleToolSchemaPolicy = googleToolSchemaPolicy; - } - - const existingProvider = config.providers[name]; - if (existingProvider?.modelCapabilities !== undefined && provConfig.modelCapabilities === undefined) { - provConfig.modelCapabilities = structuredClone(existingProvider.modelCapabilities); - } - if (existingProvider?.modelContextTiers !== undefined && provConfig.modelContextTiers === undefined) { - provConfig.modelContextTiers = structuredClone(existingProvider.modelContextTiers); - } - if (textOnly) { - const modelId = capabilityModel ?? defaultModel ?? provConfig.defaultModel; - if (!modelId) { - console.error("Error: --text-only requires --model or a default model."); - process.exit(1); + if (textOnly) { + const modelId = capabilityModel ?? defaultModel ?? provConfig.defaultModel; + if (!modelId) { + console.error("Error: --text-only requires --model or a default model."); + process.exit(1); + } + const declaration = { [modelId]: { inputModalities: ["text"] } }; + const error = modelCapabilitiesConfigError(declaration); + if (error) { console.error(`Error: ${error}.`); process.exit(1); } + provConfig.modelCapabilities = mergeModelCapabilities(provConfig.modelCapabilities, declaration); } - const declaration = { [modelId]: { inputModalities: ["text"] } }; - const error = modelCapabilitiesConfigError(declaration); - if (error) { console.error(`Error: ${error}.`); process.exit(1); } - provConfig.modelCapabilities = mergeModelCapabilities(provConfig.modelCapabilities, declaration); - } - // A --force overwrite rotates the key/endpoint but must not drop a - // user-configured price overlay (same rule as the /api/providers path and - // the login paths); there is no explicit clear/replace flag yet. - if (existingProvider?.modelCosts !== undefined && provConfig.modelCosts === undefined) { - provConfig.modelCosts = existingProvider.modelCosts; - } - if (allowPrivateNetwork) provConfig.allowPrivateNetwork = true; - // New auth/path overrides use the management owner's completed-row contract. - // Validate overrides before registration state changes; the full candidate is - // validated again by validateAndSave for every local save. - if ((authMode !== undefined || responsesPath !== undefined) - && providerManagementConfigError(name, provConfig)) { - console.error("Error: Invalid provider configuration. Authentication, destination and provider options must satisfy the provider's management rules."); - process.exitCode = 2; - return; - } - const { initializeProviderModelSelection } = await import("../providers/initial-model-selection"); - let markerCollision = false; - let removedCredentials: string[] = []; - withConfigMutationLockSync(() => { - if (provConfig.anthropicOAuthInstance) { - try { assertAnthropicInstanceLoginConfig(config, name); } - catch { markerCollision = true; return; } + // A --force overwrite rotates the key/endpoint but must not drop a + // user-configured price overlay (same rule as the /api/providers path and + // the login paths); there is no explicit clear/replace flag yet. + if (existingProvider?.modelCosts !== undefined && provConfig.modelCosts === undefined) { + provConfig.modelCosts = existingProvider.modelCosts; } - initializeProviderModelSelection(name, provConfig, existingProvider, config); - const durable = readConfigAdmissionSnapshot(); - const durableProvider = durable.kind === "read" && durable.diagnostics.source === "file" - && durable.diagnostics.error === null ? durable.diagnostics.config.providers[name] : undefined; - removedCredentials = [config.providers[name], durableProvider].flatMap(provider => provider ? providerGcpCredentialMarkers(provider) : []); - config.providers[name] = provConfig; - if (setDefault) config.defaultProvider = name; - validateAndSave(config); + if (allowPrivateNetwork) provConfig.allowPrivateNetwork = true; + // New auth/path overrides use the management owner's completed-row contract. + // Validate overrides before registration state changes; the full candidate is + // validated again by validateAndSave for every local save. + if ((authMode !== undefined || responsesPath !== undefined) + && providerManagementConfigError(name, provConfig)) { + console.error("Error: Invalid provider configuration. Authentication, destination and provider options must satisfy the provider's management rules."); + process.exitCode = 2; + return; + } + let markerCollision = false; + let removedCredentials: string[] = []; + withConfigMutationLockSync(() => { + if (provConfig.anthropicOAuthInstance) { + try { assertAnthropicInstanceLoginConfig(config, name); } + catch { markerCollision = true; return; } + } + initializeProviderModelSelection(name, provConfig, existingProvider, config); + const durable = readConfigAdmissionSnapshot(); + const durableProvider = durable.kind === "read" && durable.diagnostics.source === "file" + && durable.diagnostics.error === null ? durable.diagnostics.config.providers[name] : undefined; + removedCredentials = [config.providers[name], durableProvider].flatMap(provider => provider ? providerGcpCredentialMarkers(provider) : []); + config.providers[name] = provConfig; + if (setDefault) config.defaultProvider = name; + validateAndSave(config); + }); + if (!markerCollision) cleanupRemovedGcpCredentials(removedCredentials); + return { config, provConfig, registryEntry, markerCollision }; }); + if (!admitted) return; + const { config, provConfig, registryEntry, markerCollision } = admitted; if (markerCollision) { console.error("Error: provider configuration changed while adding Pool 2; resolve the ownership collision first."); process.exitCode = 2; return; } - cleanupRemovedGcpCredentials(removedCredentials); let sync: LocalSyncResult | undefined; if (wantsSync) { try { @@ -478,7 +485,7 @@ function handleRemove(args: string[]): void { } rejectUnknownArgs(restArgs.slice(1), "Usage: ocx provider remove [--json]"); - const config = loadConfig(); + const config = readCliConfigForMutation(); if (!hasOwnProvider(config.providers, name)) { console.error(`Provider "${name}" is not configured.`); process.exit(1); @@ -585,7 +592,7 @@ function handleSetDefault(args: string[]): void { } rejectUnknownArgs(restArgs.slice(1), "Usage: ocx provider set-default [--json]"); - const config = loadConfig(); + const config = readCliConfigForMutation(); if (!hasOwnProvider(config.providers, name)) { console.error(`Provider "${name}" is not configured. Add it first: ocx provider add ${name}`); process.exit(1); @@ -668,14 +675,14 @@ export async function handleProviderCommand(args: string[], deps: ProviderComman await handleAdd(subArgs, deps); break; case "remove": - handleRemove(subArgs); + await withConfigDirHardeningSettled(getConfigDir(), () => handleRemove(subArgs)); break; case "show": await (await import("./link-child")).printLinkChildLocalNotice("The provider settings"); handleShow(subArgs); break; case "set-default": - handleSetDefault(subArgs); + await withConfigDirHardeningSettled(getConfigDir(), () => handleSetDefault(subArgs)); break; default: { const { handleProviderRuntimeCommand } = await import("./provider-runtime"); diff --git a/src/client/state.ts b/src/client/state.ts index c51811cb1b0..deaaffe1c2f 100644 --- a/src/client/state.ts +++ b/src/client/state.ts @@ -84,7 +84,7 @@ function rawTopLevelConfig(): Record | null { export function readClientConnectionState(): ClientConnectionState { const raw = rawTopLevelConfig(); - const diagnostics = readConfigDiagnostics(); + const diagnostics = readConfigDiagnostics({ passive: true }); if (!raw) { return diagnostics.source === "default" ? { kind: "disconnected" } diff --git a/src/config/diagnostics.ts b/src/config/diagnostics.ts index c90e5e5f378..883c929146d 100644 --- a/src/config/diagnostics.ts +++ b/src/config/diagnostics.ts @@ -200,7 +200,7 @@ export function subagentDefaultSyncEffective( } export function mergeConfigDefaults(parsed: unknown): unknown { - if (!parsed || typeof parsed !== "object") return parsed; + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return parsed; const defaults = getDefaultConfig(); const raw = parsed as Record; // Same absence-is-meaningful pin as the repair merge above. @@ -839,12 +839,12 @@ function configDiagnosticsFromText(raw: string, file?: { path: string; bytes: Ui } } -export function readConfigFileSnapshot(): ConfigFileSnapshot { +export function readConfigFileSnapshot(options: { passive?: boolean } = {}): ConfigFileSnapshot { try { const path = getConfigPath(); const bytes = readFileSync(path); const raw = bytes.toString("utf-8"); - return { diagnostics: configDiagnosticsFromText(raw, { path, bytes }), raw }; + return { diagnostics: configDiagnosticsFromText(raw, options.passive ? undefined : { path, bytes }), raw }; } catch (error) { if (isMissingPathError(error)) { return { @@ -857,8 +857,9 @@ export function readConfigFileSnapshot(): ConfigFileSnapshot { } } -export function readConfigDiagnostics(): ConfigDiagnostics { - return readConfigFileSnapshot().diagnostics; +/** Passive observations project retired providers without publishing recovery backups. */ +export function readConfigDiagnostics(options: { passive?: boolean } = {}): ConfigDiagnostics { + return readConfigFileSnapshot(options).diagnostics; } /** Read-only init preflight. Occupied unsafe entries are never treated as absence. */ diff --git a/src/config/live-reconcile.ts b/src/config/live-reconcile.ts index e25bbe60ed7..3e9d14d4439 100644 --- a/src/config/live-reconcile.ts +++ b/src/config/live-reconcile.ts @@ -108,6 +108,16 @@ export function adoptPersistedModelDiscovery( baseline.disabledModels = structuredClone(persisted.disabledModels); } } + +/** Adopt the committed Advisor subtree without advancing unrelated pending live intent. */ +export function adoptPersistedAdvisor(config: OcxConfig, persisted: OcxConfig["advisor"]): void { + config.advisor = structuredClone(persisted); + const baseline = liveConfigBaseline.get(config); + // A known absent key remains rebaseable when a later disk edit adds it back. + if (baseline) baseline.advisor = structuredClone(persisted); + clearConfigTopLevelDeletionIntent(config, ["advisor"]); + if (baseline) clearConfigTopLevelDeletionIntent(baseline, ["advisor"]); +} /** * The live config retains the address of the socket Bun actually opened, while * this map retains the operator's desired address for the next process start. diff --git a/src/server/management/advisor-routes.ts b/src/server/management/advisor-routes.ts index a643f10622c..97890b01638 100644 --- a/src/server/management/advisor-routes.ts +++ b/src/server/management/advisor-routes.ts @@ -12,6 +12,9 @@ import { activateAdvisor } from "../../lib/advisor-activation"; import { jsonResponse } from "../auth-cors"; import { readManagementJsonBodyOr } from "./body"; import type { ManagementContext } from "./context"; +import { getConfigDir, withConfigDirHardeningSettled } from "../../config/paths"; +import { readConfigDiagnostics } from "../../config/diagnostics"; +import { adoptPersistedAdvisor, armDetachedConfigBaseline } from "../../config/live-reconcile"; import { ADVISOR_CONTEXT_SHARING_CONSENT_VERSION, ADVISOR_EFFORTS, @@ -184,19 +187,33 @@ async function putAdvisorSettings(ctx: ManagementContext): Promise { // `deps.` first: route tests with an in-memory fixture must never write the real config. const persist = ctx.deps.saveConfigPreservingClaudeCode ?? (await import("../../config")).saveConfigPreservingClaudeCode; - const snapshot = isRec(config.advisor) ? { ...config.advisor } : undefined; - applyPatchInMemory(config, parsed.patch); + let published = false; try { - persist(config); + return await withConfigDirHardeningSettled(getConfigDir(), () => { + const disk = ctx.deps.saveConfigPreservingClaudeCode ? undefined : readConfigDiagnostics(); + if (disk?.source === "fallback" || disk?.error) throw new Error("config_unavailable"); + const candidate = disk?.source === "file" ? disk.config : config; + // This fresh authority owns no live socket. Arm the same object passed to the + // saver so even new disk-only fields reconcile against its admitted read. + if (candidate !== config) armDetachedConfigBaseline(candidate); + const snapshot = isRec(config.advisor) ? { ...config.advisor } : undefined; + applyPatchInMemory(candidate, parsed.patch); + try { persist(candidate); } + catch (error) { + // Keep the rollback in the same synchronous phase as the live edit and save. + if (snapshot === undefined) delete config.advisor; + else config.advisor = snapshot as ManagementContext["config"]["advisor"]; + throw error; + } + published = true; + adoptPersistedAdvisor(config, candidate.advisor); + activateAdvisor(config); + return jsonResponse(advisorInfo(config), 200, req, config); + }); } catch (error) { - // Undo in memory too: a live config that serves a state the file does not hold would - // mislead every GET until the next restart. - if (snapshot === undefined) delete config.advisor; - else config.advisor = snapshot as ManagementContext["config"]["advisor"]; + if (published) throw error; return isConfigLockContention(error) ? jsonResponse({ error: { code: "config_busy", message: "Another process is saving the configuration. Try again in a moment." } }, 409, req, config) : jsonResponse({ error: { code: "write_failed", message: "The configuration could not be saved." } }, 500, req, config); } - activateAdvisor(config); - return jsonResponse(advisorInfo(config), 200, req, config); } diff --git a/structure/advisor.md b/structure/advisor.md index 9d25adccb03..d001735981e 100644 --- a/structure/advisor.md +++ b/structure/advisor.md @@ -19,6 +19,7 @@ The server composition root registers it synchronously at startup; a successful updates activation, including enable after startup and disable/reset. The runtime is loaded only when an active factory is used, so its process-local ledger is not constructed by an inactive install. `sidecar-execution.ts` reads the core-owned slot, not Advisor settings or runtime. +Advisor settings admit the complete synchronous candidate/read/patch/save/activation phase through the [config admission contract](config.md). Production uses the current disk candidate when present, preserving unrelated disk edits during admission. Successful saves adopt the committed Advisor subtree and its field-scoped live baseline together, including known absence after reset, without advancing unrelated pending live edits. Fallback or error-bearing diagnostics are refused before mutation. Deadline refusal leaves live and durable settings untouched and retains the existing `write_failed` response. Successful receipts are serialized in that callback before another settings mutation can run. Management loads `advisor-routes.ts` only for the Advisor namespace. The guard is attached through `_advisorGuard`. `tests/advisor/advisor-core-boundary.test.ts` enforces the transitive load-time boundary from router, lifecycle, Responses core and management API, including direct dynamic imports. diff --git a/structure/cli-management.md b/structure/cli-management.md index 8a3683d7c2d..c3aab111345 100644 --- a/structure/cli-management.md +++ b/structure/cli-management.md @@ -73,6 +73,8 @@ Management route declarations describe actual HTTP calls only; local Lab automat ## Provider live operations and bounded editor input +Local config set/unset/import, offline effort, custom-model add/remove and provider add/remove/default wait for config-root ACL settlement before their synchronous mutation phase. Candidate reads and validation run inside admission; invalid or unreadable authority is refused without replacing its bytes. Models/provider runners in `src/cli/dispatch.ts` use `runCliAction` for the standard usage-error message and exit 2, retaining handler `process.exitCode` on normal completion. Post-save sync keeps its existing result contract. Custom-model removal rechecks the confirmed row after any prompt and admission. The [config admission contract](config.md) owns the deadline and writer ordering. + Offline removal in `src/cli/provider.ts` snapshots GCP marker references before deletion and runs the [post-commit credential cleanup](providers/google.md#vertex-credential-markers) only after saving; live and latest durable rows are captured under the same mutation lock as deletion persistence. diff --git a/structure/config.md b/structure/config.md index 036a9b45af7..5b3301556a4 100644 --- a/structure/config.md +++ b/structure/config.md @@ -62,7 +62,7 @@ test seam and the mutation types remain re-exported through `src/config.ts`. Suc `src/config/paths.ts` owns `OPENCODEX_HOME` expansion and resolution, the config directory and `config.json` path. A relative home is resolved once per distinct raw environment value, so a later working-directory change cannot silently move the active installation. Config reads in this leaf and `src/config/load-degrade.ts` use NTFS ACLs on Windows and private POSIX modes elsewhere. Optional directory hardening follows directory symlinks/junctions but skips non-directory or unstatable targets before starting chmod or ACL work. Windows reads omit `chmod` before ACL hardening so repeated loads preserve successful object/freshness observations; external metadata changes still invalidate them. Directory flights keep their existing flush/reap lifecycle, and file/required-write failure policy is unchanged. `tests/windows/windows-config-read-acl-memo.test.ts` covers repeated loads, non-directory and directory-link behavior, external invalidation and POSIX permissions. -`withConfigDirHardeningSettled` in `src/config/paths.ts` admits synchronous mutation after the last unrelated await. Its registry includes the optional config flight, ancestors and retained timeout children; one existing ACL deadline bounds admission, including newly overlapping work. Expiry throws sanitized `WindowsSecretAclSettleTimeoutError` (`ETIMEDOUT`) without running the writer or dropping child cleanup ownership. The final empty-registry check and callback share one turn, with no await in the callback. Intercept/picker CA, token, cleanup, profile and snapshot phases re-enter after trust/discovery awaits. Config saves in `src/server/management/agent-settings-routes.ts` and `src/server/management/config-routes.ts`, Desktop intent/mode commits in `src/server/management/native-integration-routes.ts`, and gateway publication/cleanup in those routes and `src/cli/ensure-desired-integrations.ts` use the same admission. `src/cli/claude-desktop.ts` admits local profile/mode commits and the complete remote-store transaction again after hub download. Lifecycle/config locks are acquired inside admission; refresh re-read, mode/freshness checks, profile and marker publication share one callback. Live setting edits and their save/publication share the admitted synchronous callback; a refused admission leaves both live and durable state untouched. Ordered post-commit phases preserve applied-state warnings or settled compensation, including mode/fingerprint marker refusal and retained CLI intent after incomplete rollback. Synchronous leaf APIs retain their contracts. `tests/claude-integration/claude-config-root-writer-barrier.test.ts` holds parent flights and drives refusal deadlines without sleeps. Lexical/canonical aliases and uncertain identities drain conservatively; removal keeps its separate at-or-below scope and actual-exit requirement. +`withConfigDirHardeningSettled` in `src/config/paths.ts` admits synchronous mutation after the last unrelated await. Its registry includes the optional config flight, ancestors and retained timeout children; one existing ACL deadline bounds admission, including newly overlapping work. Expiry throws sanitized `WindowsSecretAclSettleTimeoutError` (`ETIMEDOUT`) without running the writer or dropping child cleanup ownership. The final empty-registry check and callback share one turn, with no await in the callback. Intercept/picker CA, token, cleanup, profile and snapshot phases re-enter after trust/discovery awaits. Config saves in `src/server/management/agent-settings-routes.ts` and `src/server/management/config-routes.ts`, Desktop intent/mode commits in `src/server/management/native-integration-routes.ts`, and gateway publication/cleanup in those routes and `src/cli/ensure-desired-integrations.ts` use the same admission. `src/cli/claude-desktop.ts` admits local profile/mode commits and the complete remote-store transaction again after hub download. Lifecycle/config locks are acquired inside admission; refresh re-read, mode/freshness checks, profile and marker publication share one callback. Live setting edits and their save/publication share the admitted synchronous callback; a refused admission leaves both live and durable state untouched. Ordered post-commit phases preserve applied-state warnings or settled compensation, including mode/fingerprint marker refusal and retained CLI intent after incomplete rollback. Synchronous leaf APIs retain their contracts. `tests/claude-integration/claude-config-root-writer-barrier.test.ts` holds parent flights and drives refusal deadlines without sleeps. Lexical/canonical aliases and uncertain identities drain conservatively; removal keeps its separate at-or-below scope and actual-exit requirement. Async local config commands (`src/cli/config-command.ts`, `src/cli/effort.ts`, `src/cli/models.ts`, `src/cli/provider.ts`) admit complete synchronous read/validate/save phases through `withConfigDirHardeningSettled`. Preliminary model selection and client-connection diagnostics use passive observations that cannot publish recovery copies. Final authority and recovery publication are re-read inside admission without starting optional hardening. `src/cli/config-mutation.ts` refuses invalid or unreadable authority, including array/non-object roots rejected before defaults repair, with `CliUsageError`; full-config commands retain fresh-install initialization when the file is absent, while set/unset retain the persisted mutation owner's missing-file refusal. Custom-model removal revalidates the confirmed row by identity and contents while retaining current siblings. Import reads its input and checks its target inside admission. Provider-add credential cleanup shares the synchronous post-save phase and retains its existing post-commit warning. Timeout leaves the mutation database and config untouched and follows existing CLI failure handling. `tests/windows/windows-config-root-async-writers.test.ts` covers held parent flights, deadline refusal, recovery copies, invalid authority and concurrent edits. `src/config/process-state.ts` derives `ocx.pid` and `runtime-port.json` from that resolved directory. It owns their byte-compatible writes, parsing, expected-PID filters, cheap liveness, full OCX command diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index d5b88fb31fa..0cc92f63148 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -32,7 +32,7 @@ Automatic activation retains its existing settings controls; dashboard quota que The companion settings contract in `src/companion/` persists menu-bar and widget preferences. `src/server/management/companion-routes.ts` exposes them and the usage timeline from `src/usage/timeline.ts`; its query and missing-measurement behavior follows [companion usage](companion.md). -Only `/api/advisor/settings` and the read-only `/api/advisor/activity` lazily load `advisor-routes.ts`, refreshing the config-scoped factory after a successful settings save; default requests stay outside [the optional Advisor subsystem](advisor.md). The Advisor page adds the adaptive policy/task allowance and a visible-page activity poll using the shared data-surface contract, and its model selector follows the [catalog selection contract](advisor.md#dashboard-model-selection), preserving unknown saved selectors and catalog failure feedback. Activity metadata retention and reasoning evidence follow that subsystem's contract. +Only `/api/advisor/settings` and the read-only `/api/advisor/activity` lazily load `advisor-routes.ts`, refreshing the config-scoped factory after a successful settings save; default requests stay outside [the optional Advisor subsystem](advisor.md). The Advisor page adds the adaptive policy/task allowance and a visible-page activity poll using the shared data-surface contract, and its model selector follows the [catalog selection contract](advisor.md#dashboard-model-selection), preserving unknown saved selectors and catalog failure feedback. Activity metadata retention and reasoning evidence follow that subsystem's contract. Advisor settings in `src/server/management/advisor-routes.ts` apply the [config admission contract](config.md) before live edits, save and activation. The route arms a detached merge baseline on its fresh disk candidate, patches and passes that same object to the saver, then adopts the saved Advisor subtree and only its live merge baseline, retaining known absence, and captures its response inside admission. Concurrent disk edits, including new fields, are reconciled before persistence. Fallback diagnostics, file diagnostics carrying an error, or a refused deadline returns the existing `write_failed` response; successful response shapes are unchanged. Native reasoning controls follow [the shared retention contract](transports/native-reasoning-retention.md#configuration-and-management). Native result continuations and function-result injection follow [the mode-specific result and control contract](transports/streaming-health.md#experimental-native-function-result-injection); this surface does not infer upstream support or alter its defaults. Explicit Codex CLI installation observation is a local CLI surface, not a management API or GUI update permission. See the [read-only observation contract](runtime.md#explicit-codex-cli-installation-observation). @@ -57,7 +57,7 @@ linked sources after the quota await to reject revoked or rotated captures. ## Compact desktop usage -The standalone `/#/tray` GUI route presents local usage and account limits without the +The standalone `/#/tray` GUI route presents local usage and account limits without full dashboard navigation. It reuses the existing API session and fetch wrapper; it has no Tauri IPC capability. Companion settings control its sections and chart. Account limits use account-level management reads rather than attributing aggregate provider diff --git a/structure/providers/google.md b/structure/providers/google.md index 117034e5656..a8064a70050 100644 --- a/structure/providers/google.md +++ b/structure/providers/google.md @@ -66,6 +66,7 @@ The management helper also owns the locked provider-deletion callback, preservin selection, custom-model/context-cap removal, persistence and subsequent marker cleanup order. `src/providers/gcp-credential-cleanup.ts` cleans removed markers after config persistence, including CLI `provider add --force` overwrites, under the config mutation lock, retaining credentials still referenced by any persisted provider. CLI overwrite captures both live and latest durable rows under that lock. Management overwrite reuses `snapshotProviderGcpCredentialMarkers`; CLI and management deletion use `commitProviderGcpCredentialRemoval` to capture live and valid durable markers and persist removal without releasing the lock. Failed saves and refused CLI additions do not clean credentials. +CLI provider add and removal keep synchronous post-save cleanup inside the same config-root ACL admission as publication; cleanup failures retain the existing warning that configuration removal remains committed. Reference retention compares OS account identities across both `gcp-sa:` and `keychain:` spellings. `src/providers/key-store.ts` leaves GCP markers stored during ordinary keychain store/restore and refuses to restore GCP JSON through a keychain alias; the shared classification diff --git a/structure/runtime.md b/structure/runtime.md index 5c149eb996f..fda2fa8e52e 100644 --- a/structure/runtime.md +++ b/structure/runtime.md @@ -167,7 +167,7 @@ their own files. `src/grok/inject.ts` holds the shared canonical config-write lo ## Lifecycle -Cross-home discovery in `src/cli/cross-home-owner.ts` checks concrete loopback address availability through `src/lib/loopback-family.ts` (re-exported by `src/cli/loopback-family.ts`) before probing each family. A synchronous ephemeral bind is immediately closed; only address/family-unavailable errors exclude that address. This prevents a disabled IPv6 loopback's off-host `::1` connect timeout from blocking startup. Other bind errors retain discovery, and a timeout on an available or unknown family still produces an indeterminate owner verdict. Live owners on either available family retain identity and attestation checks. +Cross-home discovery in `src/cli/cross-home-owner.ts` checks concrete loopback address availability through `src/lib/loopback-family.ts` (re-exported by `src/cli/loopback-family.ts`) before probing each family. A synchronous ephemeral bind is immediately closed; only address/family-unavailable errors exclude that address. This prevents a disabled IPv6 loopback's off-host `::1` connect timeout from blocking startup. Other bind errors retain discovery, and a timeout on an available or unknown family still produces an indeterminate owner verdict. Live owners on either available family retain identity and attestation checks. Local configuration writers follow the [config admission contract](config.md): eligible async CLI phases wait before acquiring synchronous persistence locks, and derive their candidate without starting optional read-path hardening. Startup catalog sync and native restore apply the [retired-native policy](catalog.md#shared-catalog). Foreground exit cleanup in `src/cli/index.ts` delegates restoration through the existing sibling/recycle teardown gates; approved external URLs follow [custom-provider consent](codex-provider-routing.md), while unapproved provider configuration and history remain unchanged. Guarded tests isolate the owner registry used by cross-home discovery beneath their selected home, including children without a registry override; production retains the shared locator. See [test state isolation](ops/test-sandbox-cleanup.md). `src/server/background-lifecycle.ts` registers low-quota protection for each live server with the OS desktop sink resolver, flushes pending saves before releasing its owner, binds its own event ledger to management requests, and unregisters it on failed startup or stop; with no pending save, owner release remains synchronous. The protection writer loads config persistence only when a threshold schedules a save; see [configuration](config.md#codex-pool-low-quota-protection). Codex quota processing has shared and Reserve scopes; retired model evidence is suppressed as diff --git a/tests/cli/cli-dispatch.test.ts b/tests/cli/cli-dispatch.test.ts index 6e76cbc7e08..3a36a246f2d 100644 --- a/tests/cli/cli-dispatch.test.ts +++ b/tests/cli/cli-dispatch.test.ts @@ -36,6 +36,25 @@ import type { OcxConfig } from "../../src/types"; * runners, which these tests never invoke, so an empty object is enough. */ const fakeDeps = {} as unknown as CliDispatchDeps; +test.each(["models", "provider"])("%s dispatch retains normal handler exit codes", async command => { + const handler = command === "models" + ? spyOn(await import("../../src/cli/models"), "handleModels") + : spyOn(await import("../../src/cli/provider"), "handleProviderCommand"); + const previousExit = process.exitCode; + try { + const args = [command, "list"]; + for (const code of [0, 1, 2, 4, 5, 79]) { + handler.mockImplementation(async () => { process.exitCode = code; }); + process.exitCode = 99; + expect(await dispatchCommand({ kind: "command", command, args }, { ...fakeDeps, args })).toBe(code); + expect(process.exitCode).toBe(code); + } + expect(handler).toHaveBeenCalledTimes(6); + expect(handler).toHaveBeenLastCalledWith(["list"]); + // Bun keeps the last code when exitCode is set to undefined, so restore an explicit 0. + } finally { handler.mockRestore(); process.exitCode = previousExit ?? 0; } +}); + describe("CLI dispatch command coverage", () => { test("every non-hidden registry command is dispatchable", () => { const aliasResolved = new Set([...DISPATCH_COMMANDS, ...DISPATCH_ALIASES.keys()]); diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 01626d51d39..dd16405bfea 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1,4 +1,5 @@ { "update-reclaim-authorizer.test.ts": "update", "update-capability-policy.test.ts": "update", "update-service-home.test.ts": "update", "update-service-home-policy.test.ts": "update", "update-home-recovery.test.ts": "update", "update-recovery-authority.test.ts": "update", "update-recovery-boundaries.test.ts": "update", "update-home-launcher.test.ts": "update", "update-home-worker.test.ts": "update", "update-mutation-boundaries.test.ts": "update", "update-mutation-guard.test.ts": "update", "update-guarded-ops.test.ts": "update", "update-effect-authority.test.ts": "update", "update-windows-wrapper-authority.test.ts": "update", "update-mutation-scanner.test.ts": "update", "refreshed-upstream-deadlines.test.ts": "responses", "local-upstream-deadlines.test.ts": "responses", "service-desktop-recovery.test.ts": "service", "provider-input-modalities.test.ts": "providers", "turn-membership.test.ts": "responses", "turn-membership-ingress.test.ts": "responses", "workbuddy-quota.test.ts": "providers", "workbuddy-login.test.ts": "oauth", "workbuddy-adapter.test.ts": "adapters", "workbuddy-provider.test.ts": "providers", "link-tunnel-history.test.ts": "clients", "link-readiness.test.ts": "clients", "link-diagnostics.test.ts": "clients", "combo-description.test.ts": "routing", "combo-description-catalog.test.ts": "codex-integration", "catalog-auto-review-override.test.ts": "codex-integration", "management-auto-review-settings.test.ts": "server", "management-auto-review-auth.test.ts": "server", "prefer-model-alias-ids.test.ts": "server", "responses-mimo-envelope.test.ts": "responses", "responses-call-id-linkage.test.ts": "responses", "responses-phase-inference.test.ts": "responses", + "windows-config-root-async-writers.test.ts": "windows", "t3code-client.test.ts": "clients", "t3code-client-lifecycle.test.ts": "clients", "fix-jsonc-cr-comments.test.ts": "lib", diff --git a/tests/windows/windows-config-root-async-writers.test.ts b/tests/windows/windows-config-root-async-writers.test.ts new file mode 100644 index 00000000000..8db6b2dbde5 --- /dev/null +++ b/tests/windows/windows-config-root-async-writers.test.ts @@ -0,0 +1,421 @@ +import { afterEach, beforeEach, expect, spyOn, test } from "bun:test"; +import { existsSync, mkdtempSync, readFileSync, readdirSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, sep } from "node:path"; +import { handleConfigCommand } from "../../src/cli/config-command"; +import { handleEffortCommand } from "../../src/cli/effort"; +import { handleModels } from "../../src/cli/models"; +import { handleProviderCommand } from "../../src/cli/provider"; +import { handleAdvisorRoutes } from "../../src/server/management/advisor-routes"; +import * as advisorActivation from "../../src/lib/advisor-activation"; +import type { OcxConfig } from "../../src/types"; +import * as paths from "../../src/config/paths"; +import * as configModule from "../../src/config"; +import { armClaudeCodeBaseline, claudeCodeBaselineArmed } from "../../src/config/live-reconcile"; +import { CliUsageError } from "../../src/cli/runtime-api"; +import { readCliConfigForMutation } from "../../src/cli/config-mutation"; +import { dispatchCommand, type CliDispatchDeps } from "../../src/cli/dispatch"; +import { advisorRunnable, resolveAdvisorSettings } from "../../src/advisor/settings"; +import { setProviderKeychainEntryFactoryForTests } from "../../src/providers/api-key-resolve"; +import { resetHardenedStateForTests, setAsyncIcaclsBeltSchedulerForTests, setAsyncIcaclsRunnerForTests, + setIcaclsRunnerForTests, setNowForTests, setPlatformForTests, windowsSecretAclReapPendingForPath, + type IcaclsResult } from "../../src/lib/windows-secret-acl"; +import { setSyntheticWindowsPrincipalForTests } from "../../src/lib/windows-user-principal"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; + +const success: IcaclsResult = { success: true, exitCode: 0, timedOut: false, stdout: "" }; +const turn = () => new Promise(done => setImmediate(done)); +function deferred() { + let resolve!: () => void; + const promise = new Promise(done => { resolve = done; }); + return { promise, resolve }; +} +const keys = ["OPENCODEX_HOME", "OPENCODEX_ACL_TIMEOUT_MS", "OPENCODEX_ACL_VERIFY_EXISTING"] as const; +let root: string, original: string; +let previous: (string | undefined)[]; +let restores: Array<() => void>; +let advisorConfig: OcxConfig | undefined; +const fixture = () => ({ port: 10100, defaultProvider: "mock", effortCap: "low", providers: { + mock: { adapter: "openai-chat", baseUrl: "https://mock.example/v1", models: ["test-model"] }, + second: { adapter: "openai-chat", baseUrl: "https://second.example/v1", models: ["second-model"] }, +}, customModels: [{ id: "fixture-model", provider: "mock", modelId: "custom-before", addedAt: "2026-01-01T00:00:00.000Z" }] }); +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "ocx-cli-writer-admission-")); + previous = keys.map(key => process.env[key]); + process.env.OPENCODEX_HOME = root; process.env.OPENCODEX_ACL_TIMEOUT_MS = "1000"; + delete process.env.OPENCODEX_ACL_VERIFY_EXISTING; + original = JSON.stringify(fixture()); + writeFileSync(join(root, "config.json"), original); + writeFileSync(join(root, "input.json"), JSON.stringify({ ...fixture(), effortCap: "high" })); + restores = []; + advisorConfig = undefined; + for (const method of ["log", "error", "warn"] as const) { + const hook = spyOn(console, method).mockImplementation(() => {}); restores.push(() => hook.mockRestore()); + } + resetHardenedStateForTests(); setPlatformForTests("win32"); + setSyntheticWindowsPrincipalForTests("*S-1-5-21-1-2-3-1001"); setIcaclsRunnerForTests(() => success); +}); +afterEach(async () => { + await paths.flushConfigDirHardeningAndReaps(root); + for (const restore of restores.reverse()) restore(); + setAsyncIcaclsRunnerForTests(null); setAsyncIcaclsBeltSchedulerForTests(null); + setNowForTests(null); setIcaclsRunnerForTests(null); setPlatformForTests(null); + setSyntheticWindowsPrincipalForTests(null); resetHardenedStateForTests(); + keys.forEach((key, index) => { + if (previous[index] === undefined) delete process.env[key]; else process.env[key] = previous[index]; + }); + removeTreeWithRetry(root); +}); +function clock() { + let now = 0; + const timers = new Map<() => void, number>(); + setNowForTests(() => now); + setAsyncIcaclsBeltSchedulerForTests((callback, milliseconds) => { + timers.set(callback, now + milliseconds); return () => { timers.delete(callback); }; + }); + return { advance(milliseconds: number) { + now += milliseconds; + for (const [callback, deadline] of timers) if (deadline <= now) { timers.delete(callback); callback(); } + } }; +} +function holdParent() { + const entered = deferred(), released = deferred(), reached = deferred(); + let held = true; + const descendants: string[] = []; + setAsyncIcaclsRunnerForTests(async () => { entered.resolve(); await released.promise; return success; }); + setIcaclsRunnerForTests(args => { + if (held && args[0]?.startsWith(root + sep)) { descendants.push(args[0]); reached.resolve(); } + return success; + }); + const barrier = paths.withConfigDirHardeningSettled; + const hook = spyOn(paths, "withConfigDirHardeningSettled").mockImplementation((dir: string, mutate: () => T) => { + const result = barrier(dir, mutate); reached.resolve(); return result; + }); + restores.push(() => hook.mockRestore()); paths.hardenConfigDir(); + return { entered: entered.promise, reached: reached.promise, arrive: reached.resolve, descendants, + release() { held = false; released.resolve(); } }; +} +const operations = ["set", "unset", "import", "effort", "model-add", "model-remove", "provider-add", "provider-remove", "provider-default", "advisor"] as const; +function requestAdvisor(liveConfig = fixture() as OcxConfig, patch: Record = { enabled: true }) { + const url = new URL("http://localhost/api/advisor/settings"); + advisorConfig = liveConfig; + if (!claudeCodeBaselineArmed(advisorConfig)) armClaudeCodeBaseline(advisorConfig); + return handleAdvisorRoutes({ url, req: new Request(url, { method: "PUT", body: JSON.stringify(patch) }), + config: advisorConfig, deps: {}, version: "test", trustedLoopbackIngress: false, guiSessionIssuance: null, + convergeCodexCatalog: async () => ({ status: "skipped", reason: "not-requested", retryable: false }), + syncClaudeAgentDefsBestEffort: async () => {}, + }); +} +function run(operation: typeof operations[number]) { + if (operation === "advisor") { + return requestAdvisor().then(async response => { + if (response?.status === 500) { expect(await response.json()).toEqual({ error: { code: "write_failed", message: "The configuration could not be saved." } }); return 1; } + expect(response?.status).toBe(200); return 0; + }); + } + if (operation === "effort") return handleEffortCommand(["high", "--json"], { findLiveProxy: async () => null }); + if (operation.startsWith("model-") || operation.startsWith("provider-")) { + const work = operation === "model-add" ? handleModels(["add", "mock", "custom-after", "--json"], { findLiveProxy: async () => null }) + : operation === "model-remove" ? handleModels(["remove", "fixture-model", "--yes", "--json"], { findLiveProxy: async () => null }) + : handleProviderCommand(operation === "provider-add" ? ["add", "third", "--adapter", "openai-chat", "--base-url", "https://third.example/v1", "--json"] + : operation === "provider-remove" ? ["remove", "second", "--json"] : ["set-default", "second", "--json"]); + return work.then(() => 0, error => { if (error.code !== "ETIMEDOUT") throw error; expect(error.code).toBe("ETIMEDOUT"); return 1; }); + } + return handleConfigCommand(operation === "import" ? ["import", join(root, "input.json"), "--yes", "--json"] + : operation === "set" ? ["set", "effortCap", "high", "--json"] : ["unset", "effortCap", "--json"]); +} +for (const operation of operations) { + test.each([false, true])(`${operation} waits for parent ACL admission (timeout=%s)`, async timeout => { + const time = clock(), parent = holdParent(); await parent.entered; + let settled = false; + const work = run(operation).then(value => { settled = true; return value; }); + void work.finally(parent.arrive); + try { + await parent.reached; await turn(); + expect(parent.descendants).toEqual([]); expect(settled).toBe(false); + if (operation === "advisor") expect(advisorConfig?.advisor).toBeUndefined(); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(original); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); + if (timeout) { + time.advance(999); await turn(); expect(settled).toBe(false); time.advance(1); + expect(await work).toBe(1); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(original); + expect(parent.descendants).toEqual([]); + if (operation === "advisor") expect(advisorConfig?.advisor).toBeUndefined(); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); + time.advance(100_000); await paths.flushConfigDirHardening(root); + expect(windowsSecretAclReapPendingForPath(root)).toBe(true); + } else if (operation === "import") { + writeFileSync(join(root, "input.json"), JSON.stringify({ ...fixture(), effortCap: "high", subagentEffortCap: "medium" })); + } else { + writeFileSync(join(root, "config.json"), JSON.stringify({ ...fixture(), subagentEffortCap: "medium" })); + } + } finally { parent.release(); await work; } + if (!timeout) { + expect(await work).toBe(0); + const saved = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + if (["set", "unset", "import", "effort"].includes(operation)) expect(saved.effortCap).toBe(operation === "unset" ? undefined : "high"); + if (operation === "model-add") expect(saved.customModels.map((row: { modelId: string }) => row.modelId)).toEqual(["custom-before", "custom-after"]); + if (operation === "model-remove") expect(saved.customModels).toBeUndefined(); + if (operation === "provider-add") expect(saved.providers.third.baseUrl).toBe("https://third.example/v1"); + if (operation === "provider-remove") expect(saved.providers.second).toBeUndefined(); + if (operation === "provider-default") expect(saved.defaultProvider).toBe("second"); + if (operation === "advisor") expect(saved.advisor.enabled).toBe(true); + expect(saved.subagentEffortCap).toBe("medium"); + } + }); +} + +test.each(["changed", "removed"])("custom-model removal refuses a %s confirmed row after admission", async change => { + const exit = new Error("fixture CLI exit"); + const hook = spyOn(process, "exit").mockImplementation(() => { throw exit; }); + restores.push(() => hook.mockRestore()); + const parent = holdParent(); await parent.entered; + const work = handleModels(["remove", "fixture-model", "--yes", "--json"], { findLiveProxy: async () => null }) + .then(() => undefined, error => error); + try { + await parent.reached; await turn(); + expect(parent.descendants).toEqual([]); + const fresh = fixture(); + if (change === "changed") fresh.customModels[0]!.modelId = "concurrent-edit"; + else fresh.customModels = []; + const bytes = JSON.stringify(fresh); + writeFileSync(join(root, "config.json"), bytes); + parent.release(); + expect(await work).toBe(exit); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(bytes); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); + } finally { parent.release(); await work; } +}); + +test("Advisor response snapshots the admitted phase before queued live edits", async () => { + const hook = spyOn(advisorActivation, "activateAdvisor").mockImplementation(config => { + queueMicrotask(() => { config.advisor!.enabled = false; }); + }); + restores.push(() => hook.mockRestore()); + const response = await requestAdvisor(); + expect(response?.status).toBe(200); + expect((await response!.json()).settings.enabled).toBe(true); + expect(advisorConfig?.advisor?.enabled).toBe(false); +}); + +const cliOperations = operations.filter(operation => operation !== "advisor"); +for (const operation of cliOperations) { + test.each([false, true])(`${operation} keeps retired-provider recovery inside admission (timeout=%s)`, async timeout => { + original = JSON.stringify({ ...fixture(), providers: { ...fixture().providers, + "opencode-free": { adapter: "openai-chat", baseUrl: "https://retired.example/v1", apiKey: "synthetic-recovery-secret" }, + } }); + writeFileSync(join(root, "config.json"), original); + const time = clock(), parent = holdParent(); await parent.entered; + const work = run(operation); + void work.then(parent.arrive, parent.arrive); + try { + await parent.reached; await turn(); + expect(parent.descendants).toEqual([]); + expect(readdirSync(root).filter(name => name.includes("pre-opencode-free-removal"))).toEqual([]); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(original); + if (timeout) { + time.advance(1000); + expect(await work).toBe(1); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(original); + expect(parent.descendants).toEqual([]); + expect(readdirSync(root).filter(name => name.includes("pre-opencode-free-removal"))).toEqual([]); + } + } finally { parent.release(); await work; } + if (!timeout) { + expect(await work).toBe(0); + const backups = readdirSync(root).filter(name => name.includes("pre-opencode-free-removal") && name.endsWith(".bak")); + expect(backups.length).toBeGreaterThan(0); + expect(backups.some(name => readFileSync(join(root, name), "utf8") === original)).toBe(true); + } + }); +} + +test("GCP cleanup completes before a parent flight queued after provider publication", async () => { + const config = fixture(); + Object.assign(config.providers.mock, { apiKey: "gcp-sa:writer-fixture" }); + writeFileSync(join(root, "config.json"), JSON.stringify(config)); + let deletions = 0; + setProviderKeychainEntryFactoryForTests(() => ({ getPassword: () => null, setPassword: () => {}, deletePassword: () => { deletions++; return true; } })); + restores.push(() => setProviderKeychainEntryFactoryForTests(null)); + const released = deferred(), entered = deferred(); + let held = false; + const descendants: string[] = []; + setAsyncIcaclsRunnerForTests(async () => { entered.resolve(); await released.promise; return success; }); + setIcaclsRunnerForTests(args => { + if (held && args[0]?.startsWith(root + sep)) descendants.push(args[0]); + return success; + }); + const barrier = paths.withConfigDirHardeningSettled; + const hook = spyOn(paths, "withConfigDirHardeningSettled").mockImplementation((dir: string, mutate: () => T) => barrier(dir, () => { + const result = mutate(); + queueMicrotask(() => { held = true; paths.hardenConfigDir(); }); + return result; + })); + restores.push(() => hook.mockRestore()); + const work = handleProviderCommand(["add", "mock", "--force", "--adapter", "openai-chat", "--base-url", "https://mock.example/v1", "--json"]); + try { + await entered.promise; await turn(); + expect(descendants).toEqual([]); + expect(deletions).toBe(1); + expect(existsSync(join(root, "local-credentials.json"))).toBe(true); + } finally { held = false; released.resolve(); await work; } +}); + +for (const operation of cliOperations) { + test.each([false, true])(`${operation} refuses malformed authority without overwriting bytes (late=%s)`, async late => { + const invalid = '{ "providers": { "operator-data": "preserve-me"'; + const exit = spyOn(process, "exit").mockImplementation(() => { throw new Error("unexpected CLI exit"); }); + restores.push(() => exit.mockRestore()); + const parent = late ? holdParent() : undefined; + if (parent) await parent.entered; + else writeFileSync(join(root, "config.json"), invalid); + const work = run(operation).then(value => value, error => error); + if (parent) { + await parent.reached; await turn(); + expect(parent.descendants).toEqual([]); + writeFileSync(join(root, "config.json"), invalid); + parent.release(); + } + try { + const outcome = await work; + if (["set", "unset", "import", "effort"].includes(operation)) expect(outcome).toBe(2); + else expect(outcome).toBeInstanceOf(CliUsageError); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(invalid); + expect(readdirSync(root).filter(name => name.startsWith("config.json.invalid-"))).toEqual([]); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); + expect(exit).not.toHaveBeenCalled(); + } finally { parent?.release(); await work; } + }); +} + +test("Advisor preserves a disk edit between admitted read and saver lock", async () => { + const live = configModule.readConfigDiagnostics().config; + armClaudeCodeBaseline(live); + const persist = configModule.saveConfigPreservingClaudeCode; + let received: OcxConfig | undefined; + const hook = spyOn(configModule, "saveConfigPreservingClaudeCode").mockImplementation(candidate => { + received = candidate; + const disk = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + disk.subagentEffortCap = "medium"; + disk.advisor = { timeoutMs: 4321 }; + writeFileSync(join(root, "config.json"), JSON.stringify(disk)); + persist(candidate); + }); + restores.push(() => hook.mockRestore()); + const response = await requestAdvisor(live); + expect(response?.status).toBe(200); + const saved = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + expect(saved.subagentEffortCap).toBe("medium"); + expect(saved.advisor).toEqual({ enabled: true, timeoutMs: 4321 }); + expect((await response!.json()).settings.timeoutMs).toBe(4321); + expect(received).toBeDefined(); + expect(claudeCodeBaselineArmed(received!)).toBe(true); +}); + +// R2 probes: the real saver must recognize a committed grant as baseline, not pending intent. +test.each(["consent", "enabled", "disabled", "subtree"])("Advisor keeps disk revocation (%s) on a later live save", async revoke => { + const hook = spyOn(advisorActivation, "activateAdvisor").mockImplementation(() => {}); + restores.push(() => hook.mockRestore()); + const initial = { ...fixture(), advisor: { model: "mock/test-model" } }; + writeFileSync(join(root, "config.json"), JSON.stringify(initial)); + const live = configModule.readConfigDiagnostics().config; + expect((await requestAdvisor(live, { enabled: true, contextSharingConsent: "v1" }))?.status).toBe(200); + const disk = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + expect(disk.advisor.contextSharingConsent).toBe("v1"); + if (revoke === "consent") delete disk.advisor.contextSharingConsent; + else if (revoke === "enabled") delete disk.advisor.enabled; + else if (revoke === "disabled") disk.advisor.enabled = false; + else delete disk.advisor; + writeFileSync(join(root, "config.json"), JSON.stringify(disk)); + live.effortCap = "high"; + configModule.saveConfigPreservingClaudeCode(live); + const saved = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + expect(saved.advisor).toEqual(disk.advisor); + expect(saved.effortCap).toBe("high"); + expect(advisorRunnable(resolveAdvisorSettings(live))).toBe(false); +}); + +test("Advisor adoption retains unrelated pending live intent and handles reset absence", async () => { + const hook = spyOn(advisorActivation, "activateAdvisor").mockImplementation(() => {}); + restores.push(() => hook.mockRestore()); + const live = configModule.readConfigDiagnostics().config; + armClaudeCodeBaseline(live); + live.effortCap = "high"; + expect((await requestAdvisor(live, { reset: true }))?.status).toBe(200); + expect(live.advisor).toBeUndefined(); + expect(live.effortCap).toBe("high"); + const disk = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + expect(disk.effortCap).toBe("low"); + disk.effortCap = "medium"; + disk.advisor = { enabled: false, timeoutMs: 4321 }; + writeFileSync(join(root, "config.json"), JSON.stringify(disk)); + configModule.saveConfigPreservingClaudeCode(live); + const saved = JSON.parse(readFileSync(join(root, "config.json"), "utf8")); + expect(saved.effortCap).toBe("high"); + expect(saved.advisor).toEqual(disk.advisor); +}); + +const nonObjectRoots = ["[]", '[{"operator-original":"synthetic-data-that-must-survive"}]', "null", "true", "42", '"operator-original"']; +test.each(nonObjectRoots)("mutation reader refuses non-object authority %s", raw => { + writeFileSync(join(root, "config.json"), raw); + expect(() => readCliConfigForMutation({ passive: true })).toThrow(CliUsageError); + expect(() => readCliConfigForMutation()).toThrow(CliUsageError); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(raw); + expect(readdirSync(root).filter(name => name.startsWith("config.json.invalid-"))).toEqual([]); +}); +for (const operation of cliOperations) { + test.each(nonObjectRoots)(`${operation} refuses non-object authority %s without changing bytes`, async raw => { + writeFileSync(join(root, "config.json"), raw); + const exit = spyOn(process, "exit").mockImplementation(() => { throw new Error("unexpected CLI exit"); }); + restores.push(() => exit.mockRestore()); + const work = operation === "model-add" + ? handleModels(["add", "openai", "review-array-model", "--json"], { findLiveProxy: async () => null }) + : run(operation); + const outcome = await work.then(value => value, error => error); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(raw); + expect(readdirSync(root).filter(name => name.startsWith("config.json.invalid-"))).toEqual([]); + if (["set", "unset", "import", "effort"].includes(operation)) expect(outcome).toBe(2); + else expect(outcome).toBeInstanceOf(CliUsageError); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); + expect(exit).not.toHaveBeenCalled(); + }); +} + +test("Advisor refuses file diagnostics with an Anthropic provenance error", async () => { + const live = configModule.readConfigDiagnostics().config; + const raw = JSON.stringify({ ...fixture(), providers: { ...fixture().providers, anthropic: { + adapter: "anthropic", authMode: "oauth", baseUrl: "https://api.anthropic.com", + anthropicOAuthInstance: "anthropic2", + } } }); + writeFileSync(join(root, "config.json"), raw); + const diagnostics = configModule.readConfigDiagnostics({ passive: true }); + expect(diagnostics.source).toBe("file"); + expect(diagnostics.error).toContain("anthropicOAuthInstance"); + const activation = spyOn(advisorActivation, "activateAdvisor").mockImplementation(() => {}); + restores.push(() => activation.mockRestore()); + const response = await requestAdvisor(live); + expect(response?.status).toBe(500); + expect(await response!.json()).toEqual({ error: { code: "write_failed", message: "The configuration could not be saved." } }); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(raw); + expect(live.advisor).toBeUndefined(); + expect(activation).not.toHaveBeenCalled(); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); +}); + +test.each(["models", "provider"])("%s dispatch reports invalid authority with standard exit two", async command => { + const raw = '{"providers":'; + writeFileSync(join(root, "config.json"), raw); + const args = command === "models" ? [command, "add", "mock", "new-model", "--json"] + : [command, "add", "third", "--adapter", "openai-chat", "--base-url", "https://third.example/v1", "--json"]; + const error = spyOn(console, "error").mockImplementation(() => {}); + restores.push(() => error.mockRestore()); + const previousExit = process.exitCode; + try { + process.exitCode = 79; + expect(await dispatchCommand({ kind: "command", command, args }, { args } as CliDispatchDeps)).toBe(2); + expect(error.mock.calls).toEqual([["Error: config.json is invalid or unreadable; run ocx config validate and repair it before retrying. Nothing was saved."]]); + expect(readFileSync(join(root, "config.json"), "utf8")).toBe(raw); + expect(existsSync(join(root, "config-mutation.sqlite"))).toBe(false); + } finally { process.exitCode = previousExit; } +});