fix: no loopback return listener when the browser is not on this machine - #99
Conversation
runCheckout started its 127.0.0.1 listener and handed Stripe its return URLs whenever the terminal could wait, including with --no-browser and inside SSH sessions. In both cases the browser runs elsewhere, so after paying the user landed on a connection-refused page for a port on the wrong machine; the terminal still recovered by polling the plan, but the landing was wrong. --no-browser now means what it says and SSH_CONNECTION / SSH_TTY / SSH_CLIENT mean the same thing: no listener, no return URLs, Stripe returns to the console billing page, and the plan poll is the only signal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
LGTM: Correct, well-scoped fix: listener skipped when the browser cannot be local, plan poll remains the completion signal; tests cover both paths.
- [nit] F1 src/billing/checkout.ts:102 — isRemoteTerminal called directly instead of via CheckoutDeps; tests mutate process.env and the suite is env-sensitive over SSH
- [nit] F2 src/billing/checkout.ts:101 — --no-browser on a local machine loses the explicit cancel signal (now ends as 10-min timeout instead of 'canceled') — deliberate per PR, worth a comment
Verdict: approve — the fix is correct and well-tested; two non-blocking nits.
What the PR does (3 files, +31/−7, all covered): runCheckout now only starts the loopback return listener when the browser can actually be on this machine — browserIsHere = !opts.noBrowser && !isRemoteTerminal() — where isRemoteTerminal() (new in src/utils/env.ts) checks SSH_CONNECTION/SSH_TTY/SSH_CLIENT. With no listener, the checkout body carries no successUrl/cancelUrl, so Stripe returns to the console page and the existing plan-poll loop is the completion signal — a path the code already handled (listener: null fallback and its test predate this change). This correctly fixes the connection-refused landing after paying from --no-browser or an SSH session.
Findings
- F1 (nit)
src/billing/checkout.ts:102—isRemoteTerminal()is called directly rather than throughCheckoutDeps, even though the function takes an injectableenvparam thatrunCheckoutnever uses. Consequence: the new SSH test mutatesprocess.env(with careful restore, fine), but every other test in the suite implicitly depends onSSH_CONNECTIONnot being set — running the suite over SSH would make e.g.sends the listener URLs…fail. Adeps.isRemotehook (or reading env via deps) would make the suite hermetic. - F2 (nit)
src/billing/checkout.ts:101— with--no-browseron a genuinely local machine (user just prefers pasting the URL into their own browser), the cancel signal is now unreachable: an explicit Stripe cancel ends as a 10-minutetimeoutinstead ofcanceled. The PR frames this as intended ("--no-browser says it does not [run here]"), which is a reasonable reading — just worth the one-line comment already present staying honest about that tradeoff.
Test guard (judged by hand — no specs:coverage script, no docs/reference/specs/): the old honours --no-browser test was retitled and rewritten; the new version asserts strictly more (no browser opened, body has no return URLs, flow completes on the plan flip) — refactor, verification strengthened, not weakened. The new SSH test mirrors it. Both tests match the fix's claims.
Reviewed at head c098d0d.
|
Release receipt — shipped in v0.2.36 (cut-release 34921739135, release 34921773207, both green). Published |
With
--no-browseror inside an SSH session the checkout flow still asked Stripe to return to a listener on127.0.0.1, which is the wrong machine. It now skips the listener there and lets Stripe return to the console page.What & why
Found while bug-hunting after #94 shipped.
runCheckoutstarted the loopback listener whenever the terminal could wait, and passed its URLs assuccessUrl/cancelUrl.--no-browserprints the URL for a browser elsewhere, and an SSH session cannot host the browser at all, so after paying the user landed on a connection-refused page for a port on another machine. The terminal still recovered, because the plan poll runs regardless, but the landing was broken and the Stripe cancel could never be seen.Tour
1. One condition decides whether the listener exists
--no-browsersays the browser is not here;SSH_CONNECTION/SSH_TTY/SSH_CLIENTmean it cannot be. Without a listener the checkout body carries no return URLs, so the API and Stripe fall back to the console billing page, and the plan poll is the only completion signal, exactly like the pre-#3021 behaviour.cli/src/billing/checkout.ts
Lines 97 to 103 in c098d0d
2. The SSH check
Environment-driven and injectable for tests.
cli/src/utils/env.ts
Lines 55 to 60 in c098d0d
3. Tests
The
--no-browsercase now asserts the body has no return URLs and the flow still completes on the plan flip; a new SSH case does the same withSSH_CONNECTIONset. Both fail against main'scheckout.ts.cli/test/billing-checkout.test.ts
Lines 234 to 253 in c098d0d
4. Remaining changes
None.
Validation
npm test504 pass; the two changed tests fail with main's handler (verified by swapping the file); typecheck against the live spec and lint clean.polylane subscription upgrade --plan starteron a UAT Free workspace prints the URL, the browser elsewhere pays, Stripe lands on the console billing page, and the terminal printsUpgraded to Starter.within the poll window.🤖 Generated with Claude Code