From c0bcdc7e4ae1a29f6856c2e10a8768ce1450ea58 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Fri, 28 Aug 2026 05:20:35 -0400 Subject: [PATCH] fix(tui): forward the clear promise from App, so AuthTab can actually await it (#2144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 12, and the best catch of the review. `onClearOAuth` was wired as `() => { void handleClearOAuth(); }`, so the promise `AuthTab` awaits resolved instantly. Everything round 6 through 9 built on that await was therefore inert in production while still passing its own tests: the pending line never showed, the repeat lock released immediately, and a rejection went unhandled instead of reaching the failure line. Passed directly now. The regression test drives the real App rather than `AuthTab` in isolation — the wiring is the contract, and only an App-level test can see it. Verified it fails when the void-ing arrow is put back. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh Signed-off-by: cliffhall --- clients/tui/__tests__/App.test.tsx | 33 ++++++++++++++++++++++++++++++ clients/tui/src/App.tsx | 9 +++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/clients/tui/__tests__/App.test.tsx b/clients/tui/__tests__/App.test.tsx index f5d56734..0a57d854 100644 --- a/clients/tui/__tests__/App.test.tsx +++ b/clients/tui/__tests__/App.test.tsx @@ -1412,6 +1412,39 @@ describe("App (mid-session auth lifecycle events)", () => { await expectFrame(r, "unreachable"); }); + // The wiring is the contract here, not just `AuthTab`'s own behavior: a + // `void`-ing arrow between them resolves instantly, which makes the pending + // state, the repeat lock and the rejection path all inert while revocation + // is still running. Driven through the real App so the arrow cannot come + // back (#2144). + it("holds the pending state until the clear actually settles", async () => { + let settle: () => void = () => {}; + h.clientSpies.clearOAuthTokens.mockImplementation( + () => + new Promise((resolve) => { + settle = () => + resolve({ status: "skipped", reason: "no_endpoint" as const }); + }), + ); + const r = await mount(oneHttp()); + await press(r, ["a", "s"]); + await waitUntil(() => h.clientSpies.clearOAuthTokens.mock.calls.length > 0); + await tick(); + + // Still running: the pending line is up and the confirmation is not. + expect(r.lastFrame() ?? "").toContain("Clearing OAuth state"); + expect(r.lastFrame() ?? "").not.toContain("OAuth state cleared"); + + // And the repeat lock is live, so a second press does not start another. + await press(r, ["s"]); + expect(h.clientSpies.clearOAuthTokens).toHaveBeenCalledTimes(1); + + settle(); + await waitUntil(() => + (r.lastFrame() ?? "").includes("OAuth state cleared"), + ); + }); + it("says nothing when there was nothing to revoke", async () => { const r = await mount(oneHttp()); await press(r, ["a", "s"]); diff --git a/clients/tui/src/App.tsx b/clients/tui/src/App.tsx index 48e75bab..9270c198 100644 --- a/clients/tui/src/App.tsx +++ b/clients/tui/src/App.tsx @@ -1899,9 +1899,12 @@ function App({ focused={ focus === "tabContentList" || focus === "tabContentDetails" } - onClearOAuth={() => { - void handleClearOAuth(); - }} + // Passed directly, NOT wrapped in a `void`-ing arrow: AuthTab + // awaits this to hold its pending state, keep its repeat lock, + // and route a rejection to the failure line. Dropping the + // promise here would resolve it instantly and make all three + // inert while revocation was still running (#2144). + onClearOAuth={handleClearOAuth} connectionStatus={inspectorStatus} /> ) : null}