Conversation
WalkthroughSplits docs to document Node (default) vs Bun (optional); adds Bun scripts and publishConfig; adds non-interactive/autopilot flags, stdin/Ink polyfills, and auto-exit hooks; refactors echo relay normalization into computeEchoRelays; adds ephemeral Nostr relay, many CLI E2E tests and helpers, and CI/release workflows and release docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Runner / runCli
participant CLI as CLI (dist/cli.js)
participant Env as Environment (IGLOO_*)
participant App as App Components (ShareAdd / KeysetLoad)
participant Relay as Ephemeral Nostr Relay
Test->>CLI: spawn with env (IGLOO_*)
CLI->>Env: read IGLOO_* flags
CLI->>App: render with inkOptions (fake stdin when disabled)
App->>Env: check autopilot / autodecrypt flags
alt Autopilot / Auto-decrypt enabled
App->>App: perform auto actions (decrypt, save)
alt echo enabled
App->>Relay: send echo event (relayUnion + logging)
Relay-->>App: ack / echo response
end
App->>CLI: render final output, schedule process.exit
else Interactive
App->>User: await input (useInput active)
end
CLI-->>Test: stdout/stderr captured
sequenceDiagram
autonumber
participant Client as Nostr Client
participant Relay as NostrRelay
participant Cache as Event Cache
participant Subs as Subscriptions
Client->>Relay: open WebSocket
Client->>Relay: REQ (filters)
Relay->>Subs: register subscription
Relay->>Cache: query matching events
Cache-->>Relay: return matches
Relay-->>Client: stream events + EOSE
Client->>Relay: EVENT (publish)
Relay->>Cache: store event
Relay->>Subs: fan-out to matching subscribers
Relay-->>Client: OK
Note over Relay,Cache: periodic cache purge removes old events
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Nitpick comments (4)
src/components/keyset/KeysetSigner.tsx (1)
624-643: Consider extracting environment flag parsing to a shared utility.The IIFE pattern for parsing environment flags is verbose and duplicated across multiple files (see similar code in KeysetList.tsx lines 22-26). This creates maintenance burden if flag parsing logic needs to change.
Consider extracting to a shared utility:
In a new file
src/utils/env.ts:export function isNonInteractive(): boolean { const rawMode = (process.env.IGLOO_DISABLE_RAW_MODE ?? '').toLowerCase(); const noInput = (process.env.IGLOO_CLI_NO_INPUT ?? '').toLowerCase(); return rawMode === '1' || rawMode === 'true' || noInput === '1' || noInput === 'true'; }Then simplify both files:
- const inputDisabled = (() => { - const a = (process.env.IGLOO_DISABLE_RAW_MODE ?? '').toLowerCase(); - const b = (process.env.IGLOO_CLI_NO_INPUT ?? '').toLowerCase(); - return a === '1' || a === 'true' || b === '1' || b === 'true'; - })(); + const inputDisabled = isNonInteractive();src/components/keyset/KeysetList.tsx (2)
20-26: Code duplication with KeysetSigner.tsx.This IIFE pattern for computing
nonInteractiveis identical to theinputDisabledlogic in KeysetSigner.tsx (lines 624-628). See the refactoring suggestion in KeysetSigner.tsx for extracting this to a shared utility.
47-53: Clean up effect dependencies and error handling.
- The effect dependencies include
state.shares.lengthandstate.errorwhich aren't used in the effect body. This causes unnecessary re-runs when shares or errors change.- The try/catch around
process.exit(0)is unnecessary sinceprocess.exit()doesn't throw exceptions.Apply this diff:
useEffect(() => { if (!nonInteractive || state.loading) return; const t = setTimeout(() => { - try { process.exit(0); } catch {} + process.exit(0); }, 10); return () => clearTimeout(t); - }, [nonInteractive, state.loading, state.shares.length, state.error]); + }, [nonInteractive, state.loading]);src/test-relay/NostrRelay.ts (1)
118-134: Stop should clear the purge intervalThe cache purge timer created on Line 118 isn’t cleared. Even after
stop()runs, that interval keeps firing, holding the relay instance in memory across tests and wasting CPU. Store the handle and clear it during shutdown.export class NostrRelay extends EventEmitter { private readonly subs: Map<string, Subscription> = new Map(); private readonly cache: NostrEvent[] = []; private readonly config: Required<NostrRelayConfig>; private wss?: WebSocketServer; private _url?: string; + private purgeTimer: NodeJS.Timeout | null = null; @@ - setInterval(() => { - this.cache.length = 0; - }, this.config.purgeIntervalSec * 1000).unref?.(); + this.purgeTimer = setInterval(() => { + this.cache.length = 0; + }, this.config.purgeIntervalSec * 1000); + this.purgeTimer.unref?.(); @@ this.subs.clear(); this.cache.length = 0; + if (this.purgeTimer) { + clearInterval(this.purgeTimer); + this.purgeTimer = null; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
README.md(2 hunks)package.json(1 hunks)src/cli.tsx(5 hunks)src/components/keyset/KeysetList.tsx(2 hunks)src/components/keyset/KeysetLoad.tsx(3 hunks)src/components/keyset/KeysetSigner.tsx(2 hunks)src/components/keyset/useShareEchoListener.ts(1 hunks)src/components/share/ShareAdd.tsx(5 hunks)src/keyset/echo.ts(1 hunks)src/test-relay/NostrRelay.ts(1 hunks)tests/cli.basic.test.ts(1 hunks)tests/cli.keys.convert.test.ts(1 hunks)tests/cli.relays.command.test.ts(1 hunks)tests/cli.share.flow.test.ts(1 hunks)tests/helpers/runCli.ts(1 hunks)tests/helpers/tmp.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
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/test-relay/NostrRelay.tssrc/components/keyset/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/keyset/KeysetList.tsxsrc/components/keyset/KeysetSigner.tsxsrc/cli.tsxsrc/keyset/echo.tssrc/components/share/ShareAdd.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/test-relay/NostrRelay.tstests/helpers/tmp.tssrc/components/keyset/KeysetLoad.tsxtests/cli.relays.command.test.tssrc/components/keyset/useShareEchoListener.tstests/cli.keys.convert.test.tssrc/components/keyset/KeysetList.tsxtests/cli.basic.test.tstests/helpers/runCli.tssrc/components/keyset/KeysetSigner.tsxsrc/cli.tsxtests/cli.share.flow.test.tssrc/keyset/echo.tssrc/components/share/ShareAdd.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ink components (, , etc.) for terminal UI and do not use HTML elements
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/keyset/KeysetList.tsxsrc/components/keyset/KeysetSigner.tsxsrc/cli.tsxsrc/components/share/ShareAdd.tsx
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/keyset/KeysetList.tsxsrc/components/keyset/KeysetSigner.tsxsrc/components/share/ShareAdd.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/KeysetLoad.tsxsrc/components/keyset/KeysetList.tsxsrc/components/keyset/KeysetSigner.tsxsrc/components/share/ShareAdd.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/keyset/KeysetList.tsxsrc/components/keyset/KeysetSigner.tsxsrc/components/share/ShareAdd.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/cli.relays.command.test.tstests/cli.keys.convert.test.tstests/cli.basic.test.tstests/cli.share.flow.test.ts
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/keyset/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/keyset/**/*.ts: Keep key exchange and credential helpers under src/keyset/
Document non-obvious flows with concise comments, especially around key lifecycle management
Files:
src/keyset/echo.ts
🧠 Learnings (2)
📚 Learning: 2025-10-03T23:45:30.161Z
Learnt from: CR
PR: FROSTR-ORG/igloo-cli#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:
README.md
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
PR: FROSTR-ORG/igloo-cli#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/cli.tsx
🧬 Code graph analysis (7)
src/components/keyset/KeysetLoad.tsx (2)
src/components/keyset/useShareEchoListener.ts (1)
useShareEchoListener(55-255)src/keyset/crypto.ts (1)
decryptShareCredential(232-284)
tests/cli.relays.command.test.ts (2)
tests/helpers/tmp.ts (1)
makeTmp(5-8)tests/helpers/runCli.ts (1)
runCli(13-69)
tests/cli.keys.convert.test.ts (2)
tests/helpers/tmp.ts (1)
makeTmp(5-8)tests/helpers/runCli.ts (1)
runCli(13-69)
tests/cli.basic.test.ts (2)
tests/helpers/tmp.ts (1)
makeTmp(5-8)tests/helpers/runCli.ts (1)
runCli(13-69)
src/cli.tsx (1)
src/components/Help.tsx (1)
Help(8-50)
tests/cli.share.flow.test.ts (3)
tests/helpers/tmp.ts (2)
makeTmp(5-8)writePasswordFile(10-14)src/test-relay/NostrRelay.ts (2)
startEphemeralRelay(207-211)url(95-97)tests/helpers/runCli.ts (1)
runCli(13-69)
src/components/share/ShareAdd.tsx (5)
src/keyset/crypto.ts (7)
randomSaltHex(130-132)deriveSecret(79-90)SHARE_FILE_PBKDF2_ITERATIONS(8-8)SHARE_FILE_PASSWORD_ENCODING(11-11)SHARE_FILE_SALT_PBKDF2_EXPANDED_BYTES(13-13)encryptPayload(92-111)SHARE_FILE_VERSION(7-7)src/keyset/types.ts (1)
ShareFileRecord(16-28)src/keyset/policy.ts (1)
createDefaultPolicy(14-20)src/keyset/storage.ts (1)
saveShareRecord(48-63)src/keyset/echo.ts (1)
sendShareEcho(15-39)
🪛 ast-grep (0.39.6)
tests/cli.keys.convert.test.ts
[warning] 30-30: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(nsec: ${expectedNsec})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 31-31: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(Public hex: ${hexPublicKey})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 32-32: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(npub: ${npub})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 52-52: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(npub: ${npub})
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
tests/cli.share.flow.test.ts
[warning] 71-71: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${name} share 1)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
tests/cli.basic.test.ts
[error] 7-7: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 7-7: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
🪛 LanguageTool
README.md
[grammar] ~19-~19: There might be a mistake here.
Context: ...js 18+ (primary target, default scripts) - Bun 1.0+ (optional; faster dev/build/tes...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...n 1.0+ (optional; faster dev/build/test) - npm 10+ (bundled with Node 20+) ## Firs...
(QB_NEW_EN)
[grammar] ~164-~164: There might be a mistake here.
Context: ...pecheck then run tests via tsx --test. - Bun (optional, faster) - `bun run dev:...
(QB_NEW_EN)
🔇 Additional comments (7)
src/components/keyset/useShareEchoListener.ts (1)
80-98: LGTM! Test relay override is cleanly implemented.The IGLOO_TEST_RELAY environment variable override provides a clean way to force relay selection in tests without modifying the underlying relay resolution logic. The short-circuit pattern is appropriate and the implementation is straightforward.
tests/helpers/tmp.ts (1)
1-15: LGTM! Clean and focused test utilities.The temporary directory and password file helpers are well-implemented with appropriate defaults. The trailing newline in the password file is good practice for file-based password inputs.
package.json (1)
7-16: Verify intended scope oftest:bunscript.The
test:bunscript only runstests/relays.test.tswhile the Nodetestscript runs all tests via the glob patterntests/**/*.test.ts. This inconsistency suggests either:
- Bun test coverage is intentionally limited to relay tests (work in progress), or
- This is an oversight and should match the Node test script.
Please clarify whether
test:bunshould run all tests like the Node version:- "test:bun": "bunx tsc --noEmit && bunx tsx --test tests/relays.test.ts", + "test:bun": "bunx tsc --noEmit && bunx tsx --test tests/**/*.test.ts",src/keyset/echo.ts (1)
20-32: LGTM! Test-mode environment flags are well-integrated.The IGLOO_SKIP_ECHO flag provides a clean way to disable echo in test scenarios, and the IGLOO_TEST_RELAY override ensures deterministic relay behavior. The implementation is consistent with similar patterns in useShareEchoListener.ts.
README.md (2)
19-42: LGTM! Clear documentation of dual runtime support.The documentation effectively communicates that Node.js is the primary runtime while positioning Bun as an optional, faster alternative. The parallel "First run" sections make it easy for users to choose their preferred runtime.
160-171: LGTM! Development workflow is well-documented.The side-by-side presentation of Node and Bun scripts makes it easy to see the parallel workflows and choose the appropriate runtime for development tasks.
tests/cli.relays.command.test.ts (1)
1-45: LGTM! Well-structured relay configuration tests.The test suite provides good coverage of relay management operations with proper isolation via temporary directories. The tests verify both exit codes and output content, and the mutation test exercises the full configuration lifecycle.
- Delegate send to sendEcho; listen via awaitShareEcho - Remove legacy hacks, notify helper, and rejection polyfill - Normalize relay handling; update tests and docs
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
tests/cli.share.flow.test.ts (1)
1-9: Reorder imports per shallow-to-deep convention.The external package import on line 8 should come before the local imports (lines 5-7), as node_modules dependencies are shallower than project-local files.
Apply this diff:
import {test, beforeEach, afterEach} from 'node:test'; import assert from 'node:assert/strict'; import fs from 'node:fs/promises'; import path from 'node:path'; +import {generateKeysetWithSecret} from '@frostr/igloo-core'; import {runCli} from './helpers/runCli.js'; import {makeTmp, writePasswordFile} from './helpers/tmp.js'; import {startEphemeralRelay} from '../src/test-relay/NostrRelay.js'; -import {generateKeysetWithSecret} from '@frostr/igloo-core'; import {buildShareId} from '../src/keyset/naming.js';Based on coding guidelines
src/components/keyset/KeysetLoad.tsx (1)
65-68: Guardprocess.envaccess for cross-runtime safetyUse a
typeof processcheck to avoid ReferenceError outside Node (e.g., bundlers/Bun settings).- const skipEcho = (() => { - const a = (process.env.IGLOO_SKIP_ECHO ?? '').toLowerCase(); - return a === '1' || a === 'true'; - })(); + const skipEcho = (() => { + const raw = typeof process !== 'undefined' ? process.env.IGLOO_SKIP_ECHO ?? '' : ''; + const a = raw.toLowerCase(); + return a === '1' || a === 'true'; + })();src/components/keyset/useShareEchoListener.ts (3)
69-71: Prefer cross‑env timer type
NodeJS.Timeoutcan clash in mixed DOM/Node typings. UseReturnType<typeof setTimeout>for Bun/Node compatibility.- const retryTimeoutRef = useRef<NodeJS.Timeout | null>(null); - const warningTimeoutRef = useRef<NodeJS.Timeout | null>(null); - const fallbackTimeoutRef = useRef<NodeJS.Timeout | null>(null); + const retryTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); + const warningTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); + const fallbackTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
98-115: Remove dead fallback fields left from previous logic
fallbackTimeoutRefandfallbackTriggeredRefare no longer used beyond clearing; simplify state/cleanup.- const fallbackTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); - const fallbackTriggeredRef = useRef(false); ... - if (fallbackTimeoutRef.current) { - clearTimeout(fallbackTimeoutRef.current); - fallbackTimeoutRef.current = null; - } + // (removed fallback timeout state)Note: Also remove the variable declarations and any remaining references in
clearPending(). Adjust imports/types accordingly.Also applies to: 189-197
74-96: Deduplicatenormalizehelper
normalizeis defined twice in the memo. Extract once above the hook for clarity and reuse.const normalizeRelayUrl = (u: string) => /^wss?:\/\//i.test(u) ? u.replace(/^http/i, 'ws') : `wss://${u}`; // then use: map(normalizeRelayUrl)src/components/share/ShareAdd.tsx (2)
376-391: Non‑interactive auto‑advance: LGTM; consider DRY env parsingThe logic is correct and aligns with the non-interactive requirement. Optionally centralize truthy env parsing to avoid repetition.
Helper (place near top-level):
const envTrue = (v?: string) => { const s = (v ?? '').toLowerCase(); return s === '1' || s === 'true'; };Then:
const nonInteractive = envTrue(process.env.IGLOO_DISABLE_RAW_MODE) || envTrue(process.env.IGLOO_TEST_AUTOPILOT);
476-498: Background echo with skip flag: LGTM; optional small cleanupThe non-blocking echo with mounted checks is solid. Optionally reuse a shared envTrue helper for IGLOO_SKIP_ECHO to keep parsing consistent across the file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/release.yml(1 hunks)RELEASING.md(1 hunks)llm/context/igloo-core-readme.md(2 hunks)package.json(3 hunks)src/components/keyset/KeysetLoad.tsx(3 hunks)src/components/keyset/useShareEchoListener.ts(3 hunks)src/components/share/ShareAdd.tsx(5 hunks)src/keyset/echo.ts(2 hunks)tests/cli.share.flow.test.ts(1 hunks)tests/helpers/runCli.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- llm/context/igloo-core-readme.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/keyset/echo.ts
- tests/helpers/runCli.ts
- package.json
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ink components (, , etc.) for terminal UI and do not use HTML elements
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/share/ShareAdd.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/KeysetLoad.tsxsrc/components/share/ShareAdd.tsxsrc/components/keyset/useShareEchoListener.ts
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/share/ShareAdd.tsxsrc/components/keyset/useShareEchoListener.ts
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/KeysetLoad.tsxsrc/components/share/ShareAdd.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/KeysetLoad.tsxsrc/components/share/ShareAdd.tsxsrc/components/keyset/useShareEchoListener.tstests/cli.share.flow.test.ts
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/share/ShareAdd.tsxsrc/components/keyset/useShareEchoListener.ts
**/*.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.share.flow.test.ts
🧬 Code graph analysis (3)
src/components/keyset/KeysetLoad.tsx (2)
src/components/keyset/useShareEchoListener.ts (1)
useShareEchoListener(49-224)src/keyset/crypto.ts (1)
decryptShareCredential(232-284)
src/components/share/ShareAdd.tsx (5)
src/keyset/crypto.ts (7)
randomSaltHex(130-132)deriveSecret(79-90)SHARE_FILE_PBKDF2_ITERATIONS(8-8)SHARE_FILE_PASSWORD_ENCODING(11-11)SHARE_FILE_SALT_PBKDF2_EXPANDED_BYTES(13-13)encryptPayload(92-111)SHARE_FILE_VERSION(7-7)src/keyset/types.ts (1)
ShareFileRecord(16-28)src/keyset/policy.ts (1)
createDefaultPolicy(14-20)src/keyset/storage.ts (1)
saveShareRecord(48-63)src/keyset/echo.ts (1)
sendShareEcho(18-39)
tests/cli.share.flow.test.ts (3)
tests/helpers/tmp.ts (2)
makeTmp(5-8)writePasswordFile(10-14)src/test-relay/NostrRelay.ts (2)
startEphemeralRelay(207-211)url(95-97)tests/helpers/runCli.ts (1)
runCli(21-82)
🪛 ast-grep (0.39.6)
tests/cli.share.flow.test.ts
[warning] 71-71: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${name} share 1)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (16)
tests/cli.share.flow.test.ts (3)
16-32: LGTM! Excellent test isolation.The setup and teardown hooks properly isolate each test with temporary directories, dedicated password files, and environment cleanup. The defensive relay cleanup check on line 28 prevents errors when tests skip relay initialization.
34-80: LGTM! Comprehensive end-to-end test coverage.The test effectively validates the complete share workflow with deterministic test data. The use of
IGLOO_TEST_AUTOPILOTandIGLOO_TEST_RELAYenables reliable CI execution. The assertions on lines 60, 66, 71-72, and 80 properly verify each step of the flow.Note: The static analysis warning on line 71 about constructing RegExp from a variable is a false positive—
nameis a test constant, not user input, so there's no ReDoS risk.
83-83: Nice fix! The assertion now validates relay activity.The change from
>= 0to> 0correctly verifies that the relay observed at least one event, preventing false passes if echo functionality breaks..github/workflows/release.yml (4)
28-36: Tag verification logic is sound.The workflow correctly extracts the git tag, parses package.json version, and ensures they match the
v<version>format before proceeding. This prevents version mismatches between tags and releases.
55-62: Release creation step looks good.The workflow uses
softprops/action-gh-release@v2withgenerate_release_notes: trueto automatically generate release notes and uploaddist/**assets. The GitHub token is correctly passed via secrets.
46-53: All npm scripts are properly defined in package.json.Verification confirms that
typecheck,test, andbuildscripts all exist in package.json. The workflow can execute these commands without errors.
22-44: Removecache: 'npm'or commitpackage-lock.jsonto fix workflow failure.The
setup-nodeaction withcache: 'npm'searches forpackage-lock.jsonin the repository root for caching, but the repository has nopackage-lock.json. This causes the cache property to fail during workflow execution.The conditional install logic (lines 38-44) cannot compensate because the workflow fails at the
setup-nodestep before reaching the install step.Options:
- Remove
cache: 'npm'at line 26 if intentionally avoiding a lockfile- Commit
package-lock.jsonif the project should use one⛔ Skipped due to learnings
Learnt from: CR PR: FROSTR-ORG/igloo-cli#0 File: AGENTS.md:0-0 Timestamp: 2025-10-08T16:24:14.475Z Learning: Install dependencies under Node.js 18 using nvm use 18RELEASING.md (3)
1-31: Documentation accurately reflects the workflow and release process.The RELEASING.md file provides clear, step-by-step instructions that align well with the GitHub Actions workflow in
.github/workflows/release.yml. The documentation covers GPG setup, tag creation, and the automated build/test/release pipeline triggered by pushing the tag.
33-53: Local npm publishing instructions are clear and complete.The documentation correctly describes the local publishing workflow, including optional review and signing steps. The reference to
publishConfig.access = publicindicates the npm package is intended to be public, which is consistent with the repository structure.
55-59: Signing and provenance notes are helpful and realistic.The documentation appropriately cautions against importing private keys into CI and explains the trade-off of local vs. CI publishing for provenance attestations. This guidance helps maintainers make informed decisions about their release security posture.
src/components/keyset/KeysetLoad.tsx (1)
70-72: Echo skip wiring looks correctPassing
undefinedto the hook when skipping echo cleanly disables listening without branching elsewhere.src/components/keyset/useShareEchoListener.ts (4)
75-80: Good: IGLOO_TEST_RELAY override with URL normalizationThe override + normalization logic is solid and test-friendly. Nice.
86-96: Union normalized group relays with defaultsMerging normalized group-derived relays with
DEFAULT_ECHO_RELAYSand falling back viaresolveRelaysWithFallbackSyncis robust.
152-165: Result handling afterawaitShareEcholooks correctShort-circuit on
controller.cancelled, set success, or keep listening with a clear message. Backoff below maintains responsiveness.
150-157: ConfirmawaitShareEchooptions shapeEnsure
{ relays, timeout: timeoutMs, eventConfig: { enableLogging: false } }matches the current@frostr/igloo-coreAPI in this branch. If the shape changed, adjust accordingly.src/components/share/ShareAdd.tsx (1)
461-465: Dual persistence path: LGTMPersisting to both the requested outputDir and the default share directory improves discoverability for other commands. The try/catch keeps UX smooth if the secondary write fails.
automated password handling, echo relay computation, and autopilot flows.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/cli.tsx (1)
130-146: Consider stronger typing for inkOptions and fakeIn.The logic correctly constructs a fake stdin and disables raw mode for non-interactive environments. However, the extensive use of
anyreduces type safety.Apply this diff to add more specific types:
-let inkOptions: any | undefined; +let inkOptions: {isRawModeSupported: boolean; stdin: NodeJS.ReadStream & {isTTY: boolean}} | undefined; if (process.env.IGLOO_DISABLE_RAW_MODE === '1' || process.env.IGLOO_DISABLE_RAW_MODE === 'true') { - const fakeIn: any = new PassThrough(); + const fakeIn = new PassThrough() as any as NodeJS.ReadStream & {isTTY: boolean}; fakeIn.isTTY = true;Note: The
as any ascast is necessary because PassThrough doesn't natively implement NodeJS.ReadStream, but this is more explicit about the intent.src/test-relay/NostrRelay.ts (1)
11-15: Consider stronger typing for Subscription fields.The
clientandfiltersfields are typed asany, which reduces type safety. Consider typingclientasWebSocketfrom thewspackage andfiltersasNostrFilter[](defining a NostrFilter type with the expected filter properties).+import {WebSocket} from 'ws'; + +type NostrFilter = { + ids?: string[]; + authors?: string[]; + kinds?: number[]; + since?: number; + until?: number; + limit?: number; + [key: `#${string}`]: string[] | undefined; +}; + type Subscription = { - client: any; + client: WebSocket; subId: string; - filters: any[]; + filters: NostrFilter[]; };src/components/share/ShareAdd.tsx (1)
265-271: Avoid redundant double-write when no outputDir is providedThe secondary save to the default share directory runs even when outputDir is undefined, causing a needless overwrite of the same file. Only perform the second save when an explicit outputDir was used.
Apply in both places:
@@ - const filepath = await saveShareRecord(record, {directory: outputDir}); - // Always persist into the default appdata share directory as well, - // so other commands (e.g. `share list`) can discover the record. - try { - await saveShareRecord(record); - } catch {} + const filepath = await saveShareRecord(record, {directory: outputDir}); + // Mirror into the default directory only when an explicit outputDir was used. + if (outputDir) { + try { + await saveShareRecord(record); + } catch {} + } @@ - const filepath = await saveShareRecord(record, {directory: outputDir}); - try { - // Also persist to the default share directory for discovery. - await saveShareRecord(record); - } catch {} + const filepath = await saveShareRecord(record, {directory: outputDir}); + if (outputDir) { + try { + // Also persist to the default share directory for discovery. + await saveShareRecord(record); + } catch {} + }Also applies to: 513-517
src/components/keyset/KeysetLoad.tsx (1)
115-129: Align auto-exit env parsing with other components and make delay configurableAccept '1' or 'true' for IGLOO_DISABLE_RAW_MODE/IGLOO_TEST_AUTOPILOT and use IGLOO_TEST_AUTOPILOT_EXIT_MS like ShareAdd for consistency.
- useEffect(() => { - if (!result) return; - const shouldAutoExit = - process.env.IGLOO_DISABLE_RAW_MODE === '1' || process.env.IGLOO_TEST_AUTOPILOT === '1'; - if (!shouldAutoExit) return; - - if (typeof window !== 'undefined') return; // ensure Node/CLI context - if (typeof process === 'undefined' || typeof process.exit !== 'function') return; - - const timer: ReturnType<typeof setTimeout> = setTimeout(() => { - try { - process.exit(0); - } catch {} - }, 20); - return () => clearTimeout(timer); - }, [result]); + useEffect(() => { + if (!result) return; + const a = (process.env.IGLOO_DISABLE_RAW_MODE ?? '').toLowerCase(); + const b = (process.env.IGLOO_TEST_AUTOPILOT ?? '').toLowerCase(); + const shouldAutoExit = a === '1' || a === 'true' || b === '1' || b === 'true'; + if (!shouldAutoExit) return; + + if (typeof window !== 'undefined') return; // ensure Node/CLI context + if (typeof process === 'undefined' || typeof process.exit !== 'function') return; + + const raw = process.env.IGLOO_TEST_AUTOPILOT_EXIT_MS; + const parsed = raw ? Number(raw) : NaN; + const delay = Number.isFinite(parsed) && parsed >= 0 ? parsed : 200; + + const timer: ReturnType<typeof setTimeout> = setTimeout(() => { + try { process.exit(0); } catch {} + }, delay); + return () => clearTimeout(timer); + }, [result]);src/components/keyset/useShareEchoListener.ts (2)
2-3: Reuse computeEchoRelays; drop local normalization/DEFAULT handlingAvoid duplicating URL normalization and defaults here; import and use computeEchoRelays.
-import {awaitShareEcho, decodeGroup, DEFAULT_ECHO_RELAYS} from '@frostr/igloo-core'; -import {resolveRelaysWithFallbackSync} from '../../keyset/relays.js'; +import {awaitShareEcho} from '@frostr/igloo-core'; +import {computeEchoRelays} from '../../keyset/echoRelays.js'; @@ - const relays = useMemo(() => { - const envOverride = typeof process !== 'undefined' ? process.env.IGLOO_TEST_RELAY : undefined; - if (envOverride && envOverride.length > 0) { - const normalize = (u: string) => (/^wss?:\/\//i.test(u) ? u.replace(/^http/i, 'ws') : `wss://${u}`); - return [normalize(envOverride)]; - } - if (!groupCredential) { - return undefined; - } - try { - const decoded = decodeGroup(groupCredential); - const fromGroup = extractRelays(decoded); - const normalize = (u: string) => (/^wss?:\/\//i.test(u) ? u.replace(/^http/i, 'ws') : `wss://${u}`); - const groupSet = new Set((fromGroup ?? []).map(normalize)); - const defaults = DEFAULT_ECHO_RELAYS.map(normalize); - const union = [...new Set([...defaults, ...groupSet])]; - if (union.length > 0) return union; - return resolveRelaysWithFallbackSync(undefined, DEFAULT_ECHO_RELAYS); - } catch { - // ignore decode failures; we'll fall back to default relays - } - return resolveRelaysWithFallbackSync(undefined, DEFAULT_ECHO_RELAYS); - }, [groupCredential]); + const relays = useMemo(() => { + const env = typeof process !== 'undefined' ? (process.env.IGLOO_TEST_RELAY ?? '').trim() : ''; + const envRelay = env.length > 0 ? env : undefined; + return computeEchoRelays(groupCredential ?? undefined, undefined, envRelay); + }, [groupCredential]); @@ - const result = await awaitShareEcho( - groupCredential, - shareCredential, - { relays, timeout: timeoutMs, eventConfig: { enableLogging: debugEnabled, customLogger: debugLogger } } - ); + const result = await awaitShareEcho(groupCredential, shareCredential, { + relays, + timeout: timeoutMs, + eventConfig: {enableLogging: debugEnabled, customLogger: debugLogger}, + });Also applies to: 74-96, 161-166
69-74: Remove unused fallback refs and cleanup codefallbackTimeoutRef and fallbackTriggeredRef are never set; they add noise. Remove their declarations and clear calls.
- const fallbackTimeoutRef = useRef<NodeJS.Timeout | null>(null); const activeShareRef = useRef<string | null>(null); - const fallbackTriggeredRef = useRef(false); @@ - if (fallbackTimeoutRef.current) { - clearTimeout(fallbackTimeoutRef.current); - fallbackTimeoutRef.current = null; - } @@ - fallbackTriggeredRef.current = false; @@ - if (fallbackTimeoutRef.current) { - clearTimeout(fallbackTimeoutRef.current); - fallbackTimeoutRef.current = null; - }Also applies to: 111-115, 130-131, 198-206
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/cli.tsx(5 hunks)src/components/keyset/KeysetLoad.tsx(4 hunks)src/components/keyset/useShareEchoListener.ts(3 hunks)src/components/share/ShareAdd.tsx(6 hunks)src/keyset/echo.ts(2 hunks)src/keyset/echoRelays.ts(1 hunks)src/test-relay/NostrRelay.ts(1 hunks)tests/echoRelays.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/keyset/echo.ts
🧰 Additional context used
📓 Path-based instructions (9)
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ink components (, , etc.) for terminal UI and do not use HTML elements
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/share/ShareAdd.tsxsrc/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/components/keyset/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/share/ShareAdd.tsxsrc/cli.tsxsrc/keyset/echoRelays.tssrc/test-relay/NostrRelay.ts
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/share/ShareAdd.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/KeysetLoad.tsxsrc/components/share/ShareAdd.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/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tstests/echoRelays.test.tssrc/components/share/ShareAdd.tsxsrc/cli.tsxsrc/keyset/echoRelays.tssrc/test-relay/NostrRelay.ts
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/KeysetLoad.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/share/ShareAdd.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/echoRelays.test.ts
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/keyset/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/keyset/**/*.ts: Keep key exchange and credential helpers under src/keyset/
Document non-obvious flows with concise comments, especially around key lifecycle management
Files:
src/keyset/echoRelays.ts
🧠 Learnings (1)
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
PR: FROSTR-ORG/igloo-cli#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/cli.tsx
🧬 Code graph analysis (6)
src/components/keyset/KeysetLoad.tsx (2)
src/keyset/crypto.ts (1)
decryptShareCredential(232-284)src/components/keyset/useShareEchoListener.ts (1)
useShareEchoListener(49-233)
src/components/keyset/useShareEchoListener.ts (1)
src/keyset/echoRelays.ts (1)
normalize(67-67)
tests/echoRelays.test.ts (1)
src/keyset/echoRelays.ts (1)
computeEchoRelays(43-65)
src/components/share/ShareAdd.tsx (5)
src/keyset/crypto.ts (7)
randomSaltHex(130-132)deriveSecret(79-90)SHARE_FILE_PBKDF2_ITERATIONS(8-8)SHARE_FILE_PASSWORD_ENCODING(11-11)SHARE_FILE_SALT_PBKDF2_EXPANDED_BYTES(13-13)encryptPayload(92-111)SHARE_FILE_VERSION(7-7)src/keyset/types.ts (1)
ShareFileRecord(16-28)src/keyset/policy.ts (1)
createDefaultPolicy(14-20)src/keyset/storage.ts (1)
saveShareRecord(48-63)src/keyset/echo.ts (1)
sendShareEcho(19-54)
src/cli.tsx (1)
src/components/Help.tsx (1)
Help(8-50)
src/keyset/echoRelays.ts (1)
src/keyset/relays.ts (1)
resolveRelaysWithFallbackSync(99-115)
🔇 Additional comments (9)
src/cli.tsx (3)
4-4: LGTM!The PassThrough import is correctly used to construct a fake stdin stream for non-interactive environments.
116-117: LGTM!The signature extension to accept optional inkOptions and propagate them to the render call is correct and enables environment-aware Ink behavior.
161-162: LGTM!The inkOptions are consistently propagated to all render calls (Help, helpable commands, and main App), ensuring uniform environment-aware Ink behavior.
Also applies to: 165-165, 174-175
src/test-relay/NostrRelay.ts (3)
33-41: LGTM!The function correctly validates tag structure with proper length checks before accessing array elements. The non-null assertion on line 38 is safe due to the length check on line 37.
151-167: LGTM! Filter limit handling is correct.The REQ handler properly respects per-filter limits as specified in NIP-01. The implementation correctly iterates through each filter independently and enforces the limit for matching events within that filter before sending EOSE.
43-76: LGTM! Filter matching implementation is solid.The function correctly implements Nostr filter matching per NIP-01, including prefix matching for ids/authors, exact matching for kinds, time windows, and tag-based filters. The defensive type checks are appropriate.
src/components/share/ShareAdd.tsx (1)
568-585: Nice: exit moved to effect with cleanupSide-effect correctly shifted out of render and made configurable; guards for Node/CLI context look good.
tests/echoRelays.test.ts (1)
1-66: Confirm test runner compatibility (Node vs Bun)Tests import 'node:test'. If your default runner is Bun, this may not execute. Ensure CI runs Node ≥18 for this suite, or switch to 'bun:test' when running under Bun.
src/keyset/echoRelays.ts (1)
9-22: LGTM: clear normalization and stable relay unionNormalization, fallback derivation, and deduplication behavior look correct and align with tests. Public API is tidy.
Also applies to: 43-65
- Auto-decrypt once with error guard; auto-exit via effect; stable hooks - stdin ref/unref polyfill runs before render - unhandledRejection only ignores benign relay case; others rethrow/log
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/keyset/KeysetLoad.tsx (2)
110-125: Auto-exit moved to effect correctly; consider defensive process checks for consistency.The auto-exit logic correctly triggers from a
useEffectrather than during render, with proper timer cleanup.However, lines 112-113 access
process.envbefore verifying thatprocessis defined (line 117). While this code runs only in Node environments whereprocessis always available, line 80 uses a defensivetypeof process !== 'undefined'check. Consider applying the same pattern here for consistency, especially given the polyfill mentions in the PR context.Apply this diff for consistency:
useEffect(() => { if (!result) return; + if (typeof process === 'undefined') return; const shouldAutoExit = process.env.IGLOO_DISABLE_RAW_MODE === '1' || process.env.IGLOO_TEST_AUTOPILOT === '1'; if (!shouldAutoExit) return; if (typeof window !== 'undefined') return; // ensure Node/CLI context - if (typeof process === 'undefined' || typeof process.exit !== 'function') return; + if (typeof process.exit !== 'function') return;
129-136: Consider defensive process check for consistency.The
skipEchoflag correctly controls echo listening behavior. However, line 130 directly accessesprocess.envwithout checkingtypeof process !== 'undefined', while line 80 uses this defensive pattern. For consistency across the component, consider applying the same guard here.Apply this diff:
const skipEcho = (() => { + if (typeof process === 'undefined') return false; const a = (process.env.IGLOO_SKIP_ECHO ?? '').toLowerCase(); return a === '1' || a === 'true'; })();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/cli.tsx(5 hunks)src/components/keyset/KeysetLoad.tsx(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ink components (, , etc.) for terminal UI and do not use HTML elements
Files:
src/components/keyset/KeysetLoad.tsxsrc/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/components/keyset/KeysetLoad.tsxsrc/cli.tsx
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/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/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/KeysetLoad.tsxsrc/cli.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/KeysetLoad.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 (1)
📚 Learning: 2025-10-08T16:24:14.475Z
Learnt from: CR
PR: FROSTR-ORG/igloo-cli#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/cli.tsx
🧬 Code graph analysis (1)
src/components/keyset/KeysetLoad.tsx (2)
src/keyset/crypto.ts (1)
decryptShareCredential(232-284)src/components/keyset/useShareEchoListener.ts (1)
useShareEchoListener(49-233)
🔇 Additional comments (8)
src/cli.tsx (4)
9-42: Well-designed unhandled rejection handler.The implementation correctly filters the benign Nostr shutdown message while preserving fail-fast behavior for genuine errors. The use of
if/elseto checkprocess.onavailability (rather than an outer try/catch) ensures registration errors are visible, addressing the previous review concern.
158-170: Correct positioning of stdin polyfills.The polyfills now execute before any
render()calls, preventing runtime errors in Bun whereref/unrefmay be missing. This addresses the previous critical review concern about initialization order.
126-129: Clean propagation of inkOptions for environment-aware rendering.The changes correctly thread
inkOptionsthrough to allrender()invocations, enabling consistent non-interactive behavior whenIGLOO_DISABLE_RAW_MODEis set.Also applies to: 185-189, 198-200
140-156: No issues found—the code is correct as written.The
isTTY=trueandisRawModeSupported=falsecombination is intentional and works as designed. The fake stdin withisTTY=trueallows Ink to render a terminal UI in non-interactive environments (CI/tests), whileisRawModeSupported=falseprevents Ink from attempting to enable raw mode, which would fail. Components likePrompt.tsxproperly check theisRawModeSupportedflag (viauseStdin()) and render non-interactive fallbacks when it's false. The no-opref/unref/setRawModemethods ensure Ink won't error if it attempts to call them. Ink v6.3.1 respects this configuration correctly.src/components/keyset/KeysetLoad.tsx (4)
25-26: LGTM! Automation state added as suggested.The
autoDecryptingandautoErrorstate variables correctly support the non-interactive decryption flow moved out of render.
66-68: LGTM! Error cleanup on share change.Clearing
autoErrorwhen the selected share changes prevents stale error messages from previous attempts.
74-105: LGTM! Auto-decrypt moved to effect with proper safeguards.The non-interactive decryption flow correctly:
- Runs outside render in a
useEffect- Guards against retry loops with the
autoErrorcheck at line 85- Uses a cancellation flag to prevent state updates after unmount
- Keeps the interactive prompt visible on failure
This addresses the past review concern about side-effects during render.
214-219: LGTM! Automation status rendered inline.The UI correctly displays automation progress and errors while keeping the interactive prompt visible, allowing users to proceed manually if auto-decryption fails.
- Fire‑and‑forget echo in autopilot with short pre‑announce grace - Fix test relay to decode ws RawData (Buffer/ArrayBuffer/etc.) - Deflake CLI flow test: skip echo, clear IGLOO_SKIP_ECHO in teardown - Add GitHub Actions CI (build+test) for staging/master
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/test-relay/NostrRelay.ts (3)
118-120: Memory leak: purge interval is never cleared.The past review comment on these lines correctly identifies that the
setIntervalcreates a recurring timer that is never stored or cleared whenstop()is called. This issue remains unresolved and should be addressed as suggested in the previous review.
127-134: Incomplete cleanup in stop().The
stop()method does not clear the purge interval timer created instart(). This is the counterpart to the memory leak issue flagged in the previous review. Ensure the interval timer is stored duringstart()and cleared here duringstop().
190-207: Unbounded cache growth between purge intervals.The past review comment correctly identifies that the cache grows without bounds until the periodic purge fires (default 30 seconds). In high-frequency test scenarios, this could lead to excessive memory consumption. The suggested solutions (max cache size limit or event deduplication by ID) remain valid.
src/components/share/ShareAdd.tsx (1)
574-591: Good fix: move process.exit scheduling into an effect with cleanup.Removes render‑time side‑effects, accepts '1'|'true', and clears the timer. Looks correct.
🧹 Nitpick comments (5)
tests/cli.share.flow.test.ts (3)
12-12: Use the password-file path (and remove the unused variable).You create passFile but never use it. Exercise the password-file flow and drop the direct --password to cover more code.
- let passFile: string; + let passFile: string; @@ - passFile = await writePasswordFile(appdataDir, 'strongpassword123'); + passFile = await writePasswordFile(appdataDir, 'strongpassword123'); @@ - '--name', name, - '--password', 'strongpassword123', + '--name', name, + '--password-file', passFile,Also applies to: 17-17, 45-46
64-64: Avoid dynamic RegExp; use a direct substring check.Safer and simpler; also resolves the static analyzer warning.
- assert.match(resList.stdout, new RegExp(`${name} share 1`)); + assert.ok(resList.stdout.includes(`${name} share 1`), 'Expected list to include the saved share entry');Based on static analysis.
49-51: Speed up the test by disabling the echo grace wait.Echo is skipped; set the grace to 0 so autopilot doesn’t sleep 500ms before printing “Share saved.”.
- env: { IGLOO_TEST_AUTOPILOT: '1', IGLOO_SKIP_ECHO: '1' }, + env: { IGLOO_TEST_AUTOPILOT: '1', IGLOO_SKIP_ECHO: '1', IGLOO_TEST_AUTOPILOT_ECHO_GRACE_MS: '0' },src/components/share/ShareAdd.tsx (2)
293-301: Skip the grace sleep when echo is disabled.Currently waits even if IGLOO_SKIP_ECHO disables echo; gate the delay on shouldEcho to cut CI time.
- // Give the fire-and-forget echo a brief head start so test relays - // can observe at least one EVENT before the harness terminates. - try { - const raw = process.env.IGLOO_TEST_AUTOPILOT_ECHO_GRACE_MS; - const parsed = raw ? Number(raw) : NaN; - const grace = Number.isFinite(parsed) && parsed >= 0 ? parsed : 500; - await new Promise(res => setTimeout(res, grace)); - } catch {} + // Give echo a brief head start only when echo is enabled. + if (shouldEcho) { + try { + const raw = process.env.IGLOO_TEST_AUTOPILOT_ECHO_GRACE_MS; + const parsed = raw ? Number(raw) : NaN; + const grace = Number.isFinite(parsed) && parsed >= 0 ? parsed : 500; + await new Promise(res => setTimeout(res, grace)); + } catch {} + }
438-441: Optional: DRY up env flag parsing.You parse on/off flags in several places; a tiny helper reduces duplication and mistakes.
Add near the top-level:
const envIsOn = (name: string) => { const v = (process.env[name] ?? '').toLowerCase(); return v === '1' || v === 'true'; };Then e.g.:
- const autopilot = envIsOn('IGLOO_TEST_AUTOPILOT');
- const shouldEcho = !envIsOn('IGLOO_SKIP_ECHO');
- const nonInteractive = envIsOn('IGLOO_DISABLE_RAW_MODE') || envIsOn('IGLOO_TEST_AUTOPILOT');
Also applies to: 535-538, 276-278, 168-170, 577-579 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 1f40c30233588a5e96eeae6a49ac827207efde5a and e039fb25bab3cf7ead16d37f1ff3cad3ce521ec7. </details> <details> <summary>📒 Files selected for processing (4)</summary> * `.github/workflows/ci.yml` (1 hunks) * `src/components/share/ShareAdd.tsx` (6 hunks) * `src/test-relay/NostrRelay.ts` (1 hunks) * `tests/cli.share.flow.test.ts` (1 hunks) </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * .github/workflows/ci.yml </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>📓 Path-based instructions (7)</summary> <details> <summary>src/**/*.tsx</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > Use Ink components (<Box>, <Text>, etc.) for terminal UI and do not use HTML elements Files: - `src/components/share/ShareAdd.tsx` </details> <details> <summary>src/**/*.{ts,tsx}</summary> **📄 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/share/ShareAdd.tsx` - `src/test-relay/NostrRelay.ts` </details> <details> <summary>src/components/**</summary> **📄 CodeRabbit inference engine (CLAUDE.md)** > All UI components should live under src/components/ Files: - `src/components/share/ShareAdd.tsx` </details> <details> <summary>src/components/**/*.tsx</summary> **📄 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/share/ShareAdd.tsx` </details> <details> <summary>**/*.{ts,tsx}</summary> **📄 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/share/ShareAdd.tsx` - `src/test-relay/NostrRelay.ts` - `tests/cli.share.flow.test.ts` </details> <details> <summary>src/components/**/*.{ts,tsx}</summary> **📄 CodeRabbit inference engine (AGENTS.md)** > Name React components using PascalCase Files: - `src/components/share/ShareAdd.tsx` </details> <details> <summary>**/*.test.ts</summary> **📄 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.share.flow.test.ts` </details> </details><details> <summary>🧬 Code graph analysis (2)</summary> <details> <summary>src/components/share/ShareAdd.tsx (5)</summary><blockquote> <details> <summary>src/keyset/crypto.ts (7)</summary> * `randomSaltHex` (130-132) * `deriveSecret` (79-90) * `SHARE_FILE_PBKDF2_ITERATIONS` (8-8) * `SHARE_FILE_PASSWORD_ENCODING` (11-11) * `SHARE_FILE_SALT_PBKDF2_EXPANDED_BYTES` (13-13) * `encryptPayload` (92-111) * `SHARE_FILE_VERSION` (7-7) </details> <details> <summary>src/keyset/types.ts (1)</summary> * `ShareFileRecord` (16-28) </details> <details> <summary>src/keyset/policy.ts (1)</summary> * `createDefaultPolicy` (14-20) </details> <details> <summary>src/keyset/storage.ts (1)</summary> * `saveShareRecord` (48-63) </details> <details> <summary>src/keyset/echo.ts (1)</summary> * `sendShareEcho` (19-54) </details> </blockquote></details> <details> <summary>tests/cli.share.flow.test.ts (2)</summary><blockquote> <details> <summary>tests/helpers/tmp.ts (2)</summary> * `makeTmp` (5-8) * `writePasswordFile` (10-14) </details> <details> <summary>tests/helpers/runCli.ts (1)</summary> * `runCli` (21-82) </details> </blockquote></details> </details><details> <summary>🪛 ast-grep (0.39.6)</summary> <details> <summary>tests/cli.share.flow.test.ts</summary> [warning] 63-63: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. Context: new RegExp(`${name} share 1`) Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES] - https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS - https://cwe.mitre.org/data/definitions/1333.html (regexp-from-variable) </details> </details> </details> <details> <summary>🔇 Additional comments (10)</summary><blockquote> <details> <summary>src/test-relay/NostrRelay.ts (9)</summary><blockquote> `1-2`: **LGTM!** Imports are correctly ordered shallow-to-deep and follow ESM conventions. --- `4-26`: **LGTM!** Type definitions appropriately model the relay configuration, subscriptions, and Nostr events. The use of `any` for client WebSocket types is acceptable for test infrastructure. --- `28-76`: **LGTM!** The helper functions correctly implement Nostr prefix matching, tag filtering, and comprehensive filter matching logic. The non-null assertion on line 38 is safe due to the preceding length check. --- `78-101`: **LGTM!** The class structure is clean with proper encapsulation. The readonly getter for `events` appropriately prevents external modifications to the cache. --- `166-182`: **LGTM!** The REQ handling correctly implements subscription logic with proper filter matching and limit enforcement. Cached events are streamed before EOSE, following the Nostr protocol. --- `184-188`: **LGTM!** The CLOSE handler properly removes the subscription using the correct composite key. --- `212-218`: **LGTM!** The close handler properly cleans up all subscriptions for the disconnected client by matching the session ID prefix. --- `141-164`: **LGTM!** The message parsing logic comprehensively handles multiple WebSocket data formats with appropriate error handling. The NOTICE response on parse failure follows the Nostr protocol. --- `222-226`: **LGTM!** The helper function provides a convenient way to create and start an ephemeral relay for testing, returning both the relay instance and connection URL. </blockquote></details> <details> <summary>src/components/share/ShareAdd.tsx (1)</summary><blockquote> `166-323`: **Autopilot flow looks solid and addresses prior concerns.** One‑shot guard, group/share validation, consistent env parsing ('1'|'true'), single now for timestamps, and dual persistence are all in place. LGTM. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
1 participant
Summary by CodeRabbit
New Features
Documentation
Tests
Chores