Repository navigation
feat(cli): billing and upgrade links carry the signed-in workspace - #7103
Conversation
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head dbc45e1b
- blocker —
Commentsratchet fails. It added 1 comment line toauth/status.ts(13.9% → 14.0%) and 2 toheygen/plan.ts(9.1% → 9.2%). See the inline comments. - important — the site doesn't read the hint yet. The body says "The site's sign-in reads it", but on hyperframes-internal
mainthe signed-out Get Pro button (PricingScreen.tsx:162) goes to/api/auth/login?return_to=…./api/auth/loginonly readsworkspace_hintfrom its own query, so a hint that comes in on/pricing?…&workspace_hint=…is dropped and the account default is still used. The site side is hyperframes-internal#8149, which is still open. Merging this first does no harm (the parameter is just ignored), but name #8149 as the dependency in the body, or land it first.
Scope
Read all 6 changed source files end-to-end, plus cloud/auth.ts and demo-next's /api/auth/login, /pricing and PricingScreen. The fails without the fix line holds: dropping the workspace_hint line makes the plan and billing tests fail (2), and at this head the changed suites pass 102/102. The edit to refusalWhenServerBlocksFree (session.upgradeUrl === null) behaves the same as before. CI's Fallow audit job was cancelled. PR captures, CodeQL, regression and preview-regression are green. No prior reviews.
| apiError: string | null; | ||
| /** The HyperFrames plan and credits, when the server (or the mock) answers. */ | ||
| plan?: Plan | null; | ||
| /** The plans link shown to a Free account, built before printing because it names the workspace. */ |
There was a problem hiding this comment.
This line fails the comment ratchet (13.9% → 14.0%). Drop it; the field name upgradeUrl already says what it holds.
|
|
||
| /** | ||
| * A plans or credits page link with what the web side reads: `from=cli`, the credits needed, the waiting step, | ||
| * and `workspace_hint` (the signed-in workspace, so the site's sign-in lands in the same one; none when signed out). |
There was a problem hiding this comment.
plan.ts goes from 9.1% to 9.2% comment share. The 2-line signedInSpace docstring at :53 plus this added line exceed what the ratchet allows. Drop the signedInSpace docstring (the name says it) and keep this line to one.
There was a problem hiding this comment.
Verdict: APPROVE · 2 resolved, 0 open, 0 new · dbc45e1b → 5787863b
Both findings are resolved. Comments passes, and the body now says the website side lands separately (hyperframes-internal#8149, still open). The diff since the last review only changes comments. CI was cancelled on both runs for this head, but every job passed in at least one of them, except Lint, which was cancelled in both. I ran bun run lint locally at this head and it is clean.
5787863 to
44f95eb
Compare
There was a problem hiding this comment.
Verdict: APPROVE · 0 resolved, 0 open, 0 new · 5787863b → 44f95ebe
The rebase is clean. Every pageUrl / reverseUpgradeUrl caller in the CLI is awaited, the reverse-start refusal goes through reverseUpgradeUrl, credits links keep #packs, and the comments stay out. At this head, the CLI suites I ran (heygen, billing, reverse, auth) pass 189/189 and tsc is clean for the changed files. Every check, CI included, is green.
What
Every plans or credits link the CLI prints or opens (
billing subscribe,billing top-up, the Free line inauth status, the credits and subscribe refusals, the reverse upgrade link) now carriesworkspace_hint=<workspace the CLI is signed in for>. The site's sign-in reads it once its change lands (a separate PR in another repo, not yet merged); until then the parameter is ignored, so merging this first is harmless. With both, the person lands in the same workspace instead of their account default. Signed out, links are unchanged.How
pageUrlinheygen/plan.tsalready built all of these links, so it is the one owner: it asksaccount.getfor the workspace bound to the current sign-in and appends the hint. The workspace is read live, never stored, so it cannot go stale after a different account signs in. No workspace is guessed: a failed read, a signed-out run or an id that is not letters, digits and hyphens (up to 64) adds nothing. Links the server sends back whole (upgrade_url,url) pass through untouched.pageUrlis now async, so its callers (billing, auth status, reverse, the v3 refusal) await it.Tests
plan.test.ts: both link kinds carry the hint fromaccount.get; signed out, a failed read and a malformed id carry none.billing.test.ts: the opened subscribe page carries the hint.workspace_hintline turns the plan and billing tests red (2 failed); restored, 176 passed across the changed suites. Oxlint and oxfmt clean.