Conversation
This commit consolidates all staging work onto a proper main-based history after recovering from a git filter-branch issue that orphaned the original staging branch. Includes all features and fixes: - IPC input validation with Zod schemas - Electron security hardening (contextIsolation, preload script) - Echo relay planning and normalization - Signer keep-alive improvements - Recovery flow enhancements - Comprehensive test coverage (445 tests) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a typed preload API and IPC schemas, a relay planner and RelayPlan type, AddShare UI and onboarding, signer keep‑alive and Signer mount-safety, SSRF-aware validation and sanitized errors, NSEC masking/auto-clear, CSP meta hardening, stricter share file modes, build/bundler updates, and many tests wiring renderer→preload→main echo/relay flows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddShare as "AddShare (Renderer)"
participant Preload as "Preload (window.electronAPI)"
participant Main as "Main Process"
participant RelayPlanner as "echoRelays.computeRelayPlan"
participant Echo as "Echo/listeners"
User->>AddShare: submit group/share + password
AddShare->>Preload: computeRelayPlan(args)
Preload->>Main: invoke('compute-relay-plan', args)
Main->>RelayPlanner: computeRelayPlan(inputs)
RelayPlanner-->>Main: RelayPlan { relays, envRelays, ... }
Main-->>Preload: { ok: true, relayPlan }
Preload-->>AddShare: relayPlan result
AddShare->>Echo: sendEcho(relayPlan.relays, shareCredential, timeout)
Echo-->>User: echoes delivered
sequenceDiagram
participant Renderer
participant Preload
participant Main
participant IPC as "IPC Handler"
Renderer->>Preload: window.electronAPI.computeRelayPlan(args)
Preload->>Main: ipcRenderer.invoke('compute-relay-plan', args)
Main->>IPC: RelayPlanArgsSchema.safeParse(args)
alt valid
IPC->>IPC: computeRelayPlan(...)
IPC-->>Main: { ok: true, relayPlan }
else invalid
IPC-->>Main: { ok: false, reason, message }
end
Main-->>Preload: response
Preload-->>Renderer: resolved result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/__tests__/__mocks__/bifrost.mock.ts (1)
177-181: Remove or fix the overly restrictive share format validation.The condition on line 179 doesn't match the actual
ShareCredentialSchemavalidation. The schema only enforces length limits (1–5000 characters), but this mock requires the digit'1'to be present in the share string. This causes valid formats likebfshare2_abcto be incorrectly rejected. Either remove this check entirely or replace it with proper format validation that matches the actual schema requirements.
🤖 Fix all issues with AI agents
In @CLAUDE.md:
- Around line 160-175: The README's relay priority list is inaccurate; update
CLAUDE.md so it documents the conditional merging logic used by
computeRelayPlan: when envRelay is present the effective priority is explicit
relays > envRelay > group relays, and when envRelay is absent the priority is
explicit relays > configured relays (relays.json) → baseRelays parameter →
DEFAULT_ECHO_RELAYS as fallbacks > group relays; reference envRelay,
computeRelayPlan, baseRelays, DEFAULT_ECHO_RELAYS and relays.json in the text
and clearly state the two distinct paths instead of a single flat five-level
order.
In @package.json:
- Line 132: Remove the unused dependency "baseline-browser-mapping" from
package.json and run a fresh install (e.g., npm ci / npm install) to update
lockfile; verify it's not required transitively by running npm ls
baseline-browser-mapping or yarn why baseline-browser-mapping, and run the
build/tests to ensure nothing breaks. Confirm the "buffer" package referenced in
webpack.config.cjs (alias polyfill for Electron renderer) remains if needed; if
"buffer" is unused, remove it similarly. Update package.json and lockfile only,
and include a short PR note stating you removed an unused dependency and
validated the build.
- Line 163: The package.json dependency for @frostr/igloo-core references a
non-existent v0.2.4 which will break installs; update the dependency to a
published version (for example "^0.1.3") or point it to the correct git/tag that
contains the needed APIs (decodeGroup, DEFAULT_ECHO_RELAYS, createConnectedNode,
sendEcho, validateGroup, validateShare, decodeShare,
recoverSecretKeyFromCredentials), then run npm install and run tests to ensure
imports resolve.
In @src/lib/clientShareManager.ts:
- Line 14: The ElectronAPI interface's getShares is typed too loosely; update
the getShares declaration in preload.ts (the ElectronAPI interface) to return
Promise<IglooShare[] | false> instead of Promise<unknown>, and import or
reference the IglooShare type in preload.ts so the signature is accurate; after
this change remove the explicit cast in clientShareManager.ts where const shares
= await window.electronAPI.getShares() as IglooShare[] | false.
In @src/main.ts:
- Around line 372-411: RelayPlanArgsSchema currently allows decodedGroup as
z.record(z.unknown()) which has no size/depth limits and can be abused; update
the decodedGroup schema to constrain its shape and add documentation via
.describe(): if the expected structure is known replace z.record with a strict
z.object({...}).strict() describing each key, otherwise use z.record(z.string(),
z.unknown()).describe('decoded group object') combined with a refinement that
limits top-level entries (e.g. max keys like 100) and a custom depth-check
helper (or a small recursive validator) to enforce max nesting depth, and
include the description text in the schema.
🧹 Nitpick comments (17)
src/lib/signer-keepalive.ts (1)
74-74: Clarify the intent of unusedselfPubkeyparameter.The
selfPubkeyparameter is defined in theKeepAliveConfiginterface but is no longer destructured or used. Consider either:
- Adding a comment in the
KeepAliveConfiginterface (line 29) explaining why this parameter is preserved (e.g., reserved for future functionality, backward compatibility)- Removing it from the interface if it's truly unnecessary
This would improve clarity for future maintainers.
src/__tests__/workflows/keyset-creation.test.ts (1)
5-6: Consider extractingtoScalarHexto shared mocks.This helper duplicates the same function in
bifrost.mock.ts. Per coding guidelines, shared mock implementations should be placed in__tests__/__mocks__/for reuse across test files.♻️ Suggested consolidation
Export from
bifrost.mock.tsand import here:-// Helper to generate realistic 64-char hex strings for scalar values -const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); +import { toScalarHex } from '../__mocks__/bifrost.mock';Then in
bifrost.mock.ts, ensure the helper is exported:export const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0');src/lib/clientShareManager.ts (1)
11-96: Consider adding runtime safety check for window.electronAPI.While the preload script should ensure
window.electronAPIexists, adding a defensive check in the constructor or first method call would make the code more robust against misconfiguration.🛡️ Optional defensive check
Add to the beginning of the
ClientShareManagerclass:class ClientShareManager { + constructor() { + if (!window.electronAPI) { + throw new Error('Electron API not available. Ensure preload script is loaded.'); + } + } + async getShares(): Promise<IglooShare[] | false> {Alternatively, add a lazy check in each method or use a private helper.
src/__tests__/components/SaveShare.test.tsx (1)
219-247: Loading state test uses inline require for mock modification.The
require('@/lib/encryption')pattern works but can be fragile with ESM/Jest configurations. Consider usingjest.mocked()with the already-imported mock for cleaner code:import { derive_secret_async } from '@/lib/encryption'; jest.mocked(derive_secret_async).mockImplementation(...);This is a minor style preference; the current approach functions correctly.
src/__tests__/workflows/share-lifecycle.test.ts (1)
5-6: Consider extractingtoScalarHexhelper to a shared test utility.This helper function is duplicated in multiple test files (also in
clientShareManager.test.ts,Signer.test.tsx). As per learnings, shared mock implementations and fixtures should be placed insrc/__tests__/__mocks__/or registered insrc/__tests__/setup.tsfor reuse.♻️ Suggested refactor
Create a shared utility in
src/__tests__/__mocks__/testHelpers.ts:// src/__tests__/__mocks__/testHelpers.ts export const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); export const toPointHex = (seed: number): string => '02' + seed.toString(16).padStart(64, '0');Then import in test files:
-// Helper to generate realistic 64-char hex strings for scalar values -const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); +import { toScalarHex } from '../__mocks__/testHelpers';src/components/AddShare.tsx (1)
395-421: Consider extractingLegacyDecodedGrouptype to shared types.The inline
LegacyDecodedGrouptype definition is specific to handling legacy group formats for IPC serialization. If this pattern is used elsewhere, consider moving it tosrc/types/index.tsfor consistency. The type casting throughunknownis unavoidable for IPC serialization but worth documenting.The relay planning integration looks correct—the IPC call properly falls back to the main process for environment variable access.
src/components/Keyset.tsx (1)
554-578: Consider adding keyboard accessibility for modal close.The Group Credential QR modal looks good and follows the same pattern as the share QR modal. However, for better accessibility, consider adding an
onKeyDownhandler to close on Escape key press, similar to how accessible modals typically work.♻️ Optional: Add Escape key handler
{showGroupQrCode && ( - <div className="fixed inset-0 bg-black/80 flex items-center justify-center backdrop-blur-sm"> + <div + className="fixed inset-0 bg-black/80 flex items-center justify-center backdrop-blur-sm" + onKeyDown={(e) => e.key === 'Escape' && setShowGroupQrCode(false)} + tabIndex={-1} + >src/__tests__/lib/echoRelays.test.ts (1)
149-222: Consider adding a test for malformed groupCredential strings.The tests cover
decodeGroupthrowing an error and empty credentials, but consider adding a test for malformed credential strings that might pass the empty check but fail decoding in unexpected ways.src/__tests__/integration/electron-security.test.ts (1)
38-41: Regex may be overly specific to formatting.The pattern
preload:\s*path\.join\(__dirname,\s*['"]preload\.js['"]\)is tightly coupled to exact code formatting. If the code uses template literals, multiline formatting, or different spacing, this test could fail despite the security setting being correctly configured.Consider a more flexible pattern:
- expect(mainTsContent).toMatch(/preload:\s*path\.join\(__dirname,\s*['"]preload\.js['"]\)/); + // Check that preload is configured (more flexible matching) + expect(mainTsContent).toMatch(/preload:/); + expect(mainTsContent).toMatch(/preload\.js/);src/components/Recover.tsx (2)
481-492: Consider removing eslint-disable comment if refs are stable.The
eslint-disable-next-line react-hooks/exhaustive-depssuggests accessing.currentin cleanup might be flagged. However, for timeout refs, accessing.currentat cleanup time is the correct pattern—you want to clear whatever timeout exists at unmount. The comment explaining this is good, but consider if the disable is truly necessary.
753-800: Consider accessibility for button states.The Copy button changes color based on
copyErrorstate. Ensure screen readers can distinguish between success and error states. Consider addingaria-liveattributes to the copy feedback area.+ {/* Copy error feedback */} + <div aria-live="polite"> {copyError && ( <div className="text-sm text-red-400 bg-red-900/20 border border-red-700/30 rounded px-3 py-2"> {copyError} </div> )} + </div>src/lib/ipcSchemas.ts (1)
70-75: RelayPlanArgsSchema allows broad decodedGroup type.The
decodedGroup: z.record(z.unknown())accepts any object structure. While flexible, this bypasses type safety. Consider using a more specific schema if the expected structure is known.If the structure is predictable:
decodedGroup: z.object({ relays: z.array(z.string()).optional(), relayUrls: z.array(z.string()).optional(), relay_urls: z.array(z.string()).optional(), threshold: z.number().optional(), group_pk: z.string().optional(), }).passthrough().optional(),src/lib/validation.ts (1)
127-153: SSRF protection covers common IPv4 ranges but not IPv6.The
isPrivateOrReservedIpfunction handles IPv4 private ranges well (RFC 1918, loopback, link-local). However, it doesn't cover IPv6 addresses like::1(loopback),fc00::/7(unique local), orfe80::/10(link-local).Consider adding IPv6 checks or documenting the limitation:
/** * Checks if an IP address is in a private or reserved range (SSRF protection) * @param ip - The IP address to check (e.g., "192.168.1.1") * @returns true if the IP is private/reserved and should be blocked * @note Currently only handles IPv4. IPv6 addresses should be validated separately. */src/lib/echoRelays.ts (2)
69-90: Redundant type assertion on line 76.The
parsedvariable is already typed as{ relays?: unknown }from line 72. The re-cast on line 76 is unnecessary.Suggested simplification
const parsed = JSON.parse(raw) as { relays?: unknown }; if (!parsed || typeof parsed !== 'object') { return undefined; } - const relays = (parsed as { relays?: unknown }).relays; + const relays = parsed.relays; if (!Array.isArray(relays)) {
143-168: Consider extracting the localdedupefunction for reuse.The
dedupefunction (lines 170-180) is defined insidecomputeRelayPlanbut performs similar work tonormalizeList. While it operates on already-normalized strings (so it's not fully redundant), extracting it to module scope would improve testability and allow direct reuse.src/main.ts (2)
33-44: Path sanitization regex has a redundant capture group.Line 41's regex for quoted Windows paths captures the backreference
\1in an unnecessary group(\1). While functionally correct (since$1in the replacement matches both opening and closing quotes), this is slightly inefficient.Minor regex cleanup
// Quoted Windows paths: "C:\path\with spaces\file.txt" -> "<path>\file.txt" - .replace(/(['"])([A-Za-z]:\\(?:[^\\]+\\)+)([^\\'"]*)(\1)/g, '$1<path>\\$3$1') + .replace(/(['"])([A-Za-z]:\\(?:[^\\]+\\)+)([^\\'"]*)\1/g, '$1<path>\\$3$1')
184-213: Relay target canonicalization duplicates logic from echoRelays.ts.The
relayTargetKeyfunction (lines 184-205) performs similar URL canonicalization todedupeKeyinechoRelays.ts. Consider importing and reusingdedupeKeyto maintain a single source of truth for URL normalization.Suggested refactor
Export
dedupeKeyfromechoRelays.ts:-export { normalizeRelayUrl }; +export { normalizeRelayUrl, dedupeKey };Then import and reuse in
main.ts:-import { computeRelayPlan } from './lib/echoRelays.js'; +import { computeRelayPlan, dedupeKey } from './lib/echoRelays.js';const relayTargetKey = (relays: string[]): string => { - const canonicalPieces = relays.map(relay => { - try { - const parsed = new URL(relay); - const protocol = parsed.protocol.toLowerCase(); - const hostname = parsed.hostname.toLowerCase(); - const port = parsed.port ? `:${parsed.port}` : ''; - return `${protocol}//${hostname}${port}${parsed.pathname}${parsed.search}${parsed.hash}`; - } catch { - const match = relay.match(/^(wss?):\/\/([^/?#]+)(.*)$/); - if (match) { - const scheme = match[1].toLowerCase(); - const authority = match[2].toLowerCase(); - const pathAndMore = match[3] ?? ''; - return `${scheme}://${authority}${pathAndMore}`; - } - return relay; - } - }); + const canonicalPieces = relays.map(dedupeKey); canonicalPieces.sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); return canonicalPieces.join('|'); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (40)
CLAUDE.mdindex.htmlpackage.jsonsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/setup.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/AddShare.tsxsrc/components/Create.tsxsrc/components/Keyset.tsxsrc/components/LoadShare.tsxsrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/components/Signer.tsxsrc/lib/clientShareManager.tssrc/lib/echoRelays.tssrc/lib/ipcSchemas.tssrc/lib/shareManager.tssrc/lib/signer-keepalive.tssrc/lib/validation.tssrc/main.tssrc/preload.tssrc/types/index.tstsconfig.preload.jsontsconfig.test.jsonwebpack.config.cjs
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with explicit types for function parameters and returns
Prefer interfaces over types for defining object shapes in TypeScript
Use @/ path alias for imports instead of relative paths
Use camelCase for functions and variables
Use UPPER_SNAKE_CASE for constants
Files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/lib/shareManager.tssrc/types/index.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/components/Create.tsxsrc/lib/signer-keepalive.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/components/AddShare.tsxsrc/lib/clientShareManager.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/integration/electron-security.test.tssrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/preload.tssrc/lib/echoRelays.tssrc/components/Signer.tsxsrc/components/LoadShare.tsxsrc/lib/ipcSchemas.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/Keyset.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/main.tssrc/lib/validation.ts
src/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/__tests__/**/*.test.{ts,tsx}: Mock @frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Use jest.clearAllMocks() in beforeEach hooks to ensure test isolation
Mock Electron's ipcRenderer and app APIs in tests
Files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
src/__tests__/integration/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test Electron IPC communication in tests/integration/ directory
Files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/integration/electron-security.test.ts
src/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests mirror runtime code under
src/__tests__/withintegration/,performance/, and__mocks__/subdirectories for feature-specific coverage
Files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/__mocks__/bifrost.mock.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: All runtime code is TypeScript with two-space indentation and required semicolons
Import third-party packages before internal modules and use the@/alias for local paths
Files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/lib/shareManager.tssrc/types/index.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/components/Create.tsxsrc/lib/signer-keepalive.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/components/AddShare.tsxsrc/lib/clientShareManager.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/integration/electron-security.test.tssrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/preload.tssrc/lib/echoRelays.tssrc/components/Signer.tsxsrc/components/LoadShare.tsxsrc/lib/ipcSchemas.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/Keyset.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/main.tssrc/lib/validation.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Jest with Testing Library powers renderer specs; place new tests beside features as
<subject>.test.tsx
Files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
src/__tests__/lib/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test utility functions in tests/lib/ directory
Files:
src/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/shareManager.test.ts
src/lib/shareManager.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/shareManager.ts: Never store plaintext share data; always encrypt with user passwords
Store shares in /igloo/shares/ directory using the defined JSON schema
Files:
src/lib/shareManager.ts
src/{components,lib,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
UI components sit under
src/components/, helpers insrc/lib/, shared types insrc/types/
Files:
src/lib/shareManager.tssrc/types/index.tssrc/components/Create.tsxsrc/lib/signer-keepalive.tssrc/components/AddShare.tsxsrc/lib/clientShareManager.tssrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/lib/echoRelays.tssrc/components/Signer.tsxsrc/components/LoadShare.tsxsrc/lib/ipcSchemas.tssrc/components/Keyset.tsxsrc/lib/validation.ts
src/**/*ipc*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Cover new IPC handlers with unit tests plus at least one scenario in
src/__tests__/integration/
Files:
src/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.ts
src/__tests__/{setup.ts,__mocks__/**}
📄 CodeRabbit inference engine (AGENTS.md)
Register shared mocks in
src/__tests__/setup.tsand reuse fixtures fromsrc/__tests__/__mocks__/
Files:
src/__tests__/setup.tssrc/__tests__/__mocks__/bifrost.mock.ts
src/__tests__/components/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test React component behavior in tests/components/ directory
Files:
src/__tests__/components/AddShare.test.tsxsrc/__tests__/components/Signer.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/LoadShare.test.tsx
src/components/[A-Z]*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for component files
Files:
src/components/Create.tsxsrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/components/Signer.tsxsrc/components/LoadShare.tsxsrc/components/Keyset.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.tsx: Use Tailwind CSS utility classes in JSX and use cn() utility for conditional classes
Avoid inline styles unless conditionally computed
Use functional components with hooks instead of class components
Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system accessTailwind utility classes style JSX; reserve inline styles for dynamic values only
Files:
src/components/Create.tsxsrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/components/Signer.tsxsrc/components/LoadShare.tsxsrc/components/Keyset.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/components/**/*.{ts,tsx}: Prefer functional React components; name them in PascalCase and helpers in camelCase
Components insrc/components/follow kebab-case filenames (e.g.,share-list.tsx)
Files:
src/components/Create.tsxsrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/components/Signer.tsxsrc/components/LoadShare.tsxsrc/components/Keyset.tsx
src/__tests__/workflows/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test end-to-end user flows in tests/workflows/ directory
Files:
src/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
src/__tests__/__mocks__/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared mock implementations in tests/mocks/ for use across test files
Files:
src/__tests__/__mocks__/bifrost.mock.ts
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main.ts: Register IPC handlers in main.ts using ipcMain.handle() for file system operations
Electron's main process uses CommonJS (require) while renderer uses ESM (import)Main Electron process lives in
src/main.ts; the renderer bootstraps fromsrc/renderer.tsx
Files:
src/main.ts
🧠 Learnings (26)
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.{ts,tsx} : Import third-party packages before internal modules and use the `@/` alias for local paths
Applied to files:
tsconfig.preload.jsonwebpack.config.cjstsconfig.test.jsonCLAUDE.md
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*ipc*.{ts,tsx} : Cover new IPC handlers with unit tests plus at least one scenario in `src/__tests__/integration/`
Applied to files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tstsconfig.test.jsonsrc/lib/clientShareManager.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/lib/ipcSchemas.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/main.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/integration/**/*.test.{ts,tsx} : Test Electron IPC communication in __tests__/integration/ directory
Applied to files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/lib/clientShareManager.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxCLAUDE.mdsrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/lib/ipcSchemas.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/Keyset.tsxsrc/main.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock Electron's ipcRenderer and app APIs in tests
Applied to files:
src/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tswebpack.config.cjssrc/__tests__/components/Recover.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/lib/clientShareManager.tssrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/lib/**/*.test.ts : Test utility functions in __tests__/lib/ directory
Applied to files:
src/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/echoRelays.test.tstsconfig.test.jsonsrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxCLAUDE.mdsrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/__tests__/**/*.{ts,tsx} : Tests mirror runtime code under `src/__tests__/` with `integration/`, `performance/`, and `__mocks__/` subdirectories for feature-specific coverage
Applied to files:
src/__tests__/lib/validation.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tstsconfig.test.jsonsrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/workflows/**/*.test.{ts,tsx} : Test end-to-end user flows in __tests__/workflows/ directory
Applied to files:
src/__tests__/lib/validation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/components/**/*.test.{ts,tsx} : Test React component behavior in __tests__/components/ directory
Applied to files:
src/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tstsconfig.test.jsonsrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/electron-security.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/shareManager.ts : Store shares in <appData>/igloo/shares/ directory using the defined JSON schema
Applied to files:
src/lib/shareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/components/AddShare.tsxsrc/lib/clientShareManager.tssrc/__tests__/lib/shareManager.test.tsCLAUDE.mdsrc/components/Recover.tsxsrc/lib/ipcSchemas.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/main.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/shareManager.ts : Never store plaintext share data; always encrypt with user passwords
Applied to files:
src/lib/shareManager.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/components/AddShare.tsxsrc/lib/clientShareManager.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/components/LoadShare.test.tsxCLAUDE.mdsrc/__tests__/workflows/keyset-creation.test.tssrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/components/LoadShare.tsxsrc/__tests__/workflows/share-lifecycle.test.tssrc/components/Keyset.tsxsrc/__tests__/__mocks__/bifrost.mock.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/encryption.ts : Use PBKDF2 with 600,000 iterations for encrypting share files
Applied to files:
src/lib/shareManager.tssrc/components/SaveShare.tsxsrc/components/LoadShare.tsxsrc/__tests__/__mocks__/bifrost.mock.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/__tests__/{setup.ts,__mocks__/**} : Register shared mocks in `src/__tests__/setup.ts` and reuse fixtures from `src/__tests__/__mocks__/`
Applied to files:
src/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/__mocks__/bifrost.mock.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/__mocks__/**/*.ts : Place shared mock implementations in __tests__/__mocks__/ for use across test files
Applied to files:
src/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tstsconfig.test.jsonsrc/__tests__/lib/shareManager.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/__mocks__/bifrost.mock.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Applied to files:
src/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/integration/clientShareManager.test.tstsconfig.test.jsonsrc/__tests__/lib/shareManager.test.tspackage.jsonsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/__mocks__/bifrost.mock.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Use jest.clearAllMocks() in beforeEach hooks to ensure test isolation
Applied to files:
src/__tests__/setup.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/integration/clientShareManager.test.tstsconfig.test.jsonsrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Electron's main process uses CommonJS (require) while renderer uses ESM (import)
Applied to files:
src/__tests__/setup.tswebpack.config.cjssrc/lib/clientShareManager.tsCLAUDE.mdpackage.jsonsrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/main.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.test.{ts,tsx} : Jest with Testing Library powers renderer specs; place new tests beside features as `<subject>.test.tsx`
Applied to files:
src/__tests__/setup.tssrc/__tests__/components/Signer.test.tsxsrc/__tests__/components/Recover.test.tsxtsconfig.test.jsonsrc/__tests__/integration/electron-security.test.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Debug flaky Electron tests with `npm test -- --runInBand` to serialize execution
Applied to files:
src/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/components/**/*.tsx : Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system access
Applied to files:
src/__tests__/setup.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/lib/clientShareManager.tssrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/components/Keyset.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Register IPC handlers in main.ts using ipcMain.handle() for file system operations
Applied to files:
src/__tests__/integration/computeRelayPlan.test.tssrc/lib/clientShareManager.tssrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/lib/ipcSchemas.tssrc/components/Keyset.tsxsrc/main.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/main.ts : Main Electron process lives in `src/main.ts`; the renderer bootstraps from `src/renderer.tsx`
Applied to files:
webpack.config.cjsCLAUDE.mdpackage.jsonsrc/__tests__/integration/electron-security.test.tssrc/preload.tssrc/main.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to **/*.{ts,tsx} : Use @/ path alias for imports instead of relative paths
Applied to files:
webpack.config.cjstsconfig.test.json
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Do not implement cryptographic logic in the desktop app; delegate all crypto operations to frostr/igloo-core
Applied to files:
src/components/AddShare.tsxCLAUDE.mdsrc/components/Signer.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.{ts,tsx} : All runtime code is TypeScript with two-space indentation and required semicolons
Applied to files:
tsconfig.test.json
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/{components,lib,types}/**/*.{ts,tsx} : UI components sit under `src/components/`, helpers in `src/lib/`, shared types in `src/types/`
Applied to files:
tsconfig.test.jsonCLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Use props and callbacks for component communication instead of external state libraries like Redux
Applied to files:
CLAUDE.md
🧬 Code graph analysis (15)
src/__tests__/lib/validation.test.ts (1)
src/lib/validation.ts (7)
VALIDATION_LIMITS(4-15)validatePassword(32-49)validatePasswordLength(468-488)validatePositiveInteger(335-369)validateShareName(389-420)sanitizeUserError(433-459)formatRelayUrl(230-304)
src/__tests__/lib/ipcSchemas.test.ts (1)
src/lib/ipcSchemas.ts (9)
ShareIdSchema(14-17)HexSaltSchema(20-23)RelayUrlSchema(26-26)ShareCredentialSchema(29-31)GroupCredentialSchema(34-36)SaveShareSchema(39-67)RelayPlanArgsSchema(70-75)EchoStartArgsSchema(80-88)EchoStopArgsSchema(91-95)
src/__tests__/integration/computeRelayPlan.test.ts (1)
src/__tests__/setup.ts (1)
mockElectronAPI(191-191)
src/__tests__/components/SaveShare.test.tsx (1)
src/lib/encryption.ts (1)
derive_secret_async(24-73)
src/components/Create.tsx (1)
src/lib/validation.ts (3)
validateShareName(389-420)VALIDATION_LIMITS(4-15)validatePositiveInteger(335-369)
src/__tests__/lib/echoRelays.test.ts (1)
src/lib/echoRelays.ts (2)
normalizeRelayUrl(221-221)computeRelayPlan(143-219)
src/__tests__/integration/clientShareManager.test.ts (1)
src/lib/clientShareManager.ts (1)
clientShareManager(98-98)
src/components/AddShare.tsx (2)
src/lib/validation.ts (2)
VALIDATION_LIMITS(4-15)sanitizeUserError(433-459)src/types/index.ts (1)
DecodedGroup(9-19)
src/lib/clientShareManager.ts (1)
src/types/index.ts (1)
IglooShare(44-55)
src/__tests__/integration/electron-security.test.ts (1)
webpack.config.cjs (1)
path(1-1)
src/components/Recover.tsx (2)
src/lib/validation.ts (1)
VALIDATION_LIMITS(4-15)src/components/ui/button.tsx (1)
Button(56-56)
src/components/SaveShare.tsx (1)
src/lib/validation.ts (1)
VALIDATION_LIMITS(4-15)
src/preload.ts (1)
src/types/index.ts (1)
RelayPlan(191-198)
src/lib/echoRelays.ts (1)
src/types/index.ts (1)
RelayPlan(191-198)
src/components/LoadShare.tsx (1)
src/lib/validation.ts (2)
VALIDATION_LIMITS(4-15)sanitizeUserError(433-459)
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLAUDE.md (1)
99-127: Document Electron security hardening (preload script, contextIsolation) in Architecture section.The PR implements significant Electron security hardening with
contextIsolationenabled and a preload script, but the Architecture section doesn't document these changes. All IPC handlers also validate input using Zod schemas, which isn't mentioned in the current documentation.Recommend adding a subsection under "### Electron Architecture" to document:
- PreLoad script location and purpose
- IPC handler registration with Zod schema validation
- contextIsolation enabled state and security implications
📋 Suggested addition to Electron Architecture section
Insert after line 116 (after Key IPC Channels):
### IPC Validation with Zod All IPC handlers validate input using Zod schemas to ensure type safety: - Main process registers handlers with validated input/output types - Renderer uses preload script to call electronAPI methods with typed arguments - contextIsolation enabled to prevent access to main process globals **Preload Script** (`src/preload.ts`): - Exposes typed `electronAPI` object to renderer - Validates all IPC arguments before sending to main process - Restricts renderer access to sensitive Electron APIs See `src/lib/ipcSchemas.ts` for schema definitions.
🤖 Fix all issues with AI agents
In @src/lib/ipcSchemas.ts:
- Around line 25-30: RelayArraySchema duplicates the inline string validation
instead of reusing RelayUrlSchema; change RelayArraySchema to use RelayUrlSchema
(e.g., z.array(RelayUrlSchema).max(50, 'Too many relay URLs')) so both
validations stay consistent; also note RelayUrlSchema currently allows empty
strings—if empty relay URLs should be rejected, add .min(1) to RelayUrlSchema
and update related tests accordingly.
In @src/preload.ts:
- Around line 24-31: The preload computeRelayPlan handler is accepting nulls
(groupCredential, decodedGroup, explicitRelays, envRelay) but the main-side Zod
schema rejects null; normalize any null values to undefined before calling
ipcRenderer.invoke for computeRelayPlan (and the duplicate handler on the
electronAPI object) so renderer-initiated nulls don't fail validation—implement
normalization by mapping each of those properties: if value === null then set
undefined (or omit the key) and then invoke compute-relay-plan with the
normalized args via ipcRenderer.invoke.
🧹 Nitpick comments (3)
src/lib/ipcSchemas.ts (2)
19-24: HexSaltSchema should enforce even-length hex.
Right now an odd-length hex string (e.g., 33 chars) passes; that’s usually invalid for byte data.Proposed change
export const HexSaltSchema = z.string() .min(32, 'Salt must be at least 32 hex characters') .max(128, 'Salt exceeds maximum length') - .regex(/^[0-9a-fA-F]+$/, 'Salt must be a valid hex string'); + .regex(/^[0-9a-fA-F]+$/, 'Salt must be a valid hex string') + .refine((s) => s.length % 2 === 0, 'Salt hex length must be even');
37-56: DecodedGroupSchema DoS guard is weaker than the comment implies.
Object.keys(obj).length <= 50doesn’t prevent huge string values / deep nesting in unknown properties (because of.passthrough()), so a renderer can still send very large payloads under a small number of keys.Consider either:
- validating only the relay-related fields you actually need (preferred), or
- adding a coarse “serialized size” cap (even a conservative one) and/or a
.catchall(...)that bounds unknown string lengths.src/lib/clientShareManager.ts (1)
86-95: Include shareId (and any provided reason/message) in openShareLocation failure logs.
Right now you lose context when multiple shares exist.Proposed tweak
async openShareLocation(shareId: string): Promise<void> { try { const result = await window.electronAPI.openShareLocation(shareId); if (!result.ok) { - console.error('Failed to open share location: operation rejected'); + console.error('Failed to open share location: operation rejected', { shareId }); } } catch (error) { console.error('Failed to open share location:', error); } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CLAUDE.mdpackage.jsonsrc/__tests__/lib/ipcSchemas.test.tssrc/lib/clientShareManager.tssrc/lib/ipcSchemas.tssrc/preload.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with explicit types for function parameters and returns
Prefer interfaces over types for defining object shapes in TypeScript
Use @/ path alias for imports instead of relative paths
Use camelCase for functions and variables
Use UPPER_SNAKE_CASE for constants
Files:
src/lib/clientShareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.tssrc/preload.ts
src/{components,lib,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
UI components sit under
src/components/, helpers insrc/lib/, shared types insrc/types/
Files:
src/lib/clientShareManager.tssrc/lib/ipcSchemas.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: All runtime code is TypeScript with two-space indentation and required semicolons
Import third-party packages before internal modules and use the@/alias for local paths
Files:
src/lib/clientShareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.tssrc/preload.ts
src/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/__tests__/**/*.test.{ts,tsx}: Mock @frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Use jest.clearAllMocks() in beforeEach hooks to ensure test isolation
Mock Electron's ipcRenderer and app APIs in tests
Files:
src/__tests__/lib/ipcSchemas.test.ts
src/__tests__/lib/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test utility functions in tests/lib/ directory
Files:
src/__tests__/lib/ipcSchemas.test.ts
src/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests mirror runtime code under
src/__tests__/withintegration/,performance/, and__mocks__/subdirectories for feature-specific coverage
Files:
src/__tests__/lib/ipcSchemas.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Jest with Testing Library powers renderer specs; place new tests beside features as
<subject>.test.tsx
Files:
src/__tests__/lib/ipcSchemas.test.ts
src/**/*ipc*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Cover new IPC handlers with unit tests plus at least one scenario in
src/__tests__/integration/
Files:
src/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.ts
🧠 Learnings (15)
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/shareManager.ts : Store shares in <appData>/igloo/shares/ directory using the defined JSON schema
Applied to files:
src/lib/clientShareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.tsCLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Register IPC handlers in main.ts using ipcMain.handle() for file system operations
Applied to files:
src/lib/clientShareManager.tssrc/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/integration/**/*.test.{ts,tsx} : Test Electron IPC communication in __tests__/integration/ directory
Applied to files:
src/lib/clientShareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/preload.tsCLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/components/**/*.tsx : Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system access
Applied to files:
src/lib/clientShareManager.tssrc/preload.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*ipc*.{ts,tsx} : Cover new IPC handlers with unit tests plus at least one scenario in `src/__tests__/integration/`
Applied to files:
src/lib/clientShareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.tssrc/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/shareManager.ts : Never store plaintext share data; always encrypt with user passwords
Applied to files:
src/lib/clientShareManager.tsCLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock Electron's ipcRenderer and app APIs in tests
Applied to files:
src/lib/clientShareManager.tssrc/__tests__/lib/ipcSchemas.test.tssrc/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Electron's main process uses CommonJS (require) while renderer uses ESM (import)
Applied to files:
src/lib/clientShareManager.tssrc/preload.tsCLAUDE.md
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/__tests__/**/*.{ts,tsx} : Tests mirror runtime code under `src/__tests__/` with `integration/`, `performance/`, and `__mocks__/` subdirectories for feature-specific coverage
Applied to files:
src/__tests__/lib/ipcSchemas.test.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/main.ts : Main Electron process lives in `src/main.ts`; the renderer bootstraps from `src/renderer.tsx`
Applied to files:
src/preload.tsCLAUDE.md
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/{components,lib,types}/**/*.{ts,tsx} : UI components sit under `src/components/`, helpers in `src/lib/`, shared types in `src/types/`
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/lib/**/*.test.ts : Test utility functions in __tests__/lib/ directory
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.{ts,tsx} : Import third-party packages before internal modules and use the `@/` alias for local paths
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Do not implement cryptographic logic in the desktop app; delegate all crypto operations to frostr/igloo-core
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Use props and callbacks for component communication instead of external state libraries like Redux
Applied to files:
CLAUDE.md
🧬 Code graph analysis (3)
src/lib/clientShareManager.ts (1)
src/types/index.ts (1)
IglooShare(44-55)
src/__tests__/lib/ipcSchemas.test.ts (1)
src/lib/ipcSchemas.ts (9)
ShareIdSchema(14-17)HexSaltSchema(20-23)RelayUrlSchema(26-26)ShareCredentialSchema(59-61)GroupCredentialSchema(64-66)SaveShareSchema(69-97)RelayPlanArgsSchema(100-105)EchoStartArgsSchema(110-118)EchoStopArgsSchema(121-125)
src/preload.ts (1)
src/types/index.ts (2)
IglooShare(44-55)RelayPlan(191-198)
🔇 Additional comments (3)
src/lib/clientShareManager.ts (1)
12-22: Good migration towindow.electronAPI; just ensure tests/mocks cover it everywhere.
This will fail hard in any environment that doesn’t install the preload bridge (unit tests, storybook-like harnesses).src/__tests__/lib/ipcSchemas.test.ts (1)
21-513: Nice coverage of boundary conditions + null-vs-undefined behavior.
Only potential concern: tests codify thatRelayUrlSchemaaccepts''; if you tighten the schema later, this will require updates (which might be desirable).src/preload.ts (1)
154-154: The project uses TypeScript 5.6.3 (from package.json), which is well above the required 4.9 forsatisfiesoperator support. ts-jest 29.3.1 is fully compatible. Thesatisfieskeyword will work without issue.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/preload.ts (4)
17-52: Consider standardizing the response pattern across all methods.The
getShares()method returnsIglooShare[] | false, while other methods use a structured response pattern like{ok: boolean; reason?: string}. This inconsistency may complicate error handling in the renderer.Consider refactoring
getShares()to return{ok: boolean; shares?: IglooShare[]; reason?: string}for consistency with other methods.
57-85: LGTM: Solid runtime validation with type guards.The runtime type guards provide essential safety for IPC messages. The validation correctly handles optional fields and null values.
Consider adding JSDoc comments to explain when these validators are used and what they protect against.
100-116: Good null normalization for Zod compatibility.The
computeRelayPlanmethod correctly normalizes null values to undefined for Zod schema compatibility, and the echo methods don't need this since they don't accept nullable parameters.However, similar to share management methods, consider adding input validation for echo methods (e.g., verify
listenerIdis non-empty,shareCredentialsis an array).
118-165: LGTM: Secure event listener implementation with proper cleanup.The event listener implementations correctly:
- Validate callbacks are functions before registration
- Apply runtime validation to incoming data
- Return cleanup functions to prevent memory leaks
- Redact sensitive data from error logs
Consider extracting the common pattern to reduce duplication:
♻️ Optional refactor to DRY up event listener pattern
// Helper to create validated event listeners function createEventListener<T>( eventName: string, validator: (data: unknown) => data is T, callback: (data: T) => void ): () => void { if (typeof callback !== 'function') { console.error(`[preload] ${eventName} requires a function callback`); return () => {}; } const handler = (_event: IpcRendererEvent, data: unknown) => { if (validator(data)) { callback(data); } else { console.error(`[preload] Received malformed ${eventName} data (type:`, typeof data, ')'); } }; ipcRenderer.on(eventName, handler); return () => { ipcRenderer.removeListener(eventName, handler); }; } // Then use it: onEchoReceived: (callback) => createEventListener('echo-received', isEchoReceivedData, callback), onEchoError: (callback) => createEventListener('echo-error', isEchoErrorData, callback),
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/__tests__/lib/ipcSchemas.test.tssrc/lib/ipcSchemas.tssrc/preload.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/tests/lib/ipcSchemas.test.ts
- src/lib/ipcSchemas.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with explicit types for function parameters and returns
Prefer interfaces over types for defining object shapes in TypeScript
Use @/ path alias for imports instead of relative paths
Use camelCase for functions and variables
Use UPPER_SNAKE_CASE for constants
Files:
src/preload.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: All runtime code is TypeScript with two-space indentation and required semicolons
Import third-party packages before internal modules and use the@/alias for local paths
Files:
src/preload.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*ipc*.{ts,tsx} : Cover new IPC handlers with unit tests plus at least one scenario in `src/__tests__/integration/`
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock Electron's ipcRenderer and app APIs in tests
Applied to files:
src/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Electron's main process uses CommonJS (require) while renderer uses ESM (import)
Applied to files:
src/preload.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/main.ts : Main Electron process lives in `src/main.ts`; the renderer bootstraps from `src/renderer.tsx`
Applied to files:
src/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/integration/**/*.test.{ts,tsx} : Test Electron IPC communication in __tests__/integration/ directory
Applied to files:
src/preload.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*ipc*.{ts,tsx} : Cover new IPC handlers with unit tests plus at least one scenario in `src/__tests__/integration/`
Applied to files:
src/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/components/**/*.tsx : Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system access
Applied to files:
src/preload.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Register IPC handlers in main.ts using ipcMain.handle() for file system operations
Applied to files:
src/preload.ts
🧬 Code graph analysis (1)
src/preload.ts (1)
src/types/index.ts (2)
IglooShare(44-55)RelayPlan(191-198)
🔇 Additional comments (3)
src/preload.ts (3)
1-13: LGTM: Clear security documentation and proper imports.The documentation clearly establishes the security boundary, and imports follow the project's coding guidelines using the @/ path alias.
89-98: Preload validation is not needed; main process validates comprehensively.The main process already implements thorough input validation for all share management IPC handlers. The
save-share,delete-share, andopen-share-locationhandlers use Zod schemas (SaveShareSchemaandShareIdSchema) that validate all required fields, enforce length limits, and check formats. Theget-shareshandler returns data directly since it doesn't accept user input.The preload correctly acts as a thin IPC bridge without redundant validation—the main process provides the security boundary where validation belongs.
166-173: LGTM: Proper TypeScript type augmentation.The use of
satisfies ElectronAPIensures type safety, and the global Window augmentation correctly types the exposed API for renderer code. Test coverage for IPC handlers is in place across integration tests (computeRelayPlan.test.ts,clientShareManager.test.ts,electron-security.test.ts) and unit tests, meeting project requirements.
…aging better onboarding modal and explanation with links
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/components/OnboardingWelcome.tsx:
- Around line 75-78: In the OnboardingWelcome component's JSX paragraph element,
escape the apostrophe in "you'll" to satisfy the linter; update the string in
that <p> node to use an escaped apostrophe (e.g., replace you'll with
you'll or use a JS string expression that contains "you'll") so the JSX no
longer triggers the unescaped-apostrophe lint warning.
🧹 Nitpick comments (2)
src/components/App.tsx (1)
37-37: Consider persisting onboarding dismissal state.Currently,
showingOnboardingresets totrueon app reload, meaning users who dismiss onboarding but haven't created shares yet will see it again after restart. This may be intentional for first-launch UX, but if not, consider persisting the dismissal state vialocalStorageor the existing share manager.src/components/OnboardingWelcome.tsx (1)
2-2: Use@/path alias for imports.Per coding guidelines, prefer the
@/path alias over relative paths for internal imports.Suggested fix
-import { Button } from './ui/button'; +import { Button } from '@/components/ui/button';
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/__tests__/components/OnboardingWelcome.test.tsxsrc/components/App.tsxsrc/components/OnboardingWelcome.tsx
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with explicit types for function parameters and returns
Prefer interfaces over types for defining object shapes in TypeScript
Use @/ path alias for imports instead of relative paths
Use camelCase for functions and variables
Use UPPER_SNAKE_CASE for constants
Files:
src/components/OnboardingWelcome.tsxsrc/__tests__/components/OnboardingWelcome.test.tsxsrc/components/App.tsx
src/components/[A-Z]*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for component files
Files:
src/components/OnboardingWelcome.tsxsrc/components/App.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.tsx: Use Tailwind CSS utility classes in JSX and use cn() utility for conditional classes
Avoid inline styles unless conditionally computed
Use functional components with hooks instead of class components
Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system accessTailwind utility classes style JSX; reserve inline styles for dynamic values only
Files:
src/components/OnboardingWelcome.tsxsrc/components/App.tsx
src/{components,lib,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
UI components sit under
src/components/, helpers insrc/lib/, shared types insrc/types/
Files:
src/components/OnboardingWelcome.tsxsrc/components/App.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: All runtime code is TypeScript with two-space indentation and required semicolons
Import third-party packages before internal modules and use the@/alias for local paths
Files:
src/components/OnboardingWelcome.tsxsrc/__tests__/components/OnboardingWelcome.test.tsxsrc/components/App.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/components/**/*.{ts,tsx}: Prefer functional React components; name them in PascalCase and helpers in camelCase
Components insrc/components/follow kebab-case filenames (e.g.,share-list.tsx)
Files:
src/components/OnboardingWelcome.tsxsrc/components/App.tsx
src/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/__tests__/**/*.test.{ts,tsx}: Mock @frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Use jest.clearAllMocks() in beforeEach hooks to ensure test isolation
Mock Electron's ipcRenderer and app APIs in tests
Files:
src/__tests__/components/OnboardingWelcome.test.tsx
src/__tests__/components/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test React component behavior in tests/components/ directory
Files:
src/__tests__/components/OnboardingWelcome.test.tsx
src/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests mirror runtime code under
src/__tests__/withintegration/,performance/, and__mocks__/subdirectories for feature-specific coverage
Files:
src/__tests__/components/OnboardingWelcome.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Jest with Testing Library powers renderer specs; place new tests beside features as
<subject>.test.tsx
Files:
src/__tests__/components/OnboardingWelcome.test.tsx
🧠 Learnings (9)
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/components/**/*.{ts,tsx} : Prefer functional React components; name them in PascalCase and helpers in camelCase
Applied to files:
src/components/OnboardingWelcome.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/components/**/*.test.{ts,tsx} : Test React component behavior in __tests__/components/ directory
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.test.{ts,tsx} : Jest with Testing Library powers renderer specs; place new tests beside features as `<subject>.test.tsx`
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*ipc*.{ts,tsx} : Cover new IPC handlers with unit tests plus at least one scenario in `src/__tests__/integration/`
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/__tests__/**/*.{ts,tsx} : Tests mirror runtime code under `src/__tests__/` with `integration/`, `performance/`, and `__mocks__/` subdirectories for feature-specific coverage
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/workflows/**/*.test.{ts,tsx} : Test end-to-end user flows in __tests__/workflows/ directory
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/integration/**/*.test.{ts,tsx} : Test Electron IPC communication in __tests__/integration/ directory
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/lib/**/*.test.ts : Test utility functions in __tests__/lib/ directory
Applied to files:
src/__tests__/components/OnboardingWelcome.test.tsx
🧬 Code graph analysis (3)
src/components/OnboardingWelcome.tsx (1)
src/components/ui/button.tsx (1)
Button(56-56)
src/__tests__/components/OnboardingWelcome.test.tsx (1)
src/components/OnboardingWelcome.tsx (1)
OnboardingWelcome(9-136)
src/components/App.tsx (1)
src/components/OnboardingWelcome.tsx (1)
OnboardingWelcome(9-136)
🪛 GitHub Check: Lint and Test (18)
src/components/OnboardingWelcome.tsx
[warning] 77-77:
' can be escaped with ', ‘, ', ’
🪛 GitHub Check: Lint and Test (20)
src/components/OnboardingWelcome.tsx
[warning] 77-77:
' can be escaped with ', ‘, ', ’
🔇 Additional comments (6)
src/components/App.tsx (1)
274-275: LGTM!The conditional rendering logic correctly prioritizes: Create view → Onboarding (for new users) → Main shares view. The callback properly dismisses onboarding by setting the flag to
false.src/__tests__/components/OnboardingWelcome.test.tsx (3)
1-11: LGTM!Test setup follows best practices: uses
@/path alias for imports and includesjest.clearAllMocks()inbeforeEachfor proper test isolation. Based on learnings.
59-104: Good security-focused test coverage.These tests verify that all external links have proper
target="_blank"andrel="noopener noreferrer"attributes, which is important for preventing tabnabbing attacks.
118-146: LGTM!The accessibility tests appropriately validate semantic HTML structure—heading hierarchy and list semantics—ensuring the component is accessible to screen readers.
src/components/OnboardingWelcome.tsx (2)
80-121: LGTM!All external links correctly include
target="_blank"withrel="noopener noreferrer"to prevent tabnabbing attacks. Good security practice.
5-9: LGTM!Component follows conventions: functional component in PascalCase, explicit prop interface, and both named and default exports for import flexibility.
Also applies to: 138-138
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/__tests__/lib/echoRelays.test.ts:
- Around line 149-164: The test is asserting calls to decodeGroup but the module
export isn't mocked; add a module-level mock for @frostr/igloo-core that
provides decodeGroup as a jest.fn() (or ensure decodeGroup is cast to jest.Mock
in the test) and then in the 'with groupCredential' test set
decodeGroup.mockReturnValue(...) before calling computeRelayPlan so the
expectation decodeGroup toHaveBeenCalledWith('bfgroup1testcredential') is valid;
reference the symbols decodeGroup and computeRelayPlan in
src/__tests__/lib/echoRelays.test.ts and ensure the
jest.mock('@frostr/igloo-core', ...) is present (or a setup.ts mock) so
decodeGroup is replaced by a mock implementation.
In @src/components/Signer.tsx:
- Line 17: The QRCodeSVG currently renders the sensitive bearer "signerSecret"
openly; change the UI so the secret QR is hidden by default and only displayed
after an explicit user action and confirmation: add a "Show QR" button and an
interstitial confirmation dialog that warns the user about shoulder‑surfing and
accidental sharing, then on confirm render the QRCodeSVG for signerSecret; keep
the group credential QR behavior unchanged, and ensure the show/hide state and
confirmation flow are tied to the component state (e.g., within Signer or the
renderSecretQRCode handler) so the secret is never rendered until the user
explicitly confirms.
🧹 Nitpick comments (22)
src/__tests__/integration/userInputValidation.test.ts (1)
221-227: SSRF protection test coverage looks solid.The test cases cover the key SSRF vectors: localhost, loopback (127.x.x.x), and private IP ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16).
Consider adding IPv6 SSRF test cases if the validation logic supports IPv6:
'wss://[::1]:8080', // IPv6 loopback 'wss://[fe80::1]:443', // IPv6 link-localsrc/lib/signer-keepalive.ts (1)
74-75: Consider makingselfPubkeyoptional in the interface.Since
selfPubkeyis no longer used in the function body (as the no-peers case now treats the tick as healthy without self-ping), consider marking it optional in theKeepAliveConfiginterface to avoid requiring callers to provide an unused value.♻️ Suggested change to KeepAliveConfig
interface KeepAliveConfig { node: BifrostNode; groupCredential: string; shareCredential: string; relays: string[]; - selfPubkey: string; + selfPubkey?: string; // Reserved for future use logger?: KeepAliveLogger; options?: Partial<KeepAliveOptions>; }Alternatively, if
selfPubkeymay be needed in the future, the current approach of keeping it required is acceptable for API stability.src/lib/clientShareManager.ts (1)
86-95: Consider returning a result instead of silently logging errors.The
openShareLocationmethod logs errors but returnsvoid, making it impossible for callers to know if the operation failed. While this may be acceptable for a fire-and-forget UI action, consider returning a boolean or the result object for consistency with other methods.♻️ Optional: Return success/failure for consistency
- async openShareLocation(shareId: string): Promise<void> { + async openShareLocation(shareId: string): Promise<boolean> { try { const result = await window.electronAPI.openShareLocation(shareId); if (!result.ok) { console.error('Failed to open share location: operation rejected'); + return false; } + return true; } catch (error) { console.error('Failed to open share location:', error); + return false; } }src/components/App.tsx (1)
37-37: Consider persisting onboarding dismissal state.Currently,
showingOnboardingdefaults totrueand resets on every component mount. Users who dismiss the onboarding will see it again after refreshing the app or navigating away and back. If this is intentional UX (e.g., onboarding should re-appear until shares are created), this is fine. Otherwise, consider persisting the dismissal tolocalStorageor via an IPC call.♻️ Optional: Persist onboarding dismissal
+ const ONBOARDING_DISMISSED_KEY = 'igloo:onboarding-dismissed'; const App: React.FC = () => { const [showingCreate, setShowingCreate] = useState(false); const [showingRecover, setShowingRecover] = useState(false); const [showingAddShare, setShowingAddShare] = useState(false); - const [showingOnboarding, setShowingOnboarding] = useState(true); + const [showingOnboarding, setShowingOnboarding] = useState(() => { + return localStorage.getItem(ONBOARDING_DISMISSED_KEY) !== 'true'; + }); // ... existing code ... + const handleGetStarted = () => { + localStorage.setItem(ONBOARDING_DISMISSED_KEY, 'true'); + setShowingOnboarding(false); + }; // In the render: - <OnboardingWelcome onGetStarted={() => setShowingOnboarding(false)} /> + <OnboardingWelcome onGetStarted={handleGetStarted} />Also applies to: 274-275
src/__tests__/workflows/keyset-creation.test.ts (1)
1-3: Consider using @/ path alias for imports.As per coding guidelines, prefer
@/path alias over relative paths for imports. These could be:-import { clientShareManager } from '../../lib/clientShareManager'; -import { mockDeriveSecret } from '../setup'; -import { validatePassword } from '../../lib/validation'; +import { clientShareManager } from '@/lib/clientShareManager'; +import { mockDeriveSecret } from '@/__tests__/setup'; +import { validatePassword } from '@/lib/validation';src/types/index.ts (1)
190-198: Well-structured RelayPlan interface.The interface correctly captures the relay planning output with distinct categories. Consider adding brief JSDoc comments to clarify the purpose of each field (e.g., what distinguishes
groupRelaysfromgroupExtras), as this would help consumers of the type understand the semantics.src/__tests__/lib/validation.test.ts (1)
8-16: Consider using @/ path alias for the import.As per coding guidelines, prefer
@/path alias over relative paths:-import { - VALIDATION_LIMITS, - validatePassword, - validatePositiveInteger, - validateShareName, - sanitizeUserError, - validatePasswordLength, - formatRelayUrl, -} from '../../lib/validation'; +import { + VALIDATION_LIMITS, + validatePassword, + validatePositiveInteger, + validateShareName, + sanitizeUserError, + validatePasswordLength, + formatRelayUrl, +} from '@/lib/validation';src/__tests__/components/OnboardingWelcome.test.tsx (1)
140-145: Consider making the list assertion more specific.Using
screen.getByRole('list')assumes there's only one list in the component. If additional lists are added later, this test could fail unexpectedly or match the wrong element.🔧 More specific list query
it('should use ordered list for How It Works steps', () => { render(<OnboardingWelcome onGetStarted={mockOnGetStarted} />); - const list = screen.getByRole('list'); - expect(list.tagName).toBe('OL'); + // Find the How It Works section and verify it contains an ordered list + const howItWorksHeading = screen.getByText('How It Works'); + const section = howItWorksHeading.closest('section') ?? howItWorksHeading.parentElement; + const list = section?.querySelector('ol'); + expect(list).toBeInTheDocument(); });src/components/AddShare.tsx (1)
395-415: IPC relay planning implementation looks correct.The relay plan is computed via the main process IPC channel, which has access to Node.js APIs and environment variables unavailable in the renderer. The error handling properly throws to be caught by the outer try/catch.
One note: The
LegacyDecodedGrouptype and double casting (as unknown as Record<string, unknown>) is verbose. Consider defining a shared IPC-compatible type or using a simpler approach.♻️ Optional: Simplify the type casting
If the IPC schema already accepts
decodedGroupas a passthrough object, you could simplify:- type LegacyDecodedGroup = DecodedGroup & { - relayUrls?: string[]; - relay_urls?: string[]; - }; - - const legacyGroup = decodedGroup as LegacyDecodedGroup; - // Compute relay plan via IPC (main process has access to Node.js APIs) - // Cast decodedGroup for IPC serialization - the main process only needs the object shape const relayPlanResult = await window.electronAPI.computeRelayPlan({ groupCredential, - decodedGroup: legacyGroup as unknown as Record<string, unknown>, + decodedGroup: decodedGroup as Record<string, unknown>, explicitRelays: undefined, envRelay: undefined });src/__tests__/integration/computeRelayPlan.test.ts (1)
29-74: Static analysis tests are comprehensive but brittle.The regex-based verification covers key handler behaviors (schema validation, env fallback, input normalization, error handling). Be aware these tests may break if the implementation code is reformatted or refactored, even if behavior remains correct. Consider adding a comment noting this trade-off.
src/__tests__/workflows/share-lifecycle.test.ts (1)
5-12: Consider extracting shared helper to avoid duplication.The
toScalarHexhelper is duplicated fromsrc/__tests__/__mocks__/bifrost.mock.ts. Consider exporting it from a shared utilities file (e.g.,__mocks__/test-utils.ts) to maintain DRY principles and ensure consistent behavior across test files.♻️ Suggested extraction
Create a shared test utility file:
// src/__tests__/__mocks__/test-utils.ts export const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); export const toPointHex = (seed: number): string => '02' + seed.toString(16).padStart(64, '0');Then import in both files:
-const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); +import { toScalarHex } from '../__mocks__/test-utils';src/__tests__/components/SaveShare.test.tsx (1)
14-26: Consider using deterministic values for crypto mock in tests.The current implementation uses
Math.random()which makes tests non-deterministic. While functional, this could cause flaky tests if assertions depend on specific salt values.♻️ Suggested improvement for deterministic testing
const mockGetRandomValues = jest.fn((array: Uint8Array) => { for (let i = 0; i < array.length; i++) { - array[i] = Math.floor(Math.random() * 256); + array[i] = i % 256; // Deterministic values based on index } return array; });src/preload.ts (1)
93-166: Consider addingas constto the exposed object for stricter type inference.The
satisfies ElectronAPIassertion validates the shape but doesn't narrow the type. This is fine for runtime but could be enhanced.Minor observation: The cast on line 95 (
as Promise<IglooShare[] | false>) could be avoided if the main process handler explicitly types its return.src/components/Keyset.tsx (1)
554-578: Group QR modal implementation is correct but duplicates share QR modal structure.The modal works correctly. For future maintainability, consider extracting a reusable
QrCodeModalcomponent since the share QR modal (lines 471-552) has similar structure.src/__tests__/integration/electron-security.test.ts (2)
101-105: Fragile regex for cleanup function detection.The regex
onEchoReceived:.*return\s*\(\)\s*=>/suses thesflag for dotall matching, which could match unintended content if the file structure changes significantly.♻️ Consider more specific patterns
- it('should return cleanup functions for event listeners', () => { - // Verify event listeners return cleanup functions - expect(preloadContent).toMatch(/onEchoReceived:.*return\s*\(\)\s*=>/s); - expect(preloadContent).toMatch(/onEchoError:.*return\s*\(\)\s*=>/s); - }); + it('should return cleanup functions for event listeners', () => { + // Verify event listeners use removeListener for cleanup + expect(preloadContent).toMatch(/ipcRenderer\.removeListener\(['"]echo-received['"]/); + expect(preloadContent).toMatch(/ipcRenderer\.removeListener\(['"]echo-error['"]/); + });
300-314: Regex for sanitizeErrorForLog path detection may be brittle.The pattern
sanitizeErrorForLog[\s\S]*?replace\([\s\S]*?<path>relies on specific formatting. Consider testing the function's behavior directly in a unit test instead.src/components/Recover.tsx (2)
481-492: Cleanup logic is correct but eslint-disable comment is misleading.The eslint-disable comment on line 489 suggests a lint issue with accessing
.currentin cleanup, but this is actually the correct pattern for cleanup functions—you want to clear whatever timeout exists at unmount time.♻️ Suggested comment improvement
- // Note: Accessing .current in cleanup is intentional for timeout refs - - // we want to clear whatever timeout is active at unmount time. useEffect(() => { return () => { if (nsecClearTimeoutRef.current) clearTimeout(nsecClearTimeoutRef.current); if (copiedTimeoutRef.current) clearTimeout(copiedTimeoutRef.current); if (copyErrorTimeoutRef.current) clearTimeout(copyErrorTimeoutRef.current); - // eslint-disable-next-line react-hooks/exhaustive-deps if (autofilledTimeoutRef.current) clearTimeout(autofilledTimeoutRef.current); }; }, []);The eslint-disable is unnecessary here—accessing refs in cleanup is valid since refs are mutable and the cleanup runs synchronously.
752-800: Consider aria-live for copy feedback accessibility.The copy success/error states change dynamically. Screen readers may not announce these changes.
♻️ Add aria-live for copy feedback
{/* Action buttons */} -<div className="flex flex-col gap-2"> +<div className="flex flex-col gap-2" aria-live="polite"> <div className="flex gap-2">src/lib/ipcSchemas.ts (1)
60-70: Consider adding format validation for credential strings.
ShareCredentialSchemaandGroupCredentialSchemaonly validate length and non-whitespace. The credentials have specific prefixes (bfshare1...,bfgroup1...).♻️ Add prefix validation for credentials
/** Share credential: base64url encoded, reasonable max for FROST shares */ export const ShareCredentialSchema = z.string() .min(1, 'Share credential is required') .max(5000, 'Share credential exceeds maximum length') - .refine(s => s.trim().length > 0, 'Share credential cannot be only whitespace'); + .refine(s => s.trim().length > 0, 'Share credential cannot be only whitespace') + .refine(s => s.startsWith('bfshare'), 'Share credential must start with bfshare'); /** Group credential: encoded group key data */ export const GroupCredentialSchema = z.string() .min(1, 'Group credential is required') .max(5000, 'Group credential exceeds maximum length') - .refine(s => s.trim().length > 0, 'Group credential cannot be only whitespace'); + .refine(s => s.trim().length > 0, 'Group credential cannot be only whitespace') + .refine(s => s.startsWith('bfgroup'), 'Group credential must start with bfgroup');src/lib/validation.ts (1)
509-519: Share name normalization regex may be too aggressive.The regex
/ share \d+$/istrips trailing " share N" but is case-insensitive. If a user intentionally names something "My SHARE 1", this would incorrectly strip it.♻️ Consider exact case matching for normalization
- // Normalize: strip trailing " share N" suffix if present (e.g., "My Keyset share 1" -> "My Keyset") - // Use regex to only match the specific pattern at the end, not arbitrary occurrences - const normalized = trimmed.replace(/ share \d+$/i, '').trim() || trimmed; + // Normalize: strip trailing " share N" suffix (lowercase) if present + // Only strip the exact pattern used by the application's naming convention + const normalized = trimmed.replace(/ share \d+$/, '').trim() || trimmed;src/main.ts (2)
5-19: Consider using@/path alias for internal imports.Per coding guidelines, prefer
@/path alias over relative paths for local imports. For example:
'./lib/shareManager.js'→'@/lib/shareManager.js''./lib/echoRelays.js'→'@/lib/echoRelays.js''./lib/ipcSchemas.js'→'@/lib/ipcSchemas.js'
208-229: Consider simplifying the URL canonicalization.The
relayTargetKeyfunction has a fallback regex parser for whennew URL()fails. This is reasonable, but the fallback could be simplified by just returning the lowercased relay string directly when URL parsing fails, since the primary goal is deduplication.♻️ Optional simplification
} catch { - const match = relay.match(/^(wss?):\/\/([^/?#]+)(.*)$/); - if (match) { - const scheme = match[1].toLowerCase(); - const authority = match[2].toLowerCase(); - const pathAndMore = match[3] ?? ''; - return `${scheme}://${authority}${pathAndMore}`; - } - return relay; + // Fallback: lowercase the entire relay URL for consistent deduplication + return relay.toLowerCase(); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (45)
CLAUDE.mdREADME.mdindex.htmlpackage.jsonsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/App.test.tsxsrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/Signer.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/validation.test.tssrc/__tests__/setup.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/AddShare.tsxsrc/components/App.tsxsrc/components/Create.tsxsrc/components/Keyset.tsxsrc/components/LoadShare.tsxsrc/components/OnboardingWelcome.tsxsrc/components/Recover.tsxsrc/components/SaveShare.tsxsrc/components/Signer.tsxsrc/lib/clientShareManager.tssrc/lib/echoRelays.tssrc/lib/ipcSchemas.tssrc/lib/shareManager.tssrc/lib/signer-keepalive.tssrc/lib/validation.tssrc/main.tssrc/preload.tssrc/types/index.tstsconfig.preload.jsontsconfig.test.jsonwebpack.config.cjs
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use strict TypeScript mode with explicit types for function parameters and returns
Prefer interfaces over types for defining object shapes in TypeScript
Use @/ path alias for imports instead of relative paths
Use camelCase for functions and variables
Use UPPER_SNAKE_CASE for constants
Files:
src/lib/signer-keepalive.tssrc/components/Signer.tsxsrc/components/Create.tsxsrc/components/SaveShare.tsxsrc/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/types/index.tssrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tssrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/App.tsxsrc/preload.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/lib/clientShareManager.tssrc/lib/shareManager.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/LoadShare.tsxsrc/lib/echoRelays.tssrc/main.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/lib/ipcSchemas.tssrc/components/Keyset.tsxsrc/lib/validation.tssrc/__tests__/lib/ipcSchemas.test.tssrc/components/OnboardingWelcome.tsxsrc/__tests__/components/App.test.tsx
src/{components,lib,types}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
UI components sit under
src/components/, helpers insrc/lib/, shared types insrc/types/
Files:
src/lib/signer-keepalive.tssrc/components/Signer.tsxsrc/components/Create.tsxsrc/components/SaveShare.tsxsrc/types/index.tssrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/App.tsxsrc/lib/clientShareManager.tssrc/lib/shareManager.tssrc/components/LoadShare.tsxsrc/lib/echoRelays.tssrc/lib/ipcSchemas.tssrc/components/Keyset.tsxsrc/lib/validation.tssrc/components/OnboardingWelcome.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: All runtime code is TypeScript with two-space indentation and required semicolons
Import third-party packages before internal modules and use the@/alias for local paths
Files:
src/lib/signer-keepalive.tssrc/components/Signer.tsxsrc/components/Create.tsxsrc/components/SaveShare.tsxsrc/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/types/index.tssrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tssrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/App.tsxsrc/preload.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/lib/clientShareManager.tssrc/lib/shareManager.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/LoadShare.tsxsrc/lib/echoRelays.tssrc/main.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/lib/ipcSchemas.tssrc/components/Keyset.tsxsrc/lib/validation.tssrc/__tests__/lib/ipcSchemas.test.tssrc/components/OnboardingWelcome.tsxsrc/__tests__/components/App.test.tsx
src/components/[A-Z]*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for component files
Files:
src/components/Signer.tsxsrc/components/Create.tsxsrc/components/SaveShare.tsxsrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/App.tsxsrc/components/LoadShare.tsxsrc/components/Keyset.tsxsrc/components/OnboardingWelcome.tsx
src/components/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/components/**/*.tsx: Use Tailwind CSS utility classes in JSX and use cn() utility for conditional classes
Avoid inline styles unless conditionally computed
Use functional components with hooks instead of class components
Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system accessTailwind utility classes style JSX; reserve inline styles for dynamic values only
Files:
src/components/Signer.tsxsrc/components/Create.tsxsrc/components/SaveShare.tsxsrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/App.tsxsrc/components/LoadShare.tsxsrc/components/Keyset.tsxsrc/components/OnboardingWelcome.tsx
src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/components/**/*.{ts,tsx}: Prefer functional React components; name them in PascalCase and helpers in camelCase
Components insrc/components/follow kebab-case filenames (e.g.,share-list.tsx)
Files:
src/components/Signer.tsxsrc/components/Create.tsxsrc/components/SaveShare.tsxsrc/components/AddShare.tsxsrc/components/Recover.tsxsrc/components/App.tsxsrc/components/LoadShare.tsxsrc/components/Keyset.tsxsrc/components/OnboardingWelcome.tsx
src/__tests__/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/__tests__/**/*.test.{ts,tsx}: Mock @frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Use jest.clearAllMocks() in beforeEach hooks to ensure test isolation
Mock Electron's ipcRenderer and app APIs in tests
Files:
src/__tests__/components/Signer.test.tsxsrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/App.test.tsx
src/__tests__/components/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test React component behavior in tests/components/ directory
Files:
src/__tests__/components/Signer.test.tsxsrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/components/App.test.tsx
src/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests mirror runtime code under
src/__tests__/withintegration/,performance/, and__mocks__/subdirectories for feature-specific coverage
Files:
src/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/App.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Jest with Testing Library powers renderer specs; place new tests beside features as
<subject>.test.tsx
Files:
src/__tests__/components/Signer.test.tsxsrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/App.test.tsx
src/__tests__/{setup.ts,__mocks__/**}
📄 CodeRabbit inference engine (AGENTS.md)
Register shared mocks in
src/__tests__/setup.tsand reuse fixtures fromsrc/__tests__/__mocks__/
Files:
src/__tests__/setup.tssrc/__tests__/__mocks__/bifrost.mock.ts
src/__tests__/integration/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test Electron IPC communication in tests/integration/ directory
Files:
src/__tests__/integration/electron-security.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/integration/userInputValidation.test.ts
src/__tests__/lib/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Test utility functions in tests/lib/ directory
Files:
src/__tests__/lib/validation.test.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.ts
src/__tests__/__mocks__/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared mock implementations in tests/mocks/ for use across test files
Files:
src/__tests__/__mocks__/bifrost.mock.ts
src/lib/shareManager.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/lib/shareManager.ts: Never store plaintext share data; always encrypt with user passwords
Store shares in /igloo/shares/ directory using the defined JSON schema
Files:
src/lib/shareManager.ts
src/__tests__/workflows/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test end-to-end user flows in tests/workflows/ directory
Files:
src/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.ts
src/main.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/main.ts: Register IPC handlers in main.ts using ipcMain.handle() for file system operations
Electron's main process uses CommonJS (require) while renderer uses ESM (import)Main Electron process lives in
src/main.ts; the renderer bootstraps fromsrc/renderer.tsx
Files:
src/main.ts
src/**/*ipc*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Cover new IPC handlers with unit tests plus at least one scenario in
src/__tests__/integration/
Files:
src/lib/ipcSchemas.tssrc/__tests__/lib/ipcSchemas.test.ts
🧠 Learnings (28)
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Do not implement cryptographic logic in the desktop app; delegate all crypto operations to frostr/igloo-core
Applied to files:
src/components/Signer.tsxsrc/components/AddShare.tsxCLAUDE.mdREADME.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/shareManager.ts : Never store plaintext share data; always encrypt with user passwords
Applied to files:
src/components/SaveShare.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tssrc/components/AddShare.tsxsrc/components/Recover.tsxCLAUDE.mdsrc/lib/clientShareManager.tssrc/lib/shareManager.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/components/LoadShare.tsxsrc/__tests__/workflows/keyset-creation.test.tssrc/components/Keyset.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/encryption.ts : Use PBKDF2 with 600,000 iterations for encrypting share files
Applied to files:
src/components/SaveShare.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/components/LoadShare.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/__tests__/**/*.{ts,tsx} : Tests mirror runtime code under `src/__tests__/` with `integration/`, `performance/`, and `__mocks__/` subdirectories for feature-specific coverage
Applied to files:
tsconfig.test.jsonsrc/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to **/*.{ts,tsx} : Use @/ path alias for imports instead of relative paths
Applied to files:
tsconfig.test.jsonwebpack.config.cjs
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/lib/**/*.test.ts : Test utility functions in __tests__/lib/ directory
Applied to files:
tsconfig.test.jsonsrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/shareManager.test.tsCLAUDE.mdsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.ts
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.{ts,tsx} : All runtime code is TypeScript with two-space indentation and required semicolons
Applied to files:
tsconfig.test.json
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*ipc*.{ts,tsx} : Cover new IPC handlers with unit tests plus at least one scenario in `src/__tests__/integration/`
Applied to files:
tsconfig.test.jsonsrc/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/shareManager.test.tssrc/components/AddShare.tsxsrc/preload.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/lib/clientShareManager.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/main.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/lib/ipcSchemas.tssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.{ts,tsx} : Import third-party packages before internal modules and use the `@/` alias for local paths
Applied to files:
tsconfig.test.jsontsconfig.preload.jsonCLAUDE.mdwebpack.config.cjs
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/**/*.test.{ts,tsx} : Jest with Testing Library powers renderer specs; place new tests beside features as `<subject>.test.tsx`
Applied to files:
tsconfig.test.jsonsrc/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/{components,lib,types}/**/*.{ts,tsx} : UI components sit under `src/components/`, helpers in `src/lib/`, shared types in `src/types/`
Applied to files:
tsconfig.test.jsonCLAUDE.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/components/**/*.test.{ts,tsx} : Test React component behavior in __tests__/components/ directory
Applied to files:
tsconfig.test.jsonsrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock frostr/igloo-core functions at the module level in tests to avoid ESM/Jest complications
Applied to files:
tsconfig.test.jsonsrc/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tspackage.jsonsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Use jest.clearAllMocks() in beforeEach hooks to ensure test isolation
Applied to files:
tsconfig.test.jsonsrc/__tests__/setup.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/__mocks__/**/*.ts : Place shared mock implementations in __tests__/__mocks__/ for use across test files
Applied to files:
tsconfig.test.jsonsrc/__tests__/setup.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tsCLAUDE.mdsrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/__tests__/{setup.ts,__mocks__/**} : Register shared mocks in `src/__tests__/setup.ts` and reuse fixtures from `src/__tests__/__mocks__/`
Applied to files:
src/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/__mocks__/bifrost.mock.tssrc/__tests__/lib/shareManager.test.tssrc/__tests__/lib/igloo-core-integration.test.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/**/*.test.{ts,tsx} : Mock Electron's ipcRenderer and app APIs in tests
Applied to files:
src/__tests__/components/Signer.test.tsxsrc/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/preload.tssrc/__tests__/workflows/share-lifecycle.test.tswebpack.config.cjssrc/__tests__/lib/ipcSchemas.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/integration/**/*.test.{ts,tsx} : Test Electron IPC communication in __tests__/integration/ directory
Applied to files:
src/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/components/Recover.test.tsxsrc/__tests__/lib/validation.test.tssrc/__tests__/components/AddShare.test.tsxsrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/__tests__/components/LoadShare.test.tsxsrc/__tests__/integration/userInputValidation.test.tssrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/lib/shareManager.test.tssrc/components/AddShare.tsxsrc/preload.tsCLAUDE.mdsrc/__tests__/lib/igloo-core-integration.test.tssrc/lib/clientShareManager.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/main.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/lib/ipcSchemas.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Electron's main process uses CommonJS (require) while renderer uses ESM (import)
Applied to files:
src/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tspackage.jsonsrc/preload.tsCLAUDE.mdsrc/lib/clientShareManager.tssrc/main.tswebpack.config.cjs
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Debug flaky Electron tests with `npm test -- --runInBand` to serialize execution
Applied to files:
src/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/workflows/share-lifecycle.test.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/components/**/*.tsx : Use ipcRenderer.invoke() in renderer process to call IPC handlers without direct file system access
Applied to files:
src/__tests__/setup.tssrc/__tests__/integration/electron-security.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/__tests__/integration/clientShareManager.test.tssrc/preload.tssrc/lib/clientShareManager.tssrc/components/Keyset.tsx
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/main.ts : Main Electron process lives in `src/main.ts`; the renderer bootstraps from `src/renderer.tsx`
Applied to files:
src/__tests__/integration/electron-security.test.tspackage.jsonsrc/preload.tsCLAUDE.mdsrc/main.tswebpack.config.cjs
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/main.ts : Register IPC handlers in main.ts using ipcMain.handle() for file system operations
Applied to files:
src/__tests__/integration/electron-security.test.tssrc/__tests__/integration/computeRelayPlan.test.tssrc/preload.tssrc/lib/clientShareManager.tssrc/main.tssrc/components/Keyset.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/lib/shareManager.ts : Store shares in <appData>/igloo/shares/ directory using the defined JSON schema
Applied to files:
src/__tests__/integration/clientShareManager.test.tssrc/__tests__/lib/shareManager.test.tssrc/components/AddShare.tsxsrc/components/Recover.tsxCLAUDE.mdsrc/lib/clientShareManager.tssrc/lib/shareManager.tssrc/__tests__/workflows/share-lifecycle.test.tssrc/lib/ipcSchemas.ts
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Applies to src/__tests__/workflows/**/*.test.{ts,tsx} : Test end-to-end user flows in __tests__/workflows/ directory
Applied to files:
src/__tests__/components/LoadShare.test.tsxsrc/__tests__/components/SaveShare.test.tsxsrc/__tests__/workflows/share-lifecycle.test.tssrc/__tests__/workflows/keyset-creation.test.tssrc/__tests__/components/OnboardingWelcome.test.tsxsrc/__tests__/lib/echoRelays.test.tssrc/__tests__/components/App.test.tsx
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Coordinate with frostr/igloo-core maintainers to add cryptographic features to the core library before using them
Applied to files:
package.jsonREADME.md
📚 Learning: 2026-01-02T23:50:16.045Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T23:50:16.045Z
Learning: Use props and callbacks for component communication instead of external state libraries like Redux
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-02T23:50:29.859Z
Learnt from: CR
Repo: FROSTR-ORG/igloo-desktop PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T23:50:29.859Z
Learning: Applies to src/components/**/*.{ts,tsx} : Prefer functional React components; name them in PascalCase and helpers in camelCase
Applied to files:
src/components/OnboardingWelcome.tsx
🧬 Code graph analysis (18)
src/components/Create.tsx (1)
src/lib/validation.ts (3)
validateShareName(488-519)VALIDATION_LIMITS(4-15)validatePositiveInteger(434-468)
src/components/SaveShare.tsx (1)
src/lib/validation.ts (1)
VALIDATION_LIMITS(4-15)
src/__tests__/lib/validation.test.ts (1)
src/lib/validation.ts (7)
VALIDATION_LIMITS(4-15)validatePassword(32-49)validatePasswordLength(567-587)validatePositiveInteger(434-468)validateShareName(488-519)sanitizeUserError(532-558)formatRelayUrl(324-403)
src/__tests__/components/AddShare.test.tsx (1)
src/__tests__/setup.ts (1)
mockElectronAPI(191-191)
src/__tests__/components/SaveShare.test.tsx (1)
src/lib/encryption.ts (1)
derive_secret_async(24-73)
src/components/AddShare.tsx (2)
src/lib/validation.ts (2)
VALIDATION_LIMITS(4-15)sanitizeUserError(532-558)src/types/index.ts (1)
DecodedGroup(9-19)
src/components/Recover.tsx (1)
src/lib/validation.ts (1)
VALIDATION_LIMITS(4-15)
src/components/App.tsx (1)
src/components/OnboardingWelcome.tsx (1)
OnboardingWelcome(9-136)
src/preload.ts (1)
src/types/index.ts (2)
IglooShare(44-55)RelayPlan(191-198)
src/lib/clientShareManager.ts (1)
src/types/index.ts (1)
IglooShare(44-55)
src/__tests__/workflows/share-lifecycle.test.ts (2)
src/__tests__/setup.ts (2)
mockElectronAPI(191-191)mockDeriveSecret(136-136)src/lib/clientShareManager.ts (1)
clientShareManager(98-98)
src/components/LoadShare.tsx (1)
src/lib/validation.ts (2)
VALIDATION_LIMITS(4-15)sanitizeUserError(532-558)
src/lib/echoRelays.ts (1)
src/types/index.ts (1)
RelayPlan(191-198)
src/__tests__/components/OnboardingWelcome.test.tsx (1)
src/components/OnboardingWelcome.tsx (1)
OnboardingWelcome(9-136)
src/__tests__/lib/echoRelays.test.ts (1)
src/lib/echoRelays.ts (2)
normalizeRelayUrl(221-221)computeRelayPlan(143-219)
src/components/Keyset.tsx (1)
src/components/ui/button.tsx (1)
Button(56-56)
src/__tests__/lib/ipcSchemas.test.ts (1)
src/lib/ipcSchemas.ts (9)
ShareIdSchema(14-17)HexSaltSchema(20-23)RelayUrlSchema(26-28)ShareCredentialSchema(61-64)GroupCredentialSchema(67-70)SaveShareSchema(73-101)RelayPlanArgsSchema(104-109)EchoStartArgsSchema(114-122)EchoStopArgsSchema(125-129)
src/components/OnboardingWelcome.tsx (1)
src/components/ui/button.tsx (1)
Button(56-56)
- Upgrade electron-builder 24.13.3 to 26.5.0 - Add tar@7.5.6 override to fix path sanitization vulnerabilities - Fix lodash prototype pollution via npm audit fix
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/components/App.tsx`:
- Around line 37-38: Replace the current onboarding toggle logic that relies
solely on showingOnboarding/hasShares by adding a "sharesLoaded" boolean state
(useState(false)), set it true only after the async share discovery completes
(where you check hasShares), and change the condition that displays onboarding
to require sharesLoaded && !hasShares (or setShowingOnboarding(false) when
sharesLoaded && hasShares). Update both places that initialize or toggle
showingOnboarding (the showingOnboarding/setShowingOnboarding usage and the
similar occurrence around the other state initialization) so onboarding is gated
until sharesLoaded is true and consider showing a lightweight loading state
while sharesLoaded is false.
In `@src/components/Create.tsx`:
- Around line 56-83: handleTotalKeysChange currently avoids cascading threshold
validation when totalKeys is invalid but does not clear any existing threshold
error, so stale thresholdError can persist; update handleTotalKeysChange to call
setThresholdError(null) (or empty string per project convention) in the branch
where result.isValid is false or result.value is null to explicitly clear
previous threshold errors, referencing validatePositiveInteger,
setThresholdError, totalKeys, threshold and thresholdResult to locate the logic
to change.
In `@src/components/Recover.tsx`:
- Around line 753-800: The JSX shows inconsistent nesting: the closing </div>
for the inner container is misplaced so only the first Button (handleCopyNsec)
appears nested inside <div className="flex gap-2"> while the other Buttons
(handleToggleNsecReveal, clearRecoveredNsec) are incorrectly outdented; fix by
moving the closing tag so that all three Button elements (the Copy button using
isNsecCopied/copyError, the Reveal/Hide button using isNsecRevealed, and the
Clear button) are children of the same inner <div className="flex gap-2"> and
then close that div before the outer container ends, adjusting indentation
accordingly.
In `@src/components/Signer.tsx`:
- Around line 251-252: The cleanup currently sets isMountedRef.current = false
and then calls addLog(), but addLog invokes setLogs (a state update) and can run
after unmount; modify the cleanup to guard the call by checking
isMountedRef.current before invoking addLog (i.e., only call addLog when
isMountedRef.current is true), ensuring any other places where cleanup or
unmount call addLog (see occurrences around addLog, isMountedRef, and setLogs at
lines ~251 and ~617-622) are similarly wrapped with the same
isMountedRef.current check to prevent state updates after unmount.
In `@src/lib/echoRelays.ts`:
- Around line 143-159: computeRelayPlan currently accepts relay URLs from
decoded group credentials without network scope validation; update the logic
that builds groupRelayCandidates (use extractGroupRelayCandidates and
decodeGroup flow) to filter out any relay whose hostname resolves to localhost,
loopback (::1), 127.0.0.1, or private IPv4 ranges (10.0.0.0/8, 172.16.0.0/12,
192.168.0.0/16) and reject malformed URLs—treat those candidates as
absent/invalid so they are not merged into the RelayPlan; implement the check
right after decoding (inside the try block where decoded is created) and ensure
explicitRelays, envRelay and baseRelays are not affected.
In `@src/lib/shareManager.ts`:
- Around line 37-41: The share data is currently written as plaintext JSON;
change share read/write logic to encrypt/decrypt share payloads using the user's
password (e.g., derive a key via PBKDF2/Argon2 and use AES-GCM) before
persisting and after loading (update functions that serialize/deserialize
shares—e.g., the routines that save/load share files referenced near
ensureSharesDirectory and the save/load share methods around lines 153–156);
after implementing encryption, enforce filesystem permissions by calling
fs.promises.chmod(this.sharesPath, 0o700) after mkdir in ensureSharesDirectory
and fs.promises.chmod(shareFilePath, 0o600) after creating or updating each
share file, and also run chmod on existing files when initializing to harden
permissions.
♻️ Duplicate comments (3)
package.json (1)
163-163: Verify@frostr/igloo-core^0.2.4 is published and resolvable.
This was flagged previously; if the version isn’t on npm, installs will fail.npm `@frostr/igloo-core` latest version 0.2.4 publishedsrc/components/Signer.tsx (1)
1197-1208: Hide the secret-share QR by default.
Displaying the secret share as a QR code is sensitive and should require explicit user confirmation before rendering. This mirrors prior feedback.src/__tests__/lib/echoRelays.test.ts (1)
1-9: Missing module-level mock for@frostr/igloo-core.The test imports
decodeGroupfrom@frostr/igloo-core(Line 2) and later casts it asjest.Mock(Line 151), but there's nojest.mock('@frostr/igloo-core', ...)at the module level. Without this, the cast is invalid and the assertions likeexpect(decodeGroup).toHaveBeenCalledWith(...)will fail or behave unexpectedly.As per coding guidelines, mock
@frostr/igloo-corefunctions at the module level to avoid ESM/Jest complications.Proposed fix
import { computeRelayPlan, normalizeRelayUrl } from '../../lib/echoRelays'; import { decodeGroup, DEFAULT_ECHO_RELAYS } from '@frostr/igloo-core'; import fs from 'node:fs'; +// Mock `@frostr/igloo-core` module +jest.mock('@frostr/igloo-core', () => ({ + decodeGroup: jest.fn(), + DEFAULT_ECHO_RELAYS: ['wss://relay.damus.io', 'wss://relay.primal.net'], +})); + // Mock fs module jest.mock('node:fs', () => ({ readFileSync: jest.fn(), }));
🧹 Nitpick comments (11)
package.json (1)
136-136: electron-builder 26.5.0 is compatible with Electron 38.1.2; however, test notarization thoroughly.electron-builder 26.5.0 contains no breaking changes—only minor fixes (dmgbuild badge-icon support, tar update, Windows spawn EINVAL fix). The version is compatible with your Electron 38.1.2 setup.
However, known reports (Nov 2025) document Electron 38 notarization failures on macOS Sequoia with "The signature of the binary is invalid" errors, even when local codesign verification passes. This is not specific to electron-builder 26.5.0 but rather an Apple signature validation edge case with newer Electron. Your custom notarization script approach (afterSign hook with notarize: false) provides some mitigation, but validate notarization during macOS builds to catch any signature or entitlement issues early.
src/__tests__/lib/igloo-core-integration.test.ts (1)
1-5: Consider centralizing shared test helpers.The
toScalarHexandtoPointHexhelpers are duplicated here and inbifrost.mock.ts. Per coding guidelines, shared mock implementations should be placed in__tests__/__mocks__/for reuse. Consider exporting these helpers frombifrost.mock.tsand importing them here.♻️ Suggested approach
In
bifrost.mock.ts, export the helpers:-const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); -const toPointHex = (seed: number): string => '02' + seed.toString(16).padStart(64, '0'); +export const toScalarHex = (seed: number): string => seed.toString(16).padStart(64, '0'); +export const toPointHex = (seed: number): string => '02' + seed.toString(16).padStart(64, '0');Then import in this test file:
import { toScalarHex, toPointHex } from '../__mocks__/bifrost.mock';src/__tests__/workflows/keyset-creation.test.ts (1)
5-6: Consider centralizingtoScalarHexif used in multiple tests.
If this helper appears in other specs, moving it tosrc/__tests__/__mocks__/orsrc/__tests__/setup.tskeeps fixtures consistent and avoids drift. Based on learnings, prefer shared test fixtures in__mocks__/setup.src/__tests__/lib/validation.test.ts (1)
8-18: Addjest.clearAllMocks()in abeforeEachfor isolation.
Even without explicit mocks here, aligning with the repo test standard keeps suites consistent.🧹 Suggested addition
import { VALIDATION_LIMITS, validatePassword, validatePositiveInteger, validateShareName, sanitizeUserError, validatePasswordLength, formatRelayUrl, } from '../../lib/validation'; +beforeEach(() => { + jest.clearAllMocks(); +});Based on learnings, ensure test isolation via
jest.clearAllMocks()inbeforeEach.src/components/OnboardingWelcome.tsx (1)
1-3: Prefer@/alias for the Button import.
Aligns with local import conventions and simplifies refactors. As per coding guidelines, prefer@/aliases for local modules.♻️ Suggested change
-import { Button } from './ui/button'; +import { Button } from '@/components/ui/button';src/components/Create.tsx (1)
294-342: Usecn()for conditional class composition.
This keeps conditional Tailwind class logic consistent across components. As per coding guidelines, prefercn()for conditional classes.♻️ Suggested refactor
-import { InputWithValidation } from "@/components/ui/input-with-validation" +import { InputWithValidation } from "@/components/ui/input-with-validation" +import { cn } from "@/lib/utils" @@ - className={`bg-gray-800/50 border-gray-700/50 text-blue-300 py-2 text-sm w-full ${totalKeysError ? 'border-red-500' : ''}`} + className={cn( + "bg-gray-800/50 border-gray-700/50 text-blue-300 py-2 text-sm w-full", + totalKeysError && "border-red-500" + )} @@ - className={`bg-gray-800/50 border-gray-700/50 text-blue-300 py-2 text-sm w-full ${thresholdError ? 'border-red-500' : ''}`} + className={cn( + "bg-gray-800/50 border-gray-700/50 text-blue-300 py-2 text-sm w-full", + thresholdError && "border-red-500" + )}src/components/Recover.tsx (2)
448-474: Consider handling clipboard permission denial explicitly.The
handleCopyNsecfunction catches clipboard errors but shows a generic message. In secure contexts, browsers may deny clipboard access, and users might benefit from more specific guidance.Suggested improvement for better UX
} catch (err) { console.error('Failed to copy NSEC:', err); // Reset success state on failure to prevent contradictory UI setIsNsecCopied(false); // Show error feedback to user - setCopyError('Failed to copy. Please reveal and copy manually.'); + const isPermissionError = err instanceof DOMException && err.name === 'NotAllowedError'; + setCopyError( + isPermissionError + ? 'Clipboard access denied. Please reveal and copy manually.' + : 'Failed to copy. Please reveal and copy manually.' + );
746-749: Minor: Masked display leaks NSEC length information.The masked display shows
{recoveredNsec.substring(0, 8)}{'•'.repeat(40)}...which reveals the first 8 characters. While the prefix is intentional for verification, consider whether this is acceptable for your threat model. NSEC keys have a predictable prefix (nsec1), so this is likely fine.src/lib/echoRelays.ts (1)
69-90: Synchronous file read may block the main thread.
readConfiguredRelaysSyncusesfs.readFileSyncwhich blocks the event loop. While this is called during relay plan computation (likely at startup or infrequently), consider whether an async version would be more appropriate for the Electron main process.The current implementation is acceptable if called infrequently, but if
computeRelayPlanis invoked on hot paths (e.g., per-request), this could cause latency spikes.src/lib/ipcSchemas.ts (2)
61-70: Consider adding format validation to credential schemas.The
ShareCredentialSchemaandGroupCredentialSchemaonly validate length and non-whitespace. If these credentials have known prefixes (e.g.,bfshare1,bfgroup1), adding a regex check would catch invalid inputs earlier.Optional prefix validation
export const ShareCredentialSchema = z.string() .min(1, 'Share credential is required') .max(5000, 'Share credential exceeds maximum length') - .refine(s => s.trim().length > 0, 'Share credential cannot be only whitespace'); + .refine(s => s.trim().length > 0, 'Share credential cannot be only whitespace') + .refine(s => /^bfshare1/i.test(s.trim()), 'Share credential must start with bfshare1'); export const GroupCredentialSchema = z.string() .min(1, 'Group credential is required') .max(5000, 'Group credential exceeds maximum length') - .refine(s => s.trim().length > 0, 'Group credential cannot be only whitespace'); + .refine(s => s.trim().length > 0, 'Group credential cannot be only whitespace') + .refine(s => /^bfgroup1/i.test(s.trim()), 'Group credential must start with bfgroup1');
86-88: Consider extending metadata schema for future fields.The
metadataobject only validatesbinder_sn. If other metadata fields are expected in the future, consider using.passthrough()or documenting the expected structure.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/App.tsx (1)
48-56: Consider adding error handling to prevent stuck loading state.If
clientShareManager.getShares()throws an error,sharesLoadedwill never becometrue, leaving users stuck on the "Loading..." screen indefinitely.🛠️ Suggested fix
useEffect(() => { // Check if there are any shares saved const checkForShares = async () => { - const shares = await clientShareManager.getShares(); - setHasShares(Array.isArray(shares) && shares.length > 0); - setSharesLoaded(true); + try { + const shares = await clientShareManager.getShares(); + setHasShares(Array.isArray(shares) && shares.length > 0); + } catch (error) { + console.error('Failed to check for shares:', error); + setHasShares(false); + } finally { + setSharesLoaded(true); + } }; checkForShares(); }, []);
♻️ Duplicate comments (3)
src/components/Recover.tsx (1)
753-800: JSX structure appears correct.The button group structure is properly nested:
- Outer
flex flex-col gap-2container (line 753)- Inner
flex gap-2container (line 754) containing all three buttons- Closing
</div>on line 800 correctly closes the inner flex containerThe indentation issue flagged in previous reviews appears to have been addressed.
src/components/Signer.tsx (2)
617-628: Cleanup still triggersaddLogduring unmount.
This mirrors the earlier concern:addLog()still runs in cleanup whileisMountedRef.currentistrue, so it can still update state after unmount. Consider flipping the flag first or skipping the log during unmount.
1199-1210: Secret share QR still rendered openly (sensitive).
This is the same issue previously raised: the share credential QR should be gated behind explicit user action/confirmation to reduce exposure risk.
🧹 Nitpick comments (5)
src/components/Recover.tsx (3)
447-474: Consider checking clipboard API availability.The
navigator.clipboardAPI may be unavailable in non-secure contexts (non-HTTPS) or in some Electron configurations. While the catch block handles failures gracefully, a pre-flight check could provide more specific error messaging.♻️ Optional: Add clipboard availability check
const handleCopyNsec = useCallback(async () => { if (!recoveredNsec) return; // Clear any previous error setCopyError(null); + + if (!navigator.clipboard?.writeText) { + setCopyError('Clipboard API not available. Please reveal and copy manually.'); + return; + } + try { await navigator.clipboard.writeText(recoveredNsec);
481-492: Inconsistent eslint-disable placement.The
eslint-disable-next-linecomment on line 489 only applies to theautofilledTimeoutRefcleanup, but all four timeout refs are accessed identically. If the suppression is needed, it should apply consistently; if it's not needed (accessing.currentin cleanup is valid), it can be removed entirely.♻️ Remove unnecessary eslint-disable or apply consistently
useEffect(() => { return () => { if (nsecClearTimeoutRef.current) clearTimeout(nsecClearTimeoutRef.current); if (copiedTimeoutRef.current) clearTimeout(copiedTimeoutRef.current); if (copyErrorTimeoutRef.current) clearTimeout(copyErrorTimeoutRef.current); - // eslint-disable-next-line react-hooks/exhaustive-deps if (autofilledTimeoutRef.current) clearTimeout(autofilledTimeoutRef.current); }; }, []);
746-749: Consider reducing visible prefix length.The masked display shows the first 8 characters of the NSEC. Since the bech32 prefix
nsec1is only 5 characters, this reveals 3 characters of actual key data. Consider reducing to 5 characters to show only the format prefix.♻️ Reduce visible prefix to format identifier only
<div className="text-xs font-mono text-gray-400"> - {recoveredNsec.substring(0, 8)}{'•'.repeat(40)}... + {recoveredNsec.substring(0, 5)}{'•'.repeat(40)}... </div>src/lib/echoRelays.ts (2)
7-11: Use interfaces for object-shape declarations (Line 7, Line 219).
Type aliases are used for object shapes; the style guide prefers interfaces here.♻️ Suggested change
-type RelayCandidateSource = { +interface RelayCandidateSource { relays?: unknown; relayUrls?: unknown; relay_urls?: unknown; -}; +} -export type ComputeRelayPlanOptions = { +export interface ComputeRelayPlanOptions { groupCredential?: string | null; decodedGroup?: RelayCandidateSource | null; explicitRelays?: string[] | null; envRelay?: string | null; baseRelays?: string[] | null; -}; +}As per coding guidelines, prefer interfaces for object shapes.
Also applies to: 219-225
186-186: Add an explicit type forfilterPrivate(Line 186).
Strict typing guidance calls for explicit parameter types.♻️ Suggested change
-const normalizeList = (candidates?: unknown, filterPrivate = false): string[] => { +const normalizeList = (candidates?: unknown, filterPrivate: boolean = false): string[] => {As per coding guidelines, function parameters should be explicitly typed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 155-158: The lockfile currently pins `@isaacs/brace-expansion` and
tar to versions lower than the overrides in package.json; update the lockfile
(e.g., run your package manager's install/lock update command) so
`@isaacs/brace-expansion` resolves to >=5.0.1 and tar resolves to >=7.5.7, or
relax the overrides in package.json to match the lockfile; specifically ensure
the override entries for "@isaacs/brace-expansion" and "tar" in package.json and
the lockfile are consistent and then commit the regenerated lockfile so the
installed versions meet the override constraints.
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
1 participant
Summary
Continuation of work from #63 after git history recovery.
Consolidates all staging branch work:
Note
Previous PR #63 was closed due to a git history issue that has been resolved. This PR contains all the same work with
proper git history.
Summary by CodeRabbit
New Features
Security Enhancements
Bug Fixes
Improvements
Tests