[cliffhall]

Closes #2847
Closes #2843

What changed

#2847 — the loopback listener accepted the first request whatever its state.

• OAuthCallbackServer.start() takes a new expectedState option. A callback whose state does not match — a success or an ?error= response — gets a short 400 ("this callback does not belong to the sign-in in progress and was ignored"), onCallback/onError are not called, and the listener keeps waiting until the matching redirect or the runner's timeout.
• expectedState is a promise because the listener must be bound (its URL is the redirect_uri) before the authorize URL — and so the state — exists. A callback that arrives before it settles is held and judged once it does (an AS that redirects instantly cannot race the runner); a held callback gets a 503 if the listener stops first. A rejected expectedState reads as "no flow" (503).
• runRunnerInteractiveOAuth hands the listener the state from the authorize URL: the step-up authorizationUrl before navigating, or the URL authenticate() returns.
• Authorization-server text is labeled: the page says "OAuth failed. The authorization server said: …" and the runner's error is OAuth authorization failed. The authorization server said: <description> (<error>). A listener-reported callback_error (our own exchange failure) stays unlabeled.
• Omitting expectedState keeps the old accept-anything behaviour, documented as only for a host that checks state itself; every production caller goes through the runner.

#2843 — non-callback requests got the "OAuth complete" page. The non-GET (405), unparseable-URL (400) and wrong-path (404) branches now send a "this address is not the OAuth callback… sign-in has not finished" page. The new 400, the 409 and the 503 have their own pages; none says sign-in finished.

Also: listener responses send Connection: close. A one-shot loopback listener gains nothing from keep-alive, and an idle kept-alive socket was holding stop()'s close() open (it took two pre-existing tests from 6s each to milliseconds).

clients/cli/__tests__/oauth-interactive.test.ts now forwards the AS redirect's state into the loopback callback, as a browser does; clients/cli/README.md gets one sentence on the state check.

Tests

• clients/web/src/test/integration/auth/node/oauth-callback-server.test.ts: page bodies for 404/405/400/409; state mismatch (success, error, missing state, JSON); held-then-matched callback; stateless flow; 503 on stop and on a rejected expectedState; and an end-to-end run of the real runner + real listener where a stateless forged ?error= lands first and the genuine callback still completes.
• clients/web/src/test/core/auth/runner-interactive-oauth.test.ts: the state hand-off on both paths, the labeled messages, the unlabeled callback_error, and the empty-error fallback.

Hand smoke

Against oauth-revocation-http.json (copied to port 18083) with the CLI waiting on 127.0.0.1:18001: / → 404 and POST → 405 with the not-the-callback page; ?error=access_denied&error_description=injected+by+another+process with no state → 400 "ignored", and the CLI kept waiting; ?code=forged&state=nope → 400; the genuine AS redirect → 200 "OAuth complete", and the CLI completed tools/list.

npm run local:gate green.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Redirect-supplied OAuth errors can bypass the authorization-server label, and the new shutdown integration test relies on a timing-sensitive sleep that can race under load.

2 open issues

• Distinguish local exchange failures from redirect-supplied errors · New
• Wait for request arrival before stopping the listener · New

What changed in this PR

This PR addresses #2847 and #2843 in the shared Node OAuth flow used by the CLI and TUI, filtering unrelated callbacks and correcting misleading response pages.

Changes:

• Passes the authorization URL’s state to the listener and holds early callbacks until that state is known.
• Adds status-specific pages, labels authorization-server errors, and closes response connections.
• Expands regression coverage and updates CLI documentation and callback simulation.

File	Description
core/​auth/​node/​runner-interactive-oauth.ts	Supplies expected state and formats callback errors.
core/​auth/​node/​oauth-callback-server.ts	Filters callbacks by state and provides status-specific responses.
clients/​web/​src/​test/​integration/​auth/​node/​oauth-callback-server.test.ts	Covers state matching, pending callbacks, response pages, and runner integration.
clients/​web/​src/​test/​core/​auth/​runner-interactive-oauth.test.ts	Tests state handoff and error-message formatting.
clients/​cli/​README.md	Documents callback state validation.
clients/​cli/​__tests__/​oauth-interactive.test.ts	Forwards state when simulating browser callbacks.

_{Review effort: Balanced}

[cliffhall]

Copilot round 1 (2 inline findings, both fixed in c58fcc8):

• Distinguish local exchange failures from redirect-supplied errors: fixed. The listener now reports source: "redirect" | "callback", and the runner labels every redirect error, including one spelled callback_error.
• Wait for request arrival before stopping the listener: fixed. Three new tests now wait on the listener's http.Server "request" event instead of a 20ms sleep.

No suppressed comments. local:gate is green on the new head. Requesting round 2.

[Copilot]

🔵 Needs a closer look

Each callback retains a new stop-signal promise reaction until sign-in ends, allowing repeated ignored requests to accumulate memory during the potentially 15-minute callback wait.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Reuse stop race to prevent promise reaction accumulation

core/​auth/​node/​oauth-callback-server.ts:299

Each callback creates a new Promise.race against stopSignal.stopped, leaving a promise reaction retained until sign-in ends even after a mismatched request has been answered. Repeated ignored requests therefore accumulate memory throughout the callback wait, which can last 15 minutes. Create the state/stop race once per start() and reuse its result across requests.

2 resolved since last review

• Distinguish local exchange failures from redirect-supplied errors
• Wait for request arrival before stopping the listener

_{Review effort: Balanced}

[cliffhall]

Copilot round 2: no inline comments. One "previously missed issue" was in the body, and it has no thread, so it is answered here.

• Each callback creates a new Promise.race against stopSignal.stopped (oauth-callback-server.ts): fixed in 2ce20aa. start() now races the expected state against the stop signal once and stores the result, and every request awaits that single promise. An ignored request no longer leaves a reaction on the stop signal for the rest of the wait. Held-callback-then-stop still answers 503, and the existing test covers it.

local:gate is green on the new head. Requesting round 3.

[Copilot]

🟢 Approval recommended

No active review issues remain for the OAuth callback state-validation and response-handling changes.

No open issues 🎉

_{Review effort: Balanced}

[cliffhall]

Copilot review loop closed. Round 3 on 2ce20aa was clean: no inline comments, no open or suppressed issues, approval recommended. Per the clean-round exit, no further round requested.