Skip to content

feat: terms/privacy notice at account creation - #53

Merged
justinhelmer merged 3 commits into
mainfrom
feat/signup-terms-notice
Aug 20, 2026
Merged

feat: terms/privacy notice at account creation#53
justinhelmer merged 3 commits into
mainfrom
feat/signup-terms-notice

Conversation

@justinhelmer

@justinhelmer justinhelmer commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Third of three fixes from the 2026-08-19 user-testing session on the CLI sign-in flow: show a Terms of Service / Privacy Policy notice at account creation.

Stack position: PR 3 of 3 from the 2026-08-19 user-testing feedback, based on main, independent of its siblings: cli#49 (per-provider auth login picker) and cli#51 (direct-to-provider OAuth routing).

Context

The CLI can create a Polylane account — via auth signup, and via auth login's Email option, which routes into the same idempotent signup flow — without ever showing the user the terms they're accepting. This PR adds the notice, with this exact wording:

By continuing, you agree to the Terms of Service and acknowledge the Privacy Policy:
  https://polylane.com/terms/
  https://polylane.com/privacy/

The verbs stay separate deliberately: EU users don't consent to a privacy policy by contract — they acknowledge it. The copy must never collapse into "you agree to our Terms and Privacy Policy".

What this PR does

  • Email signup path (emailSignup, also auth login → Email): the notice shows immediately before the account-creating POST /v1/auth/signup, gated on Enter — "Press Enter to continue, or Ctrl-C to cancel." The gate wording is neutral because signup is idempotent for an existing account, so this path is also a sign-in.
  • Provider signup path (oauthSignup): the notice joins the existing "your browser will open" note and gates on "Press Enter to create your account, or Ctrl-C to cancel." before the browser opens.
  • Scripted and non-interactive runs (--password flag, no TTY, --non-interactive) print the notice to stderr and never block — agents and CI re-invoke signup with flags per the existing flow, and blocking them on Enter would break that.
  • The notice shows exactly once per run (the two gated paths are mutually exclusive), and never on paths that don't create an account: --code verification completion, API key login, device-code login, and plain browser login. Browser signups additionally see the console's own terms UI.
  • New promptEnter util in src/utils/prompt.ts wraps the Enter gate with the repo's standard prompt behavior: ensureInteractive guard, and Ctrl-C throws CLIError('Cancelled') like every other prompt.

Verification

  • npm run typecheck — pass.
  • npm run lint — pass.
  • npm run test — 276/276 pass, including four new cases in test/signup.test.ts: the interactive email path shows the notice once and gates on Enter; a non-interactive scripted signup prints it without gating; interactive --password keeps the notice but skips the gate; the --code completion path shows no notice.

🤖 Generated with Claude Code

Every CLI path that creates an account now shows the Terms of Service /
Privacy Policy notice before the account-creating step: the email signup
POST and the provider signup browser trip. Interactive runs gate on Enter
(Ctrl-C cancels); scripted or non-interactive runs print the notice and
never block. The verbs stay separate on purpose — users agree to the
Terms but only acknowledge the Privacy Policy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Justin Helmer and others added 2 commits August 19, 2026 17:43
Node 20's mock.module cannot override a module already in the ESM cache
(22+ re-links it; 20 leaves the mock silently inert), so the real exports
now come from ?real query URLs — separate cache entries — and the env
mock registers before prompt.ts?real pulls in the canonical './env'. Fixes
the Node 20.x CI leg where the real promptPassword ran and threw
'Missing required input: Password'.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The file's noop mocks for intro/outro/note/formatOutput were silently
inert on Node 20 (mock.module cannot override an already-cached module
there), so the raw clack writes they exist to suppress still reached
stdout and could land mid-frame in the test runner's serialized reporter
stream — the exact 'Unable to deserialize cloned data' crash the mocks
document. The terms-notice change added output on paths these tests
execute, which made the latent flake bite in CI's Node 20 leg. Pull the
real exports through ?real query URLs so the canonical entries stay
unloaded until the mocks register.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM: the terms/privacy notice lands correctly on both account-creating paths, and the Node 20.x CI failure at 984cdbd is fixed by two test-only commits — all three matrix legs are green at head.

Reviewed at head 0f2abde61f2ec04812d252f0ffcc49a157256dca.

Verified

Feature (unchanged since 984cdbd — the diff since then touches only test/signup.test.ts and test/onboarding-run.test.ts):

  • Notice wording and links match the PR body exactly (TERMS_NOTICE in src/commands/auth/signup.ts). The agree/acknowledge verb split is deliberate and correct EU wording, per the code comment — noting it here so nobody re-flags it as an inconsistency.
  • Email path: notice + Enter gate sit immediately before the account-creating POST /v1/auth/signup; oauthSignup gates before oauthLogin opens the browser. The two gated paths are mutually exclusive per run.
  • Scripted/--password/non-interactive runs print the notice to stderr without blocking, keeping stdout clean for JSON consumers.
  • Show-nothing paths (--code completion, API key, device-code, plain browser login) are untouched.

The Node 20 fix (32264b8, 0f2abde):

  • Root cause checks out: Node 20's mock.module can't override an already-cached ESM module (22+ re-links it), so the tests' own pre-imports were defeating their mocks. Both files now pull real exports through ?real query URLs (separate cache entries) and register mocks before the canonical specifiers ever load.
  • Adversarial pass on the double-instance trick: the mocked modules export no classes, CLIError resolves canonically in both instances, no test does instanceof against a ?real export, and test/onboarding-run.test.ts's pre-mock imports (src/auth/onboarding-run, src/auth/oauth) don't transitively reach prompt.ts/formatter.ts — so the ordering holds and there's no identity hazard.

CI at head: all three legs green on run 32318601725 — 20.x job 96276047605, 22.x job 96276047462, 24.x job 96276047548. The previously red 20.x leg (run 32317475095, job 96272728702) is confirmed failure at the old head and superseded.

Local at 0f2abde (Node 22): npm run typecheck, npm run lint pass; full suite 271/271 (the count variance vs the PR body's 276 is the known output.test.ts counting flake, not a failure); test/signup.test.ts 14/14 and test/onboarding-run.test.ts 23/23 in isolation; npm run build + ./dist/polylane.mjs --version smoke OK.

Non-blocking follow-ups (details in the inline comments)

  • promptEnter never settles on stdin EOF (Ctrl-D) — silent exit 0 before the signup POST. Fail-safe direction, clack-wide behavior class; follow-up candidate.
  • No test covers the oauthSignup notice/gate path, and captureOutput merges stdout+stderr so the stderr routing isn't asserted; promptEnter has no unit test.
  • Pre-existing (not this PR): src/commands/workspace/create.ts:33-35 points at https://console.polylane.com/terms — a different host/wording from this PR's https://polylane.com/terms/. Worth unifying separately.
  • Cosmetic: promptEnter renders a free-text input, so typed text + Enter also passes.

Generated by Claude Code

Comment thread src/utils/prompt.ts
Comment thread test/signup.test.ts

@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: Claude reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.


Generated by Claude Code

@justinhelmer
justinhelmer merged commit 1dfece6 into main Aug 20, 2026
6 checks passed
@justinhelmer
justinhelmer deleted the feat/signup-terms-notice branch August 20, 2026 01:08
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