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..703f090dc1c 100644 --- a/src/oauth/login-flow-state.ts +++ b/src/oauth/login-flow-state.ts @@ -297,14 +297,23 @@ 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. 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 (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); + 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/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 new file mode 100644 index 00000000000..a0846fb8176 --- /dev/null +++ b/structure/decisions/ADR-7378-oauth-failed-flow-dismissal.md @@ -0,0 +1,14 @@ +# 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, 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 b66b2e9b980..1112c6fbb5d 100644 --- a/structure/providers/oauth-code-display-login.md +++ b/structure/providers/oauth-code-display-login.md @@ -24,6 +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.** 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 @@ -53,3 +58,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..f998e9dc170 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,47 @@ 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(false); + expect(cancelLoginFlow("xai", "failed-xai", { dismissSettledFailure: true })).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",