Skip to content

fix: no loopback return listener when the browser is not on this machine - #99

Merged
justinhelmer merged 1 commit into
mainfrom
fix/checkout-no-listener-remote
Sep 15, 2026
Merged

justinhelmer merged 1 commit into
mainfrom
fix/checkout-no-listener-remote

Conversation

@justinhelmer

Copy link
Copy Markdown
Contributor

With --no-browser or inside an SSH session the checkout flow still asked Stripe to return to a listener on 127.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. runCheckout started the loopback listener whenever the terminal could wait, and passed its URLs as successUrl/cancelUrl. --no-browser prints 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-browser says the browser is not here; SSH_CONNECTION / SSH_TTY / SSH_CLIENT mean 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.

const waits = deps.canWait(config);
// The loopback listener only helps when the browser runs on this machine.
// --no-browser says it does not, and an SSH session means it cannot; in
// both cases Stripe returns to the console page and the plan poll below
// is the only signal.
const browserIsHere = !opts.noBrowser && !isRemoteTerminal();
const listener = waits && browserIsHere ? await deps.startListener() : null;

2. The SSH check

Environment-driven and injectable for tests.

cli/src/utils/env.ts

Lines 55 to 60 in c098d0d

// A terminal reached over SSH cannot be the machine the browser runs on, so a
// loopback listener there would never be reached by a redirect.
export function isRemoteTerminal(env: NodeJS.ProcessEnv = process.env): boolean {
return Boolean(env.SSH_CONNECTION || env.SSH_TTY || env.SSH_CLIENT);
}

3. Tests

The --no-browser case now asserts the body has no return URLs and the flow still completes on the plan flip; a new SSH case does the same with SSH_CONNECTION set. Both fail against main's checkout.ts.

it('honours --no-browser: nothing opened, no loopback listener, Stripe returns to the console', async () => {
const h = harness({ planIds: ['free', 'starter'] });
const outcome = await runCheckout(mockConfig({ output: 'text', nonInteractive: false }), { ...base, noBrowser: true }, h.deps);
assert.equal(outcome, 'upgraded');
assert.deepEqual(h.opened, []);
assert.deepEqual(h.bodies, [{ workspaceId: 'ws_1', plan: 'starter', billingCycle: 'monthly' }]);
});
it('skips the loopback listener over SSH, where the browser is on another machine', async () => {
const prior = process.env.SSH_CONNECTION;
process.env.SSH_CONNECTION = '10.0.0.2 51000 10.0.0.1 22';
try {
const h = harness({ planIds: ['free', 'starter'] });
assert.equal(await runCheckout(mockConfig({ output: 'text', nonInteractive: false }), base, h.deps), 'upgraded');
assert.deepEqual(h.bodies, [{ workspaceId: 'ws_1', plan: 'starter', billingCycle: 'monthly' }]);
} finally {
if (prior === undefined) delete process.env.SSH_CONNECTION;
else process.env.SSH_CONNECTION = prior;
}
});

4. Remaining changes

None.

Validation

  • npm test 504 pass; the two changed tests fail with main's handler (verified by swapping the file); typecheck against the live spec and lint clean.
  • Human-gated: from an SSH session to a machine with the CLI, polylane subscription upgrade --plan starter on a UAT Free workspace prints the URL, the browser elsewhere pays, Stripe lands on the console billing page, and the terminal prints Upgraded to Starter. within the poll window.

🤖 Generated with Claude Code

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>

@coreplane-switchboard coreplane-switchboard Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:102isRemoteTerminal() is called directly rather than through CheckoutDeps, even though the function takes an injectable env param that runCheckout never uses. Consequence: the new SSH test mutates process.env (with careful restore, fine), but every other test in the suite implicitly depends on SSH_CONNECTION not being set — running the suite over SSH would make e.g. sends the listener URLs… fail. A deps.isRemote hook (or reading env via deps) would make the suite hermetic.
  • F2 (nit) src/billing/checkout.ts:101 — with --no-browser on 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-minute timeout instead of canceled. 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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.

@justinhelmer
justinhelmer merged commit 80e6750 into main Sep 15, 2026
4 checks passed
@justinhelmer
justinhelmer deleted the fix/checkout-no-listener-remote branch September 15, 2026 02:34
@justinhelmer

Copy link
Copy Markdown
Contributor Author

Release receipt — shipped in v0.2.36 (cut-release 34921739135, release 34921773207, both green). Published polylane.mjs matches checksums.txt (sha256 a5f853df103c0c400497c049ab34c5d12b209c3b450aa0e812630b2e939fb154), --version → 0.2.36, bundle carries the SSH_CONNECTION check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant