From 3eba2455b119e37b75d6f9b48bca8e96005fc9a0 Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Mon, 19 Oct 2026 12:58:53 +0000 Subject: [PATCH 1/2] fix(oauth): allow dismissing a settled failed login flow --- .../content/docs/reference/management-api.md | 5 ++- src/oauth/login-flow-state.ts | 10 ++++- .../ADR-7378-oauth-failed-flow-dismissal.md | 13 ++++++ .../providers/oauth-code-display-login.md | 6 +++ tests/oauth/oauth-status-privacy.test.ts | 42 +++++++++++++++++++ 5 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md diff --git a/docs-site/src/content/docs/reference/management-api.md b/docs-site/src/content/docs/reference/management-api.md index c5176ae8445..5325a419898 100644 --- a/docs-site/src/content/docs/reference/management-api.md +++ b/docs-site/src/content/docs/reference/management-api.md @@ -609,7 +609,7 @@ Labels are read on login/import or refresh, never inferred from quota percentage | `GET /api/key-providers` | List providers configured through API-key login | — | | `POST /api/oauth/login` | Start an OAuth login or account-add flow. `mode: "code"` starts a code-display login (Anthropic only) and returns `mode`, `flowId`, `expiresAt` | 400 unknown/invalid provider, invalid `mode`, `code_mode_unsupported`; `oauth_mutation_busy` | | `POST /api/oauth/login/code` | Submit a manual callback URL or authorization code. For a code-display flow (with `flowId`) returns the token-exchange outcome `{ok, account}` / `{ok:false, error}` | 400 invalid provider/code, `state_mismatch`, `invalid_or_expired_code`, `provider_unreachable`, `malformed_input`; 409 `no_pending_login`; `oauth_mutation_busy` | -| `POST /api/oauth/login/cancel` | Cancel a public in-progress OAuth flow; an optional `flowId` names one code-display flow (a flow of another provider is refused) | 400 unknown provider | +| `POST /api/oauth/login/cancel` | Cancel a public in-progress OAuth flow or dismiss its settled failure; an optional `flowId` names one code-display flow (a flow of another provider is refused) | 400 unknown provider | | `GET /api/oauth/status` | Poll one provider's OAuth flow | 400 unknown provider | | `POST /api/oauth/logout` | Remove the selected provider credential | 400 unknown provider; `oauth_mutation_busy` | | `GET /api/oauth/accounts` | List masked accounts; Anthropic and generic OAuth account rows include their `paused` state. Kiro rows include `autoSelectable` and a closed `skipReason` when excluded from automatic selection; an active singleton may still send. Quota remains opt-in. | 400 invalid provider | @@ -625,6 +625,9 @@ Labels are read on login/import or refresh, never inferred from quota percentage | `PUT /api/providers/keys/alias` | Set or clear a provider-key alias | 400 invalid input; 404 provider/key missing | | `GET, POST, PATCH, DELETE /api/keys` | List, create, edit, or delete data-plane admission keys | 400 invalid body/id; 404 key missing | +`POST /api/oauth/login/cancel` can also dismiss the selected settled failed login attempt. It leaves stored credentials and successful login results intact; status then reports the remaining flow or idle. Cancelling an in-progress attempt still records its cancelled outcome. + + For Anthropic, `routes` is an ordered array of `{name, match, accounts, fallback?}` rules on both settings endpoints. GET and write echoes include `routes` (`null` when absent); unified DTOs expose `routes: null` for other kinds. A supplied `routes` on another kind is rejected. Omission preserves rules, `[]` matches nothing, and `null` clears them. The `accounts` values are stored IDs; removed IDs remain valid in a rule so re-adding an account can restore routing. Management responses retain valid configured names; request logs identify a match only as `route:#` (1-based list position). If a hand-edited stored rule is invalid, both settings GETs return `routes: null` and a `routesError` diagnostic instead of presenting that rule as valid; the stored value remains available for correction. Valid or absent rules omit `routesError`. Credential list responses are deliberately masked. OAuth access tokens and complete provider API diff --git a/src/oauth/login-flow-state.ts b/src/oauth/login-flow-state.ts index 3a52a6a7151..169ec8c7554 100644 --- a/src/oauth/login-flow-state.ts +++ b/src/oauth/login-flow-state.ts @@ -297,7 +297,8 @@ export function clearLoginState(provider: string): void { /** * Cancel one login. `flowId` names it exactly; without one the provider's newest in-flight flow * is cancelled, which is every single-flow caller (GUI, CLI) unchanged. A flow id belonging to a - * different provider is refused rather than cancelled. + * different provider is refused rather than cancelled. A settled failure can be dismissed + * explicitly; credential health and the result of a successful flow remain independent. */ export function cancelLoginFlow(provider: string, flowId?: string): boolean { const flowKey = flowId ?? latestLoginFlowKey(provider); @@ -305,6 +306,13 @@ export function cancelLoginFlow(provider: string, flowId?: string): boolean { const active = loginAbort.get(flowKey); const existing = loginState.get(flowKey); if ((active?.provider ?? loginFlowProvider(flowKey)) !== provider) return false; + if (!active && existing?.done && existing.error) { + // A failed add-account attempt does not invalidate a previously signed-in account. Let + // its owner dismiss the stale UI outcome without erasing credentials or another flow. + clearManualCodeSlot(flowKey); + dropLoginFlow(flowKey); + return true; + } if (!active && (!existing || existing.done)) return false; abandonLoginFlow(flowKey, { error: "Login cancelled", errorCode: "no_pending_login" }); return true; diff --git a/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md b/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md new file mode 100644 index 00000000000..c48fab7ed83 --- /dev/null +++ b/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md @@ -0,0 +1,13 @@ +# ADR-7378: decision recorded under "OAuth code-display login" + +- Purpose: let an operator dismiss the obsolete error of a failed add-account attempt (#7378). +- Constraint: flow outcome and persisted active-account health are independent; concurrent code + logins and provider ownership already have dedicated flow-key APIs. +- Alternatives: hide every flow error whenever a credential exists, expire outcomes by time, or + reuse the explicit cancellation endpoint. Hiding errors would conceal a real failed add-account + attempt; a timer could erase an outcome before a polling caller reads it. +- Decision: extend the existing cancellation owner to drop exactly one settled failed flow with + no live controller. Reuse paste cleanup and waiter-safe flow removal, without credential I/O. +- Consequences: failures remain observable until explicitly dismissed; successful outcomes and + pending-cancellation results keep their contract. Repeated dismissal is a no-op. This provides + API recovery without an implicit login, logout, credential refresh or service restart. diff --git a/structure/providers/oauth-code-display-login.md b/structure/providers/oauth-code-display-login.md index b66b2e9b980..46321b02a07 100644 --- a/structure/providers/oauth-code-display-login.md +++ b/structure/providers/oauth-code-display-login.md @@ -24,6 +24,10 @@ unchanged. lookup is provider-scoped (`loginFlowKeys` matches `(state.provider ?? key) === provider`), so status, cancellation, clearing and the admission cap never see another provider's flows. A cancel naming a `flowId` that belongs to a different provider is refused. +- **Failed-flow dismissal.** `cancelLoginFlow` also dismisses a settled failed attempt after its + controller is gone. It drops only the selected provider-owned flow and its paste slot, so status + returns to the remaining flow or idle. Active-account credentials and successful terminal + outcomes remain untouched; cancelling a pending attempt still records its cancelled outcome. - **Expiry and limits.** A code flow's paste window is `CODE_LOGIN_TTL_MS` (10 minutes); an expired flow is reaped as `no_pending_login` the next time that provider starts a login. At most `MAX_LOGIN_FLOWS_PER_PROVIDER` (64) rows per provider are kept; settled rows are evicted first @@ -53,3 +57,5 @@ unchanged. Tests: `tests/oauth/oauth-code-login.test.ts` (flow keying, cross-provider isolation, error classification, paste contract) and the `--headless` cases in `tests/cli/cli-provider.test.ts`. + +> Decision record: [ADR-7378](../decisions/ADR-7378-oauth-failed-flow-dismissal.md) diff --git a/tests/oauth/oauth-status-privacy.test.ts b/tests/oauth/oauth-status-privacy.test.ts index fa50763c477..88c0e3ad4eb 100644 --- a/tests/oauth/oauth-status-privacy.test.ts +++ b/tests/oauth/oauth-status-privacy.test.ts @@ -3,6 +3,7 @@ import { mkdtempSync, readdirSync, readFileSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { + cancelLoginFlow, clearLoginState, getLoginStatus, getValidAccessToken, @@ -17,6 +18,7 @@ import { publicOAuthAuthenticationErrorMessage, UnsupportedOAuthProviderError, } from "../../src/oauth"; +import { loginState } from "../../src/oauth/login-flow-state"; import { getAccountSet, OAuthMutationBusyError, saveCredential, setAccountPaused } from "../../src/oauth/store"; import { handleManagementAPI } from "../../src/server/management-api"; import { handleResponses } from "../../src/server/responses"; @@ -53,6 +55,46 @@ describe("OAuth status privacy", () => { TEST_DIR = ""; }); + test("a settled failed login can be dismissed without changing the active credential (#7378)", async () => { + await saveCredential("xai", { access: "fixture-access", refresh: "fixture-refresh", + expires: Date.now() + 60_000, email: "person@example.test", accountId: "fixture-account" }); + const before = getAccountSet("xai"); + const authBytes = readFileSync(join(TEST_DIR, "auth.json")); + loginState.set("failed-xai", { provider: "xai", flowId: "failed-xai", done: true, error: PUBLIC_OAUTH_ERROR }); + expect(getLoginStatus("xai")).toMatchObject({ loggedIn: true, status: "error", error: PUBLIC_OAUTH_ERROR }); + const request = new ManagementRequest("http://localhost/api/oauth/login/cancel", { + method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify({ provider: "xai" }), + }); + const response = await handleManagementAPI(request, new URL(request.url), { + defaultProvider: "xai", providers: { xai: { adapter: "openai-responses", baseUrl: "https://api.x.ai/v1", authMode: "oauth" } }, + } as OcxConfig); + expect(response?.status).toBe(200); + expect(await response!.json()).toEqual({ ok: true, cancelled: true }); + expect(getLoginStatus("xai")).toMatchObject({ loggedIn: true, status: "idle", error: undefined }); + expect(getAccountSet("xai")).toEqual(before); + expect(readFileSync(join(TEST_DIR, "auth.json"))).toEqual(authBytes); + expect(cancelLoginFlow("xai")).toBe(false); + }); + + test("failed-flow dismissal is exact and provider scoped", () => { + loginState.set("failed-xai", { provider: "xai", flowId: "failed-xai", done: true, error: PUBLIC_OAUTH_ERROR }); + loginState.set("pending-xai", { provider: "xai", flowId: "pending-xai", done: false }); + expect(cancelLoginFlow("anthropic", "failed-xai")).toBe(false); + expect(loginState.has("failed-xai")).toBe(true); + expect(cancelLoginFlow("xai", "failed-xai")).toBe(true); + expect(loginState.get("pending-xai")?.done).toBe(false); + expect(getLoginStatus("xai").status).toBe("pending"); + }); + + test("successful terminal state remains available and pending cancellation keeps its outcome", () => { + loginState.set("completed-xai", { provider: "xai", flowId: "completed-xai", done: true }); + expect(cancelLoginFlow("xai", "completed-xai")).toBe(false); + expect(getLoginStatus("xai", true, "completed-xai").status).toBe("complete"); + loginState.set("pending-xai", { provider: "xai", flowId: "pending-xai", done: false }); + expect(cancelLoginFlow("xai", "pending-xai")).toBe(true); + expect(getLoginStatus("xai", true, "pending-xai")).toMatchObject({ status: "error", error: "Login cancelled" }); + }); + test("getLoginStatus returns a masked provider email", async () => { await saveCredential("xai", { access: "access-token", From bb9cc0e61292a81b4af0fc126790d3974e56567a Mon Sep 17 00:00:00 2001 From: Ingwannu Date: Mon, 19 Oct 2026 13:10:54 +0000 Subject: [PATCH 2/2] fix(oauth): preserve live-cancel proof for internal callers --- src/oauth/login-flow-state.ts | 9 +++++---- src/server/management/oauth-account-routes.ts | 3 ++- .../decisions/ADR-7378-oauth-failed-flow-dismissal.md | 3 ++- structure/providers/oauth-code-display-login.md | 3 ++- tests/oauth/oauth-status-privacy.test.ts | 3 ++- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/oauth/login-flow-state.ts b/src/oauth/login-flow-state.ts index 169ec8c7554..703f090dc1c 100644 --- a/src/oauth/login-flow-state.ts +++ b/src/oauth/login-flow-state.ts @@ -297,16 +297,17 @@ export function clearLoginState(provider: string): void { /** * Cancel one login. `flowId` names it exactly; without one the provider's newest in-flight flow * is cancelled, which is every single-flow caller (GUI, CLI) unchanged. A flow id belonging to a - * different provider is refused rather than cancelled. A settled failure can be dismissed - * explicitly; credential health and the result of a successful flow remain independent. + * different provider is refused rather than cancelled. Only the public OAuth cancellation + * endpoint opts into failed-outcome dismissal; callers using this boolean as proof of an + * aborted controller (Codex flow ownership) retain their original contract. */ -export function cancelLoginFlow(provider: string, flowId?: string): boolean { +export function cancelLoginFlow(provider: string, flowId?: string, options?: { dismissSettledFailure?: boolean }): boolean { const flowKey = flowId ?? latestLoginFlowKey(provider); if (flowKey === undefined) return false; const active = loginAbort.get(flowKey); const existing = loginState.get(flowKey); if ((active?.provider ?? loginFlowProvider(flowKey)) !== provider) return false; - if (!active && existing?.done && existing.error) { + if (options?.dismissSettledFailure && !active && existing?.done && existing.error) { // A failed add-account attempt does not invalidate a previously signed-in account. Let // its owner dismiss the stale UI outcome without erasing credentials or another flow. clearManualCodeSlot(flowKey); diff --git a/src/server/management/oauth-account-routes.ts b/src/server/management/oauth-account-routes.ts index 865995ecbf9..853e9d4d13b 100644 --- a/src/server/management/oauth-account-routes.ts +++ b/src/server/management/oauth-account-routes.ts @@ -374,7 +374,8 @@ export async function handleOauthAccountRoutes(ctx: ManagementContext): Promise< // Without a flow id this cancels the provider's newest in-flight login, which is what the // single-user GUI has always meant by "Cancel". A host running concurrent code logins for // several users passes the flowId it was given at /api/oauth/login. - const cancelled = cancelLoginFlow(provider, typeof body.flowId === "string" ? body.flowId : undefined); + const cancelled = cancelLoginFlow(provider, typeof body.flowId === "string" ? body.flowId : undefined, + { dismissSettledFailure: true }); return jsonResponse({ ok: true, cancelled }); } diff --git a/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md b/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md index c48fab7ed83..a0846fb8176 100644 --- a/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md +++ b/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md @@ -7,7 +7,8 @@ reuse the explicit cancellation endpoint. Hiding errors would conceal a real failed add-account attempt; a timer could erase an outcome before a polling caller reads it. - Decision: extend the existing cancellation owner to drop exactly one settled failed flow with - no live controller. Reuse paste cleanup and waiter-safe flow removal, without credential I/O. + no live controller, only when the public endpoint explicitly opts in. Internal callers retain + the boolean meaning of live cancellation. Reuse paste cleanup and waiter-safe flow removal, without credential I/O. - Consequences: failures remain observable until explicitly dismissed; successful outcomes and pending-cancellation results keep their contract. Repeated dismissal is a no-op. This provides API recovery without an implicit login, logout, credential refresh or service restart. diff --git a/structure/providers/oauth-code-display-login.md b/structure/providers/oauth-code-display-login.md index 46321b02a07..1112c6fbb5d 100644 --- a/structure/providers/oauth-code-display-login.md +++ b/structure/providers/oauth-code-display-login.md @@ -24,10 +24,11 @@ unchanged. lookup is provider-scoped (`loginFlowKeys` matches `(state.provider ?? key) === provider`), so status, cancellation, clearing and the admission cap never see another provider's flows. A cancel naming a `flowId` that belongs to a different provider is refused. -- **Failed-flow dismissal.** `cancelLoginFlow` also dismisses a settled failed attempt after its +- **Failed-flow dismissal.** The public cancellation endpoint opts into dismissing a settled failed attempt after its controller is gone. It drops only the selected provider-owned flow and its paste slot, so status returns to the remaining flow or idle. Active-account credentials and successful terminal outcomes remain untouched; cancelling a pending attempt still records its cancelled outcome. + The internal default still reports only live cancellation, preserving Codex flow ownership. - **Expiry and limits.** A code flow's paste window is `CODE_LOGIN_TTL_MS` (10 minutes); an expired flow is reaped as `no_pending_login` the next time that provider starts a login. At most `MAX_LOGIN_FLOWS_PER_PROVIDER` (64) rows per provider are kept; settled rows are evicted first diff --git a/tests/oauth/oauth-status-privacy.test.ts b/tests/oauth/oauth-status-privacy.test.ts index 88c0e3ad4eb..f998e9dc170 100644 --- a/tests/oauth/oauth-status-privacy.test.ts +++ b/tests/oauth/oauth-status-privacy.test.ts @@ -81,7 +81,8 @@ describe("OAuth status privacy", () => { loginState.set("pending-xai", { provider: "xai", flowId: "pending-xai", done: false }); expect(cancelLoginFlow("anthropic", "failed-xai")).toBe(false); expect(loginState.has("failed-xai")).toBe(true); - expect(cancelLoginFlow("xai", "failed-xai")).toBe(true); + expect(cancelLoginFlow("xai", "failed-xai")).toBe(false); + expect(cancelLoginFlow("xai", "failed-xai", { dismissSettledFailure: true })).toBe(true); expect(loginState.get("pending-xai")?.done).toBe(false); expect(getLoginStatus("xai").status).toBe("pending"); });