Normalize Nostr subscribe filters; add --debug-echo; doc updates#7
Normalize Nostr subscribe filters; add --debug-echo; doc updates#7AustinKelsay merged 1 commit intomasterfrom
Conversation
- Add global SimplePool subscribeMany shim (src/polyfills/nostr.ts), import at startup. - Remove duplicate signer patch; KeysetSigner now imports shared polyfill. - Add --debug-echo (-E) flag to toggle IGLOO_DEBUG_ECHO without env vars. - Echo listener logs active relay set when debug enabled. - Update Help panel, README, and LLM context with echo testing tips.
WalkthroughThis PR adds debugging infrastructure and a runtime polyfill to igloo-cli. It introduces a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The changes are largely additive and follow consistent patterns (flag handling, polyfill boilerplate, documentation). The polyfill implementation is straightforward filter normalization with defensive try/catch. No complex logic density or surprising interactions across files—each modification is focused and isolated in its domain. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/cli.tsx (1)
133-141: Consider extracting the boolean coercion logic.The
toBoolfunction duplicates similar logic inKeysetSigner.tsx(parseBooleanFlag, lines 70-86). Consider extracting this to a shared utility module to maintain consistency and reduce duplication.Example location:
src/utils/flags.tsexport function toBool(value: string | boolean | undefined): boolean { if (typeof value === 'boolean') return value; if (typeof value === 'string') { const v = value.trim().toLowerCase(); if (['1', 'true', 'yes', 'on'].includes(v)) return true; if (['0', 'false', 'no', 'off'].includes(v)) return false; } return false; }src/polyfills/nostr.ts (1)
1-25: Runtime prototype patching requires caution.The polyfill achieves its goal of normalizing filters for relay compatibility, but runtime prototype modification is inherently fragile. Here are some observations:
Positives:
- Idempotent check via
__iglooFilterNormalizePatchedprevents double-patching- Silent failure (try/catch) won't break the app if nostr-tools changes
- Centralized in a polyfill vs. scattered runtime patches
- Preserves original function with proper
call(this, ...)Potential concerns:
- The normalization logic assumes
filters[0]is always the filter object whenfilters.length === 1. This works for the current nostr-tools API but could break if the library changes its signature.- The
__iglooFilterNormalizePatchedproperty is added to the prototype, which pollutes the SimplePool prototype. Consider using a WeakSet or module-level flag instead.- No TypeScript types for the patched method, which could lead to type safety issues.
Suggested improvements:
- Add a module-level flag instead of polluting the prototype:
+let patchApplied = false; + try { const proto = (SimplePool as any)?.prototype; - if (proto && !proto.__iglooFilterNormalizePatched) { + if (proto && !patchApplied) { const original = proto.subscribeMany; if (typeof original === 'function') { proto.subscribeMany = function patchedSubscribeMany(this: unknown, relays: unknown, filters: unknown, params: unknown) { const normalized = Array.isArray(filters) && filters.length === 1 && filters[0] && typeof filters[0] === 'object' && !Array.isArray(filters[0]) ? filters[0] : filters; return original.call(this, relays, normalized, params); }; - Object.defineProperty(proto, '__iglooFilterNormalizePatched', {value: true}); + patchApplied = true; } } } catch { // Best-effort only; if nostr-tools changes, we fail silently. }
- Consider adding a comment explaining which relay implementations require this normalization and linking to any relevant issue tracker entries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(1 hunks)llm/context/igloo-core-readme.md(2 hunks)src/cli.tsx(4 hunks)src/components/Help.tsx(1 hunks)src/components/keyset/KeysetSigner.tsx(1 hunks)src/components/keyset/useShareEchoListener.ts(1 hunks)src/polyfills/nostr.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
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/polyfills/nostr.tssrc/components/keyset/KeysetSigner.tsxsrc/components/Help.tsxsrc/cli.tsxsrc/components/keyset/useShareEchoListener.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:
src/polyfills/nostr.tssrc/components/keyset/KeysetSigner.tsxsrc/components/Help.tsxsrc/cli.tsxsrc/components/keyset/useShareEchoListener.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/KeysetSigner.tsxsrc/components/Help.tsxsrc/cli.tsx
src/components/**
📄 CodeRabbit inference engine (CLAUDE.md)
All UI components should live under src/components/
Files:
src/components/keyset/KeysetSigner.tsxsrc/components/Help.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/KeysetSigner.tsxsrc/components/Help.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React components using PascalCase
Files:
src/components/keyset/KeysetSigner.tsxsrc/components/Help.tsxsrc/components/keyset/useShareEchoListener.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
llm/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep protocol prompts and agent scripts in llm/ and update them when UX flows change
Files:
llm/context/igloo-core-readme.md
🔇 Additional comments (9)
src/components/Help.tsx (1)
47-47: LGTM!The help text clearly documents the new
--debug-echoflag and its purpose. The description is concise and consistent with other flag documentation in this file.src/components/keyset/useShareEchoListener.ts (1)
161-166: LGTM!The debug logging is well-guarded and consistent with the existing debugging pattern established earlier in this function. The try/catch wrapper around
console.logprevents crashes in environments where console might not be available.The placement before
awaitShareEchois ideal for debugging relay connectivity issues.llm/context/igloo-core-readme.md (2)
324-324: LGTM!The CLI debugging tip clearly explains how to enable echo logs and ensure relay overlap between desktop and CLI. The guidance is practical and actionable.
372-372: LGTM!The troubleshooting note accurately documents the filter normalization behavior and provides clear guidance for users implementing custom listeners outside the CLI.
README.md (1)
158-170: LGTM!The Echo debugging section is well-written with clear examples. The explanation of filter normalization provides useful context for users troubleshooting echo-related issues.
src/components/keyset/KeysetSigner.tsx (1)
20-21: LGTM!The transition from runtime patching to a centralized polyfill import is a cleaner architecture. The comment clearly explains the purpose, and the
.jsextension follows the TypeScript ESM requirements specified in the coding guidelines.The polyfill's silent failure mode (try/catch in nostr.ts) ensures this won't break if nostr-tools changes.
src/cli.tsx (3)
2-2: LGTM!Importing the nostr polyfill at the CLI entry point ensures the SimplePool patch is applied before any nostr-tools usage. This guarantees consistent filter normalization behavior across the application.
118-122: LGTM!The short alias mapping follows the established pattern for
-t→--thresholdand-T→--total. The cleanup of the short flag prevents duplication.
157-164: LGTM!The IIFE appropriately initializes
IGLOO_DEBUG_ECHObased on the--debug-echoflag after argument parsing but before rendering. This ensures the environment variable is available to all components that check it.The fallback to
(flags as any).debugEchohandles potential camelCase variants gracefully.
1 participant
Summary by CodeRabbit
New Features
--debug-echoflag for verbose echo logging during testingIGLOO_TEST_RELAYenvironment variable to pin a specific relayBug Fixes
Documentation