fix(sdk): guest pass issuance + idempotent enroll handling - #42
Merged
Conversation
…t B)
New SDK surface:
- Badge.init() — async factory, issues guest pass on first run
- badge.headers() — { "Kya-Token": "gp_v1_..." }
- badge.shouldNudge() / nudgeMessage() — upgrade nudge (v1: stub)
- badge.destroy() — cleanup (v1: no-op)
- issueGuestPass() — POST /api/badge/guest-pass with cache
- loadCachedGuestPass() — read ~/.kya/guest_token with TTL check
- Offline fallback when API unreachable
TDD: 12 new tests, 104/104 SDK tests pass, 199 total.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two P0 bugs discovered during Batch 1+2 smoke test review:
- B1: guest-pass.ts was not sending iss: "sdk" in the fetch body,
causing every guest pass request to fail with 400 (API requires iss
field). Badge.init() silently fell back to offline mode.
- B2: guest-pass.ts read data.token but the API returns data.guest_token,
so the cached token was always undefined. Badge.headers() returned
{ "Kya-Token": "undefined" }.
Also fixed P1 in badge-token.ts:
- B4: enrollAndCacheBadgeToken() now handles { existing: true } responses
gracefully — returns cached token if available instead of logging a
misleading "missing badge_token" error.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a framework-agnostic Badge identity primitive with guest-pass lifecycle (issue/load/cache), re-exports guest-pass utilities, and updates badge-token enrollment to handle idempotent enroll responses by reusing an in-memory cached token or returning null when unavailable. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Badge as Badge.init()
participant Storage as SDK Storage
participant Cache as Guest Pass Cache
participant API as Guest Pass API
App->>Badge: init(opts)
Badge->>Storage: determine installId
Storage-->>Badge: installId
Badge->>Cache: loadCachedGuestPass()
alt valid cached token
Cache-->>Badge: GuestPassResult
Badge-->>App: return Badge (identity: guest)
else no cached token / expired
Badge->>API: issueGuestPass(installId,...)
alt API ok (guest_token)
API-->>Badge: guest_token, expires_at
Badge->>Cache: cacheGuestPass(token, expiresAt)
Cache-->>Badge: stored
Badge-->>App: return Badge (identity: guest)
else API failure / network error
Badge-->>App: return Badge (identity: offline, token: offline_${installId})
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
Covers Badge.init(), headers(), enrollAndCacheBadgeToken(), credential storage, and KYA_API_URL / KYA_API_KEY / KYA_EXTENDED_AUTH env vars. Documents the idempotent enrollment caveat (token only returned on first 201, must be persisted by caller). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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 (2)
packages/sdk/src/badge-token.ts (1)
63-70: Variablecachedshadows outer scope declaration.Line 65 declares
const cachedwhich shadows thecachedvariable from line 32. While functionally correct (the outercachedis already falsy when reaching this point), the shadowing reduces clarity.♻️ Suggested rename for clarity
if (data.existing && !data.badge_token) { // Already enrolled today — use cached token if we have one - const cached = badgeTokenCache.get(merchant); - if (cached) return cached; + const existingToken = badgeTokenCache.get(merchant); + if (existingToken) return existingToken; // No cached token (process restarted) — can't recover until next day process.stderr.write(`[badge] already enrolled at ${merchant} today but no cached token — persist badge_token on first enrollment\n`); return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/badge-token.ts` around lines 63 - 70, The inner declaration of const cached in the badge enrollment branch shadows an outer cached variable; rename the inner variable (e.g., to cachedToken or cachedFromCache) and update its usages and return so the shadowing is removed—look for the badgeTokenCache.get(merchant) call in the block that checks data.existing && !data.badge_token and replace the inner cached identifier consistently.packages/sdk/src/guest-pass.test.ts (1)
70-75: Consider adding tests for expired and malformed cache scenarios.The
loadCachedGuestPass()tests only cover the "no cache exists" case. Consider adding tests for:
- Expired cache (returns
null)- Malformed JSON cache (returns
null)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/guest-pass.test.ts` around lines 70 - 75, Add two unit tests for loadCachedGuestPass(): one that writes a cached guest pass JSON with an expiration timestamp in the past and asserts loadCachedGuestPass() returns null, and another that writes a malformed JSON string (e.g., truncated or invalid) to the same cache key and asserts loadCachedGuestPass() returns null; ensure each test sets up the local storage key used by loadCachedGuestPass (and clears it after) so tests are isolated and reference the loadCachedGuestPass() symbol directly in the new it(...) blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/src/guest-pass.test.ts`:
- Around line 37-55: The test mock in guest-pass.test.ts uses the wrong response
field ("token") while issueGuestPass (in guest-pass.ts) expects
data.guest_token; update the mocked JSON returned by mockFetch in the "calls
/api/badge/guest-pass and returns token" test to use guest_token (and matching
expires_at and identity_type) so the mock shape matches the real API and
exercise issueGuestPass parsing logic correctly.
In `@packages/sdk/src/guest-pass.ts`:
- Around line 100-106: The cacheGuestPass function currently creates KYA_DIR and
writes CACHE_FILE without explicit permissions; update it to set secure modes
like storage.ts does: when calling mkdirSync(KYA_DIR) pass { recursive: true,
mode: 0o700 } and when writing the cache file ensure the file is created with
mode 0o600 (use writeFileSync(CACHE_FILE, ..., { encoding: "utf-8", mode: 0o600
}) and, for the case the file already exists, follow-up with
fs.chmodSync(CACHE_FILE, 0o600) to enforce permissions); keep the existing
silent catch behavior for failures.
---
Nitpick comments:
In `@packages/sdk/src/badge-token.ts`:
- Around line 63-70: The inner declaration of const cached in the badge
enrollment branch shadows an outer cached variable; rename the inner variable
(e.g., to cachedToken or cachedFromCache) and update its usages and return so
the shadowing is removed—look for the badgeTokenCache.get(merchant) call in the
block that checks data.existing && !data.badge_token and replace the inner
cached identifier consistently.
In `@packages/sdk/src/guest-pass.test.ts`:
- Around line 70-75: Add two unit tests for loadCachedGuestPass(): one that
writes a cached guest pass JSON with an expiration timestamp in the past and
asserts loadCachedGuestPass() returns null, and another that writes a malformed
JSON string (e.g., truncated or invalid) to the same cache key and asserts
loadCachedGuestPass() returns null; ensure each test sets up the local storage
key used by loadCachedGuestPass (and clears it after) so tests are isolated and
reference the loadCachedGuestPass() symbol directly in the new it(...) blocks.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2abea87a-e39c-457e-b4ef-d68dbb9f5958
📒 Files selected for processing (6)
packages/sdk/src/badge-token.tspackages/sdk/src/badge.test.tspackages/sdk/src/badge.tspackages/sdk/src/guest-pass.test.tspackages/sdk/src/guest-pass.tspackages/sdk/src/index.ts
Comment on lines
+37
to
+55
| it("calls /api/badge/guest-pass and returns token", async () => { | ||
| mockFetch.mockResolvedValueOnce({ | ||
| ok: true, | ||
| json: async () => ({ | ||
| token: "gp_v1_fresh_token", | ||
| expires_at: "2026-04-07T00:00:00Z", | ||
| identity_type: "guest", | ||
| }), | ||
| }); | ||
|
|
||
| const result = await issueGuestPass("inst-aaaa-bbbb-cccc-dddddddddddd"); | ||
| expect(result).toBeDefined(); | ||
| expect(result!.token).toBe("gp_v1_fresh_token"); | ||
| expect(result!.identityType).toBe("guest"); | ||
|
|
||
| const [url, opts] = mockFetch.mock.calls[0]; | ||
| expect(url).toContain("/api/badge/guest-pass"); | ||
| expect(opts.method).toBe("POST"); | ||
| }); |
There was a problem hiding this comment.
Test mock uses wrong field name — should be guest_token not token.
The PR objective B2 states the API returns data.guest_token, and guest-pass.ts:58 correctly reads data.guest_token. However, this test mock returns { token: "..." } instead of { guest_token: "..." }.
The test passes because mockFetch returns a fake response that issueGuestPass parses, but the mock shape doesn't match the real API response. This could mask bugs if the test is later used to validate parsing logic.
🐛 Proposed fix to align mock with API response
mockFetch.mockResolvedValueOnce({
ok: true,
json: async () => ({
- token: "gp_v1_fresh_token",
+ guest_token: "gp_v1_fresh_token",
expires_at: "2026-04-07T00:00:00Z",
- identity_type: "guest",
}),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/guest-pass.test.ts` around lines 37 - 55, The test mock in
guest-pass.test.ts uses the wrong response field ("token") while issueGuestPass
(in guest-pass.ts) expects data.guest_token; update the mocked JSON returned by
mockFetch in the "calls /api/badge/guest-pass and returns token" test to use
guest_token (and matching expires_at and identity_type) so the mock shape
matches the real API and exercise issueGuestPass parsing logic correctly.
Comment on lines
+100
to
+106
| export function cacheGuestPass(token: string, expiresAt: string): void { | ||
| try { | ||
| mkdirSync(KYA_DIR, { recursive: true }); | ||
| writeFileSync(CACHE_FILE, JSON.stringify({ token, expiresAt }), "utf-8"); | ||
| } catch { | ||
| // Silent failure — cache is a performance optimization, not required | ||
| } |
There was a problem hiding this comment.
Missing file permissions on cache directory and file.
The storage.ts module creates ~/.kya with mode: 0o700 and files with mode: 0o600 to protect credentials. This function creates the same directory and writes a credential file without explicit permissions, potentially exposing the guest token to other users on shared systems.
🛡️ Proposed fix to add restrictive permissions
export function cacheGuestPass(token: string, expiresAt: string): void {
try {
- mkdirSync(KYA_DIR, { recursive: true });
- writeFileSync(CACHE_FILE, JSON.stringify({ token, expiresAt }), "utf-8");
+ mkdirSync(KYA_DIR, { recursive: true, mode: 0o700 });
+ writeFileSync(CACHE_FILE, JSON.stringify({ token, expiresAt }), { encoding: "utf-8", mode: 0o600 });
} catch {
// Silent failure — cache is a performance optimization, not required
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function cacheGuestPass(token: string, expiresAt: string): void { | |
| try { | |
| mkdirSync(KYA_DIR, { recursive: true }); | |
| writeFileSync(CACHE_FILE, JSON.stringify({ token, expiresAt }), "utf-8"); | |
| } catch { | |
| // Silent failure — cache is a performance optimization, not required | |
| } | |
| export function cacheGuestPass(token: string, expiresAt: string): void { | |
| try { | |
| mkdirSync(KYA_DIR, { recursive: true, mode: 0o700 }); | |
| writeFileSync(CACHE_FILE, JSON.stringify({ token, expiresAt }), { encoding: "utf-8", mode: 0o600 }); | |
| } catch { | |
| // Silent failure — cache is a performance optimization, not required | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/src/guest-pass.ts` around lines 100 - 106, The cacheGuestPass
function currently creates KYA_DIR and writes CACHE_FILE without explicit
permissions; update it to set secure modes like storage.ts does: when calling
mkdirSync(KYA_DIR) pass { recursive: true, mode: 0o700 } and when writing the
cache file ensure the file is created with mode 0o600 (use
writeFileSync(CACHE_FILE, ..., { encoding: "utf-8", mode: 0o600 }) and, for the
case the file already exists, follow-up with fs.chmodSync(CACHE_FILE, 0o600) to
enforce permissions); keep the existing silent catch behavior for failures.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sdk/README.md`:
- Around line 41-45: The fenced diagram block is missing a language identifier
which trips markdown linting; update the code fence surrounding the lifecycle
diagram (the block containing "Badge.init() → guest pass (gp_v1_*) → enroll
at merchant → badge token (kya_*)" and the arrows/cached/consent notes) to
include a language tag such as text (i.e., replace ``` with ```text) so the
markdown linter recognizes the block.
- Around line 113-117: The fenced code block showing the directory listing
starting with "~/.kya/" is missing a language identifier; update that block to
include a language (e.g., "text" or "bash") after the opening backticks so
markdown linters accept it — locate the block containing the lines "~/.kya/",
"install_id", and "guest_token" in packages/sdk/README.md and change the opening
"```" to "```text" (or "```bash").
- Around line 88-99: Update the README description for enrollAndCacheBadgeToken
to clarify re-enrollment semantics: state that enrollAndCacheBadgeToken
(implemented in packages/sdk/src/badge-token.ts) will return the cached badge
token if it was previously issued in the same process, and only returns null
when no in-memory cache exists (e.g., after a process restart), rather than
always returning null on same-day re-enrollment; keep the note about requiring
KYA_API_KEY and persisting the token from the first call.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 34e39898-65cd-4aeb-8854-0496a55d2f17
📒 Files selected for processing (1)
packages/sdk/README.md
Comment on lines
+41
to
+45
| ``` | ||
| Badge.init() → guest pass (gp_v1_*) → enroll at merchant → badge token (kya_*) | ||
| ↑ ↑ | ||
| cached in ~/.kya/ requires consent key (pk_*) | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to the fenced code block.
The lifecycle diagram should specify a language (e.g., text or mermaid) to satisfy markdown linting rules.
📝 Proposed fix
-```
+```text
Badge.init() → guest pass (gp_v1_*) → enroll at merchant → badge token (kya_*)
↑ ↑
cached in ~/.kya/ requires consent key (pk_*)</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @packages/sdk/README.md around lines 41 - 45, The fenced diagram block is
missing a language identifier which trips markdown linting; update the code
fence surrounding the lifecycle diagram (the block containing "Badge.init() →
guest pass (gp_v1_) → enroll at merchant → badge token (kya_)" and the
arrows/cached/consent notes) to include a language tag such as text (i.e.,
replace withtext) so the markdown linter recognizes the block.
</details>
<!-- fingerprinting:phantom:triton:puma:1ff752b7-4f5b-4c44-b064-95767326b113 -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+88
to
+99
| ### `enrollAndCacheBadgeToken(merchant)` | ||
|
|
||
| Enroll at a merchant and receive a `kya_*` badge token. Requires `KYA_API_KEY`. | ||
|
|
||
| ```typescript | ||
| import { enrollAndCacheBadgeToken } from '@kyalabs/badge-sdk' | ||
|
|
||
| const token = await enrollAndCacheBadgeToken('store.example.com') | ||
| // "kya_abc123..." | ||
| ``` | ||
|
|
||
| **Important:** The badge token is only returned on the first enrollment per merchant per day. Re-enrollment on the same day returns `null` (the token hash is one-way). Persist the token from the first call. |
There was a problem hiding this comment.
Clarify the re-enrollment behavior.
Line 99 states "Re-enrollment on the same day returns null", but this is incomplete. Per packages/sdk/src/badge-token.ts lines 58-71, re-enrollment returns the cached token if available in memory, and only returns null if the process restarted and no cached token exists.
This distinction is critical for linked repository consumers (e.g., mcp-server) that rely on receiving the token string on re-enrollment within the same process.
📝 Proposed fix
-**Important:** The badge token is only returned on the first enrollment per merchant per day. Re-enrollment on the same day returns `null` (the token hash is one-way). Persist the token from the first call.
+**Important:** The badge token is only returned on the first enrollment per merchant per day. Re-enrollment on the same day returns the cached token if available in memory, or `null` if the process restarted (the API-stored token is hashed and cannot be recovered). Persist the token from the first 201 response to survive process restarts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sdk/README.md` around lines 88 - 99, Update the README description
for enrollAndCacheBadgeToken to clarify re-enrollment semantics: state that
enrollAndCacheBadgeToken (implemented in packages/sdk/src/badge-token.ts) will
return the cached badge token if it was previously issued in the same process,
and only returns null when no in-memory cache exists (e.g., after a process
restart), rather than always returning null on same-day re-enrollment; keep the
note about requiring KYA_API_KEY and persisting the token from the first call.
Comment on lines
+113
to
+117
| ``` | ||
| ~/.kya/ | ||
| install_id # persistent UUID | ||
| guest_token # cached guest pass { token, expiresAt } | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to the fenced code block.
The directory structure should specify a language (e.g., text or bash) to satisfy markdown linting rules.
📝 Proposed fix
-```
+```text
~/.kya/
install_id # persistent UUID
guest_token # cached guest pass { token, expiresAt }</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @packages/sdk/README.md around lines 113 - 117, The fenced code block showing
the directory listing starting with "/.kya/" is missing a language identifier;/.kya/", "install_id", and "guest_token" in packages/sdk/README.md
update that block to include a language (e.g., "text" or "bash") after the
opening backticks so markdown linters accept it — locate the block containing
the lines "
and change the opening "" to "text" (or "```bash").
</details>
<!-- fingerprinting:phantom:triton:puma:1ff752b7-4f5b-4c44-b064-95767326b113 -->
<!-- This is an auto-generated comment by CodeRabbit -->
…st_token) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
This was referenced Apr 16, 2026
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.
Summary
guest-pass.tswas not sendingiss: "sdk"— every SDK guest pass request failed with 400,Badge.init()silently fell back to offline modeguest-pass.tsreaddata.tokenbut API returnsdata.guest_token— cached token was alwaysundefined,Badge.headers()returned{ "Kya-Token": "undefined" }badge-token.tsnow handles{ existing: true }idempotent enroll responses gracefully instead of logging misleading "missing badge_token" errorDiscovered during comprehensive Batch 1+2 smoke test review. See
internalops/specs/roadmaps/ProveIt/batch2_smoke_test.mdfor full test plan.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests