fix(licenses): show real activation count (token-independent), fail loud on machine mgmt#271
Merged
Merged
Conversation
…an admin-token call The account Licenses page showed '0 of N activations' for every customer. Root cause: the count came from the authed Keygen machine-list call (getLicenseMachines), which requires KEYGEN_API_TOKEN. Whenever that token was absent or the call failed, resolveLicenseReference silently fell back to a path that hardcoded machines:[] -> a misleading 0. Proper fix (token-independent by design): - Keygen's PUBLIC validate-key response carries the authoritative activation count at relationships.machines.meta.count. Map it into LicenseDetail as activationCount and render 'X of Y' from that, never machines.length. - resolveLicenseReference is now validate-key-primary: correct status + count with no credential. The machine DETAIL list is optional enrichment, attempted only when a token is configured (canManageMachines()); failure leaves the list empty but the count intact -- an honest 'N (details unavailable)', never a wrong 0. - The plugin-facing validateLicense() count now comes from validate-key too. - Fail LOUD: authHeaders() throws KeygenAuthNotConfiguredError when the token is missing; the machine-deactivation route surfaces it as a clear 503 instead of a vague 500. Verified against live Keygen: validate-key returns machines.meta.count for a known 1-activation license.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds Keygen auth gating, derives activation counts from validate-key metadata, conditionally enriches customer licenses with machine lists, and updates the machine deactivation route and UI to use the new license data. ChangesActivation count and Keygen auth guard
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant licenseClient
participant KeygenAPI
Client->>licenseClient: validateLicense(key)
licenseClient->>KeygenAPI: validateLicenseKey(key)
KeygenAPI-->>licenseClient: license + machines.meta.count
licenseClient->>licenseClient: canAuthenticate()?
alt token present
licenseClient->>KeygenAPI: fetch machine list
KeygenAPI-->>licenseClient: machines (best-effort)
else no token
licenseClient-->>licenseClient: skip machine list, use activationCount
end
licenseClient-->>Client: activationsCount from license.activationCount
sequenceDiagram
participant Caller
participant resolveLicenseReference
participant licenseClient
participant enrichWithMachineList
Caller->>resolveLicenseReference: resolve(licenseRef)
resolveLicenseReference->>licenseClient: validateLicenseKey(key)
licenseClient-->>resolveLicenseReference: base license + activationCount
resolveLicenseReference->>enrichWithMachineList: enrich(base, id)
enrichWithMachineList->>licenseClient: canManageMachines()?
alt token configured
enrichWithMachineList->>licenseClient: getLicenseMachines(id)
licenseClient-->>enrichWithMachineList: machines list
enrichWithMachineList-->>resolveLicenseReference: base with machines
else no token or failure
enrichWithMachineList-->>resolveLicenseReference: unmodified base license
end
resolveLicenseReference-->>Caller: resolved license
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Use validate-key's canonical license.id (base.id) instead of reference.id so key-only and re-issued (stale-id) references still fetch their machine detail list when a token is configured. Count was already correct; this fixes the detail-list/deactivation UI for those references.
Contributor
Author
|
Live verification (review item #1 — the load-bearing assumption): confirmed against prod Keygen that the public for license Deferred (pre-existing, not a regression): the Discord member-removal route ( |
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acfed4374c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/account/licenses-client.tsx (1)
445-445: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMachine list section can vanish even with a positive activation count.
license.machines.length > 0gates the "Activated machines" section, but per the new contractmachinescan be empty whileactivationCount > 0(unauthenticated enrichment). Users would see e.g. "2 of 5 activations" with no list/management controls beneath it, with no explanation. Since this is an accepted tradeoff per the PR description, consider (optionally) surfacing a note like "machine details unavailable" whenactivationCount > 0 && machines.length === 0for clarity, rather than silently omitting the section.🤖 Prompt for 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. In `@src/components/account/licenses-client.tsx` at line 445, The Activated machines section in licenses-client.tsx is currently hidden whenever `license.machines.length > 0` is false, which can make the UI disappear even when `activationCount` is positive. Update the rendering condition around the machine list in `licenses-client.tsx` so it also handles the unauthenticated enrichment case where `activationCount > 0` but `machines` is empty. Use the `license.machines`, `license.activationCount`, and the existing “Activated machines” block to locate the change, and optionally add a small note such as “machine details unavailable” when counts exist but no machine records are present.
🤖 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 `@src/lib/customer-licenses.test.ts`:
- Around line 37-38: The test helper wrapper for canManageMachines is causing a
TypeScript spread/signature mismatch because mockCanManageMachines is a
zero-argument vi.fn and does not accept forwarded args. Update the
canManageMachines mock in customer-licenses.test.ts to call
mockCanManageMachines directly with no parameters, matching the production
canManageMachines contract and removing the unnecessary (...args: unknown[])
wrapper.
In `@src/lib/customer-licenses.ts`:
- Around line 45-59: Use the validated license id for machine enrichment:
enrichWithMachineList currently receives reference.id, which can be missing even
after resolveLicenseReference() successfully validates a key-only license.
Update the caller in customer license resolution to pass base.id into
enrichWithMachineList so machine details are still fetched when only the
validated base license has an id and a Keygen token is available.
---
Nitpick comments:
In `@src/components/account/licenses-client.tsx`:
- Line 445: The Activated machines section in licenses-client.tsx is currently
hidden whenever `license.machines.length > 0` is false, which can make the UI
disappear even when `activationCount` is positive. Update the rendering
condition around the machine list in `licenses-client.tsx` so it also handles
the unauthenticated enrichment case where `activationCount > 0` but `machines`
is empty. Use the `license.machines`, `license.activationCount`, and the
existing “Activated machines” block to locate the change, and optionally add a
small note such as “machine details unavailable” when counts exist but no
machine records are present.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 99f2bff3-120d-4e92-a30d-6802a386d67a
📒 Files selected for processing (9)
src/app/api/account/licenses/[licenseId]/machines/[machineId]/route.test.tssrc/app/api/account/licenses/[licenseId]/machines/[machineId]/route.tssrc/components/account/licenses-client.test.tsxsrc/components/account/licenses-client.tsxsrc/lib/customer-licenses.test.tssrc/lib/customer-licenses.tssrc/services/core/external/license-client.test.tssrc/services/core/external/license-client.tssrc/types/license.ts
…ichment CI type-check (tsc over test files, which next build skips) failed: three test LicenseDetail factories lacked the now-required activationCount, and a mock wrapper spread args into a zero-arg vi.fn. Add activationCount to the factories and drop the spread (CodeRabbit). Also address Codex P2: enrichWithMachineList no longer overwrites the authoritative validate-key activationCount with machines.length — getLicenseMachines returns a single paginated page, so a partial page could undercount. Keep the validate-key count; use the fetched machines as the detail list only.
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
Author
Triage — review + CI (fix skill)
The greptile bot comments are a trial-credit notice, not findings. Replying to + resolving each thread after the E2E fix pushes. |
authHeaders() now fails loud (KeygenAuthNotConfiguredError) when the token is absent; the mock harness set none, so the license/machine flows 500'd. The mock ignores auth, but the value must be present — matching prod, where KEYGEN_API_TOKEN is now configured — so the id-path resolves and machine-management e2e tests run.
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Contributor
Author
✅ Merge-readyAll review threads replied-to + resolved (4/4), all CI green:
Final commits this round:
|
|
🚀 Preview: https://wcpos-jnp08kuhi-wcpos.vercel.app |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix "0 of N activations" — properly (token-independent by design)
Every customer's Licenses page showed "0 of N activations" even when machines were active. This is the code half of the fix; the infra half (setting
KEYGEN_API_TOKENin prod) was handled separately.Root cause
Activation count came from the authed Keygen machine-list call (
getLicenseMachines), which needsKEYGEN_API_TOKEN. When the token was absent (as it was in prod) or the call failed,resolveLicenseReferencesilently fell back to a path that hardcodedmachines: []→ a misleading0. The same bug existed in the plugin-facingvalidateLicense.The proper fix — don't depend on an admin token for a read customers need
Keygen's public
validate-keyresponse already carries the authoritative activation count atrelationships.machines.meta.count— no credential required. So:LicenseDetailgainsactivationCount, mapped from that field. The UI renders "X of Y" from it, nevermachines.length.resolveLicenseReferenceis now validate-key-primary: correct status + count with zero credentials. The machine detail list (fingerprints, for management/deactivation) is optional enrichment, attempted only when a token is configured. If enrichment is unavailable or fails, the list is empty but the count stays correct — honest "N activations (details unavailable)", never a wrong0.validateLicensecount now comes from validate-key too.authHeaders()throwsKeygenAuthNotConfiguredErrorwhen the token is missing (instead of silently sendingBearer undefined→ 401 → "0"). The machine-deactivation route surfaces it as a clear 503 "Machine management is not configured" rather than a vague 500.Net effect: activation counts are correct with or without the admin token; the token only gates machine management (list + deactivate), which now fails loudly when misconfigured.
Verified against live Keygen
for a license the customer would see as "1 of 2".
Validation
vitest run), incl. new tests: correct count with no token; enrichment when token present; count preserved when enrichment fails;KeygenAuthNotConfiguredError→ 503.next buildclean, ESLint clean.Independent of #268 (view-as Orders fields) — touches different files.
Summary by CodeRabbit
New Features
Bug Fixes
Tests