From fd2f1cd03d6d99f2d27280ee12aed9a40a4b572e Mon Sep 17 00:00:00 2001 From: JUN Date: Fri, 18 Sep 2026 06:47:15 +0900 Subject: [PATCH] fix(reauth): preserve polling through cancellation HTTP races --- gui/src/components/use-main-device-reauth.ts | 62 ++++++++++--------- .../main-device-reauth-ownership.test.tsx | 32 ++++++++++ structure/design-methodology.md | 2 +- structure/gui-and-management-api.md | 2 +- structure/overview.md | 2 + 5 files changed, 68 insertions(+), 32 deletions(-) diff --git a/gui/src/components/use-main-device-reauth.ts b/gui/src/components/use-main-device-reauth.ts index ecaf3eb0ac3..626e004a5c1 100644 --- a/gui/src/components/use-main-device-reauth.ts +++ b/gui/src/components/use-main-device-reauth.ts @@ -182,7 +182,8 @@ export function useMainDeviceReauth(apiBase: string, onCompleted: () => void) { if (!isCurrent() || flowRef.current !== flowId) return; if (!res.ok) { // Ownership continues from the Cancel click, including while DELETE - // is unresolved. Never offer a replacement POST during that window. + // is unresolved. Never offer a replacement POST during that window, + // and keep the existing poll cadence so a later terminal status remains observable. const cancellationRequested = cancellationRequestedFlowRef.current === flowId; setState(current => { if (!isCurrent() || flowRef.current !== flowId) return current; @@ -191,35 +192,36 @@ export function useMainDeviceReauth(apiBase: string, onCompleted: () => void) { ? current : { phase: "failed", code: failureCode(dto.code) }; }); - return; - } - lastUrl = allowedVerificationUrl(dto.verificationUrl) || lastUrl; - lastCode = humanCode(dto.deviceCode) || lastCode; - if (dto.status === "pending" || dto.status === "committing") { - const pendingState: CancellableState = { - phase: dto.status, - flowId, - verificationUrl: lastUrl, - deviceCode: lastCode, - }; - lastCancellableStateRef.current = pendingState; - setState(current => (current.phase === "pending" || current.phase === "committing") - && current.flowId === flowId && current.cancelFailed - ? { ...pendingState, cancelFailed: true } - : pendingState); - } else if (dto.status === "succeeded") { - flowRef.current = null; - setState({ phase: "succeeded" }); - onCompleted(); - return; - } else if (dto.status === "cancelled") { - flowRef.current = null; - setState({ phase: "cancelled" }); - return; - } else if (dto.status === "failed") { - flowRef.current = null; - setState({ phase: "failed", code: failureCode(dto.code) }); - return; + if (!cancellationRequested) return; + } else { + lastUrl = allowedVerificationUrl(dto.verificationUrl) || lastUrl; + lastCode = humanCode(dto.deviceCode) || lastCode; + if (dto.status === "pending" || dto.status === "committing") { + const pendingState: CancellableState = { + phase: dto.status, + flowId, + verificationUrl: lastUrl, + deviceCode: lastCode, + }; + lastCancellableStateRef.current = pendingState; + setState(current => (current.phase === "pending" || current.phase === "committing") + && current.flowId === flowId && current.cancelFailed + ? { ...pendingState, cancelFailed: true } + : pendingState); + } else if (dto.status === "succeeded") { + flowRef.current = null; + setState({ phase: "succeeded" }); + onCompleted(); + return; + } else if (dto.status === "cancelled") { + flowRef.current = null; + setState({ phase: "cancelled" }); + return; + } else if (dto.status === "failed") { + flowRef.current = null; + setState({ phase: "failed", code: failureCode(dto.code) }); + return; + } } } catch { if (!isCurrent() || flowRef.current !== flowId) return; diff --git a/gui/tests/main-device-reauth-ownership.test.tsx b/gui/tests/main-device-reauth-ownership.test.tsx index 93a9637ee62..917cb5f80d1 100644 --- a/gui/tests/main-device-reauth-ownership.test.tsx +++ b/gui/tests/main-device-reauth-ownership.test.tsx @@ -329,6 +329,38 @@ test("two successful cancellation replies complete the same flow only once", asy }); for (const phase of ["pending", "committing"] as const) { + test.each(["poll-first", "cancel-first"] as const)(`raced polling HTTP error and cancellation failure still observe ${phase} completion (%s)`, async order => { + await mount(); + await beginFlow("A"); + if (phase === "committing") { + await act(async () => { for (const wake of sleepers.splice(0)) wake(); }); + await reply(take("GET", "A"), { status: "committing" }); + } + await act(async () => { for (const wake of sleepers.splice(0)) wake(); }); + const poll = take("GET", "A"); + await invoke(() => hook.cancel()); + const cancellation = take("DELETE", "A"); + + if (order === "poll-first") { + await reply(poll, { code: "unavailable" }, 503); + await reply(cancellation, { code: "unavailable" }, 503); + } else { + await reply(cancellation, { code: "unavailable" }, 503); + await reply(poll, { code: "unavailable" }, 503); + } + expect(hook.state).toEqual({ + phase, flowId: "A", verificationUrl: "https://auth.openai.com/codex/device", + deviceCode: "ABCD-1234", cancelFailed: true, + }); + expect(requests.filter(pending => pending.method === "POST")).toHaveLength(1); + + await act(async () => { for (const wake of sleepers.splice(0)) wake(); }); + await reply(take("GET", "A"), { status: "succeeded" }); + expect(hook.state.phase).toBe("succeeded"); + expect(completed).toBe(1); + expect(requests.filter(pending => pending.method === "POST")).toHaveLength(1); + }); + for (const failure of ["network", "http", "nonterminal"] as const) { test.each(["poll-first", "cancel-first"] as const)(`raced polling HTTP error keeps ${phase} cancellation ${failure} retryable in the card (%s)`, async order => { await mount(false, true); diff --git a/structure/design-methodology.md b/structure/design-methodology.md index 5d5baac9704..0daf6382e96 100644 --- a/structure/design-methodology.md +++ b/structure/design-methodology.md @@ -55,4 +55,4 @@ Cline uses the existing file-integration page, tabs, status badge and rollback d Account quota surfaces use [safe probe diagnostics](transports/inventory.md#account-quota-failure-diagnostics) separately from quota validity, credential health and routing authority. -Native-main reauthentication separates polling lifetime from flow ownership: a non-2xx GET stops polling, while cancellation requested for the same flow preserves the pending/committing device state even before DELETE settles. A retryable DELETE failure preserves or restores Cancel retry without a second login POST. Trusted terminal results release ownership. +Native-main reauthentication separates polling lifetime from flow ownership: a non-2xx GET normally stops polling, while cancellation requested for the same owned flow preserves the pending/committing device state and existing polling cadence even before DELETE settles. A retryable DELETE failure preserves or restores Cancel retry without a second login POST, and later trusted terminal results remain observable and release ownership. diff --git a/structure/gui-and-management-api.md b/structure/gui-and-management-api.md index 5d992a10468..80841acfdf0 100644 --- a/structure/gui-and-management-api.md +++ b/structure/gui-and-management-api.md @@ -344,7 +344,7 @@ single forms, and the shell pattern is the part worth keeping stable: | Subagents | Featured-roster selection workspace (`gui/src/components/subagents-workspace/`). | | Combos | Rail, detail panel, and an add flow (`gui/src/components/ComboWorkspace.tsx`). | | Add provider | Catalog browser plus form and OAuth panes (`gui/src/components/provider-catalog/`, `gui/src/components/AddProviderModal.tsx`). The catalog browses four tabs — Accounts, Free, Local, Paid — where Local is a catalog-only bucket peeled out of `bucketPresets` after `presetTier` has classified; the workspace `providerTier` stays three-way, so the rail, the free-paid sort and the Free count still treat a local runtime as free. Search sits above the tabs and reaches every tab at once: while a query is live the list renders all four groups with headings and the strip becomes jump chips with counts rather than a tablist, because moving the selected tab would change the row kind under the user (a preset-select button becomes a login row). ArrowDown from the search input focuses the first enabled result action; if none is available, focus stays in the input. The tab strip wraps within narrow modals. Every nonempty note has a full-text button so narrow rows never hide content permanently; the native note dialog closes during teardown and restores focus to its trigger. Provider notes clamp to two lines and open in full in a stacked native `` owned by `AddProviderModal`, which also owns the search text so its `window` Escape handler can unwind popup, then query, then dialog. | -| Codex accounts | Account pool cards, add-account flow, switch and reset modals (`gui/src/components/CodexAccountPool.tsx`, `gui/src/components/AddCodexAccountModal.tsx`), plus the generic account-targeting picker opt-in on `gui/src/pages/codex-set-multiauth.tsx`. Add/delete/login completion is projected to one boolean before presentation; pending catalog work is a warning, not a failed account mutation. The main card's native-main device reauth (#3898) is owned by `gui/src/components/use-main-device-reauth.ts`: the dedicated `/api/codex-auth/main/reauth-device` namespace only — never the pool login route — with flowId-owned polling, an allowlisted verification URL, and no token fields accepted from payloads. The main-device reauth hook retains flow ownership from the Cancel click, while DELETE is unresolved and after retryable failure; polling normally continues. A concurrent GET HTTP error cannot expose a replacement login POST before DELETE settles. If a retryable DELETE failure races with a non-2xx GET while the flow is pending or committing, either response order preserves same-flow Cancel retry and restores the last server-provided device code, verification URL, and phase when needed. The GET HTTP failure still stops polling without starting a second login POST. The cancellation-failure indication survives pending status updates until a trusted terminal result releases ownership. A successful DELETE with a terminal `failed` DTO releases it and uses the same closed failure-code mapping as polling; only `succeeded` notifies login completion. Unrecognized or nonterminal DTO status values remain retryable. A DELETE response with HTTP 404 and code `unknown_flow` releases the expired flow and shows the existing generic failure state so device re-login is available again; it claims neither login success nor confirmed cancellation. Confirmed cancellation also makes device re-login available. Start, polling and cancellation completions verify their controller or flow ownership after asynchronous response reads; replaced flows and unmounted hooks cannot update a newer flow or notify completion. Effect setup restores mounted state after the StrictMode development cleanup cycle. | +| Codex accounts | Account pool cards, add-account flow, switch and reset modals (`gui/src/components/CodexAccountPool.tsx`, `gui/src/components/AddCodexAccountModal.tsx`), plus the generic account-targeting picker opt-in on `gui/src/pages/codex-set-multiauth.tsx`. Add/delete/login completion is projected to one boolean before presentation; pending catalog work is a warning, not a failed account mutation. The main card's native-main device reauth (#3898) is owned by `gui/src/components/use-main-device-reauth.ts`: the dedicated `/api/codex-auth/main/reauth-device` namespace only — never the pool login route — with flowId-owned polling, an allowlisted verification URL, and no token fields accepted from payloads. The main-device reauth hook retains flow ownership from the Cancel click, while DELETE is unresolved and after retryable failure; polling normally continues. A concurrent GET HTTP error cannot expose a replacement login POST before DELETE settles. If a retryable DELETE failure races with a non-2xx GET while the flow is pending or committing, either response order preserves same-flow Cancel retry, restores the last server-provided device code, verification URL, and phase when needed, and keeps the existing poll cadence so a later terminal result remains observable. Outside same-flow cancellation ownership, a GET HTTP failure still stops polling without starting a second login POST. The cancellation-failure indication survives pending status updates until a trusted terminal result releases ownership. A successful DELETE with a terminal `failed` DTO releases it and uses the same closed failure-code mapping as polling; only `succeeded` notifies login completion. Unrecognized or nonterminal DTO status values remain retryable. A DELETE response with HTTP 404 and code `unknown_flow` releases the expired flow and shows the existing generic failure state so device re-login is available again; it claims neither login success nor confirmed cancellation. Confirmed cancellation also makes device re-login available. Start, polling and cancellation completions verify their controller or flow ownership after asynchronous response reads; replaced flows and unmounted hooks cannot update a newer flow or notify completion. Effect setup restores mounted state after the StrictMode development cleanup cycle. | | Dashboard overview | Overview, Providers, and Models tabs at the page level (`gui/src/pages/Dashboard.tsx`), the 30-day token and coverage stats in the overview head (`gui/src/pages/dashboard-overview-head.tsx`), and the effort-cap, injection, maintenance, sidecar, and memory panels below it (`gui/src/pages/dashboard-overview-panels.tsx`). | The native-main reauth poller captures an immutable accepted flow id for queued callbacks. diff --git a/structure/overview.md b/structure/overview.md index a6cb5206c85..39e11fb7d13 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -154,6 +154,8 @@ Raw reasoning content and provider-authored summaries remain distinct on the Res Connected-browser pairing and dashboard failure meanings follow the [management UI contract](gui-and-management-api.md#dashboard-surfaces); machine enrollment alone does not authenticate a browser. +Native-main reauthentication keeps its existing polling cadence when a non-2xx status races with retryable cancellation for the same owned flow; the [dashboard flow-ownership contract](gui-and-management-api.md#dashboard-surfaces) defines terminal release and completion notification. + Cline CLI is a managed file integration: its provider settings and catalog share one recoverable journal operation. The [paired-file contract](clients/integrations.md#cline-paired-files) defines its stop/restart requirement. Pool quota producers and account commands follow the [bounded raw-observation contract](providers/openai-tiers.md#bounded-pool-quota-observations), separate from the latest display snapshot and capacity estimates.