Skip to content

first pass test coverage overhaul#14

Merged
AustinKelsay merged 2 commits intostagingfrom
refactor/test-coverage-1
Jan 5, 2026
Merged

first pass test coverage overhaul#14
AustinKelsay merged 2 commits intostagingfrom
refactor/test-coverage-1

Conversation

@AustinKelsay
Copy link
Copy Markdown
Member

@AustinKelsay AustinKelsay commented Jan 5, 2026

Summary by CodeRabbit

  • Documentation

    • Replaced npm commands with Bun equivalents across developer guides and examples; added a new test-architecture guide describing testing patterns and workflows.
  • Tests

    • Added extensive test coverage across crypto, policy, storage, CLI parsing, naming, and path utilities — 97 new tests, bringing the suite to 164 passing tests.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 5, 2026

📝 Walkthrough

Walkthrough

Extracts CLI argument parsing into a new reusable module and adds extensive test coverage (multiple new test suites and updates). Documentation and examples switch npm commands to Bun equivalents. Minor test helper signature change for runCli.

Changes

Cohort / File(s) Summary
Documentation
AGENTS.md, CLAUDE.md, llm/context/testing/test-architecture.md, llm/implementation/test-coverage-implementation.md
Replaced npm command examples with Bun equivalents; added test-architecture doc and updated test-coverage documentation/metrics.
CLI parsing extraction
src/lib/parseArgv.ts, src/cli.tsx
New src/lib/parseArgv.ts exporting parseArgv, toBool, and types ParsedArgs/Flags; src/cli.tsx now imports parsing logic instead of defining it inline.
Test helper change
tests/helpers/runCli.js, tests/cli.basic.test.ts
runCli signature changed to accept options (successPattern, env, timeout) and to return { stdout, timedOut }; tests updated to use successPattern-driven termination.
New/Expanded test suites
tests/crypto.test.ts, tests/policy.test.ts, tests/storage.test.ts, tests/cli.parsing.test.ts, tests/naming.test.ts, tests/paths.test.ts
Added comprehensive tests for crypto, policy, storage I/O, CLI parsing and boolean coercion, naming utilities, and platform path behavior (total ~97 new tests across these files).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 I hopped a parser out into the light,
Tests multiplied through day and night,
Crypto, paths, and shares in a row,
Bun commands whisper, ready to go—
A tiny rabbit cheers the new test glow.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'first pass test coverage overhaul' directly reflects the main changes in the pull request, which involve adding comprehensive test coverage across multiple modules and documentation updates.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

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

@AustinKelsay
Copy link
Copy Markdown
Member Author

@CodeRabbit full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 5, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Fix all issues with AI Agents 🤖
In @llm/context/testing/test-architecture.md:
- Line 255: Replace the redundant phrase "CLI interface" in the heading/text
"Integration with the CLI interface" with either "Integration with the CLI" or
"Integration with the command-line interface" so the wording is not repetitive;
update any other occurrences of "CLI interface" in the document to the chosen
form for consistency.

In @llm/implementation/test-coverage-implementation.md:
- Line 5: The two test-metrics statements in the file are inconsistent: update
both occurrences so they report the same, verified test results; locate the
phrases "163 passing, 1 pre-existing failure" and "164 passing, 0 failing"
(around lines where the summary & detailed results are written) and replace them
with the actual, verified output from the test run (e.g., "164 passing, 0
failing" or "163 passing, 1 failing") and ensure any adjacent totals (like "97
new tests" and "total 164 tests") align with that same verified result before
committing.

In @src/lib/parseArgv.ts:
- Around line 30-50: The long-form flag parsing treats `--flag=` as a boolean
because the check `inline !== undefined && inline.length > 0` rejects an empty
string; update the logic in the `value.startsWith('--')` branch (the `inline`,
`name`, `equalsIndex` handling that assigns `flags[name] = inline` or falls back
to peeking `argv[index+1]`) so that if an `=` is present (`equalsIndex !== -1`)
you always assign the RHS (even if empty) i.e. set `flags[name] = inline` when
`inline !== undefined` (or when `equalsIndex !== -1`), and only treat it as
boolean when there is no `=` and no next non-flag arg; add/adjust tests to
assert `--flag=` yields an empty string for `flags["flag"]`.
🧹 Nitpick comments (3)
llm/context/testing/test-architecture.md (1)

17-17: Add language specifier to fenced code block.

The fenced code block starting at line 17 should specify a language for syntax highlighting and better rendering.

🔎 Proposed fix
-```
+```text
 tests/
 ├── helpers/
src/cli.tsx (1)

47-47: Consider removing redundant comment.

This comment restates what the imports already convey. The imports on lines 8-9 are self-documenting.

🔎 Suggested change
-// parseArgv, toBool, ParsedArgs, and Flags are now imported from ./lib/parseArgv.js
+
src/lib/parseArgv.ts (1)

92-100: LGTM! Robust boolean coercion.

The toBool function handles common truthy/falsy string values and defaults to false for unrecognized inputs. This is a sensible defensive default.

Consider adding a JSDoc comment to document the recognized values and the default behavior:

📝 Optional documentation enhancement
+/**
+ * Converts a value to boolean.
+ * Recognizes: '1', 'true', 'yes', 'on' (true) and '0', 'false', 'no', 'off' (false).
+ * Defaults to false for undefined or unrecognized strings.
+ */
 export function toBool(value: string | boolean | undefined): boolean {
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f07c0fc and 3aba615.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • AGENTS.md
  • CLAUDE.md
  • llm/context/testing/test-architecture.md
  • llm/implementation/test-coverage-implementation.md
  • src/cli.tsx
  • src/lib/parseArgv.ts
  • tests/cli.basic.test.ts
  • tests/cli.parsing.test.ts
  • tests/crypto.test.ts
  • tests/naming.test.ts
  • tests/paths.test.ts
  • tests/policy.test.ts
  • tests/storage.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/paths.test.ts
  • tests/naming.test.ts
  • tests/crypto.test.ts
  • tests/storage.test.ts
  • tests/policy.test.ts
  • tests/cli.basic.test.ts
  • tests/cli.parsing.test.ts
**/*.{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:

  • tests/paths.test.ts
  • tests/naming.test.ts
  • tests/crypto.test.ts
  • src/lib/parseArgv.ts
  • tests/storage.test.ts
  • tests/policy.test.ts
  • tests/cli.basic.test.ts
  • src/cli.tsx
  • tests/cli.parsing.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.md
  • llm/implementation/test-coverage-implementation.md
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/lib/parseArgv.ts
  • src/cli.tsx
src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use Ink components (, , etc.) for terminal UI and do not use HTML elements

Files:

  • src/cli.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 (23)
📚 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/paths.test.ts
  • llm/context/testing/test-architecture.md
  • tests/crypto.test.ts
  • tests/policy.test.ts
  • tests/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/paths.test.ts
  • llm/context/testing/test-architecture.md
  • tests/crypto.test.ts
  • tests/policy.test.ts
  • tests/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/__tests__/**/*.ts : Allow tests under src/__tests__/

Applied to files:

  • tests/paths.test.ts
  • 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: 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.md
  • AGENTS.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.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 src/keyset/**/*.ts : Keep key exchange and credential helpers under src/keyset/

Applied to files:

  • tests/crypto.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 : When adding command flags, extend parseArgv() in cli.tsx for any custom parsing logic

Applied to files:

  • src/lib/parseArgv.ts
  • src/cli.tsx
  • tests/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.ts
  • src/cli.tsx
  • tests/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 : Extract flags from props in App.tsx and pass them to command components; add validation/defaults as needed

Applied to files:

  • src/lib/parseArgv.ts
  • CLAUDE.md
  • src/cli.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/cli.tsx : Keep CLI flags lower-case (e.g., --verbose)

Applied to files:

  • src/lib/parseArgv.ts
  • AGENTS.md
  • src/cli.tsx
  • tests/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/keyset/**/*.ts : Document non-obvious flows with concise comments, especially around key lifecycle management

Applied to files:

  • tests/storage.test.ts
  • AGENTS.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 src/cli.tsx : Keep the CLI entrypoint in src/cli.tsx and use it to bootstrap Ink rendering

Applied to files:

  • AGENTS.md
  • tests/cli.basic.test.ts
  • CLAUDE.md
  • src/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 src/**/*.tsx : Use Ink components (<Box>, <Text>, etc.) for terminal UI and do not use HTML elements

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 tsup.config.ts : tsup entry must be src/cli.tsx

Applied to files:

  • AGENTS.md
  • CLAUDE.md
  • src/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.md
  • tests/cli.basic.test.ts
  • CLAUDE.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/**/*.tsx : When adding a new command, implement it as a component under src/components/

Applied to files:

  • AGENTS.md
  • CLAUDE.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/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:

  • CLAUDE.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 src/App.tsx : Place routes and screen wiring in src/App.tsx

Applied to files:

  • CLAUDE.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/App.tsx : Route commands via a switch statement in App.tsx based on a normalized command string

Applied to files:

  • CLAUDE.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 src/components/**/*.tsx : Store reusable UI components under src/components/

Applied to files:

  • CLAUDE.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 src/components/**/*.{ts,tsx} : Name React components using PascalCase

Applied to files:

  • CLAUDE.md
🧬 Code graph analysis (6)
tests/paths.test.ts (1)
src/keyset/paths.ts (2)
  • getAppDataPath (4-20)
  • getShareDirectory (22-24)
tests/naming.test.ts (2)
src/keyset/paths.ts (1)
  • getShareDirectory (22-24)
tests/helpers/tmp.ts (1)
  • makeTmp (5-8)
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/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)
tests/cli.basic.test.ts (1)
tests/helpers/runCli.ts (1)
  • runCli (21-82)
tests/cli.parsing.test.ts (1)
src/lib/parseArgv.ts (2)
  • parseArgv (11-90)
  • toBool (92-100)
🪛 LanguageTool
llm/context/testing/test-architecture.md

[style] ~255-~255: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”.
Context: ... and persistence - Integration with the CLI interface --- ## Testing Best Practices ### 1....

(ACRONYM_TAUTOLOGY)

🪛 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 (32)
tests/paths.test.ts (1)

1-131: Excellent test coverage!

This test file demonstrates thorough coverage of the path utilities with proper environment isolation, platform-specific behavior validation, and edge case handling. The pattern of saving/restoring environment variables in try/finally blocks ensures test reliability.

Based on learnings, test files should exercise both happy paths and failure modes—this file does exactly that.

llm/context/testing/test-architecture.md (1)

1-309: Well-structured test architecture documentation!

This document provides comprehensive guidance on the testing approach, file organization, and best practices. It will be valuable for contributors understanding the test infrastructure.

tests/naming.test.ts (1)

1-143: Comprehensive naming utility test coverage!

This test file provides thorough coverage of the naming utilities with proper test isolation using temporary directories and environment variable management. The tests cover both pure functions (slugification, ID building) and file I/O operations (existence checks) following project conventions.

Based on learnings, test files should exercise both happy paths and failure modes—this file does that well, including empty input handling, missing directories, and edge cases.

tests/crypto.test.ts (1)

1-471: Excellent comprehensive cryptographic test coverage!

This test file demonstrates thorough validation of the cryptographic operations with excellent coverage of:

  • Happy paths (successful encryption/decryption)
  • Error cases (wrong keys, corrupted data, truncated ciphertext)
  • Legacy format compatibility (v0 with 12-byte IV and 32 iterations)
  • Edge cases (determinism, uniqueness, format validation)

The long timeout on the wrong password test (line 301) is properly justified in the comment. Based on learnings, test files should exercise both happy paths and failure modes—this file exemplifies that approach.

llm/implementation/test-coverage-implementation.md (1)

1-177: Comprehensive test coverage documentation!

This document provides excellent tracking of the test implementation effort, clearly documenting the scope, coverage, and metrics for the new test suite. The organization by module with priority levels is particularly helpful.

AGENTS.md (1)

7-7: LGTM!

Documentation correctly updated to reflect the Bun-based workflow. The commands are consistent with the project's migration from npm to Bun.

Also applies to: 13-13

CLAUDE.md (2)

15-34: LGTM!

Development commands section correctly migrated to Bun equivalents. The workflow remains consistent with the updated AGENTS.md.


140-141: LGTM!

"Adding a New Command" instructions correctly updated to use Bun commands.

src/cli.tsx (1)

8-9: LGTM!

Good refactoring to extract parsing utilities into a dedicated module for testability. Import paths correctly use .js extensions as required by TypeScript ESM. As per coding guidelines, this follows the pattern of keeping CLI flags lower-case.

tests/cli.basic.test.ts (2)

21-30: LGTM!

Good use of successPattern to handle the animated intro screen that runs indefinitely. The comments clearly explain the rationale, and the timedOut assertion appropriately verifies graceful termination.


32-91: LGTM!

Test cases cover essential CLI functionality including version, help, namespace help routing, and commands with no shares. Both happy paths and informative error states are tested, aligning with coding guidelines.

tests/cli.parsing.test.ts (5)

1-4: LGTM!

Good setup with proper imports using .js extension as required by TypeScript ESM guidelines.


9-28: LGTM!

Comprehensive coverage of positional argument parsing including the empty array edge case.


46-74: LGTM!

Tests correctly exercise all three flag forms (--flag value, --flag=value, -f value) as specified in the coding guidelines. Boolean flag detection is also covered.


80-96: LGTM!

Good attention to detail by verifying that original short flags are deleted after aliasing, ensuring clean flag output.


137-169: LGTM!

Thorough toBool coverage including valid true/false values, case insensitivity, boolean passthrough, undefined handling, and invalid string fallback. As per coding guidelines, both happy paths and failure modes are exercised.

tests/storage.test.ts (5)

1-15: LGTM!

Proper imports with .js extensions and appropriate test utilities. Good use of node:assert/strict for stricter equality checks.


36-46: LGTM!

Excellent test isolation pattern using temporary directories and IGLOO_APPDATA override. Proper cleanup in afterEach prevents test pollution.


93-180: LGTM!

Comprehensive readShareFiles tests covering edge cases: missing directory, empty directory, valid JSON parsing, non-JSON filtering, malformed JSON handling, filepath attachment, and policy defaulting. As per coding guidelines, both happy paths and failure modes are exercised.


186-265: LGTM!

Good coverage of saveShareRecord including automatic version/timestamp injection, peer pruning, and directory override support.


321-355: LGTM!

The roundtrip integration test validates end-to-end save/load consistency including policy normalization. This is an excellent pattern for ensuring data integrity.

tests/policy.test.ts (6)

1-14: LGTM!

Clean imports with proper .js extensions and comprehensive coverage of policy utilities.


20-38: LGTM!

Well-designed test helper factories that reduce boilerplate while maintaining flexibility through overrides.


90-242: LGTM!

Excellent coverage of boolean coercion including all accepted string formats (true/false, 1/0, yes/no, on/off), case insensitivity, and fallback behavior for invalid values.


263-285: LGTM!

Good edge case testing for pubkey normalization, verifying that the 02 prefix is stripped while preserving policy values.


404-416: LGTM!

Important behavior test: verifying that upsertPeerPolicy removes peers when their settings match defaults, avoiding redundant storage.


508-535: LGTM!

Excellent immutability verification using JSON serialization comparison. This ensures all policy functions follow functional programming principles and don't cause unexpected side effects.

src/lib/parseArgv.ts (5)

1-9: LGTM! Clean type definitions.

The Flags and ParsedArgs types are well-structured and provide clear contracts for the parsing API.


11-28: LGTM! Global flag handling is correct.

The initialization and global flag detection for --help/-h and --version/-v are straightforward and follow the expected conventions.


52-62: LGTM! Short-form parsing is correct.

The short-form flag parsing correctly handles -f value and -f (boolean) forms as required. Note that this implementation treats multi-character short flags like -abc as a single flag named "abc" rather than expanding to -a -b -c. This appears intentional given the project's requirements.


67-81: LGTM! Elegant alias mapping approach.

The post-processing approach to map short aliases (-t--threshold, -T--total, -E--debug-echo) is clean and correctly preserves explicit long-form flags when both are provided.


83-89: LGTM! Well-structured return value.

The default command of 'intro' and the clean separation of command/args/flags aligns with the expected CLI behavior.

@AustinKelsay
Copy link
Copy Markdown
Member Author

@CodeRabbit full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 5, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (3)
llm/context/testing/test-architecture.md (1)

255-255: Fix redundant phrase "CLI interface".

The phrase "CLI interface" is redundant since CLI already stands for "Command Line Interface." Use either "CLI" or "command-line interface."

🔎 Proposed fix
-- Integration with the CLI interface
+- Integration with the CLI
src/lib/parseArgv.ts (1)

30-51: Past review issue resolved - long-form flag parsing now handles all cases correctly.

The --flag= edge case from the previous review has been fixed. Lines 36-39 now correctly assign the RHS substring when an equals sign is present, including empty strings. The fallback logic for --flag value and boolean flags is also correct.

llm/implementation/test-coverage-implementation.md (1)

1-143: Past review issue resolved - test metrics are now consistent.

The inconsistency between line 5 and lines 139-140 has been fixed. Both sections now correctly report "164 passing, 0 failing." The documentation is comprehensive and well-structured.

🧹 Nitpick comments (3)
llm/context/testing/test-architecture.md (3)

8-10: Inconsistent tooling commands with other documentation.

This file uses npm test and npm run test:bun, while AGENTS.md and CLAUDE.md have been updated to use bun test and bun run commands throughout. Consider updating these references to maintain consistency across the documentation.

🔎 Suggested updates for consistency
-npm test           # Full test suite (typecheck + build + test)
-npm run test:bun   # Alternative Bun runtime
+bun test           # Full test suite (typecheck + build + test)
+bun run test:bun   # Alternative Bun runtime (if applicable)
 npx tsx --test tests/specific.test.ts  # Run single test file

288-290: Update test runner command for consistency.

The command examples use npm test, which is inconsistent with the Bun migration documented in AGENTS.md and CLAUDE.md.

🔎 Suggested update
 ```bash
 # Full suite (recommended)
-npm test
+bun test
 
 # Individual test file
 npx tsx --test tests/crypto.test.ts

305-309: Update final test command reference.

The final example uses npm test, which should be updated to match the Bun migration.

🔎 Suggested update
 4. For CLI tests, use `runCli()` helper
-5. Run `npm test` to verify
+5. Run `bun test` to verify
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f07c0fc and b462248.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • AGENTS.md
  • CLAUDE.md
  • llm/context/testing/test-architecture.md
  • llm/implementation/test-coverage-implementation.md
  • src/cli.tsx
  • src/lib/parseArgv.ts
  • tests/cli.basic.test.ts
  • tests/cli.parsing.test.ts
  • tests/crypto.test.ts
  • tests/naming.test.ts
  • tests/paths.test.ts
  • tests/policy.test.ts
  • tests/storage.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/cli.parsing.test.ts
  • tests/paths.test.ts
  • tests/naming.test.ts
  • tests/storage.test.ts
  • tests/crypto.test.ts
  • tests/policy.test.ts
  • tests/cli.basic.test.ts
**/*.{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:

  • tests/cli.parsing.test.ts
  • tests/paths.test.ts
  • tests/naming.test.ts
  • tests/storage.test.ts
  • tests/crypto.test.ts
  • src/cli.tsx
  • src/lib/parseArgv.ts
  • tests/policy.test.ts
  • tests/cli.basic.test.ts
src/**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Use Ink components (, , etc.) for terminal UI and do not use HTML elements

Files:

  • src/cli.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
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/cli.tsx
  • src/lib/parseArgv.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.md
  • llm/implementation/test-coverage-implementation.md
🧠 Learnings (25)
📓 Common learnings
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
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
📚 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:

  • tests/cli.parsing.test.ts
  • src/cli.tsx
  • src/lib/parseArgv.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:

  • tests/cli.parsing.test.ts
  • src/cli.tsx
  • src/lib/parseArgv.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/cli.parsing.test.ts
  • tests/paths.test.ts
  • tests/crypto.test.ts
  • tests/policy.test.ts
  • llm/context/testing/test-architecture.md
  • llm/implementation/test-coverage-implementation.md
  • tests/cli.basic.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:

  • tests/cli.parsing.test.ts
  • AGENTS.md
  • src/cli.tsx
  • src/lib/parseArgv.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 : Tests under src/__tests__/ should exercise both happy paths and failure modes

Applied to files:

  • tests/cli.parsing.test.ts
  • tests/paths.test.ts
  • tests/crypto.test.ts
  • tests/policy.test.ts
  • llm/context/testing/test-architecture.md
  • tests/cli.basic.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:

  • AGENTS.md
  • CLAUDE.md
  • src/cli.tsx
  • tests/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/**/*.tsx : Use Ink components (<Box>, <Text>, etc.) for terminal UI and do not use HTML elements

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 tsup.config.ts : tsup entry must be src/cli.tsx

Applied to files:

  • AGENTS.md
  • CLAUDE.md
  • src/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-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:

  • AGENTS.md
  • tests/policy.test.ts
  • 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 src/keyset/**/*.ts : Document non-obvious flows with concise comments, especially around key lifecycle management

Applied to files:

  • AGENTS.md
  • tests/storage.test.ts
  • tests/crypto.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/Intro.tsx : When adding a new command, update command examples in Intro.tsx if applicable

Applied to files:

  • AGENTS.md
  • CLAUDE.md
  • tests/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.md
  • CLAUDE.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 llm/** : Keep protocol prompts and agent scripts in llm/ and update them when UX flows change

Applied to files:

  • AGENTS.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 src/__tests__/**/*.ts : Allow tests under src/__tests__/

Applied to files:

  • tests/paths.test.ts
  • llm/context/testing/test-architecture.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/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:

  • CLAUDE.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 src/App.tsx : Place routes and screen wiring in src/App.tsx

Applied to files:

  • CLAUDE.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/App.tsx : Route commands via a switch statement in App.tsx based on a normalized command string

Applied to files:

  • CLAUDE.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 src/components/**/*.tsx : Store reusable UI components under src/components/

Applied to files:

  • CLAUDE.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/App.tsx : Extract flags from props in App.tsx and pass them to command components; add validation/defaults as needed

Applied to files:

  • CLAUDE.md
  • src/cli.tsx
  • src/lib/parseArgv.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/components/**/*.{ts,tsx} : Name React components using PascalCase

Applied to files:

  • CLAUDE.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 src/keyset/**/*.ts : Keep key exchange and credential helpers under src/keyset/

Applied to files:

  • tests/storage.test.ts
  • tests/crypto.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: 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.md
🧬 Code graph analysis (6)
tests/cli.parsing.test.ts (1)
src/lib/parseArgv.ts (2)
  • parseArgv (11-91)
  • toBool (93-101)
tests/paths.test.ts (1)
src/keyset/paths.ts (2)
  • getAppDataPath (4-20)
  • getShareDirectory (22-24)
tests/naming.test.ts (2)
src/keyset/paths.ts (1)
  • getShareDirectory (22-24)
tests/helpers/tmp.ts (1)
  • makeTmp (5-8)
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/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)
tests/cli.basic.test.ts (1)
tests/helpers/runCli.ts (1)
  • runCli (21-82)
🔇 Additional comments (42)
tests/paths.test.ts (2)

11-96: Comprehensive test coverage for getAppDataPath.

The tests cover environment override, empty-string fallback, platform-specific paths, and absolute path validation. Environment cleanup is properly handled in all cases.


102-131: Good coverage for getShareDirectory.

Tests verify correct path composition (appending igloo/shares) and absolute path guarantee. Environment handling is consistent with the rest of the test suite.

tests/crypto.test.ts (9)

28-53: Constants validation is thorough.

Tests verify both type and exact values for all crypto constants, ensuring the API surface is stable and correct.


59-71: Version-based iteration selection is well-tested.

Tests cover all branches: undefined (legacy), version 0 (legacy), and version 1+ (current iterations).


77-118: deriveSecret tests are comprehensive.

Tests validate determinism, output format, and proper differentiation based on password, salt, iterations, and encoding parameters.


124-160: encryptPayload tests cover key behaviors.

Tests validate output structure, base64url encoding, deterministic behavior with explicit IV, and randomness with random IV generation.


166-223: Excellent coverage of decryptPayload including failure modes.

Tests cover successful decryption, error cases (wrong key, truncated/corrupted ciphertext), and backward compatibility with legacy 12-byte IV format. This aligns well with testing best practices for exercising both happy paths and failure modes.


229-242: randomSaltHex tests validate format and uniqueness.

Tests verify the expected 32-character hex format and statistical uniqueness across multiple calls.


248-270: assertShareCredentialFormat tests cover validation logic.

Tests verify acceptance of valid bfshare prefixes and rejection of invalid formats including other credential types and edge cases.


276-435: decryptShareCredential tests are comprehensive.

Tests cover v1 format, legacy v0 format, wrong password failure, metadata-driven parameters, and return value structure. The timeout annotation on Line 301 is appropriately documented explaining the performance characteristic of brute-force iteration attempts.


441-471: Integration test validates end-to-end workflow.

The roundtrip test exercises the complete lifecycle: salt generation, key derivation, encryption, record creation, and decryption with verification of all parameters.

tests/naming.test.ts (4)

18-46: slugifyKeysetName tests cover transformation rules thoroughly.

Tests validate lowercasing, space-to-hyphen replacement, special character removal, hyphen trimming, and fallback behavior for empty/invalid inputs.


52-62: buildShareId tests verify correct ID composition.

Tests validate the expected format (slug_share_index) and confirm that slugification is applied to the name parameter.


68-89: buildShareFilePath test validates path construction.

Test verifies correct directory resolution, ID composition, and .json extension with proper environment cleanup.


95-143: keysetNameExists tests exercise file I/O scenarios.

Tests cover positive cases (matching files exist), negative cases (no matching files), and edge cases (missing directory). Temporary directory setup and cleanup is properly handled.

tests/storage.test.ts (6)

21-46: Test infrastructure is well-organized.

The createTestRecord helper reduces duplication, and beforeEach/afterEach hooks ensure proper test isolation with temporary directories.


52-87: ensureShareDirectory tests are comprehensive.

Tests validate directory creation, override behavior, idempotency, and path correctness. Good coverage of expected behaviors.


93-180: readShareFiles tests cover diverse scenarios.

Tests validate handling of missing/empty directories, valid JSON parsing, filtering non-JSON files, silent skipping of malformed JSON, filepath attachment, and policy normalization. Excellent coverage of both happy paths and failure modes.


186-265: saveShareRecord tests validate storage behavior.

Tests cover file creation, filename derivation from ID, metadata injection (version, savedAt), policy normalization (empty peer pruning), and directory override configuration.


271-315: loadShareRecord tests cover all access patterns.

Tests validate loading by ID and filepath, proper handling of missing files (returns undefined), and policy normalization on loaded data.


321-355: Integration test validates complete storage lifecycle.

The roundtrip test exercises save and load operations with complex data including policy configuration, verifying data integrity across the persistence boundary.

tests/policy.test.ts (11)

20-38: Test helpers streamline test setup.

The createTestPolicy and createTestRecord helpers reduce duplication and provide consistent test data across the suite.


44-50: Constant validation is appropriate.

Tests verify the expected default values for policy defaults.


56-84: createDefaultPolicy tests are comprehensive.

Tests validate default values, empty peers initialization, explicit timestamp usage, and automatic timestamp generation.


90-242: coerceBoolean tests are exceptionally thorough.

Tests comprehensively validate boolean coercion from various string formats ("true"/"false", "1"/"0", "yes"/"no", "on"/"off"), case insensitivity, and fallback behavior for invalid values. This extensive coverage is valuable for robust user input handling.


248-335: ensurePolicy tests validate normalization logic.

Tests cover default policy creation, pubkey normalization (02 prefix stripping), peer preservation, empty key filtering, and timestamp fallback behavior.


341-373: setPolicyDefaults tests validate update behavior.

Tests verify default updates, peer re-normalization, and timestamp updates when policy defaults change.


379-431: upsertPeerPolicy tests cover upsert logic thoroughly.

Tests validate peer addition, updates, automatic removal when matching defaults, and handling of non-standard pubkey formats.


437-462: removePeerPolicy tests validate deletion behavior.

Tests cover peer deletion, graceful no-op for missing peers, and timestamp updates.


468-475: updatePolicyTimestamp test validates behavior and immutability.

Test verifies timestamp update and confirms the original object is not mutated.


481-502: pruneEmptyPeers tests validate pruning logic.

Tests verify removal of empty peers object and preservation of non-empty peer collections.


508-535: Immutability test is excellent and important.

This comprehensive test verifies that all policy manipulation functions (updatePolicyTimestamp, setPolicyDefaults, upsertPeerPolicy, removePeerPolicy, pruneEmptyPeers) return new objects without mutating inputs. This is critical for preventing bugs in functional code patterns.

AGENTS.md (1)

7-7: LGTM! Bun migration is consistent.

The documentation updates correctly reflect the project's migration from npm to Bun across all development commands.

CLAUDE.md (1)

15-34: LGTM! Complete and consistent Bun migration.

All npm commands have been properly updated to use Bun equivalents throughout the development workflow documentation.

src/cli.tsx (1)

8-9: LGTM! Clean extraction of parsing logic.

Moving parseArgv and toBool to a dedicated module improves testability and follows the separation of concerns principle. The imports correctly use .js extensions per TypeScript ESM requirements.

tests/cli.parsing.test.ts (1)

1-181: LGTM! Comprehensive and well-structured test suite.

This test file provides excellent coverage of the CLI argument parsing functionality:

  • 22 tests total covering all documented parseArgv and toBool behaviors
  • Happy paths and edge cases: empty arrays, positionals, flags (long/short forms), aliases, boolean detection, empty values
  • Well-organized: Clear section headers and focused test cases
  • Follows guidelines: Uses node:test and node:assert/strict, tests both success and failure scenarios

The test suite validates critical CLI parsing requirements including --flag value, --flag=value, and -f value forms, plus alias transformations as per the coding guidelines.

tests/cli.basic.test.ts (1)

21-30: LGTM! Clean solution for testing animated content.

The successPattern approach correctly handles the infinite animation while providing clear test assertions. The explicit timeout check with a descriptive message improves test diagnostics.

src/lib/parseArgv.ts (5)

1-9: LGTM! Clear type definitions.

The exported types provide a clean interface for the parsing module.


11-28: LGTM! Clean initialization and global flag handling.

Help and version flags are correctly detected and isolated from regular flag processing.


53-66: LGTM! Short-form parsing is consistent with long-form logic.

The implementation correctly handles -f value and -f (boolean) forms. Note that bundled short flags (e.g., -abc as three separate flags) are not supported—this is treated as a single flag name "abc", which is acceptable for this CLI's requirements.


68-91: LGTM! Alias handling and return structure are correct.

The post-processing correctly promotes short-form aliases to their long-form equivalents only when the long-form is not already set, preventing conflicts. The default command of 'intro' aligns with the CLI's behavior.


93-101: LGTM! Robust boolean coercion.

The toBool function handles a comprehensive set of string representations with case-insensitivity and whitespace trimming. The default to false for unrecognized values is a safe choice.

@AustinKelsay AustinKelsay merged commit 4241541 into staging Jan 5, 2026
2 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 5, 2026
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