diff --git a/devlog/_plan/260826_wp7e_presence_driven_oauth_failover/030_post_merge_f5.md b/devlog/_plan/260826_wp7e_presence_driven_oauth_failover/030_post_merge_f5.md new file mode 100644 index 00000000000..b0b406fd020 --- /dev/null +++ b/devlog/_plan/260826_wp7e_presence_driven_oauth_failover/030_post_merge_f5.md @@ -0,0 +1,39 @@ +# 030 — closing F5: the Add Provider path also had to stop dropping the opt-out + +`011_audit_response.md` scoped F5 out with a reason: `/api/providers` POST replaces a provider +row wholesale and already discards every unrecognized key, so it looked like a general payload +contract problem rather than this feature's problem. + +That reasoning was half right and the conclusion was wrong. + +## Why it could not stay deferred + +Every other field this path drops fails toward something neutral — a missing `modelCosts` means +the Usage estimate falls back to registry prices, a missing `contextWindow` means the seed value +applies. Losing `oauthAccountFailover` does not fail toward neutral. Activation is now +presence-driven, so deleting an operator's `enabled: false` does not restore a default: it +**enables** rotation across their second subscription account, as a side effect of an edit that +had nothing to do with failover. + +That is the same failure shape as F4 (login rebuilding the row), which was accepted as blocking. +The two paths differ only in which unrelated action triggers the loss. + +## What changed + +One preservation line in `src/server/management/provider-routes.ts`, next to the ones that +already exist for `apiKeyPool`, `modelCosts`, `requestPacing`, and the context-window maps. The +comment there records why absence means "not carried" rather than "deleted": `ProviderPayload` +(`gui/src/provider-payload.ts`) structurally cannot express the field, so the dashboard's +add/edit form can never send it. Deletion goes through PATCH with an explicit null, exactly as +#1409 established for context windows. + +No GUI change. Widening `ProviderPayload` would be the wrong fix for the same reason it was the +wrong fix for `modelCosts`: the form has no control for this setting, so a payload member would +only give it a way to send `undefined` and re-create the problem. + +## Verification + +A regression next to the existing `modelCosts` overwrite test in +`tests/management-provider-validation.test.ts`, driven against a real server: create a provider +with `enabled: false`, POST an overwrite without the field, assert the opt-out survived. +Falsified by disabling the preservation branch — the test fails, 74 pass / 1 fail. diff --git a/src/server/management/provider-routes.ts b/src/server/management/provider-routes.ts index 5c107135f85..d542306885b 100644 --- a/src/server/management/provider-routes.ts +++ b/src/server/management/provider-routes.ts @@ -596,6 +596,12 @@ export async function handleProviderRoutes(ctx: ManagementContext): Promise { } }); + test("provider POST overwrite preserves the account-failover opt-out when the payload omits it (#2568d)", async () => { + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; + saveConfig(config("127.0.0.1")); + + const server = startServer(0); + try { + const create = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "custom-failover", + provider: { + adapter: "openai-chat", + baseUrl: "https://api.example.test/v1", + oauthAccountFailover: { enabled: false }, + }, + }), + }); + expect(create.status).toBe(200); + + // Losing this one is worse than losing a cosmetic field: activation is presence-driven, + // so dropping the opt-out does not fall back to a neutral default — it ENABLES rotation + // across the operator's second subscription account, as a side effect of an edit that had + // nothing to do with failover. + const overwrite = await fetch(new URL("/api/providers", server.url), { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + name: "custom-failover", + provider: { adapter: "openai-chat", baseUrl: "https://api.example.test/v1" }, + }), + }); + expect(overwrite.status).toBe(200); + expect(loadConfig().providers["custom-failover"]?.oauthAccountFailover).toEqual({ enabled: false }); + } finally { + await server.stop(true); + } + }); + // #1409: the add/edit form's payload type has no member for contextWindow or // modelContextWindows, so an overwrite arrives without them. Registry enrichment then fills // the absent fields from the seed and the stored row loses the user's values — for