Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 138 additions & 7 deletions src/components/settings/acp-agent-settings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1307,13 +1307,29 @@ describe("patchCodexConfigTomlText — codeg's requires_openai_auth default", ()

// The three structured controls that reach ensureCodexProviderDefaults. Each
// must behave identically — the bug in issue #406 fired through all of them.
// `seedsFromEmpty` marks the ones that may CREATE the provider: the WebSocket
// toggle carries no URL, so on an unbound config it writes the `[features]`
// key instead of installing an empty provider (#520).
const ENTRY_POINTS: Array<{
label: string
patch: Parameters<typeof patchCodexConfigTomlText>[1]
seedsFromEmpty: boolean
}> = [
{ label: "API base URL", patch: { apiBaseUrl: "https://new.example/v1" } },
{ label: "WebSocket toggle", patch: { supportsWebsockets: true } },
{ label: "model provider", patch: { modelProvider: "codeg" } },
{
label: "API base URL",
patch: { apiBaseUrl: "https://new.example/v1" },
seedsFromEmpty: true,
},
{
label: "WebSocket toggle",
patch: { supportsWebsockets: true },
seedsFromEmpty: false,
},
{
label: "model provider",
patch: { modelProvider: "codeg" },
seedsFromEmpty: true,
},
]

const BOUND_PROVIDER = [
Expand All @@ -1325,7 +1341,7 @@ describe("patchCodexConfigTomlText — codeg's requires_openai_auth default", ()
'wire_api = "responses"',
].join("\n")

for (const { label, patch } of ENTRY_POINTS) {
for (const { label, patch, seedsFromEmpty } of ENTRY_POINTS) {
describe(`via the ${label} control`, () => {
it("keeps an explicit false", () => {
const toml = `${BOUND_PROVIDER}\nrequires_openai_auth = false\n`
Expand All @@ -1343,9 +1359,16 @@ describe("patchCodexConfigTomlText — codeg's requires_openai_auth default", ()
).toBe(true)
})

it("seeds a brand-new provider from an empty config", () => {
expect(authFlagOf(patchCodexConfigTomlText("", patch))).toBe(true)
})
it(
seedsFromEmpty
? "seeds a brand-new provider from an empty config"
: "does not seed a provider from an empty config",
() => {
expect(authFlagOf(patchCodexConfigTomlText("", patch))).toBe(
seedsFromEmpty ? true : undefined
)
}
)

it("stands down for a provider using actor authorization", () => {
const toml = [
Expand Down Expand Up @@ -1482,6 +1505,114 @@ describe("patchCodexConfigTomlText — codeg's requires_openai_auth default", ()
})
})

// config.toml is shared with the codex CLI and the native Codex app. Adopting
// `codeg` as the model provider from a control that carries no URL writes
// `model_provider = "codeg"` beside `[model_providers.codeg] base_url = ""`, and
// codex then fails every request at the builder stage ("stream disconnected
// before completion: builder error") in every client reading that file. Nothing
// in codeg's UI undoes it. See issue #520.
describe("patchCodexConfigTomlText: an unbound config stays unbound", () => {
// What the panel really sends: the draft's provider is "" while nothing is
// bound, and it rides along on the URL and WebSocket patches.
const UNBOUND_DRAFT_PROVIDER = ""
const UNBOUND = 'model = "gpt-5.6-sol"\n'
const BOUND = [
'model_provider = "acme"',
"",
"[model_providers.acme]",
'base_url = "https://acme.example/v1"',
].join("\n")

function codegProviderOf(configTomlText: string): {
root: string | undefined
table: unknown
} {
const parsed = parseTomlDocument(configTomlText) as {
model_provider?: unknown
model_providers?: Record<string, unknown>
}
return {
root:
typeof parsed.model_provider === "string"
? parsed.model_provider
: undefined,
table: parsed.model_providers?.codeg,
}
}

for (const enabled of [true, false]) {
it(`turning WebSockets ${enabled ? "on" : "off"} binds no provider`, () => {
const after = patchCodexConfigTomlText(UNBOUND, {
modelProvider: UNBOUND_DRAFT_PROVIDER,
supportsWebsockets: enabled,
})
const { root, table } = codegProviderOf(after)
expect(root).toBeUndefined()
expect(table).toBeUndefined()
expect(after).not.toContain("base_url")
})
}

// Without a provider the `[features]` key IS the setting, and it is what the
// reader falls back to, so the switch has to keep round-tripping.
it("still moves the WebSocket switch, through the feature key", () => {
const on = patchCodexConfigTomlText(UNBOUND, {
modelProvider: UNBOUND_DRAFT_PROVIDER,
supportsWebsockets: true,
})
expect(on).toContain("responses_websockets_v2 = true")
expect(extractCodexImportantValues("", on).supportsWebsockets).toBe(true)

const off = patchCodexConfigTomlText(on, {
modelProvider: UNBOUND_DRAFT_PROVIDER,
supportsWebsockets: false,
})
expect(off).not.toContain("responses_websockets_v2")
expect(extractCodexImportantValues("", off).supportsWebsockets).toBe(false)
expect(codegProviderOf(off).root).toBeUndefined()
})

it("clearing the API base URL binds no provider", () => {
const after = patchCodexConfigTomlText(UNBOUND, {
modelProvider: UNBOUND_DRAFT_PROVIDER,
apiBaseUrl: "",
})
expect(codegProviderOf(after).table).toBeUndefined()
expect(after).not.toContain("model_providers")
})

it("typing an API base URL still binds codeg", () => {
const after = patchCodexConfigTomlText(UNBOUND, {
modelProvider: UNBOUND_DRAFT_PROVIDER,
apiBaseUrl: "https://new.example/v1",
})
expect(codegProviderOf(after).root).toBe("codeg")
expect(after).toContain('base_url = "https://new.example/v1"')
})

it("writes supports_websockets onto the provider the user did bind", () => {
const after = patchCodexConfigTomlText(BOUND, { supportsWebsockets: true })
const parsed = parseTomlDocument(after) as {
model_provider?: string
model_providers?: Record<string, { supports_websockets?: unknown }>
}
expect(parsed.model_provider).toBe("acme")
expect(parsed.model_providers?.acme?.supports_websockets).toBe(true)
expect(parsed.model_providers?.codeg).toBeUndefined()
})

it("still clears a bound provider's base URL", () => {
const after = patchCodexConfigTomlText(BOUND, { apiBaseUrl: "" })
const parsed = parseTomlDocument(after) as {
model_provider?: string
model_providers?: Record<string, { base_url?: unknown }>
}
expect(parsed.model_provider).toBe("acme")
expect(parsed.model_providers?.acme).toBeDefined()
expect(parsed.model_providers?.acme?.base_url).toBeUndefined()
})
})

// The panel used to ship a Reasoning Effort dropdown, and the writer rewrote
// `model_reasoning_effort` on EVERY patch with "high" as the parse default. So
// touching any unrelated control injected a key the user never asked for and
Expand Down
65 changes: 40 additions & 25 deletions src/components/settings/acp-agent-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3061,44 +3061,59 @@ export function patchCodexConfigTomlText(
}
if (typeof patch.apiBaseUrl === "string") {
const tomlValues = extractCodexTomlImportantValues(nextTomlText)
// Falling back to `codeg` is only right when this patch actually carries a
// URL for it. Clearing the box must not conjure a provider whose entire
// content would be `base_url = ""`. See the websockets arm below.
const modelProvider =
patch.modelProvider?.trim() ||
tomlValues.modelProvider.trim() ||
CODEX_DEFAULT_MODEL_PROVIDER
if (!tomlValues.modelProvider.trim() && patch.apiBaseUrl.trim()) {
nextTomlText = updateTomlRootStringKey(
(patch.apiBaseUrl.trim() ? CODEX_DEFAULT_MODEL_PROVIDER : "")
if (modelProvider) {
if (!tomlValues.modelProvider.trim() && patch.apiBaseUrl.trim()) {
nextTomlText = updateTomlRootStringKey(
nextTomlText,
"model_provider",
modelProvider
)
}
nextTomlText = patchCodexProviderBaseUrl(
nextTomlText,
"model_provider",
modelProvider
modelProvider,
patch.apiBaseUrl
)
nextTomlText = ensureCodexProviderDefaults(nextTomlText, modelProvider)
}
nextTomlText = patchCodexProviderBaseUrl(
nextTomlText,
modelProvider,
patch.apiBaseUrl
)
nextTomlText = ensureCodexProviderDefaults(nextTomlText, modelProvider)
}
if (typeof patch.supportsWebsockets === "boolean") {
const tomlValues = extractCodexTomlImportantValues(nextTomlText)
// No `|| CODEX_DEFAULT_MODEL_PROVIDER` here. config.toml is shared with the
// codex CLI and the native Codex app, and adopting `codeg` from a switch
// that has nothing to do with providers installs
// `model_provider = "codeg"` + `[model_providers.codeg] base_url = ""` into
// a file the user never pointed at codeg. codex then fails every request at
// the builder stage ("stream disconnected before completion: builder
// error") in every client, and nothing inside codeg undoes it. See #520.
const modelProvider =
patch.modelProvider?.trim() ||
tomlValues.modelProvider.trim() ||
CODEX_DEFAULT_MODEL_PROVIDER
if (!tomlValues.modelProvider.trim()) {
nextTomlText = updateTomlRootStringKey(
patch.modelProvider?.trim() || tomlValues.modelProvider.trim()
if (modelProvider) {
nextTomlText = patchCodexProviderField(
nextTomlText,
"model_provider",
modelProvider
modelProvider,
"supports_websockets",
`supports_websockets = ${patch.supportsWebsockets ? "true" : "false"}`
)
nextTomlText = ensureCodexProviderDefaults(nextTomlText, modelProvider)
} else {
// Unbound: the `[features]` key IS the setting, and it is what the reader
// falls back to. Write it here so the normalization below reads the value
// the switch just moved to instead of the one already on disk.
nextTomlText = upsertTomlSectionBooleanKey(
nextTomlText,
"features",
"responses_websockets_v2",
patch.supportsWebsockets ? true : null
)
}
nextTomlText = patchCodexProviderField(
nextTomlText,
modelProvider,
"supports_websockets",
`supports_websockets = ${patch.supportsWebsockets ? "true" : "false"}`
)
nextTomlText = ensureCodexProviderDefaults(nextTomlText, modelProvider)
}
const normalizedTomlValues = extractCodexTomlImportantValues(nextTomlText)
if (normalizedTomlValues.model.trim()) {
Expand Down
Loading