Tune echo relays and keyset exit flow#8
Conversation
WalkthroughThis pull request refactors relay computation logic to use a centralized Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as ShareSaver
participant Env as Environment
participant Process as Node Process
CLI->>Env: Read IGLOO_DISABLE_RAW_MODE<br/>IGLOO_CLI_NO_INPUT
Env-->>CLI: inputDisabled, isAutomated flags
CLI->>CLI: Compute shouldPrompt =<br/>!(isAutomated && inputDisabled)
alt shouldPrompt = true
CLI->>CLI: Render "Press Enter to finish"
Note over CLI: Interactive mode
else shouldPrompt = false
alt No active share
CLI->>Process: Schedule process.exit(0)
Note over CLI,Process: Auto-exit path
end
alt autoState → 'done' or 'error'
CLI->>Process: Schedule process.exit(0/1)
Note over CLI,Process: Automated exit
end
end
sequenceDiagram
participant Listener as useShareEchoListener
participant ComputeFn as computeEchoRelays
participant Env as Environment
participant Relays as Relay Endpoints
Listener->>Env: Read IGLOO_TEST_RELAY
Env-->>Listener: envRelay value
Listener->>ComputeFn: computeEchoRelays(groupCred,<br/>undefined, envRelay)
alt envRelay provided
ComputeFn->>ComputeFn: Prioritize envRelay<br/>over defaults
Note over ComputeFn: Override mode
else envRelay not provided
ComputeFn->>ComputeFn: Use explicit relays<br/>+ group defaults
Note over ComputeFn: Normal resolution
end
ComputeFn-->>Listener: Deduped relay list
Listener->>Relays: Connect to relays
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span multiple domains—UI updates, new automation control flow, relay computation refactoring, and test updates—requiring separate reasoning for each area. Logic modifications in ShareSaver and echoRelays introduce conditional paths and environment-based behavior that need careful verification, while the relay refactoring consolidates decoding logic into a centralized function and adjusts test expectations accordingly. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/keyset/ShareSaver.tsx (1)
145-172: Consider extracting the exit delay constant.The 20ms delay appears in both exit paths (lines 155 and 170). Consider extracting this as a named constant for clarity and maintainability.
Apply this diff to improve readability:
+const EXIT_DELAY_MS = 20; // Brief delay to ensure UI renders before exit + export function ShareSaver({ keysetName, groupCredential, shareCredentials, onComplete, autoPassword, outputDir }: ShareSaverProps) { ... useEffect(() => { if (!done || shouldPrompt) return; - const timer = setTimeout(() => { + const timer = setTimeout(() => { try { if (typeof process !== 'undefined' && typeof process.exit === 'function') { process.exit(0); } } catch {} - }, 20); + }, EXIT_DELAY_MS); return () => clearTimeout(timer); }, [done, shouldPrompt]); useEffect(() => { if (!isAutomated || shouldPrompt) return; if (autoState !== 'done' && autoState !== 'error') return; const code = autoState === 'done' ? 0 : 1; - const timer = setTimeout(() => { + const timer = setTimeout(() => { try { if (typeof process !== 'undefined' && typeof process.exit === 'function') { process.exit(code); } } catch {} - }, 20); + }, EXIT_DELAY_MS); return () => clearTimeout(timer); }, [isAutomated, shouldPrompt, autoState]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/Setup.tsx(1 hunks)src/components/keyset/ShareSaver.tsx(3 hunks)src/components/keyset/useShareEchoListener.ts(2 hunks)src/keyset/echo.ts(1 hunks)src/keyset/echoRelays.ts(1 hunks)tests/echoRelays.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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
**/*.{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/echoRelays.test.tssrc/components/keyset/ShareSaver.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/Setup.tsxsrc/keyset/echo.tssrc/keyset/echoRelays.ts
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Ink components (, , etc.) for terminal UI and do not use HTML elements
Files:
src/components/keyset/ShareSaver.tsxsrc/components/Setup.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/ShareSaver.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/Setup.tsxsrc/keyset/echo.tssrc/keyset/echoRelays.ts
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/components/keyset/ShareSaver.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/Setup.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/ShareSaver.tsxsrc/components/Setup.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/ShareSaver.tsxsrc/components/keyset/useShareEchoListener.tssrc/components/Setup.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.tssrc/keyset/echoRelays.ts
🧬 Code graph analysis (2)
tests/echoRelays.test.ts (1)
src/keyset/echoRelays.ts (1)
computeEchoRelays(43-84)
src/components/keyset/useShareEchoListener.ts (1)
src/keyset/echoRelays.ts (1)
computeEchoRelays(43-84)
🔇 Additional comments (9)
src/components/Setup.tsx (1)
15-15: LGTM! Clearer onboarding message.The updated text "Bootstrap your FROSTR signing circle" is more specific and descriptive than the previous "Get started with FROSTR".
src/keyset/echo.ts (1)
31-32: LGTM! Helpful clarification.The comment clearly documents the test isolation behavior when
IGLOO_TEST_RELAYis set, improving code maintainability.src/keyset/echoRelays.ts (2)
50-68: LGTM! Well-documented environment override logic.The envRelay handling correctly implements test isolation:
- Returns only envRelay when no explicit relays (full isolation)
- Merges envRelay with explicit relays when provided (controlled overlap)
- Skips group/base relays in both env cases to prevent accidental public connections
The deduplication logic properly preserves original casing while using case-insensitive keys.
77-77: LGTM! Correctly excludes envRelay from final deduplication.Since envRelay is now handled separately in the conditional block above (lines 50-68), removing it from the final deduplication pass is correct and prevents double-processing.
src/components/keyset/ShareSaver.tsx (2)
76-81: LGTM! Environment-driven input control.The
inputDisabledandshouldPromptflags correctly handle non-interactive/automated environments by checking bothIGLOO_DISABLE_RAW_MODEandIGLOO_CLI_NO_INPUT.
125-141: LGTM! Conditional finish prompt.The finish prompt is correctly rendered only when
shouldPromptis true, preventing hangs in non-interactive environments.src/components/keyset/useShareEchoListener.ts (1)
58-62: LGTM! Excellent refactoring.Replacing the local relay resolution logic with a centralized
computeEchoRelayscall:
- Eliminates code duplication
- Simplifies the hook
- Ensures consistent relay behavior across the codebase
- Properly handles
IGLOO_TEST_RELAYfor test isolationtests/echoRelays.test.ts (2)
23-46: LGTM! Test clarified for no env override scenario.The test name and parameters now explicitly indicate this tests the non-override path, and the expected results correctly exclude the env relay.
65-83: LGTM! Comprehensive coverage of env override behavior.The new tests properly verify:
- Env override with explicit relays → explicit + env (no group/base)
- Env override without explicit relays → only env (full isolation)
This ensures the test isolation behavior works as designed.
Summary by CodeRabbit
New Features
UI Changes
Tests