Repository navigation
fix(listen): stop when Hookdeck ends the key's access, and show the MFA setup link - #477
rahulbhadja wants to merge 1 commit into
Conversation
…FA setup link listen reconnected forever after a revoke (4003) or a refused reconnect (401/403). It now stops, says why and exits 1. MFA refusals show the dashboard link to set it up. Co-Authored-By: Claude <noreply@anthropic.com>
|
@leggetter FYI, this stops |
leggetter
left a comment
There was a problem hiding this comment.
Approving. I ran the new unit tests with each fix reverted and they go red, so they're guarding the right things.
The slice 0 acceptance failure is the MCP harness flake (no JSON-RPC response with id 2), not this change. It's shown up on other PRs and passes locally. I'll re-run it before merging.
Not blocking: is an acceptance test against the built binary worth adding? Point --ws-base at a fake websocket server under the listen tag and check for exit 1, the message printed once and a single connect. The unit tests call proxy.Run() directly, so they don't cover that path.
| case http.StatusUnauthorized: | ||
| return "This CLI key is no longer valid. Run `hookdeck login` to log in again." | ||
| default: | ||
| return "Hookdeck refused the connection: this CLI key can't use this project. Run `hookdeck project use` to pick another one, or `hookdeck login`." |
There was a problem hiding this comment.
Nit: a 403 from a corporate proxy or CDN lands here too, and until the API sends a reason, this is what everyone sees. Maybe state the status, e.g. Hookdeck refused the connection (HTTP 403), rather than blame the key?
What
hookdeck listenstops, with the reason, when Hookdeck won't accept its key again, instead of reconnecting forever. API errors for an organization that requires MFA now include the link to set it up.Why
listenretried every reconnect forever. When the key was revoked (close 4003), or a reconnect was refused with 401 (key revoked or expired) or 403 (no access to the project, or MFA required), it looped on "Connection lost, reconnecting..." and never said why.redirect), and the CLI dropped it.Change
pkg/websocket: a 401 or 403 on connect, or a 4003 close, is recorded asAccessEndedError, with the server's message when it sends one and a default otherwise. 4003 is no longer logged as an error with the "file a GitHub issue" hint.pkg/listen/proxy: when access has ended,listenreports it through the renderer, stops, and exits 1. 503, network drops, 1001, 1006 and 4001 still reconnect as before.pkg/hookdeckandpkg/cmd: API errors keep theredirectpath. A 403 whose path starts with/setup-mfaaddsSet it up here: <dashboard>/setup-mfa?.... Only a path is accepted, joined to the CLI's own dashboard address, so a response can't point the user to another site. The error decoder only takesredirectwhen it's a string, so other bodies decode as before.Example:
An upcoming API update adds the reason to refused connections (for example "This CLI key can't reach this project"), and the CLI shows it.
Testing
listenstops after one connect when refused, and when revoked while connected; the MFA redirect is kept and a non-string one is ignored; the setup link is built from the CLI's dashboard and ignored for other sites, other paths and other errors. Each fix's tests fail without it.go test -short ./pkg/...andgo vet ./...pass.listenfrom the dashboard stopped it with the revoked message (exit 1); removing the member stopped theirs with "This CLI key can't reach this project..." (exit 1); an MFA organization showed the setup link forlistenandgateway source list; restarting the server reconnected and kept listening.Not in this PR
listenreading its sources and starting its session.🤖 Generated with Claude Code