diff --git a/docs-site/src/content/docs/guides/codex-integration.md b/docs-site/src/content/docs/guides/codex-integration.md index 3afb7af916f..529ce12decc 100644 --- a/docs-site/src/content/docs/guides/codex-integration.md +++ b/docs-site/src/content/docs/guides/codex-integration.md @@ -668,6 +668,13 @@ Codex history metadata restoration. Tools that manage a custom provider often ta provider id; replacing the active id can make those intact sessions disappear from Codex's history view. The same protection applies to an external provider selected by a legacy root profile. +While an external provider owns `config.toml`, the settings report that +`GET /api/settings` and `ocx system settings` return describes the Desktop authless and +client-compaction switches — and the Codex sign-in requirement — as controlled by that +provider instead of showing the effective state OpenCodex would produce. Flipping either +switch still stores the preference, but `config.toml` is not rewritten; the stored value +takes effect if you switch Codex back to a provider OpenCodex manages and rerun `ocx start`. + Keep one tool as the owner of Codex provider configuration. To use OpenCodex behind an existing provider manager, point that provider at `http://127.0.0.1:10100/v1` with Responses passthrough (`wire_api = "responses"` in Codex TOML), not Chat Completions translation. When proxy API auth is diff --git a/src/cli/system-command.ts b/src/cli/system-command.ts index 4e08fcbc49c..f18a0351ea7 100644 --- a/src/cli/system-command.ts +++ b/src/cli/system-command.ts @@ -60,6 +60,7 @@ function desktopSwitchApplyReason(reason: unknown): string { if (reason === "not_requested") return "no desktop switch rewrite was requested"; if (reason === "proxy_not_running") return "the proxy is not running"; if (reason === "integration_disabled") return "Codex integration is disabled"; + if (reason === "external_provider") return "an external model provider owns config.toml"; if (reason === "write_lock_busy") return "the Codex config write lock is busy"; if (reason === "injection_refused") return "Codex config injection was refused"; return "the rewrite could not be completed"; @@ -76,8 +77,13 @@ function settingsUpdateLines( const lines: string[] = []; const appendSwitch = (key: string, label: string): boolean => { const state = recordValue(switches[key]); - if (!state || typeof state.stored !== "boolean" || typeof state.effective !== "boolean") return false; + if (!state || typeof state.stored !== "boolean" + || (typeof state.effective !== "boolean" && state.effective !== null)) return false; lines.push(`${label}: stored ${state.stored ? "on" : "off"}.`); + if (state.effective === null) { + lines.push(`${label}: effective state is controlled by the external model provider.`); + return true; + } // The effective value is always stated, even when it matches. Printing it only on a // mismatch would make silence ambiguous — the reader could not tell "the stored value is // in force" from "this build does not report effective state", and that ambiguity is a @@ -104,7 +110,10 @@ function settingsUpdateLines( lines.push("Codex config: ~/.codex/config.toml was rewritten."); } else { const detail = typeof apply.detail === "string" && apply.detail.length > 0 ? ` Details: ${apply.detail}` : ""; - lines.push(`Codex config: ~/.codex/config.toml was not rewritten because ${desktopSwitchApplyReason(apply.reason)}.${detail} Run 'ocx sync' to apply the stored settings.`); + const retry = apply.reason === "external_provider" + ? "" + : " Run 'ocx sync' to apply the stored settings."; + lines.push(`Codex config: ~/.codex/config.toml was not rewritten because ${desktopSwitchApplyReason(apply.reason)}.${detail}${retry}`); } lines.push(`Auth source: ${authSource.summary}`); return lines; diff --git a/src/codex/desktop-switches.ts b/src/codex/desktop-switches.ts index 4e8126c075d..7bb6d53c258 100644 --- a/src/codex/desktop-switches.ts +++ b/src/codex/desktop-switches.ts @@ -1,5 +1,6 @@ import type { OcxConfig } from "../types"; import { shouldSyncCodexOnStart } from "./desired-state"; +import { tomlString } from "./paths"; import { isEffectiveCodexClientCompaction, isEffectiveCodexDesktopAuthless, @@ -11,7 +12,7 @@ export type CodexDesktopSwitchInertReason = export interface CodexDesktopSwitchState { stored: boolean; - effective: boolean; + effective: boolean | null; inertReason?: CodexDesktopSwitchInertReason; } @@ -19,6 +20,7 @@ export type CodexDesktopSwitchApplyReason = | "not_requested" | "proxy_not_running" | "integration_disabled" + | "external_provider" | "write_lock_busy" | "injection_refused"; @@ -35,7 +37,7 @@ export interface CodexDesktopSwitchReport { codexDesktopAuthless: CodexDesktopSwitchState; codexClientCompaction: CodexDesktopSwitchState; apply: CodexDesktopSwitchApply; - authSource: { presentsCodexAccount: boolean; summary: string }; + authSource: { presentsCodexAccount: boolean | null; summary: string }; } type DesktopSwitchConfig = Pick< @@ -50,9 +52,10 @@ type DesktopSwitchConfig = Pick< function describeSwitch( stored: boolean, - effective: boolean, + effective: boolean | null, config: Pick, ): CodexDesktopSwitchState { + if (effective === null) return { stored, effective }; if (!stored || effective) return { stored, effective }; return { stored, @@ -68,15 +71,21 @@ export function describeCodexDesktopSwitches( apply: CodexDesktopSwitchApply, ): CodexDesktopSwitchReport { const authlessStored = config.codexDesktopAuthless === true; - const authlessEffective = isEffectiveCodexDesktopAuthless(config); + const externallyOwned = !apply.applied && apply.reason === "external_provider"; + const authlessEffective = externallyOwned ? null : isEffectiveCodexDesktopAuthless(config); const compactionStored = config.codexClientCompaction === true; - const compactionEffective = isEffectiveCodexClientCompaction(config); + const compactionEffective = externallyOwned ? null : isEffectiveCodexClientCompaction(config); return { codexDesktopAuthless: describeSwitch(authlessStored, authlessEffective, config), codexClientCompaction: describeSwitch(compactionStored, compactionEffective, config), apply, - authSource: authlessEffective + authSource: externallyOwned + ? { + presentsCodexAccount: null, + summary: "An external model provider owns Codex sign-in behavior; its account requirement was not changed.", + } + : authlessEffective ? { presentsCodexAccount: false, summary: "The Codex app will not require its own account sign-in.", @@ -88,6 +97,26 @@ export function describeCodexDesktopSwitches( }; } +/** + * The apply record for a report that attempted no rewrite. `not_requested` alone would have + * the report claiming OpenCodex's stored-versus-effective state as live, so the read path + * consults the same ownership predicate the injector does and reports external ownership + * instead — a settings GET and a switch-free PUT then agree with an attempted apply. + */ +export async function observedCodexDesktopSwitchApply(): Promise { + // Same lazy boundary as applyCodexDesktopSwitches: the ownership predicate lives in the + // injection graph, which the settings read path must not pull in at module scope. + const { currentExternalCodexModelProvider } = await import("./inject/config-toml"); + const provider = currentExternalCodexModelProvider(); + if (!provider) return { applied: false, reason: "not_requested", retryable: false }; + return { + applied: false, + reason: "external_provider", + retryable: false, + detail: `config.toml selects the external model_provider ${tomlString(provider)}.`, + }; +} + export async function applyCodexDesktopSwitches( config: OcxConfig, ): Promise { @@ -115,6 +144,14 @@ export async function applyCodexDesktopSwitches( detail: result.message, }; } + if (result.success && result.configApplied === false) { + return { + applied: false, + reason: "external_provider", + retryable: false, + detail: result.message, + }; + } if (result.success) { // history_paginated_requires_native_writer stands down only the legacy relabel; // apply still writes the routing and catalog half for paginated Codex homes. diff --git a/src/codex/inject.ts b/src/codex/inject.ts index 54c83105898..dafcd0506ef 100644 --- a/src/codex/inject.ts +++ b/src/codex/inject.ts @@ -149,6 +149,8 @@ function runClientWriteGuard(guard: InjectCodexOptions["beforeClientWrite"]): vo export interface CodexInjectResult { success: boolean; message: string; + /** False when injection intentionally preserves configuration owned by another provider. */ + configApplied?: false; /** * Structured read-only history preflight refusal; never parsed from display text. * @@ -240,6 +242,7 @@ async function injectCodexConfigImpl( : undefined; return { success: true, + configApplied: false, ...(nativeSubagentDefaultsWarning ? { nativeSubagentDefaultsWarning } : {}), diff --git a/src/server/management/config-routes.ts b/src/server/management/config-routes.ts index 6db5ca3f35c..d267a537471 100644 --- a/src/server/management/config-routes.ts +++ b/src/server/management/config-routes.ts @@ -8,6 +8,7 @@ import { catalogModelSlug, invalidateCodexModelsCache, nativeContextLimits, nati import { applyCodexDesktopSwitches, describeCodexDesktopSwitches, + observedCodexDesktopSwitchApply, type CodexDesktopSwitchApply, } from "../../codex/desktop-switches"; import { @@ -339,11 +340,7 @@ export async function handleConfigRoutes(ctx: ManagementContext): Promise { } }); + test("reports externally owned switch and authentication state without claiming a rewrite", async () => { + const { deps } = fakeRuntime(() => ({ + ok: true, + codexDesktopSwitches: { + codexDesktopAuthless: { stored: true, effective: null }, + codexClientCompaction: { stored: false, effective: null }, + apply: { applied: false, reason: "external_provider", retryable: false }, + authSource: { + presentsCodexAccount: null, + summary: "An external model provider owns Codex sign-in behavior; its account requirement was not changed.", + }, + }, + })); + const logSpy = spyOn(console, "log").mockImplementation(() => {}); + try { + expect(await handleSystemCommand(["settings", "--desktop-authless", "on"], deps)).toBe(0); + const output = logSpy.mock.calls.flat().join("\n"); + expect(output).toContain("effective state is controlled by the external model provider"); + expect(output).toContain("was not rewritten because an external model provider owns config.toml"); + expect(output).toContain("Auth source: An external model provider owns Codex sign-in behavior"); + expect(output).not.toContain("was rewritten."); + expect(output).not.toContain("ocx sync"); + } finally { + logSpy.mockRestore(); + } + }); + test("keeps the legacy success line when an older server omits the switch report", async () => { const { deps } = fakeRuntime((_req, body) => ({ ok: true, ...body })); const logSpy = spyOn(console, "log").mockImplementation(() => {}); diff --git a/tests/codex-integration/codex-inject-integration.test.ts b/tests/codex-integration/codex-inject-integration.test.ts index e88b4446d4d..344af3b3025 100644 --- a/tests/codex-integration/codex-inject-integration.test.ts +++ b/tests/codex-integration/codex-inject-integration.test.ts @@ -1488,6 +1488,7 @@ describe("injectCodexConfig integration (Design B)", () => { expect(result.success).toBe(true); expect(result.message).toContain("routing NOT injected"); expect(result.message).toContain('external model_provider "custom"'); + expect(result.configApplied).toBe(false); expect(result.message).toContain("http://127.0.0.1:10100/v1"); expect(result.message).toContain("Responses passthrough"); expect(result.nativeSubagentDefaultsWarning).toContain("external model_provider"); diff --git a/tests/config/settings-desktop-switch-apply.test.ts b/tests/config/settings-desktop-switch-apply.test.ts index 1eeb84067ce..c871d662796 100644 --- a/tests/config/settings-desktop-switch-apply.test.ts +++ b/tests/config/settings-desktop-switch-apply.test.ts @@ -1,10 +1,12 @@ import { expect, spyOn, test } from "bun:test"; -import { mkdirSync, mkdtempSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { removeTreeWithRetry } from "../helpers/remove-tree"; +import { repoRoot } from "../helpers/repo-root"; -test("PUT /api/settings reports Codex write-lock contention as retryable", async () => { +test("PUT /api/settings reports why Codex desktop switches were not applied", async () => { const root = mkdtempSync(join(tmpdir(), "ocx-settings-desktop-switch-")); const codexHome = join(root, "codex"); mkdirSync(codexHome, { recursive: true }); @@ -67,6 +69,38 @@ test("PUT /api/settings reports Codex write-lock contention as retryable", async }, }); expect(injectionSpy).toHaveBeenCalledTimes(1); + + injectionSpy.mockResolvedValue({ + success: true, + configApplied: false, + message: 'Codex routing NOT injected: external model_provider "custom" owns config.toml.', + }); + const externalRequest = new Request("http://127.0.0.1:10100/api/settings", { + method: "PUT", + headers: { host: "127.0.0.1:10100", "content-type": "application/json" }, + body: JSON.stringify({ codexClientCompaction: true }), + }); + const externalResponse = await handleManagementAPI( + externalRequest, + new URL(externalRequest.url), + config, + { + saveConfigPreservingClaudeCode: () => {}, + getCachedStartupHealth: async () => startupHealthFixture(), + createManagementConvergeCodex: catalogConvergenceFactory(() => {}), + }, + ); + + expect(externalResponse!.status).toBe(200); + expect(await externalResponse!.json()).toMatchObject({ + codexDesktopSwitches: { + codexDesktopAuthless: { effective: null }, + codexClientCompaction: { effective: null }, + apply: { applied: false, reason: "external_provider", retryable: false }, + authSource: { presentsCodexAccount: null }, + }, + }); + expect(injectionSpy).toHaveBeenCalledTimes(2); } finally { injectionSpy.mockRestore(); if (previousOcxHome === undefined) delete process.env.OPENCODEX_HOME; @@ -76,3 +110,71 @@ test("PUT /api/settings reports Codex write-lock contention as retryable", async removeTreeWithRetry(root); } }); + +test("GET /api/settings reports external Codex ownership without an apply attempt", () => { + // The ownership predicate reads CODEX_CONFIG_PATH, which is bound to CODEX_HOME at module + // load, so an externally owned config.toml must live in a home fixed before the child + // process starts — mutating process.env here would not move the already-bound path. + const root = mkdtempSync(join(tmpdir(), "ocx-settings-external-get-")); + const codexHome = join(root, "codex"); + mkdirSync(codexHome, { recursive: true }); + writeFileSync(join(codexHome, "config.toml"), 'model_provider = "custom"\n', "utf8"); + + const script = ` + const { handleManagementAPI } = await import("./src/server/management-api"); + const { startupHealthFixture } = await import("./tests/helpers/startup-health"); + const config = JSON.parse(process.env.OCX_TEST_ROUTE_CONFIG); + const request = new Request("http://127.0.0.1:10100/api/settings", { + // Same requirement as the in-process cases: managementRequestOrigin derives the + // allowed origin from the Host header, and a constructed Request carries none. + headers: { host: "127.0.0.1:10100" }, + }); + const response = await handleManagementAPI(request, new URL(request.url), config, { + getCachedStartupHealth: async () => startupHealthFixture(), + }); + console.log(JSON.stringify({ status: response.status, body: await response.json() })); + `; + const child = spawnSync(process.execPath, ["--eval", script], { + cwd: repoRoot(), + env: { + ...process.env, + CODEX_HOME: codexHome, + OPENCODEX_HOME: join(root, "opencodex"), + OCX_TEST_ROUTE_CONFIG: JSON.stringify({ + port: 10100, + defaultProvider: "openai", + codexDesktopAuthless: true, + codexClientCompaction: true, + providers: { + openai: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + apiKey: "sk-secret-value", + defaultModel: "gpt-test", + }, + }, + }), + }, + encoding: "utf8", + timeout: 30_000, + }); + try { + if (child.status !== 0) { + throw new Error(`isolated settings GET failed: ${child.stderr || child.stdout}`); + } + const line = child.stdout.trim().split("\n").filter(Boolean).at(-1); + expect(line).toBeDefined(); + const response = JSON.parse(line!) as { status: number; body: Record }; + expect(response.status).toBe(200); + expect(response.body).toMatchObject({ + codexDesktopSwitches: { + codexDesktopAuthless: { stored: true, effective: null }, + codexClientCompaction: { stored: true, effective: null }, + apply: { applied: false, reason: "external_provider", retryable: false }, + authSource: { presentsCodexAccount: null }, + }, + }); + } finally { + removeTreeWithRetry(root); + } +}, 15_000); diff --git a/tests/responses/chat-inline-document-bytes.test.ts b/tests/responses/chat-inline-document-bytes.test.ts index dd88fa05788..a718c1c376a 100644 --- a/tests/responses/chat-inline-document-bytes.test.ts +++ b/tests/responses/chat-inline-document-bytes.test.ts @@ -28,6 +28,10 @@ const chatProvider: OcxProviderConfig = { adapter: "openai-chat", baseUrl: "https://gateway.example.internal/v1", apiKey: "k", + // The wire role folds to `system` unless a destination is recorded as accepting + // `developer`; the document test asserts the role a turn keeps, so it declares the + // destination rather than asserting the default. + foldDeveloperRoleToSystem: false, }; const anthropicProvider = { adapter: "anthropic",