Conversation
qr code display for shares and group
Feature/frostr icon
first pass test coverage overhaul
📝 WalkthroughWalkthroughThis PR introduces QR code support to the igloo-cli with new UI components, refactors CLI argument parsing into a reusable module, adds comprehensive test coverage across six new test files totaling 97 tests, updates documentation to reflect bun-based workflows and restructured command namespaces, and enhances the visual experience with animated ASCII icons. Changes
Sequence DiagramsequenceDiagram
participant User as User (Raw Mode)
participant KL as KeysetLoad Component
participant Input as useInput/stdin
participant KS as KeysetLoad State<br/>(qrVisible)
participant CD as CredentialDisplay
participant QR as QRCodeDisplay
participant QRC as qrcode.toString()
participant Term as Terminal
User->>Input: Press 'Q' key
Input->>KL: onInput handler triggered
KL->>KS: Toggle qrVisible state
KS->>CD: Re-render with qrVisible
alt showQR === true and qrVisible === true
CD->>QR: Render QRCodeDisplay
QR->>QRC: Generate QR string (async)
QRC-->>QR: QR string
QR->>Term: Render label + QR code in terminal
else showQR === false or qrVisible === false
CD->>Term: Render credential text only
end
Term-->>User: Display toggles between text and QR
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/naming.test.ts (1)
68-89: Consider using beforeEach/afterEach for environment variable manipulation.The inline
try/finallypattern works but is inconsistent with the rest of the file. Lines 97-105 usebeforeEach/afterEachfor the same purpose, which is cleaner and more maintainable.🔎 Proposed refactor to align with existing patterns
-test('buildShareFilePath builds full path with directory and .json', () => { - // Set up temp directory for consistent path testing - const originalAppdata = process.env.IGLOO_APPDATA; - process.env.IGLOO_APPDATA = '/tmp/test-appdata'; - - try { - const result = buildShareFilePath('test', 1); - const expectedDir = getShareDirectory(); - const expectedId = buildShareId('test', 1); - - assert.equal(result, path.join(expectedDir, `${expectedId}.json`)); - assert.ok(result.endsWith('.json')); - assert.ok(result.includes('test_share_1')); - } finally { - // Restore original env - if (originalAppdata !== undefined) { - process.env.IGLOO_APPDATA = originalAppdata; - } else { - delete process.env.IGLOO_APPDATA; - } - } -}); +test('buildShareFilePath builds full path with directory and .json', () => { + const originalAppdata = process.env.IGLOO_APPDATA; + process.env.IGLOO_APPDATA = '/tmp/test-appdata'; + + const result = buildShareFilePath('test', 1); + const expectedDir = getShareDirectory(); + const expectedId = buildShareId('test', 1); + + assert.equal(result, path.join(expectedDir, `${expectedId}.json`)); + assert.ok(result.endsWith('.json')); + assert.ok(result.includes('test_share_1')); + + // Restore original env + if (originalAppdata !== undefined) { + process.env.IGLOO_APPDATA = originalAppdata; + } else { + delete process.env.IGLOO_APPDATA; + } +});Alternatively, move this test below line 106 to share the existing
beforeEach/afterEachsetup and eliminate env manipulation entirely.llm/context/testing/test-architecture.md (1)
17-35: Minor: Add language identifier to fenced code block.The fenced code block at line 17 is missing a language specifier. Consider adding
textor leaving it as is if the style is intentional for directory tree visualizations.🔎 Proposed fix
-``` +```text tests/ ├── helpers/src/lib/parseArgv.ts (1)
68-82: Alias normalization logic is correct.The normalization correctly maps short flags to their long-form equivalents while preserving long-form values if both are provided. The precedence behavior (long-form wins) is reasonable and user-friendly.
Optional: Add brief comment for clarity
Consider adding a comment above line 68 to document the purpose:
+ // Normalize short flag aliases to their long-form equivalents if (flags.t !== undefined && flags.threshold === undefined) {This is purely optional as the code is self-explanatory.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
AGENTS.mdCLAUDE.mdllm/context/testing/test-architecture.mdllm/implementation/test-coverage-implementation.mdpackage.jsonsrc/cli.tsxsrc/components/Help.tsxsrc/components/Intro.tsxsrc/components/keyset/KeysetCreate.tsxsrc/components/keyset/KeysetLoad.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/lib/parseArgv.tstests/cli.basic.test.tstests/cli.parsing.test.tstests/crypto.test.tstests/naming.test.tstests/paths.test.tstests/policy.test.tstests/storage.test.ts
🧰 Additional context used
📓 Path-based instructions (10)
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ink components (, , etc.) for terminal UI and do not use HTML elements
Files:
src/components/keyset/KeysetCreate.tsxsrc/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/components/Intro.tsxsrc/cli.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All import specifiers must include .js extensions (TypeScript ESM requirement) even though source files are .ts/.tsx
Files:
src/components/keyset/KeysetCreate.tsxsrc/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/lib/parseArgv.tssrc/components/Intro.tsxsrc/cli.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsx
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/components/keyset/KeysetCreate.tsxsrc/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/components/Intro.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
When adding a new command, implement it as a component under src/components/
Store reusable UI components under src/components/
Files:
src/components/keyset/KeysetCreate.tsxsrc/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/components/Intro.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript with ESM, 2-space indentation, single quotes, and trailing commas
Order imports from shallow-to-deep
Use camelCase for utility function and variable names
Use SCREAMING_SNAKE_CASE for constants
Files:
src/components/keyset/KeysetCreate.tsxtests/policy.test.tssrc/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/lib/parseArgv.tstests/storage.test.tssrc/components/Intro.tsxtests/paths.test.tstests/crypto.test.tssrc/cli.tsxtests/naming.test.tssrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsxtests/cli.parsing.test.tstests/cli.basic.test.ts
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/KeysetCreate.tsxsrc/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxsrc/components/Intro.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsx
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Allow colocated test files named feature.test.ts beside implementations
Test files should exercise both happy paths and failure modes
Files:
tests/policy.test.tstests/storage.test.tstests/paths.test.tstests/crypto.test.tstests/naming.test.tstests/cli.parsing.test.tstests/cli.basic.test.ts
llm/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep protocol prompts and agent scripts in llm/ and update them when UX flows change
Files:
llm/context/testing/test-architecture.mdllm/implementation/test-coverage-implementation.md
src/components/Intro.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
When adding a new command, update command examples in Intro.tsx if applicable
Files:
src/components/Intro.tsx
src/cli.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/cli.tsx: Argument parsing is handled manually in cli.tsx via parseArgv() and must support --flag value, --flag=value, and -f value forms
When adding command flags, extend parseArgv() in cli.tsx for any custom parsing logic
src/cli.tsx: Keep the CLI entrypoint in src/cli.tsx and use it to bootstrap Ink rendering
Keep CLI flags lower-case (e.g., --verbose)
Files:
src/cli.tsx
🧠 Learnings (19)
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/App.tsx : Extract flags from props in App.tsx and pass them to command components; add validation/defaults as needed
Applied to files:
src/components/keyset/KeysetCreate.tsxsrc/lib/parseArgv.tssrc/cli.tsxCLAUDE.mdsrc/components/keyset/KeysetLoad.tsx
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to src/keyset/**/*.ts : Document non-obvious flows with concise comments, especially around key lifecycle management
Applied to files:
src/components/keyset/KeysetCreate.tsxtests/policy.test.tsAGENTS.mdtests/storage.test.tstests/crypto.test.tssrc/components/keyset/KeysetLoad.tsx
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to src/keyset/**/*.ts : Keep key exchange and credential helpers under src/keyset/
Applied to files:
src/components/keyset/KeysetCreate.tsxtests/policy.test.tstests/crypto.test.tssrc/components/keyset/KeysetLoad.tsx
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to src/__tests__/**/*.ts : Tests under src/__tests__/ should exercise both happy paths and failure modes
Applied to files:
tests/policy.test.tsllm/context/testing/test-architecture.mdtests/storage.test.tstests/paths.test.tstests/crypto.test.tstests/naming.test.tstests/cli.parsing.test.ts
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to **/*.test.ts : Test files should exercise both happy paths and failure modes
Applied to files:
tests/policy.test.tsllm/context/testing/test-architecture.mdtests/storage.test.tstests/paths.test.tstests/cli.parsing.test.ts
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/**/*.tsx : Use Ink components (<Box>, <Text>, etc.) for terminal UI and do not use HTML elements
Applied to files:
src/components/Help.tsxsrc/components/keyset/ShareSaver.tsxsrc/components/ui/QRCodeDisplay.tsxAGENTS.mdsrc/components/Intro.tsxsrc/components/ui/CredentialDisplay.tsxsrc/components/keyset/KeysetLoad.tsx
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Run the full test suite (npm test) before opening a PR and record any manual validation steps in the PR description
Applied to files:
llm/context/testing/test-architecture.mdAGENTS.md
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Favor type coverage first; add node:test or vitest specs when logic branches or data transforms appear
Applied to files:
llm/context/testing/test-architecture.md
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to **/*.test.ts : Allow colocated test files named feature.test.ts beside implementations
Applied to files:
llm/context/testing/test-architecture.mdtests/paths.test.tstests/naming.test.ts
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to src/__tests__/**/*.ts : Allow tests under src/__tests__/
Applied to files:
llm/context/testing/test-architecture.mdtests/paths.test.ts
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to src/cli.tsx : Keep the CLI entrypoint in src/cli.tsx and use it to bootstrap Ink rendering
Applied to files:
src/components/ui/QRCodeDisplay.tsxAGENTS.mdpackage.jsonsrc/components/Intro.tsxsrc/cli.tsxCLAUDE.mdsrc/components/keyset/KeysetLoad.tsxtests/cli.basic.test.ts
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to tsup.config.ts : tsup entry must be src/cli.tsx
Applied to files:
AGENTS.mdsrc/cli.tsx
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to tsup.config.ts : Build output should be dist/cli.js and include a shebang (#!/usr/bin/env node)
Applied to files:
AGENTS.md
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/components/Intro.tsx : When adding a new command, update command examples in Intro.tsx if applicable
Applied to files:
AGENTS.mdsrc/components/Intro.tsxsrc/cli.tsxCLAUDE.mdtests/cli.basic.test.ts
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/components/**/*.tsx : When adding a new command, implement it as a component under src/components/
Applied to files:
AGENTS.mdCLAUDE.md
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/cli.tsx : When adding command flags, extend parseArgv() in cli.tsx for any custom parsing logic
Applied to files:
src/lib/parseArgv.tssrc/cli.tsxCLAUDE.mdtests/cli.parsing.test.ts
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/cli.tsx : Argument parsing is handled manually in cli.tsx via parseArgv() and must support --flag value, --flag=value, and -f value forms
Applied to files:
src/lib/parseArgv.tssrc/cli.tsxCLAUDE.mdtests/cli.parsing.test.ts
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-08T16:24:14.475Z
Learning: Applies to src/cli.tsx : Keep CLI flags lower-case (e.g., --verbose)
Applied to files:
src/lib/parseArgv.tssrc/cli.tsxCLAUDE.mdtests/cli.parsing.test.ts
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-cli PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-03T23:45:30.161Z
Learning: Applies to src/App.tsx : When adding a new command, import the new component in App.tsx and add a corresponding case to the switch
Applied to files:
src/cli.tsx
🧬 Code graph analysis (10)
tests/policy.test.ts (2)
src/keyset/types.ts (1)
ShareFileRecord(16-28)src/keyset/policy.ts (8)
DEFAULT_POLICY_DEFAULTS(9-12)createDefaultPolicy(14-20)ensurePolicy(97-100)setPolicyDefaults(109-131)upsertPeerPolicy(133-171)removePeerPolicy(173-190)updatePolicyTimestamp(102-107)pruneEmptyPeers(192-199)
src/components/keyset/ShareSaver.tsx (1)
src/components/ui/CredentialDisplay.tsx (1)
CredentialDisplay(13-31)
tests/storage.test.ts (5)
src/keyset/types.ts (1)
ShareFileRecord(16-28)tests/helpers/tmp.ts (1)
makeTmp(5-8)src/keyset/storage.ts (4)
ensureShareDirectory(8-12)readShareFiles(14-46)saveShareRecord(48-63)loadShareRecord(65-103)src/keyset/paths.ts (1)
getShareDirectory(22-24)src/keyset/crypto.ts (1)
SHARE_FILE_VERSION(7-7)
tests/paths.test.ts (1)
src/keyset/paths.ts (2)
getAppDataPath(4-20)getShareDirectory(22-24)
tests/crypto.test.ts (1)
src/keyset/crypto.ts (16)
SHARE_FILE_VERSION(7-7)SHARE_FILE_PBKDF2_ITERATIONS(8-8)SHARE_FILE_PBKDF2_PREVIOUS_ITERATIONS(9-9)SHARE_FILE_PBKDF2_LEGACY_ITERATIONS(10-10)SHARE_FILE_PASSWORD_ENCODING(11-11)SHARE_FILE_SALT_LENGTH_BYTES(12-12)SHARE_FILE_SALT_PBKDF2_EXPANDED_BYTES(13-13)SHARE_FILE_IV_LENGTH_BYTES(14-14)SHARE_FILE_IV_LEGACY_LENGTH_BYTES(15-15)getIterationsForShareVersion(51-57)deriveSecret(79-90)encryptPayload(92-111)decryptPayload(113-128)randomSaltHex(130-132)assertShareCredentialFormat(134-138)decryptShareCredential(232-284)
tests/naming.test.ts (2)
src/keyset/paths.ts (1)
getShareDirectory(22-24)tests/helpers/tmp.ts (1)
makeTmp(5-8)
src/components/ui/CredentialDisplay.tsx (1)
src/components/ui/QRCodeDisplay.tsx (1)
QRCodeDisplay(12-78)
src/components/keyset/KeysetLoad.tsx (1)
src/components/ui/CredentialDisplay.tsx (1)
CredentialDisplay(13-31)
tests/cli.parsing.test.ts (1)
src/lib/parseArgv.ts (2)
parseArgv(11-91)toBool(93-101)
tests/cli.basic.test.ts (1)
tests/helpers/runCli.ts (1)
runCli(21-82)
🪛 markdownlint-cli2 (0.18.1)
llm/context/testing/test-architecture.md
17-17: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (41)
AGENTS.md (2)
7-7: LGTM! Documentation correctly reflects bun-based workflow.The updated commands align with the bun scripts defined in
package.jsonand provide clear guidance for developers using the bun runtime.
13-13: LGTM! Test command documentation updated appropriately.The change from npm-based to bun-based test execution is consistent with the project's tooling direction.
src/components/Intro.tsx (3)
1-1: LGTM! React 19 compatible import pattern.Using named imports for
useStateanduseEffectis the standard approach and works correctly with React 19.1.1.
4-86: LGTM! Well-structured animation frames.The ASCII art frames create a smooth rotation effect with clear comments indicating the rotation angle. The 8-frame sequence provides good visual continuity.
94-102: LGTM! Animation logic is correct and properly cleaned up.The animation implementation:
- Correctly cycles through frames using modulo arithmetic
- Properly cleans up the interval on unmount
- Uses an appropriate 200ms delay for smooth animation
Also applies to: 107-107
tests/paths.test.ts (6)
1-5: LGTM! Correct imports with .js extension.The imports correctly use the
.jsextension for the TypeScript ESM requirement, as specified in the coding guidelines.
11-24: LGTM! Proper environment variable isolation.The test correctly:
- Saves the original environment variable
- Sets a test value
- Verifies the override behavior
- Properly restores or deletes the variable in the finally block
This follows the environment-variable testing patterns documented in the test architecture.
26-41: LGTM! Edge case properly tested.The test correctly verifies that an empty string in
IGLOO_APPDATAfalls through to platform-specific logic rather than being used as-is. The assertions confirm non-empty fallback behavior.
43-81: LGTM! Comprehensive platform-specific testing.The test validates platform-appropriate paths for:
- macOS:
~/Library/Application Support- Windows:
AppDataorappdata- Linux:
XDG_CONFIG_HOMEor~/.configThis approach tests actual platform behavior while maintaining environment isolation.
83-96: LGTM! Absolute path requirements verified.Both tests correctly assert that returned paths are absolute and meet structural expectations (e.g., ending with
igloo/shares). The cleanup logic is consistent across all tests.Also applies to: 117-131
102-115: LGTM! Path construction properly validated.The test verifies that
getShareDirectorycorrectly appendsigloo/sharesto the app data path, using environment variable override for isolation.package.json (1)
37-37: No action required. Theqrcodepackage is already at the latest version (1.5.4) with no known security vulnerabilities. The@types/qrcodedependency (^1.5.6) is also current and secure.tests/naming.test.ts (1)
1-143: LGTM! Comprehensive test coverage for naming utilities.The test suite thoroughly covers all naming functions with both happy paths and edge cases:
- Slugification logic (casing, special chars, trimming, empty inputs)
- Share ID construction with slugification
- File path building
- Name existence checking with filesystem interactions
Test organization is clear with section markers, and proper cleanup is used throughout.
tests/storage.test.ts (1)
1-355: LGTM! Excellent test coverage for storage functionality.The test suite is comprehensive and well-organized:
- ensureShareDirectory: Tests creation, idempotence, overrides, and path correctness
- readShareFiles: Tests empty directories, valid/invalid JSON, non-JSON files, malformed data, filepath attachment, and policy normalization
- saveShareRecord: Tests file creation, version/timestamp injection, policy pruning, and directory overrides
- loadShareRecord: Tests loading by ID/filepath, missing files, and policy normalization
- Integration: Validates full roundtrip consistency
Test structure follows best practices with proper setup/teardown, helper functions, and clear section organization.
Based on learnings, the test suite appropriately exercises both happy paths and failure modes for the keyset storage functionality.
tests/policy.test.ts (1)
1-535: LGTM! Exceptional test coverage for policy functionality.The test suite is extremely thorough and well-crafted:
- Constants & Defaults: Validates exported constants and default policy creation
- Boolean Coercion: Extensive testing of string-to-boolean conversion (true/false, 1/0, yes/no, on/off, case insensitivity, invalid values)
- Policy Management: Tests normalization, peer pubkey handling, empty string filtering, timestamp fallbacks
- CRUD Operations: Comprehensive coverage of setPolicyDefaults, upsertPeerPolicy, removePeerPolicy with edge cases
- Data Integrity: Excellent immutability test verifying all functions return new objects without mutating inputs
The test organization with clear section markers and helper functions makes the suite highly maintainable.
Based on learnings, the test suite appropriately exercises both happy paths and failure modes for the policy module.
tests/crypto.test.ts (1)
1-471: LGTM! Comprehensive crypto test coverage with excellent edge case handling.The test suite thoroughly validates the crypto module:
- Constants: Type and value validation for all exported constants
- Key Derivation: Tests determinism, sensitivity to inputs (password/salt/iterations/encoding)
- Encryption: Validates output format, base64url encoding, determinism with explicit IVs, and randomness
- Decryption: Tests successful roundtrips, wrong keys, corrupted/truncated data, and legacy 12-byte IV compatibility
- Share Credentials: Tests v1 and legacy formats, wrong passwords (with appropriate timeout), metadata-driven parameters
- Integration: Full roundtrip validation ensuring consistency
The manual construction of legacy-format encrypted payloads (lines 202-223, 325-357) demonstrates thorough testing of backward compatibility.
Based on learnings, the test suite appropriately exercises both happy paths and failure modes for the crypto module.
CLAUDE.md (1)
1-147: LGTM! Excellent documentation updates reflecting the new architecture.The documentation comprehensively captures the PR changes:
- Workflow Updates: Clear migration from npm to bun-based commands
- Command Structure: Well-organized namespace hierarchy (share, keyset, keys) with clear examples
- Architecture: Detailed component organization showing the new structure
- Core Library: Documents the non-UI utilities under
src/keyset/- Dependencies: Updated to reflect @frostr/bifrost, @frostr/igloo-core, and other key packages
- Environment Variables: New debug and test options documented
The guidance is clear and actionable for developers working with the codebase.
src/components/keyset/KeysetCreate.tsx (2)
133-133: LGTM: QR flag derivation is clean and handles both naming conventions.The logic correctly checks both
flags.qrandflags['show-qr']with explicit boolean comparison, and the bracket notation is appropriate for the hyphenated flag name.
499-499: LGTM: QR flag properly forwarded to ShareSaver.The
showQRprop is cleanly passed through to the ShareSaver component, enabling QR code display functionality.src/cli.tsx (1)
8-9: LGTM: Clean refactoring to centralized argument parsing.Moving
parseArgv,toBool, and related types to a dedicated module improves code organization and testability. The comment on line 47 helpfully documents the change.Also applies to: 47-47
src/components/Help.tsx (1)
4-14: LGTM: Nice visual enhancement with proper constant naming.The ASCII art icon adds visual polish to the help screen. The constant follows the SCREAMING_SNAKE_CASE convention and is rendered correctly using Ink's Text component.
Also applies to: 24-26
src/components/ui/CredentialDisplay.tsx (1)
1-31: LGTM: Well-designed component with clear single responsibility.This new component provides a clean abstraction for displaying credentials with optional QR codes. The implementation correctly uses Ink primitives, follows TypeScript conventions, and includes sensible defaults for colors. The conditional QR rendering integrates well with the QRCodeDisplay component.
src/components/keyset/KeysetLoad.tsx (4)
2-2: LGTM: Clean integration of QR visibility support.The imports, flag derivation, and state initialization are well-structured. The optional chaining on
flagsis correct given thatflagsis optional in the component props.Also applies to: 6-6, 30-32
147-151: LGTM: Keyboard handler properly guarded.The Q key toggle is appropriately limited to raw mode and the result phase via the
isActivecondition, preventing interference with interactive prompts.
289-305: LGTM: CredentialDisplay integration enhances UX.Replacing plain text credentials with the CredentialDisplay component provides a consistent presentation and seamless QR code support. The toggle hint is appropriately conditional on raw mode availability.
336-352: LGTM: Finish prompt complements the interactive flow.The finish prompt in raw mode provides a clean exit mechanism. The auto-exit logic for non-interactive environments (lines 119-134) ensures the component doesn't hang in test scenarios.
tests/cli.basic.test.ts (1)
21-30: LGTM! Pattern-based completion detection is the right approach.The use of
successPatternto detect when the intro screen content renders is appropriate for testing continuously running animated UI. The comment clearly explains the rationale, and thetimedOutassertion ensures the test doesn't hang indefinitely.llm/context/testing/test-architecture.md (1)
1-309: Excellent test architecture documentation.This comprehensive documentation clearly describes the test framework, organization, patterns, helpers, and best practices. It will be valuable for maintainers and contributors to understand the testing strategy and conventions.
src/components/keyset/ShareSaver.tsx (4)
1-32: LGTM! Clean integration of QR display prop.The addition of the
showQRprop with a default value offalseis well-designed, making QR code display opt-in. The imports follow the project's ESM conventions with.jsextensions.
59-100: LGTM! Well-implemented QR toggle with good UX.The keyboard interaction is properly scoped via
isActive: shouldPrompt, ensuring the Q key toggle only works when user input is expected. The hint message clearly indicates the current state and available action.
102-131: LGTM! Good refactoring to use CredentialDisplay component.Migrating from inline Box/Text blocks to the
CredentialDisplaycomponent reduces duplication and centralizes credential rendering logic. The QR visibility is correctly controlled via theqrVisiblestate.
391-453: LGTM! Consistent QR hint placement across render phases.The QR hint is appropriately shown only when
shouldPromptis true, ensuring users see the toggle instruction in interactive modes while keeping automated/non-interactive runs clean.tests/cli.parsing.test.ts (3)
1-40: LGTM! Well-structured basic positional tests.The tests correctly validate default command behavior, positional parsing, and help/version flag detection. The import paths follow the project's ESM conventions with
.jsextensions.
42-142: LGTM! Comprehensive flag parsing coverage.The tests thoroughly exercise flag parsing including inline formats (
--flag=value), empty values, aliases, and boolean detection. The alias tests correctly verify that original short flags are deleted after mapping, preventing duplicate entries.
144-181: LGTM! Thorough toBool function coverage.The tests validate all expected string values, case-insensitivity, boolean passthrough, and fallback behavior for undefined and invalid inputs. This provides excellent coverage of the boolean coercion utility.
src/components/ui/QRCodeDisplay.tsx (3)
1-19: LGTM! Clean component setup with good defaults.The props are well-typed with sensible defaults (
small = truefor compact terminal output,labelColor = 'cyan'for consistency). The state structure appropriately separates QR string content from error messages.
21-52: LGTM! Proper async handling with cancellation support.The cancellation pattern correctly prevents state updates after component unmount, avoiding memory leaks and React warnings. The QR generation uses appropriate parameters for terminal display (
type: 'terminal',errorCorrectionLevel: 'M').
54-78: LGTM! Clear three-state rendering with good UX.The rendering logic appropriately handles error, loading, and success states with clear visual feedback. The component correctly uses Ink's
BoxandTextcomponents for terminal UI, adhering to the project's coding guidelines.src/lib/parseArgv.ts (3)
1-9: LGTM! Clean type definitions.The type definitions are well-structured and appropriately capture the parsing result. The
Flagstype correctly allows both string and boolean values, matching the parsing logic below.
11-66: Excellent implementation of CLI argument parsing.The parsing logic correctly handles all required forms per the learnings:
--flag=value(including empty string for--flag=)--flag value(space-separated)-f value(short form with value)Boolean flags are appropriately defaulted when no value follows, and the edge case of standalone
-is correctly handled at line 53. Based on learnings, this aligns with the manual parsing requirements.
93-101: Well-implemented boolean conversion utility.The
toBoolfunction handles common boolean string representations correctly with sensible normalization (trim + lowercase). The default tofalsefor undefined or unrecognized values is a safe and reasonable choice.
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.