Tell the user to run kcap login when a hook is rejected with HTTP 401 - #513
Conversation
A credential the local token store believes is usable but the server rejects surfaces today as an opaque Claude hook-error banner (bare "HTTP 401", exit 1) or, on session-start/session-end, as complete silence. Neither says recording has stopped, and neither says `kcap login`. Spec rides the implementation PR per the repo spec convention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five tasks: collect the notice wording into a Core type (verbatim move of the two existing strings), nudge from the shared stop path, nudge from the session-start arm that drops in silence today, make the vendor stderr line actionable, then README plus full-suite and AOT verification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tightens the AI-1835 branch before PR: replaces a NotInParallel key that protected nothing with a bare serialization, corrects the README and an AgentHookPoster doc comment now made false by the change, folds the duplicated 401-vs-other stderr ternary into AuthLapseNotice.VendorStderrLine so the two call sites can't diverge, and adds coverage for the pre-flight auth-lapse arm of ClaudeHookCommand.HandleCore that had none. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR Summary by QodoShow 'kcap login' guidance when hooks get HTTP 401 (Claude + vendors)
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99af1235d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| error, so you no longer have to run `kcap whoami` to work out why recording stopped. Other agents' | ||
| hooks print the same advice to stderr instead of an in-session notice, since not every agent |
There was a problem hiding this comment.
When a locally valid credential is rejected with HTTP 401 during a Cursor session, this promise does not hold: CursorHookCommand bypasses AgentHookPoster, and its direct POST paths only convert non-success responses into DrainOutcome.Drop or false without writing the new advice to stderr. Cursor users therefore still receive no explanation or kcap login recovery step—the exact failure this change is intended to address—so the Cursor POST path should recognize 401 as well (or the documentation and claimed vendor scope must exclude Cursor).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99af1235d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| error, so you no longer have to run `kcap whoami` to work out why recording stopped. Other agents' | ||
| hooks print the same advice to stderr instead of an in-session notice, since not every agent |
There was a problem hiding this comment.
When a locally valid credential is rejected with HTTP 401 during a Cursor session, this promise does not hold: CursorHookCommand bypasses AgentHookPoster, and its direct POST paths only convert non-success responses into DrainOutcome.Drop or false without writing the new advice to stderr. Cursor users therefore still receive no explanation or kcap login recovery step—the exact failure this change is intended to address—so the Cursor POST path should recognize 401 as well (or the documentation and claimed vendor scope must exclude Cursor).
Useful? React with 👍 / 👎.
Code Review by Qodo
1. AI-1835 in docs
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| **Issue:** #509 / AI-1835 | ||
| **Siblings:** #510 (a 401'd lifecycle payload is dropped, not spooled), #511 (the daemon does not |
There was a problem hiding this comment.
1. ai-1835 in docs 📘 Rule violation ⚙ Maintainability
New documentation includes Linear issue identifiers (AI-1835), which the checklist disallows in repo comments/docs. This can leak internal tracking references and reduces long-term stability of public-facing docs.
Agent Prompt
## Issue description
New markdown docs reference Linear IDs (e.g., `AI-1835`), which violates the rule to avoid Linear identifiers in comments/docs.
## Issue Context
Use GitHub issue/PR references (e.g., `#509`) when an issue reference is necessary.
## Fix Focus Areas
- docs/superpowers/specs/2026-08-10-hook-401-login-nudge-design.md[5-6]
- docs/superpowers/plans/2026-08-10-hook-401-login-nudge.md[11-13]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Cursor was the one vendor the nudge missed. It does not route its recording POST through AgentHookPoster — it POSTs directly and uses the poster only for the IsAuthLapsed predicate — so a 401 returned false/DrainOutcome.Drop in silence, leaving Cursor users with exactly the unexplained failure this change exists to remove. TryPostHookAsync (the live path) now writes the same stderr line. The spool-drain lambda stays silent on purpose: it replays many entries per pass and would repeat the line for each one. Found by the Codex PR reviewer. The design doc had asserted Cursor shared the poster; corrected there with a note rather than a silent rewrite. Also trims two comment blocks flagged as over-narrated, keeping the load-bearing rationale (why exit 0, why stop-only, why this is the arm's only stdout write). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bot review responses✅ Codex — "Emit the login advice from Cursor hooks" (P2): valid, fixed in 0e0d150This was a real gap and the reasoning behind it was mine, not the implementation's. For the record on how it got through: an earlier task review raised exactly this as a "cannot verify from diff" item and asked for a spot-check of the vendor call sites. I dismissed it on the grounds that both poster entry points were edited so the wiring didn't matter — which only holds if the vendor calls one of them. Cursor doesn't. Fix: The design doc asserted Cursor shared the poster; corrected there with a dated note rather than a silent rewrite. ❌ Qodo — "Linear issue identifiers in docs": not applicable in this repoThe rule this cites is There is a reason for the asymmetry: the team convention is that the reviewed spec lives on the Linear issue as the durable record, with the repo copy riding the implementation PR. A spec that cannot name its own Linear issue cannot be traced back to that record. The GitHub number No change. ◐ Qodo — "Overly verbose new comments": partly fair, trimmed in 0e0d150Trimmed both flagged blocks in Left Verification after the fix: The caveat in the PR description still stands and is the one thing worth a human's attention: the |
…-hook-401-login-nudge
#516 landed AuthRejectionNotice on main while this branch was in review, carrying a StoredCredentialState vocabulary whose LooksValid case is exactly this branch's "the server rejected a locally-valid credential". Two near-identically-named auth-notice types is the drift the original type existed to prevent, so there is now one. The surfaces keep different renderings of the shared states, because their length budgets genuinely differ: Render() stays the MCP form (several sentences in a tool result), RecordingNotice() is the one-line form a Claude systemMessage and a vendor stderr line can carry. FromAuthStatus() maps the AuthStatus the hook already holds onto the states, so the per-turn hook path pays none of the disk reads ForPersistentUnauthorizedAsync makes to classify. No wording changes: every rendered string is byte-identical, including WrongServer still rendering as the not-authenticated line in the short form. Naming both servers there would be more truthful and is now a one-line follow-up, but it is a behaviour change and not this PR's. AuthLapseNoticeTests folded into AuthRejectionNoticeTests, plus coverage for the AuthStatus mapping and the WrongServer short-form choice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Folded into
|
| Member | Surface | Form |
|---|---|---|
Render(state, stored, target) |
MCP tool results | several sentences (unchanged) |
RecordingNotice(state) |
Claude systemMessage, vendor stderr |
one line |
VendorStderrLine(tag, endpoint, code) |
vendor stderr | status line, enriched only on 401 |
FromAuthStatus(status) |
hook pre-flight arm | maps the AuthStatus the hook already holds |
Sharing the enum rather than the prose is deliberate: the states are what must not drift, and a five-sentence paragraph inside a systemMessage would be unreadable. FromAuthStatus matters for a second reason — the hook path is per-turn and budget-bounded, so it must not pay the two disk reads ForPersistentUnauthorizedAsync makes just to name a state it already knows.
No wording changed. Every rendered string is byte-identical, including WrongServer still rendering as the not-authenticated line in the short form. Naming both servers there (as Render does) would be more truthful and is now a one-line change, but it is a behaviour change and not this PR's to make.
AuthLapseNoticeTests folded into AuthRejectionNoticeTests, plus new coverage for the AuthStatus mapping and for the WrongServer short-form choice so it can't be changed silently.
Verification: AuthRejectionNoticeTests 12/12, ClaudeHookCommandTests 44/44, AgentHookPosterTests 9/9, CursorHookCommandTests 42/42, AOT publish clean.
The CI failure is not from this branch
NotAuthenticated_MakesNoRequest_AndReturnsZero (added by #517 / AI-1283, not touched here) fails on main's own CI at tip commit 032f477d9 with the identical assertion, so it is red independently of this PR.
It's order-dependent rather than broken logic: it asserts no HTTP request is made when unauthenticated, but AppConfig.SetResolvedState and the token store are process-global statics and the test carries no NotInParallel. Any test running concurrently that seeds auth makes it issue the request it asserts never happens. It passes 10/10 in isolation. Worth someone's attention as a separate issue — it will redden every PR until it's isolated.
The ubuntu leg also showed McpTelemetryTests.No_argument_data_is_carried (Single() on an empty sink) once — same species, a static TelemetryState.PathOverride guarded only by a keyed NotInParallel. It did not appear in any of four local full-suite runs, nor on main's ubuntu leg. Watching whether it recurs.
Current main is merged in as of 3d7caff.
1 participant
Closes #509
AI-1835
The bug
A credential lapsed mid-session and Claude Code showed only this:
Nothing said recording had stopped, and nothing said
kcap login. Finding out took a hunch and a manualkcap whoami.There are two distinct auth lapses, and only one was handled:
Expired/NotAuthenticated/WrongServer)systemMessagenudge — but only onsession-startThis was the second.
CreateClientWithAuthStatusAsyncreportsAuthStatus.Okfor any locally-valid token, and the hook path builds its client withautoRetryUnauthorized: false, so the 401 was final — it fell to the shared failure arm, which wrote a bareHTTP 401to stderr and returned 1. Claude Code renders any non-zero, non-2 hook exit as that opaque banner. The other events were worse than opaque:session-start,session-endandsubagent-stopclass a 401 as permanent, drop the payload, and return 0 without a word.The fix
A 401 becomes a recognized outcome instead of a generic failure.
stopandsession-startexit 0 and write{"systemMessage": "[kcap] The server rejected your credentials (HTTP 401) — session recording is paused. Run 'kcap login' to resume."}. Exit 0 is load-bearing: it's what replaces the hook-error banner with a clean notice.notification,subagent-start) exit 0 silently.notificationfires on every permission prompt, so nudging there would stack duplicate notices inside one turn. No throttle state on disk is needed as a result.systemMessage: their stdout is a strict handshake contract the vendor parses. Their stderr line now names the fix instead. BothAgentHookPosterentry points were edited, so it applies whichever one a vendor uses.AuthLapseNotice, so the pre-flight nudge and the server-rejection nudge can't drift apart.decision: "block"was deliberately not used. It's the only way to hand text to the agent from aStophook, but it stops Claude from stopping and costs an extra model turn per lapsed turn. The user is the one who runskcap login, so the notice goes to the user.401 only — a 403 is an authorization decision, not a dead credential, and
kcap loginwouldn't fix it.Deliberately out of scope
kcap logincannot recover the session #510). Because 401 is classed permanent, thesession-start/session-endevent that hit the 401 is already gone — logging in doesn't recover it. Making 401 retryable means changing the drop rule inHookSpool/LifecycleSpoolDrainfor every vendor, and it contradicts the existing reasoning atAgentHookPoster.cs:212. Separate change, real blast radius.kcap login(Daemon does not reliably pick up a new credential afterkcap login(live hub pins handshake identity; profile/tenant change never heals) #511). A live SignalR hub pins its handshake identity, and a profile/tenant change yieldsWrongServerforever without explaining itself.GetValidTokensForServerAsyncalready refreshes proactively, so a 401 after it means the server rejected a fresh token — a revoked session or an org mismatch, neither of which a refresh heals — and WorkOS refresh tokens are single-use.Tests
New coverage:
stop/401 → exit 0 + notice;session-start/401 → exit 0 + notice;notification/401 → exit 0, no notice;stop/500 → exit 1, no notice; the vendor 401 stderr line (and its non-401 bare-status counterpart); and the pre-flight lapse arm, which turned out to have had no test at all despite the design assuming one — including theWrongServer→ "not authenticated" mapping.ClaudeHookCommandTests44/44,AgentHookPosterTests9/9,AuthLapseNoticeTests5/5, integration 217/217.dotnet publish -c Releaseshows no IL3050/IL2026.The wider unit suite has ~63 failures on
mainitself in a local macOS environment (Codexconfig.toml,kcap uninstall, CLI-runner flood tests, daemon teardown/quarantine timing), churning in both directions run-to-run; this branch measured 62-66 with none in hook, 401, or auth-lapse tests. CI is the authority on those.One thing a reviewer should know
The design rests on Claude Code honouring
systemMessagefor theStopevent. That's documented in the installed CLI bundle ("systemMessage — Display a message to the user (all hooks)", with a workedStopexample), and kcap already ships asystemMessagewrite on thesession-startpath. It has not been confirmed end-to-end interactively, because headless-pdoesn't fireStophooks at all. If the contract doesn't hold forStop, the failure is silent — the user sees nothing where they previously saw an opaque banner. Thesession-starthalf is an improvement either way, since it was previously silent.🤖 Generated with Claude Code