Skip to content

Fix database corruption recovery and add error boundary - #45

Merged
scotej merged 7 commits into
mainfrom
claude/production-readiness-yg17gr
Jul 9, 2026
Merged

Fix database corruption recovery and add error boundary#45
scotej merged 7 commits into
mainfrom
claude/production-readiness-yg17gr

Conversation

@scotej

@scotej scotej commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

This PR addresses critical reliability issues: database corruption recovery now properly cleans up SQLite sidecars and detects corruption errors that were previously missed, and a new React error boundary prevents uncaught render errors from crashing the entire app. Additional fixes improve screen capture resilience, pairing reliability, and data integrity across multiple surfaces.

Key Changes

Database & Corruption Recovery

  • Clean up SQLite rollback-journal/WAL/SHM sidecars when renaming a corrupt database, preventing them from corrupting the newly-created file
  • Detect SQLITE_CORRUPT and SQLITE_NOTADB errors from integrity_check itself (not just verdict mismatches), fixing recovery for truncated files and garbage headers—the exact corruption cases the recovery was built for
  • Add comprehensive unit tests for truncated DBs, bad magic headers, and healthy DBs to prevent regression

Error Handling & UI Resilience

  • Add ErrorBoundary component to catch uncaught render errors and offer a retry that remounts the subtree, preventing a single feature fault from crashing the entire app window and disconnecting P2P/sessions
  • Add error strings to strings.ts for the boundary's recovery UI

Screen Capture & AI

  • Fix multi-display handling in 'all' mode: when one display ends, drop only that track and keep compositing survivors instead of tearing down all AI capture and showing the permission overlay
  • Add dropScreenTrack() helper to cleanly unhook listeners and stop streams for individual dead displays
  • Add device-loss recovery in SessionView with track 'ended' listener to surface the same recovery banner as initial acquisition failures

Data Integrity & Filtering

  • Add filterWho parameter to deriveTopDistractions() and deriveTopicTimeline() to exclude peer broadcast events from local user's report, preventing peers' AI alerts from inflating the local user's distraction list and deduction total
  • Add filterWho to computeTiming() and computeRecurringReasons() for consistent cross-session insights
  • Add unit tests verifying peer events are excluded when filtering by local user's ed_pubkey
  • Fix CSV formula injection by prefixing string cells starting with =, +, -, @, or tab with a single quote (numeric cells unaffected)

Pairing & Networking

  • Add replay guard to inbox subscription: track (from_ed_pubkey, nonce) pairs and drop duplicate invites until TTL expires, preventing strangers from re-broadcasting captured envelopes to spam invites
  • Add isSessionLive() guard to invite retry manager to prevent delivering retries for sessions the host has already left
  • Add pairingRelaysUnreachable() function that checks both Nostr and MQTT transports, fixing false "network down" verdict when MQTT is available but all curated Nostr relays are blocked
  • Update AddFriendDialog to reset phase on any close (user- or parent-driven) and abort in-flight pairing to prevent orphaned relay sockets

Sidecar & Process Management

  • Add shutting_down flag to sidecar state to prevent respawning after explicit kill at app exit/relaunch
  • Fix AI dialog cleanup on main window close to prevent orphaned floating window keeping the process alive
  • Add return statement after prevent_close() to avoid double-handling the close event

Shortcut Management

  • Fix global shortcut re-registration to detect when both actions share one combo (hand-edited settings.json) and avoid unregistering a still-live binding

CI/CD & Quality

  • Expand Rust CI to compile both shipped host targets (macOS aarch64 + Windows x86_64) on every push/PR, catching platform-specific compilation failures before release
  • Add accessibility gate (Storybook build + axe-core audit) to release-prep workflow to prevent shipping over a11y regressions
  • Add requirement that CI must pass on the exact main SHA being tagged before release, preventing irreversible version bump from outrunning test results

Documentation & Testing

  • Update README to clarify multi-relay architecture and improve friend-code pairing flow description
  • Add Storybook story for `ErrorBoundary

https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD

Summary by CodeRabbit

  • New Features

    • Added an error recovery screen so the app can stay usable after a render failure.
    • Improved friend pairing with offline friend codes and clearer safety-number confirmation.
    • Added support for multiple display captures during screen sharing.
  • Bug Fixes

    • Prevents downloads, restarts, and relaunches from getting stuck or leaving background processes behind.
    • Improves shortcut handling so existing key bindings are not broken.
    • Makes report and stats views reflect the current user more accurately.
  • Security & Reliability

    • Strengthened CSV export protection and input validation for malformed values.

claude added 6 commits July 9, 2026 07:24
Production-readiness audit findings across the Rust core:

- Corrupt-DB recovery (PR-1): is_definitely_corrupt() classified only an
  explicit non-"ok" integrity_check verdict as corruption, so the two most
  common real signatures — a truncated file (SQLITE_CORRUPT) and a damaged
  header (SQLITE_NOTADB), where integrity_check ERRORS rather than returning
  a row — fell through to "unproven" and the app bricked on every launch
  after a power-loss/force-kill. Classify by SQLite error code, clean up the
  -journal/-wal/-shm sidecars on rename, and add corruption-signature tests.
- AI dialog can't close (PR-2): the ai-dialog capability granted only
  core:default, which omits core:window:allow-close, so Esc/blur/X all
  silently failed. Grant allow-close.
- Model download hang (PR-6): no read timeout meant a mid-stream stall
  blocked bytes_stream().next() forever, freezing the UI and permanently
  locking the model_id. Add a 60s per-read idle timeout.
- Relaunch orphans sidecar (PR-7): system_relaunch_app's app.restart()
  skips RunEvent::Exit where the only sidecar kill lives. Kill first.
- Boot panic (PR-8): a global-shortcut OS conflict propagated out of setup()
  into build().expect() and panicked before first paint. Register
  best-effort; a failed binding is inert until rebound in Settings.
- Stranded main window (PR-25): closing main with minimize-to-tray off while
  the AI dialog is open left the process alive but unreachable. Destroy the
  dialog on that close path so the runtime exits.
- Sidecar respawn at quit (PR-26): the crash-restart watcher could spawn a
  fresh llama-server after kill_blocking already ran. Add a shutting_down
  flag re-checked after the backoff, before respawn.
- PTT shortcut (PR-33): changing one shortcut when both shared a combo (via
  hand-edited settings.json) unregistered the other. Only unregister the old
  combo when the other action isn't still using it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
…e loss

- Report + Stats misattribution (PR-5): deriveTopicTimeline,
  deriveTopDistractions, and the cross-session computeTiming /
  computeRecurringReasons walked every audit event regardless of signer.
  Peers broadcast topic_set and ai_alert, which are persisted locally under
  the same session_id, so an ordinary multi-peer session made a friend's
  topic appear as the local user switching topics and a friend's distractions
  count toward the local user's "top distractions", −deduction total, and
  cross-session "recurring distractions" — inconsistent with the local-only
  score gauge and focused-time trend. Thread the local ed_pubkey through all
  four aggregations and filter to it. Added who-filter regression tests.

- Multi-monitor capture teardown (PR-4): onScreenTrackEnded latched
  captureDenied and tore down ALL AI capture the moment ANY screen track
  ended, so in "All displays" mode unplugging a secondary monitor stopped
  focus detection and popped the (misleading) screen-permission overlay while
  the primary was still live. Discriminate: drop only the dead display and
  keep compositing survivors; latch only when the last live display is gone.

- Local media track death (audit critique): the session's camera/mic stream
  had no `ended` listener, so a mid-session device loss (unplug, OS privacy
  revocation, another app grabbing the camera — common on Windows) left peers
  staring at a frozen tile and silently killed the AI face path. Attach an
  ended listener that surfaces the existing "Try again" recovery banner.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
…uard

- Orphaned pairing room on programmatic close (PR-11): Radix's controlled
  Dialog only fires onOpenChange for user-driven closes, so a contact-card
  deep link arriving mid-pairing (Home.setAddOpen(false)) left the in-flight
  pairing's trystero room + Nostr/MQTT relay sockets orphaned — the abort
  never ran. Tear down on any `open` transition.

- Invite retry could yank a friend into a dead session (PR-9): a retry is
  queued only after the up-to-15s send times out, so a session ending during
  that window escaped cancelAll and later pulled the friend into an empty
  room. Guard delivery with an injected isSessionLive check — a retry never
  fires for a session that isn't the host's current live one.

- Inbox replay (PR-18): the inbox topic/password derive solely from the
  recipient's public ed pubkey, so a stranger with a contact card can
  re-broadcast a captured, still-unexpired envelope to re-fire the invite
  toast + OS notification. Dedup on (from_ed_pubkey, box nonce) with a
  TTL-bounded seen set.

- "Network down" hint misfired in the MQTT-fallback case (PR-21): pairing
  races Nostr + MQTT, but the reachability check read only Nostr sockets, so
  on a network blocking every Nostr relay (but allowing MQTT) it wrongly told
  the user their network was down and pushed them to cancel a pairing MQTT
  would complete. Add a transport-aware pairingRelaysUnreachable() judging
  both socket maps; keep the Nostr-only signal for the invite path.

- Deep link dropped mid-session (PR-23): PairDeepLinkBoot rendered only in
  the non-session tail, so a studyvis:// link clicked during a live session
  reached zero listeners and was silently lost. Render the full tail in the
  active-session branch too.

- Legacy pairing name unsanitized (PR-29): the 12-word hello's display_name
  is unsigned and was stored/rendered raw, unlike the ContactCard path which
  caps at 32 bytes and strips bidi/zero-width chars. Apply the same
  normalization (shared normalizeUntrustedName) so a peer can't spoof another
  friend's row or bloat the DB.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
… misc

- CSV formula injection (PR-10): a peer/stranger-chosen friend display name
  flows verbatim into the stats CSV; a name beginning with = + - @ (or tab/CR)
  would execute on open in Excel/Sheets/LibreOffice (=HYPERLINK exfil, DDE
  command exec). Prefix a quote on string cells starting with a trigger;
  numeric cells (incl. negatives) are untouched.

- Strict hex decoding (PR-34): hexToBytes used parseInt per byte, silently
  accepting a valid prefix or a leading sign/space ('1g'→0x01, '-a'→wraps).
  In a key-custodial wire format that must fail closed — validate the whole
  string against /^[0-9a-fA-F]*$/ first.

- No error boundary (PR-31): any uncaught render throw unmounted the whole
  tree to a blank window, taking the always-on inbox/presence + live session
  down with it. Add a top-level ErrorBoundary around the routed content with
  a calm "Try again" that remounts the subtree.

- Theme FOUC (PR-28): ThemeProvider's layout effect wrote the pre-hydration
  fallback 'dark' class, stripping the 'light' class the index.html pre-paint
  script set from the boot cache and flashing dark for light/auto users. Defer
  to the boot script until an authoritative mode exists (store ready or a
  Storybook defaultMode).

- Sidecar start/stop race (PR-13): start() unconditionally set status
  'running' after its await, clobbering an interleaved stop() and leaving the
  store 'running' on a killed process. Bail if a stop intervened.

- AI copy honesty (PR-24): the enable-toggle help promised screen access is
  requested "when you start your first session", but enabling AI requests it
  immediately. Reword both strings to match the shipped behavior.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
- Windows Rust never compiled pre-tag (PR-17): the only Rust CI job built
  aarch64-apple-darwin, so every #[cfg(target_os = "windows")] path (custom
  chrome, sidecar PATH block, windows-native keyring) first compiled inside
  release.yml AFTER release-prep pushed the immutable tag — a Windows-only
  break burned a version. Make the rust job a macOS + Windows matrix so both
  shipped host targets are checked/clippied/tested on every push and PR.

- Release-prep gate weaker than CI (PR-19): the one-click gate ran no Rust
  compilation and skipped the check-a11y gate, so an axe-core or
  clippy::correctness regression could reach a cut release. Add the a11y gate
  directly, and require the exact main SHA's CI run (which compiles + clippies
  + tests both host targets) to have concluded success before the irreversible
  bump/tag/push.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
…ledger

- README first-run now describes the default offline ContactCard swap +
  safety number (PR-3), not the retired 12-word flow; idle footprint reads
  "a handful of WebSockets to a small curated relay set" not "one" (PR-15);
  model picker "four tiers" not three (PR-16); versioning names v1.3.1 as
  current (PR-14); tech-stack pairing bullet mentions the offline code.
- PLAN §3 idle footprint + §5 model count corrected (PR-15, PR-16).
- ARCHITECTURE §6 Strategies rewritten to describe the shipped Nostr+MQTT
  pairing race instead of "MQTT not yet wired" (PR-32); §14 threat model
  gains rows for invite replay (mitigated), invite delivery false-positive
  (accepted), and presence spoofing (accepted).
- CHANGELOG v1.0.5 per-arch-DMG bullet annotated as superseded by the
  Intel-drop (aarch64-only today) (PR-36).
- ISSUES.md records the production-readiness audit round: I20–I45 fixed,
  I46–I50 deferred/accepted with reasons.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
Copilot AI review requested due to automatic review settings July 9, 2026 09:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@scotej, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 39 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c9bc5a31-fddf-4515-8fb4-03c5812cd48c

📥 Commits

Reviewing files that changed from the base of the PR and between ed1f509 and 6cdbd88.

📒 Files selected for processing (7)
  • .github/workflows/release-prep.yml
  • src-tauri/src/commands/system.rs
  • src-tauri/src/db/mod.rs
  • src/features/friends/inviteRetry.ts
  • src/lib/fileExport.ts
  • src/routes/Home.tsx
  • tests/unit/file-export.test.ts
📝 Walkthrough

Walkthrough

This PR bundles CI/release workflow updates, extensive documentation revisions, Rust desktop reliability fixes (sidecar shutdown, DB corruption detection, shortcut/relaunch handling, download timeouts), a new React ErrorBoundary, theme hydration gating, friends/pairing security hardening (name sanitization, invite replay guard, session-liveness gating, MQTT diagnostics), per-signer report/stats scoping, media-track recovery, and encoding/CSV export hardening.

Changes

CI and Release Workflow

Layer / File(s) Summary
Rust CI matrix build
.github/workflows/ci.yml
Runs the rust job on macOS and Windows via a matrix, fetching the correct llama-server prebuild per target.
Release gate hardening
.github/workflows/release-prep.yml
Adds Storybook build, Playwright/axe-core accessibility audit, and a check requiring a successful ci.yml run for the release commit.

Documentation and Audit Records

Layer / File(s) Summary
Architecture threat model
ARCHITECTURE.md
Documents MQTT wired for pairing only, BitTorrent still unwired, and adds three threat-model rows for invite replay, inbox delivery ambiguity, and presence forgery.
Release notes and product docs
CHANGELOG.md, PLAN.md, README.md
Updates networking footprint description, model-picker options, onboarding friend-code flow, and current release version to v1.3.1.
Audit ledger expansion
ISSUES.md
Adds a v1.3.1 production-readiness audit section and appends ledger entries I20–I50 covering fixes, triage, and deferrals.

Desktop Rust Reliability

Layer / File(s) Summary
Model download timeout
src-tauri/src/commands/models.rs
Adds an HTTP read timeout to bound stalls during streaming downloads.
Sidecar shutdown guard
src-tauri/src/commands/sidecar.rs
Adds a shutting_down flag checked by the restart watcher to prevent respawn after shutdown begins.
Shortcut sharing and relaunch cleanup
src-tauri/src/commands/system.rs
Avoids breaking shared shortcuts on swap and kills the sidecar before app relaunch.
DB corruption detection
src-tauri/src/db/mod.rs
Classifies specific SQLite errors as definitely corrupt and cleans up sidecar DB files during recovery, with expanded tests.
App lifecycle cleanup
src-tauri/src/lib.rs, src-tauri/capabilities/ai-dialog.json
Destroys the AI dialog window on quit confirmation, makes shortcut registration best-effort, and grants window-close capability.

Error Boundary and Theme Hydration

Layer / File(s) Summary
ErrorBoundary component
src/components/ErrorBoundary.tsx, src/stories/ErrorBoundary.stories.tsx, src/strings.ts
Adds a class-based error boundary with retry UI, Storybook stories, and error copy.
Route wrapping
src/App.tsx
Wraps Routes in the new ErrorBoundary.
Theme hydration gate
src/design/theme.tsx
Defers theme class application until an authoritative mode is available.

Friends Pairing and Networking Hardening

Layer / File(s) Summary
Name sanitization
src/features/friends/contactCard.ts, src/features/friends/pair.ts
Adds normalizeUntrustedName and applies it to pairing-derived display names.
Invite replay guard
src/features/friends/inbox.ts
Dedupes received invite envelopes within the invite TTL before forwarding.
Invite retry session gating
src/features/friends/invite.ts, src/features/friends/inviteRetry.ts, tests/unit/inviteRetry.test.ts
Drops queued retries when the host session is no longer live.
MQTT pairing diagnostics
src/lib/relayDiagnostics.ts, src/lib/trystero/index.ts, src/features/friends/AddFriendDialog.tsx
Adds combined Nostr/MQTT relay reachability check used for pairing UI hints.
Dialog close cleanup
src/features/friends/AddFriendDialog.tsx
Resets pairing phase and cancels in-flight requests/timers on dialog close.
Active-session mounting
src/routes/Home.tsx
Keeps deep-link and contact-import UI mounted during an active session.

Per-Signer Report Scoping and Media Recovery

Layer / File(s) Summary
Report data filtering
src/features/session/reportData.ts, tests/unit/report-data.test.ts
Adds optional filterWho to distraction/timeline derivations.
Report wiring
src/features/session/reportSerialize.ts, src/features/session/Report.tsx
Passes local identity into report derivation calls.
Stats insights filtering
src/features/stats/statsInsights.ts, src/features/stats/Dashboard.tsx, tests/unit/stats-insights.test.ts
Adds filterWho scoping to insight computations and wires local identity in Dashboard.
Media/sidecar recovery
src/features/session/SessionView.tsx, src/features/ai/sampleLoop.ts, src/features/ai/sidecar.ts
Handles per-display screen-track ended events without latching denial prematurely, and guards sidecar start against interleaved stop.

Encoding and Export Hardening

Layer / File(s) Summary
Hex decoding validation
src/lib/encoding.ts, tests/unit/identity.test.ts
Rejects malformed hex strings before decoding.
CSV formula injection guard
src/lib/fileExport.ts, tests/unit/file-export.test.ts
Neutralizes formula-trigger characters in string CSV cells while preserving numeric formatting.

Estimated code review effort: 4 (Complex) | ~75 minutes

Possibly related PRs

  • scotej/studyvis#42: Modifies the same AddFriendDialog.tsx pairing phase/state flow that this PR's teardown and diagnostics changes build on.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is clear and accurately names two major changes in the PR: database corruption recovery and the new error boundary.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/production-readiness-yg17gr

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
src/lib/fileExport.ts (1)

92-94: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add \n to the formula-trigger character class.

A string starting with \n (e.g. "\n=cmd") bypasses the prefix guard because \n is not in the trigger regex, yet it is a known whitespace-based obfuscation vector. The cell gets CSV-quoted as "\n=cmd", but if the spreadsheet trims leading whitespace from cell values, the exposed =cmd would execute as a formula. Adding \n to the trigger set closes this gap.

🛡️ Proposed fix
-  if (/^[=+\-@\t\r]/.test(s)) {
+  if (/^[=+\-@\t\r\n]/.test(s)) {
     s = `'${s}`
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/fileExport.ts` around lines 92 - 94, The prefix guard in fileExport’s
string sanitization is missing newline from the formula-trigger check, so values
like a leading “\n=…” can bypass protection. Update the regex in the export
logic that handles the string prefixing step to include “\n” alongside the
existing trigger characters, keeping the behavior consistent with the
surrounding CSV escaping in fileExport.
src/features/friends/inviteRetry.ts (1)

39-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Comment says "and at registration" but register has no isSessionLive check.

The docstring at line 43 states the guard is "Checked at delivery time (and at registration)", but register (lines 90-101) performs no isSessionLive check — only onPresenceOnline does. Consider correcting to "Checked at delivery time" to match the implementation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/friends/inviteRetry.ts` around lines 39 - 46, The docstring in
inviteRetry’s session-live guard is inaccurate because register does not
validate isSessionLive, only the delivery path does. Update the comment near the
isSessionLive field in inviteRetry to reflect the actual behavior by removing
the “and at registration” claim and describing the guard as being checked at
delivery time only, so it matches register and onPresenceOnline.
src/features/session/reportData.ts (1)

77-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider extracting the filterWho normalization into a shared helper.

The pattern const only = filterWho && filterWho.length > 0 ? filterWho.toLowerCase() : null is duplicated four times across reportData.ts (lines 77-78, 166-167) and statsInsights.ts (lines 94-95, 123-124). A small shared utility would keep the normalization logic in one place.

♻️ Optional helper extraction
// e.g. in src/lib/encoding.ts or a new src/lib/filterWho.ts
export function normalizeFilterWho(filterWho?: string | null): string | null {
  return filterWho && filterWho.length > 0 ? filterWho.toLowerCase() : null
}

Then replace each inline normalization:

-  const only =
-    filterWho && filterWho.length > 0 ? filterWho.toLowerCase() : null
+  const only = normalizeFilterWho(filterWho)

Also applies to: 166-167

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/features/session/reportData.ts` around lines 77 - 78, The `filterWho`
normalization logic is duplicated in `reportData.ts` and `statsInsights.ts`, so
extract it into a shared helper and reuse it everywhere. Add a small utility
such as `normalizeFilterWho` in a common location, then replace the inline
`const only = filterWho && filterWho.length > 0 ? filterWho.toLowerCase() :
null` pattern in `reportData.ts` and `statsInsights.ts` with that helper to keep
the behavior identical in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/release-prep.yml:
- Around line 86-107: The release-prep workflow has a TOCTOU gap: the gate job
verifies CI for one main SHA, but the prep job can later checkout a newer main
commit and release it unverified. Update the gate job to expose the verified SHA
as an output from the CI check step, then in the prep job add a verification
step after checkout that compares the checked-out commit against that output and
fails if they differ. Use the existing “Require CI success on this main commit”
step and the prep job checkout step to keep the release pinned to the exact SHA
CI approved.

In `@src-tauri/src/db/mod.rs`:
- Around line 77-83: The sidecar cleanup in the recovery path ignores all delete
errors, which can leave stale SQLite files next to a newly recreated app.db.
Update the loop in the DB recovery logic around the sidecar removal in
src-tauri/src/db/mod.rs so it still skips NotFound, but aborts recovery on any
other fs::remove_file failure (permission, lock, I/O, etc.) by propagating or
returning the error before recreating the database.

In `@src-tauri/src/lib.rs`:
- Around line 575-587: Keep the in-memory shortcut state aligned with what was
actually registered at startup. In the setup loop that calls
manager.register(shortcut), make sure failed registrations are not retained in
ShortcutBindings, so later rebinding through system_set_global_shortcut does not
try to unregister an OS-level shortcut that was never registered. Update the
registration flow around global_shortcut, ShortcutBindings, and
system_set_global_shortcut so the stored binding state only reflects successful
registrations.

In `@src/routes/Home.tsx`:
- Around line 276-288: The pair-words deep-link flow in Home.tsx is still firing
during an active session, but `AddFriendDialog` is only mounted in the main
view, so `handlePairDeepLink` currently sets state that cannot render and later
reappears stale. Update `handlePairDeepLink` to follow the same active-session
guard pattern already used for the leave-session paths (for example, the checks
around the session guards in the Home component), and only call
`setView('main')`, `setDeepLinkWords`, and `setAddOpen(true)` when no session is
active. If a session is active, clear or ignore the pair-words deep-link state
so `AddFriendDialog` does not open unexpectedly after the session ends.

---

Nitpick comments:
In `@src/features/friends/inviteRetry.ts`:
- Around line 39-46: The docstring in inviteRetry’s session-live guard is
inaccurate because register does not validate isSessionLive, only the delivery
path does. Update the comment near the isSessionLive field in inviteRetry to
reflect the actual behavior by removing the “and at registration” claim and
describing the guard as being checked at delivery time only, so it matches
register and onPresenceOnline.

In `@src/features/session/reportData.ts`:
- Around line 77-78: The `filterWho` normalization logic is duplicated in
`reportData.ts` and `statsInsights.ts`, so extract it into a shared helper and
reuse it everywhere. Add a small utility such as `normalizeFilterWho` in a
common location, then replace the inline `const only = filterWho &&
filterWho.length > 0 ? filterWho.toLowerCase() : null` pattern in
`reportData.ts` and `statsInsights.ts` with that helper to keep the behavior
identical in one place.

In `@src/lib/fileExport.ts`:
- Around line 92-94: The prefix guard in fileExport’s string sanitization is
missing newline from the formula-trigger check, so values like a leading “\n=…”
can bypass protection. Update the regex in the export logic that handles the
string prefixing step to include “\n” alongside the existing trigger characters,
keeping the behavior consistent with the surrounding CSV escaping in fileExport.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 75769de1-b340-4839-8fac-89090e606f1f

📥 Commits

Reviewing files that changed from the base of the PR and between c51e8aa and ed1f509.

📒 Files selected for processing (42)
  • .github/workflows/ci.yml
  • .github/workflows/release-prep.yml
  • ARCHITECTURE.md
  • CHANGELOG.md
  • ISSUES.md
  • PLAN.md
  • README.md
  • src-tauri/capabilities/ai-dialog.json
  • src-tauri/src/commands/models.rs
  • src-tauri/src/commands/sidecar.rs
  • src-tauri/src/commands/system.rs
  • src-tauri/src/db/mod.rs
  • src-tauri/src/lib.rs
  • src/App.tsx
  • src/components/ErrorBoundary.tsx
  • src/design/theme.tsx
  • src/features/ai/sampleLoop.ts
  • src/features/ai/sidecar.ts
  • src/features/friends/AddFriendDialog.tsx
  • src/features/friends/contactCard.ts
  • src/features/friends/inbox.ts
  • src/features/friends/invite.ts
  • src/features/friends/inviteRetry.ts
  • src/features/friends/pair.ts
  • src/features/session/Report.tsx
  • src/features/session/SessionView.tsx
  • src/features/session/reportData.ts
  • src/features/session/reportSerialize.ts
  • src/features/stats/Dashboard.tsx
  • src/features/stats/statsInsights.ts
  • src/lib/encoding.ts
  • src/lib/fileExport.ts
  • src/lib/relayDiagnostics.ts
  • src/lib/trystero/index.ts
  • src/routes/Home.tsx
  • src/stories/ErrorBoundary.stories.tsx
  • src/strings.ts
  • tests/unit/file-export.test.ts
  • tests/unit/identity.test.ts
  • tests/unit/inviteRetry.test.ts
  • tests/unit/report-data.test.ts
  • tests/unit/stats-insights.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Frontend
  • GitHub Check: Rust (Windows)
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Single source of truth for design tokens: Every color, spacing, font, radius, shadow, motion, and z-index value comes from src/design/tokens.ts. No raw hex, no arbitrary px, no inline cubic-bezier outside tokens.ts.
Single source of truth for user-facing strings: Toast and notification copy lives in src/strings.ts. JSX text and aria-label literals should follow voice guidelines (see DESIGN-SYSTEM.md §14) and prefer strings.ts.
No comments unless the why is non-obvious: Identifiers carry the meaning; code reads top-to-bottom.

Files:

  • src/lib/relayDiagnostics.ts
  • src/design/theme.tsx
  • src/stories/ErrorBoundary.stories.tsx
  • src/routes/Home.tsx
  • src/lib/encoding.ts
  • src/features/friends/contactCard.ts
  • src/lib/fileExport.ts
  • src/features/friends/pair.ts
  • src/features/session/reportSerialize.ts
  • src/App.tsx
  • src/features/ai/sidecar.ts
  • src/features/session/SessionView.tsx
  • src/features/friends/invite.ts
  • src/features/friends/inbox.ts
  • src/components/ErrorBoundary.tsx
  • src/features/friends/inviteRetry.ts
  • src/features/session/reportData.ts
  • src/features/stats/Dashboard.tsx
  • src/strings.ts
  • src/features/session/Report.tsx
  • src/features/stats/statsInsights.ts
  • src/lib/trystero/index.ts
  • src/features/ai/sampleLoop.ts
  • src/features/friends/AddFriendDialog.tsx
**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Quality gate: Run npm run test (vitest in node-env); no RTL/jsdom component tests — component behavior covered by Storybook + axe-core.

Files:

  • tests/unit/report-data.test.ts
  • tests/unit/identity.test.ts
  • tests/unit/file-export.test.ts
  • tests/unit/inviteRetry.test.ts
  • tests/unit/stats-insights.test.ts
src/components/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/components/**/*.{ts,tsx}: Component layering wall: src/components/ui/ is the only place Radix / shadcn primitives may be imported. src/components/ composes from ui/, src/design/, and shared utils. Reverse imports are an ESLint error.
Accessibility is a gate: WCAG AA on every text/background pairing in both themes; axe-core over every Storybook story; no information by color alone; reduced-motion is a global kill switch.

Files:

  • src/components/ErrorBoundary.tsx
src-tauri/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Quality gate: Run cargo test && cargo fmt --check && cargo clippy in src-tauri/ before committing Rust changes.

Files:

  • src-tauri/src/commands/models.rs
  • src-tauri/src/commands/sidecar.rs
  • src-tauri/src/lib.rs
  • src-tauri/src/db/mod.rs
  • src-tauri/src/commands/system.rs
🪛 LanguageTool
ISSUES.md

[style] ~39-~39: The noun “invitation” is usually used instead of ‘invite’ in formal writing.
Context: ...inviteRetry.ts` | An invite retry queued after the 15s send timeout...

(AN_INVITE)


[uncategorized] ~55-~55: The official name of this software platform is spelled with a capital “H”.
Context: ... | | I43 | Sev3 | .github/workflows/ci.yml ...

(GITHUB)


[uncategorized] ~56-~56: The official name of this software platform is spelled with a capital “H”.
Context: ... | | I44 | Sev3 | .github/workflows/release-prep.yml ...

(GITHUB)

🔇 Additional comments (41)
ARCHITECTURE.md (1)

137-139: LGTM!

Also applies to: 704-706

ISSUES.md (1)

9-10: LGTM!

Also applies to: 11-12, 13-21, 22-31, 32-41, 42-49, 50-55, 56-62

CHANGELOG.md (1)

312-316: LGTM!

PLAN.md (1)

24-24: LGTM!

Also applies to: 85-85

README.md (1)

25-28: LGTM!

Also applies to: 93-102, 141-142, 278-285, 308-311

src/components/ErrorBoundary.tsx (1)

1-79: LGTM!

src/stories/ErrorBoundary.stories.tsx (1)

1-36: LGTM!

src/strings.ts (1)

32-36: LGTM!

Also applies to: 921-921, 994-994

src/App.tsx (1)

4-4: LGTM!

Also applies to: 53-58

src/design/theme.tsx (1)

65-80: LGTM!

src-tauri/src/commands/models.rs (1)

34-47: LGTM!

Also applies to: 160-171

src-tauri/src/commands/sidecar.rs (1)

57-63: LGTM!

Also applies to: 102-102, 501-512

src-tauri/src/commands/system.rs (1)

174-200: LGTM!

Also applies to: 404-409

src-tauri/src/db/mod.rs (1)

120-159: LGTM!

Also applies to: 161-229

src-tauri/src/lib.rs (1)

184-196: LGTM!

src-tauri/capabilities/ai-dialog.json (1)

6-6: LGTM!

src/lib/encoding.ts (1)

14-27: LGTM!

tests/unit/identity.test.ts (1)

289-300: LGTM!

tests/unit/file-export.test.ts (1)

44-57: LGTM!

If you add \n to the trigger regex per the suggestion in fileExport.ts, consider adding a corresponding assertion here, e.g. expect(csvCell('\n=cmd')).toBe("'\n=cmd").

.github/workflows/ci.yml (2)

63-81: LGTM!


93-100: LGTM!

.github/workflows/release-prep.yml (1)

67-77: LGTM!

src/features/friends/contactCard.ts (1)

191-202: LGTM!

src/features/friends/pair.ts (1)

5-5: LGTM!

Also applies to: 177-180

src/features/friends/invite.ts (1)

6-6: LGTM!

Also applies to: 25-31

src/features/friends/inviteRetry.ts (1)

118-124: LGTM!

tests/unit/inviteRetry.test.ts (1)

26-52: LGTM!

src/lib/relayDiagnostics.ts (1)

1-1: LGTM!

Also applies to: 42-59

src/lib/trystero/index.ts (1)

1-4: LGTM!

Also applies to: 57-69

src/features/friends/AddFriendDialog.tsx (1)

5-5: LGTM!

Also applies to: 99-103, 116-134, 179-184

src/features/friends/inbox.ts (1)

10-10: LGTM!

Also applies to: 177-196

src/features/session/reportData.ts (1)

67-104: LGTM!

Also applies to: 154-198

tests/unit/report-data.test.ts (1)

150-171: LGTM!

Also applies to: 237-255

src/features/session/reportSerialize.ts (1)

83-89: LGTM!

src/features/session/Report.tsx (1)

249-255: LGTM!

src/features/stats/statsInsights.ts (1)

84-111: LGTM!

Also applies to: 119-139, 161-175

src/features/stats/Dashboard.tsx (1)

18-18: LGTM!

Also applies to: 96-100, 113-113, 129-129

tests/unit/stats-insights.test.ts (1)

153-176: LGTM!

src/features/session/SessionView.tsx (1)

269-269: LGTM!

Also applies to: 293-315, 340-340

src/features/ai/sampleLoop.ts (1)

455-512: LGTM!

src/features/ai/sidecar.ts (1)

167-175: LGTM!

Comment thread .github/workflows/release-prep.yml
Comment thread src-tauri/src/db/mod.rs
Comment thread src-tauri/src/lib.rs
Comment thread src/routes/Home.tsx
Valid findings from the automated review, all on code this PR introduced:

- release-prep TOCTOU (Major): the gate verified CI for its own `main`
  checkout, but the prep job re-checked-out `main` independently — if main
  advanced between them, prep could bump/tag/release an unverified commit.
  Output the gate-verified SHA and assert the prep checkout matches it.
- Corrupt-DB sidecar cleanup (Major): removal of the -journal/-wal/-shm
  sidecars ignored ALL errors, so a permission/lock/I/O failure would leave a
  stale WAL beside the freshly-recreated app.db (the exact corruption vector
  the cleanup exists to prevent). Skip NotFound, abort recovery on any other
  error.
- Global-shortcut rebind (Major): after a best-effort boot registration
  failed (PR-8), ShortcutBindings still held the unregistered combo, so
  rebinding that action tried to unregister a never-registered OS shortcut and
  wedged. Gate the unregister on manager.is_registered(old).
- Pair-words deep link during a session (Major): the contact-card path was
  fixed (mounted in the active-session tail), but the legacy pair-words path
  set addOpen state that AddFriendDialog — only mounted in the main view —
  couldn't render, then popped open stale after the session ended. Guard it
  with the existing "leave session first" pattern.
- CSV injection (nitpick): add `\n` to the formula-trigger class so a
  leading-newline cell can't re-expose a trigger if the spreadsheet trims it.
- inviteRetry doc accuracy (nitpick): the guard is checked only at delivery
  time, not "at registration" — corrected the comment.

Skipped: the filterWho-normalization helper extraction (CodeRabbit-rated
low-value; a 2-line ternary, and the house rule leans against churn), the
docstring-coverage warning (conflicts with the repo's "no comments unless the
why is non-obvious" rule), and LanguageTool nits (file-path and domain-term
false positives).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rLddBnJ8Qq4RsnaFURAPD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants