From 85e3215534a203a74e4225f599388632a52cead6 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Sun, 16 Aug 2026 22:11:40 -0400
Subject: [PATCH] fix(tui): retire the disconnect token on a server switch, not
 just clear
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Copilot round 3 (PR #2039): clearing the banner when `selectedServer`
changes is not enough. Start disconnecting A, switch A → B → A, and by
the time the original request rejects both guards match again — the
attempt token is unchanged and the selected server is A once more — so
the stale failure lands on the freshly re-selected A. Bump the token on
every switch, so a round trip invalidates prior disconnects the same way
a one-way switch does.

New test drives exactly that interleaving; it fails without the bump
(the one-way test does not, which is why this needed its own).

Also attaches the before/after TUI screenshots the review asked for —
the review was right that "no UI change" stopped being true once the
banner was added — and corrects the PR description accordingly.

Signed-off-by: cliffhall <cliff@futurescale.com>
---
 clients/tui/__tests__/App.test.tsx | 24 ++++++++++++++++++++++++
 clients/tui/src/App.tsx            |  6 +++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/clients/tui/__tests__/App.test.tsx b/clients/tui/__tests__/App.test.tsx
index 4f7a25c8..11b65af5 100644
--- a/clients/tui/__tests__/App.test.tsx
+++ b/clients/tui/__tests__/App.test.tsx
@@ -722,6 +722,30 @@ describe("App (foundation)", () => {
     expect(r.lastFrame() ?? "").not.toContain("stale-alpha-failure");
   });
 
+  it("drops a stale disconnect rejection across an A → B → A round trip", async () => {
+    // The server-name check alone passes here: by the time the rejection
+    // lands, alpha is selected again. Only retiring the attempt token on
+    // every switch catches it.
+    h.ctrl.status = "connected";
+    let rejectDisconnect: (err: Error) => void = () => {};
+    h.disconnect.mockImplementationOnce(
+      () =>
+        new Promise<void>((_resolve, reject) => {
+          rejectDisconnect = reject;
+        }),
+    );
+    const r = await mount(stdioServer());
+    r.stdin.write("d"); // alpha's disconnect starts, and hangs
+    await tick();
+    await press(r, [DOWN]); // alpha -> beta
+    await expectFrame(r, "b.js");
+    await press(r, [UP]); // beta -> alpha again
+    await expectFrame(r, "s.js");
+    rejectDisconnect(new Error("round-trip-failure"));
+    await waitUntil(() => (r.lastFrame() ?? "").includes("round-trip-failure"));
+    expect(r.lastFrame() ?? "").not.toContain("round-trip-failure");
+  });
+
   it("switches tabs via accelerator keys", async () => {
     const r = await mount(stdioServer());
     await press(r, ["t"]); // tools tab (server is auto-selected)
diff --git a/clients/tui/src/App.tsx b/clients/tui/src/App.tsx
index 6e04f04e..240bbd82 100644
--- a/clients/tui/src/App.tsx
+++ b/clients/tui/src/App.tsx
@@ -446,7 +446,11 @@ function App({
     setOauthStatus("idle");
     setOauthMessage(null);
     // The header banner is server-scoped, so a failure from the server we just
-    // left must not be read as this one's.
+    // left must not be read as this one's. Clearing is not enough: a switch
+    // away and back (A → B → A) would leave both the attempt token and the
+    // server name matching again, so the stale rejection would land on the
+    // re-selected A. Retiring the token on every switch is what closes that.
+    disconnectAttemptRef.current++;
     setDisconnectError(null);
     const stepUp = pendingStepUpRef.current;
     if (stepUp && selectedServer && stepUp.serverName !== selectedServer) {