Skip to content

feat(impersonate): nudge to billing add-on when impersonation is blocked#376

Merged
rafa-thayto merged 3 commits into
mainfrom
rafa-thayto/impersonate-limit-upgrade-nudge-branch-gate-tests-impersonate-impersonate-impers
Jul 10, 2026
Merged

feat(impersonate): nudge to billing add-on when impersonation is blocked#376
rafa-thayto merged 3 commits into
mainfrom
rafa-thayto/impersonate-limit-upgrade-nudge-branch-gate-tests-impersonate-impersonate-impers

Conversation

@rafa-thayto

Copy link
Copy Markdown
Contributor

Summary

Kyle asked (via the Slack thread on clerk impersonate) whether hitting the impersonation limit nudges customers toward the add-on. Today it doesn't. This adds that nudge so the upgrade path is as frictionless as possible now that the CLI makes it easier to hit the limit.

When POST /v1/actor_tokens is rejected by billing, clerk impersonate now guides you to the billing page instead of just printing a terse error:

  • 402 (plan gate)Impersonation is available as an add-on.
  • 422 (billing-period limit)You've reached your impersonation limit this billing period.

Both attach the account-level billing page (<dashboard>/settings/billing, from getDashboardUrl(), honoring CLERK_DASHBOARD_URL) as docsUrl, and the command still exits non-zero (no session was ever created). How the URL is surfaced follows the output mode:

Condition Behavior
Agent mode JSON error with the billing URL as docsUrl. Never opens a browser.
--print Prints the URL beneath the error. No prompt, no browser.
--yes Opens the billing page immediately (prints it if headless). No prompt.
TTY human, no --yes Prompts "Add more impersonations now?" (default Yes); opens on yes.
Non-TTY human, no --yes Prints the URL beneath the error. No prompt, no browser.

Other changes

  • Drops the (used X/Y this billing period) templating — the copy is deliberately numberless (BAPI has no quota-read endpoint, so counts can't be shown proactively or reliably).
  • Removes the now-unused limit/used fields from BillingError (impersonate was the only consumer and no longer passes them).
  • Documents the behavior in the command README.

Out of scope

  • A proactive "X/5 remaining" counter — impossible without a BAPI quota-read endpoint. This is reactive-on-failure only.

Test Plan

  • bun run format:check · bun run lint · bun run typecheck — all clean
  • bun run test — 1896 pass (adds gating-branch coverage for agent / --print / --yes / TTY-prompt / non-TTY, plus wire-level 402/422 mapping and docsUrl assertions)
  • Drove the real bapiRequest → BapiError(402/422) → nudge stack against a local stub BAPI: 402 → add-on message, 422 → limit message, both with docsUrl = <dashboard>/settings/billing; confirmed openBrowser is invoked with the billing URL on the --yes branch.
  • bun run test:e2e:opnot run locally: the runner needs the AI Enablement 1Password vault, which this environment can't access. The e2e suite has no impersonate coverage, so it wouldn't exercise this change regardless; CI will run it on the PR.

Notes

Minor bump changeset included (clerk).

When `clerk impersonate` is blocked by billing — 402 (plan gate) or 422
(billing-period limit) — surface a value-framed, numberless message and point
to the account-level billing page (`<dashboard>/settings/billing`) as docsUrl.
In an interactive terminal, offer to open the page (default Yes); `--yes` opens
it directly, while `--print` and agent mode surface the URL without opening a
browser. Every branch still exits non-zero since no session was created.

Drops the used/limit quota templating (and the now-unused BillingError
limit/used fields) since BAPI has no quota-read endpoint to make counts
reliable.
@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 086b8d3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
clerk Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 650cd453-5a27-4982-aa0e-1c87572bb921

📥 Commits

Reviewing files that changed from the base of the PR and between d16eb38 and 086b8d3.

📒 Files selected for processing (1)
  • packages/cli-core/src/commands/impersonate/impersonate.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go (manual)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/cli-core/src/commands/impersonate/impersonate.test.ts

📝 Walkthrough

Walkthrough

The impersonate command now maps HTTP 402 and 422 billing failures to BillingError instances with a billing-page docsUrl, omits quota counts from messages, and conditionally prompts or opens the billing page based on execution mode and flags. BillingError no longer stores limit or used. Tests and documentation cover the updated errors and navigation behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • clerk/cli#373: Updates clerk impersonate BAPI 402/422 error handling and related tests.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: nudging impersonation users to the billing add-on when blocked.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the impersonation billing upgrade behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 7 linked repositories, but your current plan allows 1. Analyzed clerk/clerk_go, skipped clerk/dashboard, clerk/accounts, clerk/backoffice, clerk/clerk, clerk/clerk-docs, clerk/cloudflare-workers.


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli-core/src/commands/impersonate/impersonate.test.ts`:
- Around line 287-344: The non-TTY billing test is missing an assertion that the
upgrade nudge is skipped, so it could still pass even if nudgeToBilling()
wrongly prompts. Update the impersonate tests around BillingError handling to
assert the billing prompt/mockConfirm call count or exact calls, ensuring only
the pre-flight confirmation runs in the non-TTY case and no upgrade prompt is
shown.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 266cbca4-2603-4c52-866f-481a1a365c96

📥 Commits

Reviewing files that changed from the base of the PR and between f0f8d23 and d16eb38.

📒 Files selected for processing (6)
  • .changeset/impersonate-upgrade-nudge.md
  • packages/cli-core/src/commands/impersonate/README.md
  • packages/cli-core/src/commands/impersonate/impersonate.test.ts
  • packages/cli-core/src/commands/impersonate/impersonate.ts
  • packages/cli-core/src/lib/errors.test.ts
  • packages/cli-core/src/lib/errors.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • clerk/clerk_go (manual)

Comment thread packages/cli-core/src/commands/impersonate/impersonate.test.ts
The prior assertion only checked that the browser never opened, which
would still pass if nudgeToBilling() incorrectly prompted in non-TTY
mode. Assert the confirm call count so a regression there is caught.
@rafa-thayto rafa-thayto merged commit 7603dde into main Jul 10, 2026
10 checks passed
@rafa-thayto rafa-thayto deleted the rafa-thayto/impersonate-limit-upgrade-nudge-branch-gate-tests-impersonate-impersonate-impers branch July 10, 2026 23:13
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.

2 participants