feat: Add Cairn mobile companion app and desktop Device Sync

[ddutchie]

What does this PR do?

Introduces the Cairn mobile companion app (an offline-first Expo iOS/Android client) and the desktop-side Device Sync feature that connects the two via a shared cloud folder using an append-only oplog. This branch adds the v2.4.0 changelog, a mobile-app viability/planning doc, and updates .gitignore to track example env files and ignore mobile/node_modules.

Type of change

• Bug fix
• New feature
• Refactor / code quality
• Docs / changelog
• Tests

Screenshots / recording

Checklist

• npm run type-check:all passes
• npm run lint passes
• npm test passes (runs npm run compile first so electron/bundle-guard.test.ts actually executes — npm run test:bundle to run just that)
• npm run test:e2e passes (run before merging UI changes or cutting a release)
• No hardcoded colours — CSS variables only (var(--accent), var(--text-primary), etc.)
• No text-[Npx] pixel font classes — rem equivalents only (text-[0.714rem], text-xs, etc.)
• New IPC handlers wrapped in handle() and return IpcResult<T>
• New DB migrations appended (not edited) in schema.ts
• New SQL goes in electron/db/queries.ts — single source of truth (imported by both Electron main process and MCP server); never construct a Database instance outside db/client.ts (Electron) or mcp-server.ts (MCP runtime)
• New dependencies or devDependencies added to ROLE_MAP in scripts/generate-licenses.js and licenses.json regenerated
• New MCP tools registered in electron/mcp/tools/index.ts dispatch + electron/lib/tool-schemas.ts Zod schema
• If you added/removed a --external:<pkg> flag in the compile script: updated the appropriate allowlist group (RUNTIME_PROVIDED / OPTIONAL_TRANSITIVE / SUBPROCESS_ONLY / SHIPPED_NATIVE) in electron/bundle-guard.test.ts and (if SHIPPED_NATIVE) shipped the package via electron-builder.yml

Notes for reviewer

This diff currently covers the documentation and repo-hygiene groundwork for the mobile/Device Sync effort:

• .gitignore: now force-tracks .env.example / **/.env.example (template files with no secrets) while still ignoring real .env* files, and ignores /mobile/node_modules.
• changelogs/v2.4.0.md: user-facing release notes describing the mobile companion app, Device Sync, conflict-safe reconciliation (last-writer-wins + preserved "conflicted copy"), and the pluggable OpenAI-compatible mobile AI backend.
• docs/plans/mobile-app-viability.md: a design/planning document covering the sync engine strategy (synced-folder oplog, tombstones, changelog/oplog, HLC/CAS concurrency, and the gaps in the current schema that must be closed).

Key decisions worth confirming:

• Sync transport is a synced-folder oplog (no backend); only append-only oplog files transit the shared folder, never the DB.
• Scope is 2 personal devices / single user, which simplifies the conflict model.
• The viability doc calls out schema gaps (missing tombstones, no changelog/oplog, tags lacking timestamps, advisory-only version) — please review whether those are accurately captured before the corresponding schema.ts migrations land in a follow-up PR.

Since this branch appears to be docs + config only, the code-oriented checklist items above are not yet applicable and will be addressed as the implementation PRs follow.

Summary by CodeRabbit

• New Features
  • Added a mobile companion app (projects, notes, tasks/board, search, chat, sync, and conflict resolution).
  • Introduced offline-first device sync with a configurable shared sync folder and sync status metrics.
  • Added AI-powered chat with provider auto-selection, attachments, and editable chat UI.
• Bug Fixes
  • Prevented deleted items from being resurrected during sync and improved hard-delete safety.
• Documentation
  • Added mobile setup/readme, environment examples, and sync/viability planning docs.
• Tests / Chores
  • Expanded sync/formatting/shared unit tests; updated linting/type-check scripts and sync-related tooling/config.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds a shared sync/notes library, Electron Device Sync support, and a new Expo mobile app with local storage, iCloud-based sync, chat, markdown rendering, and core project/note/task screens. Desktop utilities are partially moved to shared modules, and several desktop entrypoints are updated to consume the new shared APIs.

Changes

Shared sync, notes, and format library

Layer / File(s)	Summary
Shared sync core shared/sync/*, shared/package.json, tsconfig.shared*.json, vitest.config.ts	Adds HLC, sync engine, transport, conflict helpers, sync schema/contracts, shared exports, and shared test/typecheck coverage.
Shared notes, format, and UI utilities shared/format/*, shared/notes/*, shared/ui/constants.ts, shared/shared.test.ts, shared/README.md	Adds portable date formatting, folder trees, markdown preprocessing, AI action prompts, syntax palette, text stripping, and UI constants with tests and docs.

Electron Device Sync integration

Layer / File(s)	Summary
Electron Device Sync integration electron/db/*, electron/file-watcher.ts, electron/sync/*, electron/ipc/handlers.ts, electron/main.ts, electron/preload.ts, src/components/settings/*, src/types/index.ts, tsconfig.electron.json, changelogs/v2.4.0.md	Adds sync schema migrations, file-watcher orphan handling, desktop sync orchestration, IPC/preload wiring, background sync scheduling, and the Device Sync settings UI.
Desktop refactor to shared modules src/components/notes/*, src/lib/*, src/components/chat/chat-panel/MarkdownContent.tsx, src/components/notes/NoteMarkdownPreview.tsx, src/components/notes/note-editor.tsx	Replaces local notes/format/syntax implementations with shared re-exports and hardens markdown rendering with rehypeEscapeUnknownTags.

Cairn mobile app

Layer / File(s)	Summary
Project scaffolding, build tooling, and docs mobile/app.json, mobile/babel.config.js, mobile/eslint.config.js, mobile/metro.config.js, mobile/package.json, mobile/plugins/*, mobile/scripts/*, mobile/tsconfig.json, mobile/.env.example, mobile/.gitignore, mobile/README.md, mobile/eas.json, .github/workflows/release-mobile.yml, scripts/publishupdate.sh, scripts/releasemobile.sh, docs/plans/mobile-app-viability.md, package.json, eslint.config.mjs, .gitignore	Adds Expo config, build tooling, native signing/iCloud plugins, release/update scripts, CI workflow, env/docs, and mobile-specific typecheck/lint setup.
Theme and shared UI primitives mobile/src/theme.ts, mobile/src/components/{GlassBar,PressableScale,Skeleton,ProjectIcon,TagChips,toolbar-icons,TabScreen,TabStack,ConflictBanner,SyncStatusBadge,UpdateBanner}.tsx	Defines theme tokens and reusable UI primitives.
Local SQLite schema, queries, and chat persistence mobile/src/db/*	Adds the mobile schema, migrations, query layer, sync adapter, and local chat storage.
iCloud-based sync controller and transport mobile/src/sync/*	Implements the auto-sync scheduler, iCloud folder resolution, oplog transport, sync round-trip, and sync status hooks.
Local AI chat: providers, agent loop, tools, and settings UI mobile/src/chat/*, mobile/src/components/AiSettingsSheet.tsx, mobile/app/(tabs)/chat/*	Adds provider selection, OpenAI/Rork streaming providers, AI config persistence, attachments, agent/tool execution, settings, and chat UI.
Markdown, code, and picker rendering components mobile/src/components/{MarkdownView,MathView,MermaidView,CodeBlock,WebViewRenderer,NoteEditorToolbar,WikilinkPickerSheet,TagPickerSheet,DueDatePickerSheet}.tsx	Implements offline markdown/math/mermaid rendering and editor pickers.
Root layout, tabs, projects, search, notes, and cards screens mobile/app/_layout.tsx, mobile/app/(tabs)/*, mobile/app/note/*, mobile/app/card/*	Implements app bootstrap and core CRUD screens.
Board, sync status, and conflicts screens mobile/app/project/[id].tsx, mobile/src/components/DraggableBoard.tsx, mobile/app/sync.tsx, mobile/app/conflicts.tsx	Implements the project board, sync detail page, and conflict resolution UI.

Estimated code review effort: 5 (Critical) | ~180 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ElectronMain as electron/main.ts
  participant DesktopSync as electron/sync/desktop-sync.ts
  participant SyncEngine as shared/sync/engine.ts
  participant SharedFolder
  participant MobileSync as mobile/src/sync/sync.ts
  ElectronMain->>DesktopSync: runFullSync()
  DesktopSync->>SyncEngine: drainPending / backfill
  DesktopSync->>SharedFolder: writeOplogFile()
  MobileSync->>SharedFolder: writeOwnOplog()
  DesktopSync->>SharedFolder: readPeerOplogs()
  MobileSync->>SharedFolder: readPeerOplogs()
  DesktopSync->>SyncEngine: applyRemote(entries)
  MobileSync->>SyncEngine: applyRemote(entries)

Loading

Possibly related PRs

• ddutchie/cairn#53: It also changes electron/ipc/handlers.ts orchestration around IPC handler registration, which is directly related to the new sync handler registration wiring.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding the Cairn mobile companion app and desktop Device Sync.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ddutchie/expomobile

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 15

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (14)

electron/db/queries.ts-238-242 (1)

238-242: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the delete-safety doc to point at suppressNextChange().
electron/file-watcher.ts doesn’t define markDeleted(); the guard lives in suppressNextChange() backed by suppressedNoteIds.

🤖 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 `@electron/db/queries.ts` around lines 238 - 242, The delete-safety doc is
pointing to the wrong file-watcher API and should reference the actual guard
used by the watcher. Update the comment near the delete logic in queries.ts to
mention suppressNextChange() and its backing suppressedNoteIds state in
electron/file-watcher.ts, replacing the stale markDeleted() reference so the
documentation matches the current implementation.

docs/plans/mobile-app-viability.md-48-55 (1)

48-55: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the broken table header.

The second table has 4 cells per row, but the header only declares 3 columns, so markdownlint will drop the source references. Add a Source column or move the references out of the table.

🔧 Proposed fix

-| Gap | Consequence if ignored | Required addition |
+| Gap | Consequence if ignored | Required addition | Source |

🤖 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 `@docs/plans/mobile-app-viability.md` around lines 48 - 55, The markdown table
in the mobile-app-viability plan has mismatched columns because the header only
defines three fields while the body rows include a source reference column.
Update the table header to include a fourth column for the references, or move
the source references out of the table, so the entries under the schema.ts,
registry.ts, queries.ts, mcp/tools/notes.ts, notes-files.ts, and notes-io.ts
references remain intact and render correctly.

Source: Linters/SAST tools

docs/plans/mobile-app-viability.md-67-83 (1)

67-83: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Label the architecture diagram fence.

The unlabeled fenced block triggers markdownlint MD040. Use text (or mermaid if you want structured rendering) so the block stays lint-clean.

🔧 Proposed fix

-```
+```text

🤖 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 `@docs/plans/mobile-app-viability.md` around lines 67 - 83, The architecture
diagram fenced block is unlabeled, which triggers markdownlint MD040. Update the
fenced block in the mobile-app-viability diagram section to include a language
tag such as text (or mermaid if you want structured rendering) so the block
remains lint-clean while preserving the diagram content.

Source: Linters/SAST tools

mobile/src/chat/providers/rork.ts-42-42 (1)

42-42: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Preserve the base path in the Rork request URL. new URL("/agent/chat", base) drops any path in EXPO_PUBLIC_TOOLKIT_URL (https://host/api → https://host/agent/chat), which breaks the {base}/agent/chat contract. Use a relative segment here, as in openai.ts.

🤖 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 `@mobile/src/chat/providers/rork.ts` at line 42, The Rork request URL is
dropping the configured base path because `new URL("/agent/chat", base)` resets
to the host root, breaking the `{base}/agent/chat` contract. Update the URL
construction in `rork.ts` around the `expoFetch` call so it preserves any path
already present in `EXPO_PUBLIC_TOOLKIT_URL`, following the same
relative-segment approach used in `openai.ts`.

mobile/app/conflicts.tsx-63-76 (1)

63-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

"Original was deleted" message shown for empty (not deleted) originals.

c.original?.content ?? "(original was deleted)" conflates c.original being null (truly deleted) with c.original.content being null (a live, but empty, note). When the original exists but has no content, this incorrectly tells the user it was deleted.

🐛 Proposed fix

                 <VersionBlock
                   label="This device (original)"
-                  body={c.original?.content ?? "(original was deleted)"}
+                  body={c.original ? (c.original.content ?? "") : "(original was deleted)"}
                   onOpen={c.original ? () => router.push(`/note/${c.original!.id}`) : undefined}
                   styles={styles}
                 />

🤖 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 `@mobile/app/conflicts.tsx` around lines 63 - 76, The original note display in
conflicts.tsx is using c.original?.content ?? "(original was deleted)", which
mixes up a missing original with an existing note that simply has empty content.
Update the VersionBlock body logic in the conflicts screen so it checks
c.original first and only shows the deleted message when c.original is
null/undefined; if the note exists but content is empty, render an empty/blank
body instead. Use the c.original and VersionBlock references to locate the
affected rendering.

mobile/src/components/MathView.tsx-24-33 (1)

24-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Error handler references window.__postError before it's defined.

The KaTeX <script> (lines 24-32) calls window.__postError in its catch, but that function is only defined inside buildAutoHeightScript(), which runs in the next <script> tag (line 33). If katex.render throws synchronously (e.g. bundled KATEX_JS fails to init, or an unexpected non-parse error since throwOnError: false only suppresses LaTeX parse errors), window.__postError is undefined at that point, so the && guard silently swallows the error — onError never fires and the view is left blank instead of falling back.

Swap the script order so the auto-height/error script (which defines __postError) runs before the KaTeX render script.

🐛 Proposed fix

     )}</head><body>
       <div class="cairn-math" id="m"></div>
       <script>${KATEX_JS}</script>
+      <script>${buildAutoHeightScript()}</script>
       <script>
         try {
           katex.render(${safe}, document.getElementById('m'), {
             displayMode: ${display ? "true" : "false"},
             throwOnError: false,
             output: 'html',
           });
         } catch (e) { window.__postError && window.__postError(e && e.message ? e.message : e); }
       </script>
-      <script>${buildAutoHeightScript()}</script>
     </body></html>`;

🤖 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 `@mobile/src/components/MathView.tsx` around lines 24 - 33, The KaTeX render
block in MathView calls window.__postError before it is defined, so synchronous
render failures can be swallowed and onError won’t run. Update MathView so the
script from buildAutoHeightScript() executes before the katex.render script,
ensuring __postError is available when the catch handler runs.

mobile/src/components/NoteEditorToolbar.tsx-181-212 (1)

181-212: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clear custom AI input on dismiss.
onDismiss only blurs the body field, but NoteEditorToolbar stays mounted while editing is true, so showCustom/customPrompt can survive a keyboard close and reappear on the next focus. Reset those states in the dismiss path (or when editing ends).

🤖 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 `@mobile/src/components/NoteEditorToolbar.tsx` around lines 181 - 212, The
custom AI prompt state in NoteEditorToolbar can persist after the keyboard is
dismissed because onDismiss only blurs the main field while showCustom and
customPrompt remain set. Update the dismiss flow in NoteEditorToolbar (and/or
the parent editing-close path) to reset showCustom to false and clear
customPrompt so the custom input does not reappear on the next focus.

mobile/src/components/WikilinkPickerSheet.tsx-51-79 (1)

51-79: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Focus the search input on modal open mobile/src/components/WikilinkPickerSheet.tsx:51-79

autoFocus inside Modal is flaky here; use a ref and call .focus() from onShow so the search field grabs focus every time the sheet opens.

🤖 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 `@mobile/src/components/WikilinkPickerSheet.tsx` around lines 51 - 79, The
search field in WikilinkPickerSheet should be focused reliably when the modal
opens, since relying on autoFocus inside Modal is inconsistent. Add a ref to the
TextInput in WikilinkPickerSheet and use the Modal onShow callback to call
.focus() each time visible becomes true, keeping the existing search input
behavior unchanged.

mobile/src/sync/controller.ts-111-116 (1)

111-116: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

stopAutoSync doesn't prevent an in-flight sync's queued rerun.

If stopAutoSync() is called while a sync is in-flight and pendingRerun is true, the finally block still fires void requestSync("rerun") after teardown, since nothing checks started. Given the doc comment says this teardown path is "mainly for tests / hot-reload cleanliness," a stray sync after "stop" can leak state between test cases.

🔧 Proposed fix

   } finally {
     inFlight = false;
-    if (pendingRerun) {
+    if (pendingRerun && started) {
       pendingRerun = false;
       void requestSync("rerun");
     }
   }

Also applies to: 156-166

🤖 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 `@mobile/src/sync/controller.ts` around lines 111 - 116, The in-flight sync
cleanup in stopAutoSync/requestSync still schedules a queued rerun after
teardown because the finally block unconditionally calls requestSync("rerun")
when pendingRerun is set. Update the rerun path in controller.ts to check that
syncing is still started before clearing pendingRerun and invoking requestSync,
so stopAutoSync can fully suppress any queued rerun. Use the existing started
flag and the requestSync / stopAutoSync flow to keep the teardown path from
triggering work after shutdown.

shared/notes/text.ts-11-17 (1)

11-17: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

stripMarkdown can merge words together when markdown syntax abuts text.

Matched characters are replaced with an empty string before whitespace collapsing. This works when markdown tokens are already surrounded by spaces (as in the test fixtures), but for constructs without surrounding whitespace — e.g. a link [text](url) — removing [, ], (, ) directly concatenates text and url into a single run with no separating space, degrading search-index/preview quality.

Replacing matched characters with a space instead of an empty string fixes this while preserving all existing test expectations (traced by hand against shared/shared.test.ts's stripMarkdown cases).

🐛 Suggested fix

 export function stripMarkdown(md: string): string {
   if (!md) return "";
   return md
-    .replace(/[#*_`>[\]()!-]/g, "")
+    .replace(/[#*_`>[\]()!-]/g, " ")
     .replace(/\s+/g, " ")
     .trim();
 }

🤖 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 `@shared/notes/text.ts` around lines 11 - 17, stripMarkdown currently removes
markdown punctuation in a way that can concatenate adjacent words, especially
for patterns like links. Update stripMarkdown in text.ts to replace matched
markdown characters with a space instead of deleting them, while keeping the
existing whitespace-collapsing and trim behavior. This should preserve the
current stripMarkdown expectations in shared.test.ts and prevent text runs from
merging when syntax abuts content.

shared/format/date.ts-25-42 (1)

25-42: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use UTC calendar-day math here. Dividing the local-midnight delta by 86400000 is DST-sensitive; on a 23-hour day it can return 0 for the next calendar day. Compute the difference from UTC midnights or a calendar-day helper instead.

🤖 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 `@shared/format/date.ts` around lines 25 - 42, formatDateCompact currently
computes day differences using local-midnight timestamps divided by 86400000,
which can be DST-sensitive. Update the logic in formatDateCompact to compare UTC
calendar days instead, either by normalizing both dates to UTC midnight before
subtracting or by using a calendar-day helper, while keeping the existing
Today/Yesterday/d ago formatting behavior.

shared/format/date.ts-65-76 (1)

65-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle yyyy-MM-dd due dates without new Date(...). dueDate is stored as a date key, so parsing it as a timestamp can shift the day in negative UTC offsets and misclassify overdue/today/upcoming. Compare the date string directly here, or convert via UTC date parts; formatDate(dueDate) has the same issue where this value is displayed.

🤖 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 `@shared/format/date.ts` around lines 65 - 76, The date comparison in
getDueDateStatus is parsing a yyyy-MM-dd key with new Date, which can shift the
day by timezone and misclassify the status. Update getDueDateStatus to compare
the date string safely without relying on local timestamp parsing, or convert
the input using UTC date parts before comparing to today; also review formatDate
for the same parsing pattern and make it use the same safe date handling.

shared/sync/hlc.ts-36-43 (1)

36-43: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

decodeHlc has no validation against malformed stamps.

If a stamp string is malformed (e.g. a partially-corrupted line surviving JSON parsing in a peer's .ndjson oplog file, or any future encoding drift), parseInt(p, 16)/parseInt(c, 16) silently return NaN. compareHlc would then propagate NaN (V8 treats NaN comparator results as equal), silently breaking the total order that the whole reconcile/compaction pipeline depends on for correctness — a bad outcome for a system whose stated goal is "zero data loss." readPeerOplogs (shared/sync/transport.ts) already guards against corrupt lines, but nothing guards against corrupt fields within an otherwise-valid JSON line.

Consider validating in decodeHlc and failing loudly (letting the caller decide whether to skip or abort that entry) rather than degrading silently.

🛡️ Proposed validation

 export function decodeHlc(stamp: string): HlcParts {
   const [p, c, ...rest] = stamp.split(":");
+  const physical = parseInt(p, 16);
+  const counter = parseInt(c, 16);
+  if (!p || !c || Number.isNaN(physical) || Number.isNaN(counter)) {
+    throw new Error(`Invalid HLC stamp: ${stamp}`);
+  }
   return {
-    physical: parseInt(p, 16),
-    counter: parseInt(c, 16),
+    physical,
+    counter,
     deviceId: rest.join(":"),
   };
 }

🤖 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 `@shared/sync/hlc.ts` around lines 36 - 43, decodeHlc currently accepts
malformed stamp fields and can return NaN, which breaks Hlc ordering silently.
Add explicit validation inside decodeHlc to ensure the stamp splits into valid
hex physical/counter parts before parsing, and throw a clear error on malformed
input instead of returning invalid HlcParts. Keep the fix localized to decodeHlc
and ensure compareHlc only ever receives validated values.

shared/sync/engine.ts-582-593 (1)

582-593: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Let SQLite apply defaults in tombstone shells (shared/sync/engine.ts:582-593)

PRAGMA table_info(...).dflt_value is raw SQL text, so DEFAULT 'active' gets inserted as the literal string "'active'". Skip columns that already have a real default and only synthesize placeholders for NOT NULL columns without one; otherwise the tombstone row stores corrupted values.

🤖 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 `@shared/sync/engine.ts` around lines 582 - 593, The tombstone placeholder
logic in insertTombstoneShell is currently copying db_cols_notnull defaults into
the row as raw SQL text, which can corrupt values when the default is a quoted
literal. Update insertTombstoneShell to let SQLite apply real defaults by
skipping NOT NULL columns that already have a dflt_value from db_cols_notnull,
and only synthesizing placeholder values for NOT NULL columns with no default
before calling writeRow.

🧹 Nitpick comments (21)

electron/main.ts (2)

428-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

require() + manually-typed consts bypass compile-time checking of the sync module contract.

desktopSync, writeNoteFile/deleteNoteFile, suppressNextChange, getProjectName, and getNoteById are all pulled in via require() with hand-written type annotations rather than static import. This likely stems from electron/sync/**/*.ts being excluded from tsconfig.electron.json (see that file's review), but for the other requires (./notes-files, ./file-watcher, ./ipc/result-helpers, ./db/queries) which are NOT excluded, static imports would restore real type-checking against the actual exported signatures instead of relying on manually maintained type strings that can silently drift out of sync.

♻️ Suggested direction for the non-excluded requires

-  // eslint-disable-next-line `@typescript-eslint/no-require-imports`
-  const { writeNoteFile, deleteNoteFile } = require("./notes-files");
-  // eslint-disable-next-line `@typescript-eslint/no-require-imports`
-  const { suppressNextChange } = require("./file-watcher");
-  // eslint-disable-next-line `@typescript-eslint/no-require-imports`
-  const { getProjectName } = require("./ipc/result-helpers");
-  // eslint-disable-next-line `@typescript-eslint/no-require-imports`
-  const { getNoteById } = require("./db/queries");
+  import { writeNoteFile, deleteNoteFile } from "./notes-files";
+  import { suppressNextChange } from "./file-watcher";
+  import { getProjectName } from "./ipc/result-helpers";
+  import { getNoteById } from "./db/queries";

(Move these to top-level imports if there's no circular-dependency reason for the lazy require pattern.)

Also applies to: 438-445

🤖 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 `@electron/main.ts` around lines 428 - 433, Replace the manually typed
require-based bindings in main.ts with static imports for the non-excluded
modules so TypeScript checks the वास्तविक exported contracts instead of relying
on hand-written signatures. Update the bindings around desktopSync,
writeNoteFile/deleteNoteFile, suppressNextChange, getProjectName, and
getNoteById to use the actual module exports from sync/desktop-sync,
notes-files, file-watcher, ipc/result-helpers, and db/queries where possible,
keeping lazy require only if there is a proven circular-dependency need. Ensure
the existing identifiers still map cleanly to the imported symbols after the
refactor.

---

481-503: 📐 Maintainability & Code Quality | 🔵 Trivial

Consider extracting the background-sync wiring into a helper.

Timers, powerMonitor, focus, and before-quit wiring are all inlined in this one block, flagged as high complexity in the line-range summary. Extracting into a small wireBackgroundSync(ctx, win, desktopSync) helper (in electron/sync/ or a new module) would isolate lifecycle bookkeeping from the rest of window setup and make the listener-cleanup story easier to audit.

🤖 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 `@electron/main.ts` around lines 481 - 503, The background-sync setup is too
complex in the current window initialization block because timers and lifecycle
listeners are all wired inline. Move the drain interval, full sync interval, and
the `powerMonitor`/`focus`/`before-quit` subscriptions out of the main flow into
a helper such as `wireBackgroundSync(ctx, win, desktopSync)` so the lifecycle
bookkeeping is isolated. Keep the helper responsible for registering and
cleaning up the listeners/intervals, and call it from the existing setup code in
`electron/main.ts`.

electron/db/schema.ts (1)

605-608: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Deduplicate the SYNCABLE list to prevent v25/v26 divergence.

The same table list is declared independently in v25 (lines 605-608) and v26 (lines 660-663). If a future syncable table is added to the column/tables migration but not the trigger migration (or vice-versa), rows would gain hlc/deleted_at without capture triggers (or triggers referencing columns that don't exist), silently breaking sync for that entity. Hoist it to a single module-level constant (or import the shared SYNCABLE_TABLES) and reference it from both.

Note: the OpenGrep command-injection.exec-js hints on lines 619-620/665-684 are false positives — these are db.exec SQL statements interpolating a hardcoded constant list, not shell commands.

🤖 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 `@electron/db/schema.ts` around lines 605 - 608, The SYNCABLE table list is
duplicated between the v25 and v26 schema logic, which can cause them to drift
and break sync for new tables. Hoist the shared table names into a single
module-level constant (or reuse the existing shared SYNCABLE_TABLES) and
reference it from both the migration/trigger paths in schema.ts, especially the
sections that define the SYNCABLE array for the column/table migration and
trigger migration. Keep the SQL exec calls unchanged; only centralize the list
so both versions stay in sync.

Source: Linters/SAST tools

mobile/app/(tabs)/chat/index.tsx (1)

89-100: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Minor: image context dropped on history restore. When rebuilding conversation.current, user turns are re-added via userMessage(h.content) without their stored images, so the agent loses image context across relaunches even though the UI bubbles keep them. If restoring image context matters, pass the attachments back through. Otherwise this is an acceptable tradeoff to keep the payload small.

🤖 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 `@mobile/app/`(tabs)/chat/index.tsx around lines 89 - 100, The chat history
restore logic in useEffect is dropping image context when rebuilding
conversation.current. Update the history replay in chat/index.tsx so user turns
restored from loadChatHistory() pass h.images back into userMessage along with
h.content, matching the data already preserved in setMessages. Keep the
assistantMessage path unchanged and ensure the restored conversation state stays
consistent with the UI bubbles.

mobile/src/components/AiSettingsSheet.tsx (1)

103-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Stale comment. The comment claims the key is only rewritten "if the field changed", but setOpenAIApiKey(apiKey) is called unconditionally. Either drop the comment or implement the change-detection it describes (e.g., skip write when apiKey equals the loaded value).

🤖 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 `@mobile/src/components/AiSettingsSheet.tsx` around lines 103 - 105, The
comment around setOpenAIApiKey(apiKey) in AiSettingsSheet is stale because the
write happens unconditionally. Update the logic in the save handler to either
actually compare apiKey against the loaded value before calling setOpenAIApiKey,
or remove the comment if unconditional writes are intended. Use the existing
loaded-value state in AiSettingsSheet to keep the behavior and comment
consistent.

mobile/src/components/TagPickerSheet.tsx (1)

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

Prefer the withAlpha helper over a hardcoded rgba() backdrop.

The file already imports withAlpha and uses it elsewhere; the modal backdrop is the only spot still hardcoding an rgba string.

♻️ Proposed fix

-    backdrop: { flex: 1, backgroundColor: "rgba(0,0,0,0.4)" },
+    backdrop: { flex: 1, backgroundColor: withAlpha("`#000000`", 0.4) },

As per coding guidelines, **/*.{css,scss,ts,tsx}: "Use color-mix(in srgb, var(--token) X%, transparent) for alpha variants and never hardcode rgba()" — applying the intent via the project's own withAlpha alpha-mixing helper since RN has no CSS variables/color-mix.

🤖 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 `@mobile/src/components/TagPickerSheet.tsx` at line 95, The modal backdrop in
TagPickerSheet still hardcodes an rgba string instead of using the existing
withAlpha helper. Update the backdrop style in the TagPickerSheet component to
build the semi-transparent color through withAlpha, matching the other color
usage in this file and keeping the backdrop alpha consistent with the project’s
color handling conventions.

Source: Coding guidelines

mobile/src/components/DueDatePickerSheet.tsx (1)

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

Same hardcoded rgba() backdrop as TagPickerSheet.tsx.

♻️ Proposed fix

-import { useTheme, elevation, type Theme } from "`@/theme`";
+import { useTheme, withAlpha, elevation, type Theme } from "`@/theme`";
@@
-    backdrop: { flex: 1, backgroundColor: "rgba(0,0,0,0.4)" },
+    backdrop: { flex: 1, backgroundColor: withAlpha("`#000000`", 0.4) },

As per coding guidelines, **/*.{css,scss,ts,tsx}: "never hardcode rgba()" — using the project's withAlpha helper keeps this consistent with the rest of the codebase.

🤖 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 `@mobile/src/components/DueDatePickerSheet.tsx` at line 75, The backdrop style
in DueDatePickerSheet uses a hardcoded rgba() value, which should be replaced to
match the codebase guideline. Update the backdrop color to use the shared
withAlpha helper instead of the literal rgba string, and keep the change within
the backdrop style object in DueDatePickerSheet so it stays consistent with
TagPickerSheet and other sheet components.

Source: Coding guidelines

mobile/src/components/WebViewRenderer.tsx (1)

48-59: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Navigation is not actually blocked despite the "hardened against navigation" comment.

originWhitelist={["*"]} is required for static source={{ html }} content, but on its own it does not prevent in-webview navigation — it just allows loading local HTML. There is no onShouldStartLoadWithRequest handler here, so any link/navigation triggered from within the rendered content (now or in future consumers of this shared renderer) would not actually be intercepted, contradicting the "hardened against navigation" comment.

Since content today is generated from JSON.stringify-escaped values (math/mermaid source), risk is currently low, but the comment overstates the real protection. Consider adding a handler that rejects any navigation away from the initial about:blank/inline document.

🛡️ Suggested hardening

       <WebView
         ref={ref}
         originWhitelist={["*"]}
         source={{ html }}
         style={[styles.web, { opacity: loading ? 0 : 1 }]}
         scrollEnabled={false}
         showsVerticalScrollIndicator={false}
         showsHorizontalScrollIndicator={false}
         onMessage={onMessage}
+        onShouldStartLoadWithRequest={(req) => req.url === "about:blank"}
         // Static, bundled content only — hardened against navigation.
         javaScriptEnabled
         setSupportMultipleWindows={false}
       />

🤖 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 `@mobile/src/components/WebViewRenderer.tsx` around lines 48 - 59, The WebView
in WebViewRenderer is not actually hardened against navigation because
originWhitelist only permits the inline HTML source and does not block
in-webview navigations. Add an onShouldStartLoadWithRequest handler to the
WebView that rejects any request other than the initial inline/about:blank load,
and keep the existing static html rendering and onMessage behavior unchanged.
Update the surrounding comment so it accurately reflects the protection provided
by WebViewRenderer.

mobile/src/components/NoteEditorToolbar.tsx (2)

179-181: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

styles is rebuilt on every render instead of memoized.

ScrollAffordanceRow in this same file memoizes makeStyles(t) via useMemo (line 95), and WebViewRenderer/WikilinkPickerSheet do the same. Here makeStyles(t) runs unmemoized on every render of NoteEditorToolbar, including every keystroke in the custom-prompt TextInput (customPrompt state changes trigger a re-render of the whole component), needlessly recreating the StyleSheet object each time.

♻️ Proposed fix

-  const styles = makeStyles(t);
+  const styles = useMemo(() => makeStyles(t), [t]);

🤖 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 `@mobile/src/components/NoteEditorToolbar.tsx` around lines 179 - 181,
`NoteEditorToolbar` is recreating `makeStyles(t)` on every render, including
keystrokes from `customPrompt`, instead of memoizing the style object. Update
the `styles` initialization in `NoteEditorToolbar` to use the same `useMemo`
pattern already used by `ScrollAffordanceRow`, `WebViewRenderer`, and
`WikilinkPickerSheet`, so `makeStyles(t)` only runs when the theme changes.

---

231-243: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Icon-only buttons have no accessibility labels.

The AI action chips and formatting buttons render only an Icon (and label text for AI chips is present, but the formatting-row Pressables at lines 280-291 are icon-only with no accessibilityLabel/accessibilityRole). Screen-reader users can't tell what each formatting button (bold, italic, h1, wikilink, etc.) does.

♿ Proposed fix (formatting buttons)

                 <Pressable
                   key={id}
                   disabled={disabled}
                   onPress={() => onFormat(id)}
                   style={[styles.fmtBtn, disabled && styles.disabled]}
                   hitSlop={4}
+                  accessibilityRole="button"
+                  accessibilityLabel={id}
                 >

Also applies to: 280-291

🤖 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 `@mobile/src/components/NoteEditorToolbar.tsx` around lines 231 - 243, The
formatting-row Pressable controls in NoteEditorToolbar are icon-only and lack
screen-reader labels/roles, so they need explicit accessibility metadata. Update
the Pressable buttons in the formatting button section (and verify the AI chip
buttons in the same component) to provide an accessibilityRole and a descriptive
accessibilityLabel derived from each action name, so users can identify bold,
italic, h1, wikilink, and similar actions without relying on the icon. Make sure
the labels are tied to the existing button identifiers used in
NoteEditorToolbar/handleAI so each control remains discoverable even if the icon
changes.

mobile/src/db/schema.ts (1)

19-22: 📐 Maintainability & Code Quality | 🔵 Trivial

No forward migration path for schema changes.

CREATE TABLE IF NOT EXISTS only creates tables that don't yet exist; it won't add new columns to already-installed devices when the mobile schema evolves later (desktop already needed explicit v25/v26 migrations for its own sync columns). Once this ships, a similar versioned-migration mechanism (e.g., a schema_version row in sync_state + ALTER TABLE steps) will be needed for the mobile DB too.

🤖 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 `@mobile/src/db/schema.ts` around lines 19 - 22, The mobile schema setup in
MOBILE_SCHEMA_SQL currently only relies on CREATE TABLE IF NOT EXISTS, so
existing installs will never pick up added columns when the schema evolves. Add
a versioned migration path around the schema initialization in schema.ts, using
a persisted version marker such as a schema_version entry in sync_state and
applying explicit ALTER TABLE migrations for future changes. Make the migration
logic part of the mobile DB initialization flow so existing devices can be
upgraded safely as the schema changes over time.

mobile/src/db/chat-store.ts (1)

38-62: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider capping chat history growth and moving image data off SQLite rows.

images are stored as full data URIs inline in the chat_local table, and loadChatHistory loads the entire unbounded history on every mount. Over long-term use this can bloat the local DB and slow reads. Consider storing image files on the filesystem (keeping only a path/reference in the row) and/or paginating loadChatHistory.

🤖 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 `@mobile/src/db/chat-store.ts` around lines 38 - 62, The chat history loading
and persistence in loadChatHistory and saveChatMessage currently keep full image
data URIs inline and read the entire unbounded table each time, which will bloat
the local DB and slow startup. Update StoredMessage handling so images are
stored as filesystem references or paths instead of raw data, and change
loadChatHistory to fetch only a bounded/paginated set of recent messages rather
than all rows at once.

shared/README.md (1)

24-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

"Contents" section is stale — missing format/, notes/, and ui/.

This cohort adds format/date.ts, several notes/* modules, and ui/constants.ts, but the README's Contents section only documents sync/. Worth updating so new consumers can discover the full shared surface.

🤖 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 `@shared/README.md` around lines 24 - 36, The README Contents section is out of
date because it only documents the sync area and omits the newly added format/,
notes/, and ui/ modules. Update the Contents list in shared/README.md to include
the new shared surface, referencing the existing section structure under the
contents header and the module names such as format/date.ts, the notes/*
modules, and ui/constants.ts so readers can discover them.

tsconfig.shared.json (1)

13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Node types are applied across both the portable shared/** library and Node-only Electron paths.

This single tsconfig type-checks shared/**/*.ts (documented as platform-neutral, consumed by both Electron and the Expo mobile app) together with electron/db/**/*.ts and electron/sync/**/*.ts (Node-only) under "types": ["node"]. This lets Node globals (process, Buffer, etc.) type-check inside shared/** files even though they won't exist at runtime on React Native/mobile.

Consider splitting into a portable tsconfig.shared.json (no "types": ["node"], include: ["shared/**/*.ts"]) and a separate config for the Electron-only sync/db paths that legitimately need Node types.

Also applies to: 15-15

🤖 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 `@tsconfig.shared.json` at line 13, The shared TypeScript config is currently
applying Node types to both portable shared code and Electron-only code, which
leaks Node globals into platform-neutral modules. Split the current setup so the
portable config used by shared code no longer includes "types": ["node"] and
only covers shared/**/*.ts, then create a separate Electron-only config for the
db/sync paths that still needs Node typings. Update the relevant tsconfig files
so shared/** stays platform-neutral while electron/db/** and electron/sync/**
continue to type-check with Node types.

mobile/app/card/new.tsx (2)

20-20: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

listColumns(project) re-queried on every keystroke.

Same pattern as card/[id].tsx: columns is recomputed on every render since title/description/priority are controlled state, causing a redundant synchronous SQLite query per keystroke.

🛠️ Proposed fix

-  const columns: ColumnRow[] = project ? listColumns(project) : [];
+  const columns: ColumnRow[] = useMemo(() => (project ? listColumns(project) : []), [project]);

🤖 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 `@mobile/app/card/new.tsx` at line 20, The columns list is being recomputed on
every render in the new-card screen, which triggers a redundant SQLite query
whenever controlled inputs change. Update the `new.tsx` component to memoize or
otherwise cache the result of `listColumns(project)` so it only recalculates
when `project` changes, following the same approach used in `card/[id].tsx`;
keep the fix localized around the `columns` assignment and any related
render-time derived state.

---

61-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hardcoded "#fff" instead of theme token.

Same as in card/[id].tsx — the column chip below (Line 81) uses t.accentFg for the equivalent text-on-accent case; prefer that token here for consistency.

♻️ Proposed fix

-              <Text style={[styles.chipText, { color: priority === p ? "`#fff`" : t.textSecondary }]}>{p}</Text>
+              <Text style={[styles.chipText, { color: priority === p ? t.accentFg : t.textSecondary }]}>{p}</Text>

🤖 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 `@mobile/app/card/new.tsx` around lines 61 - 67, The priority chip in the card
creation screen is using a hardcoded white text color instead of the theme token
used elsewhere. Update the Text styling inside the Pressable map in new.tsx to
use the same accent foreground token pattern as card/[id].tsx (the equivalent
text-on-accent case) so the chip text follows theme consistency and avoids
hardcoded color values.

mobile/app/card/[id].tsx (1)

108-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hardcoded "#fff" instead of theme token.

The active-priority chip text uses a raw "#fff" while the column chip a few lines below (Line 125) uses t.accentFg for the same "text on accent-colored background" purpose. Prefer the theme token for consistency (dark/light/contrast handling).

♻️ Proposed fix

-              <Text style={[styles.priorityText, { color: priority === p ? "`#fff`" : t.textSecondary }]}>{p}</Text>
+              <Text style={[styles.priorityText, { color: priority === p ? t.accentFg : t.textSecondary }]}>{p}</Text>

🤖 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 `@mobile/app/card/`[id].tsx around lines 108 - 111, The active priority chip
text in the priority selector is using a hardcoded white color instead of the
theme token. Update the Text styling in the priority chip render path to use the
same theme-based foreground token used by the column chip (for example, the
theme’s accent foreground value) so the color stays consistent across themes and
contrast modes.

mobile/app/(tabs)/search/index.tsx (1)

40-48: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Consider debouncing search-as-you-type.

onChange/switchScope run a synchronous SQLite query on every keystroke via onChangeText. With local SQLite and a LIMIT 50 this is likely fine in practice, but a small debounce would avoid redundant queries on fast typing.

Also applies to: 67-74

🤖 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 `@mobile/app/`(tabs)/search/index.tsx around lines 40 - 48, The search handlers
in the search screen are firing a SQLite query on every keystroke, so add a
small debounce around the query trigger in the relevant search flow. Update the
`onChange` and `switchScope` logic in the search component so they only call
`run` after input stabilizes, using the existing `query`, `scope`, and `run`
symbols to locate the search-as-you-type path. Ensure the debounce is applied
consistently where the same pattern appears elsewhere in the search screen so
rapid typing or scope changes do not launch redundant synchronous queries.

shared/sync/engine.ts (1)

192-220: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoidable extra SELECT per row in put, drainPending, and backfill.

All three methods update a row's hlc column via an UPDATE/INSERT, then immediately call readRow(...) again just to build the oplog payload, when the previously-fetched row object (merged with the new hlc) already has everything needed in memory. This doubles the DB round-trips on every drained change and on every backfilled row — drainPending runs on the periodic background sync tick, so this is on a recurring hot path, not just the one-time backfill.

♻️ Example fix for `drainPending` (same pattern applies to `put`/`backfill`)

         } else {
           this.db.prepare(`UPDATE ${entity} SET hlc = ? WHERE id = ?`).run(stamp, entity_id);
-          this.appendOplog({ hlc: stamp, origin: this.deviceId, entity, entity_id, op: "put", payload: readRow(this.db, entity, entity_id) ?? null });
+          this.appendOplog({ hlc: stamp, origin: this.deviceId, entity, entity_id, op: "put", payload: { ...row, hlc: stamp } });
         }

Also applies to: 255-260, 294-301

🤖 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 `@shared/sync/engine.ts` around lines 192 - 220, The oplog payload creation in
the row publish paths is doing an extra read after the row was already fetched
and updated in memory. In the affected methods like drainPending, put, and
backfill in the sync engine, reuse the existing row object (merged with the new
hlc value) when calling appendOplog instead of invoking readRow again. Keep the
existing row variable/shape as the source of truth for the payload so each
UPDATE/INSERT only needs one database round-trip.

shared/sync/transport.ts (2)

19-21: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Unsanitized deviceId embedded in filename.

oplogFileName interpolates deviceId directly into a path segment with no validation. If deviceId ever contains path separators or .., writeOplogFile/readPeerOplogs could write/read outside syncFolder. Given deviceId is presumably generated internally (UUID-like), exploitability is low today, but nothing here enforces that invariant at the boundary where this transport reads/writes files.

🛡️ Proposed guard

 function oplogFileName(deviceId: string): string {
+  if (!/^[A-Za-z0-9_-]+$/.test(deviceId)) {
+    throw new Error(`Invalid deviceId for oplog filename: ${deviceId}`);
+  }
   return `oplog-${deviceId}.ndjson`;
 }

🤖 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 `@shared/sync/transport.ts` around lines 19 - 21, The filename helper
oplogFileName currently trusts deviceId and can build unsafe paths; add
validation or sanitization at the boundary before it is used by writeOplogFile
and readPeerOplogs. Update oplogFileName to reject any deviceId containing path
separators, traversal tokens, or other non-identifier characters, or normalize
it to a safe form, and ensure callers in the transport flow cannot pass an
untrusted raw value.

---

42-51: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

No shape validation on parsed remote oplog entries.

JSON.parse(trimmed) as OplogEntry trusts the cast; a peer running a mismatched schema version, or a partially-written line that happens to parse as valid JSON but with a different shape, would pass through to applyRemote and could cause downstream failures in the sync engine rather than being safely skipped here like the malformed-JSON case already is.

🤖 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 `@shared/sync/transport.ts` around lines 42 - 51, The parsing loop in
transport.ts only catches malformed JSON, but JSON.parse(trimmed) is still
blindly cast to OplogEntry and can let mismatched or partial shapes reach
applyRemote. Add shape validation immediately after parsing in the existing
line-processing logic so only entries that match the expected OplogEntry
structure are pushed to out, and skip anything invalid the same way the current
catch block skips bad JSON.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4f5149ac-e020-4eae-8bfa-7e68c058bb61

📥 Commits

Reviewing files that changed from the base of the PR and between 397d1e9 and ca7692c.

⛔ Files ignored due to path filters (3)

• mobile/assets/icon.png is excluded by !**/*.png
• mobile/assets/splashIcon.png is excluded by !**/*.png
• mobile/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (123)

• .gitignore
• changelogs/v2.4.0.md
• docs/plans/mobile-app-viability.md
• electron/db/queries.test.ts
• electron/db/queries.ts
• electron/db/schema.ts
• electron/file-watcher.ts
• electron/ipc/handlers.ts
• electron/main.ts
• electron/preload.ts
• electron/sync/desktop-sync.ts
• electron/sync/sync-handlers.ts
• eslint.config.mjs
• mobile/.env.example
• mobile/.gitignore
• mobile/README.md
• mobile/app.json
• mobile/app/(tabs)/_layout.tsx
• mobile/app/(tabs)/chat/_layout.tsx
• mobile/app/(tabs)/chat/index.tsx
• mobile/app/(tabs)/index/_layout.tsx
• mobile/app/(tabs)/index/index.tsx
• mobile/app/(tabs)/search/_layout.tsx
• mobile/app/(tabs)/search/index.tsx
• mobile/app/_layout.tsx
• mobile/app/card/[id].tsx
• mobile/app/card/new.tsx
• mobile/app/conflicts.tsx
• mobile/app/note/[id].tsx
• mobile/app/note/new.tsx
• mobile/app/project/[id].tsx
• mobile/app/sync.tsx
• mobile/babel.config.js
• mobile/eslint.config.js
• mobile/metro.config.js
• mobile/package.json
• mobile/plugins/withAppleSigning.js
• mobile/plugins/withICloudContainer.js
• mobile/scripts/gen-webview-assets.js
• mobile/scripts/link-shared.js
• mobile/src/chat/agent.ts
• mobile/src/chat/ai-config.ts
• mobile/src/chat/attachments.ts
• mobile/src/chat/providers/index.ts
• mobile/src/chat/providers/openai.ts
• mobile/src/chat/providers/rork.ts
• mobile/src/chat/providers/types.ts
• mobile/src/chat/tools.ts
• mobile/src/components/AiSettingsSheet.tsx
• mobile/src/components/CodeBlock.tsx
• mobile/src/components/ConflictBanner.tsx
• mobile/src/components/DraggableBoard.tsx
• mobile/src/components/DueDatePickerSheet.tsx
• mobile/src/components/GlassBar.tsx
• mobile/src/components/MarkdownView.tsx
• mobile/src/components/MathView.tsx
• mobile/src/components/MermaidView.tsx
• mobile/src/components/NoteEditorToolbar.tsx
• mobile/src/components/PressableScale.tsx
• mobile/src/components/ProjectIcon.tsx
• mobile/src/components/Skeleton.tsx
• mobile/src/components/SyncStatusBadge.tsx
• mobile/src/components/TabScreen.tsx
• mobile/src/components/TabStack.tsx
• mobile/src/components/TagChips.tsx
• mobile/src/components/TagPickerSheet.tsx
• mobile/src/components/WebViewRenderer.tsx
• mobile/src/components/WikilinkPickerSheet.tsx
• mobile/src/components/toolbar-icons.ts
• mobile/src/db/chat-store.ts
• mobile/src/db/expo-sync-db.ts
• mobile/src/db/index.ts
• mobile/src/db/queries.ts
• mobile/src/db/schema.ts
• mobile/src/sync/controller.ts
• mobile/src/sync/folder.ts
• mobile/src/sync/fs-transport.ts
• mobile/src/sync/sync.ts
• mobile/src/sync/useSyncStatus.ts
• mobile/src/sync/write-signal.ts
• mobile/src/theme.ts
• mobile/src/types.d.ts
• mobile/src/webview-assets/katex-assets.ts
• mobile/src/webview-assets/mermaid-assets.ts
• mobile/tsconfig.json
• package.json
• shared/README.md
• shared/format/date.ts
• shared/format/index.ts
• shared/notes/ai-actions.ts
• shared/notes/folder-tree.ts
• shared/notes/format.ts
• shared/notes/markdown.ts
• shared/notes/syntax-palette.ts
• shared/notes/text.ts
• shared/package.json
• shared/shared.test.ts
• shared/sync/conflict.test.ts
• shared/sync/conflict.ts
• shared/sync/db-adapter.ts
• shared/sync/engine.test.ts
• shared/sync/engine.ts
• shared/sync/hlc.ts
• shared/sync/index.ts
• shared/sync/schema.ts
• shared/sync/transport.ts
• shared/ui/constants.ts
• src/components/chat/chat-panel/MarkdownContent.tsx
• src/components/notes/NoteMarkdownPreview.tsx
• src/components/notes/ai-text-toolbar.tsx
• src/components/notes/note-editor-utils.ts
• src/components/notes/note-editor.tsx
• src/components/notes/notes-view/buildFolderTree.ts
• src/components/settings/SyncSettings.tsx
• src/components/settings/settings-view.tsx
• src/lib/markdown/pipeline.tsx
• src/lib/syntax-palette.ts
• src/lib/utils.ts
• src/types/index.ts
• tsconfig.electron.json
• tsconfig.json
• tsconfig.shared.json
• vitest.config.ts

[coderabbitai]

🧹 Nitpick comments (1)

tsconfig.shared-node.json (1)

15-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant include entries.

electron/db/schema.ts and electron/db/queries.ts are already matched by the electron/db/**/*.ts glob on the next line.

🧹 Proposed cleanup

   "include": [
     "shared/sync/transport.ts",
     "shared/sync/transport.test.ts",
-    "electron/db/schema.ts",
-    "electron/db/queries.ts",
     "electron/db/**/*.ts",
     "electron/sync/**/*.ts"
   ],

🤖 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 `@tsconfig.shared-node.json` around lines 15 - 22, The include list in
tsconfig.shared-node.json has redundant explicit entries because
electron/db/schema.ts and electron/db/queries.ts are already covered by the
electron/db/**/*.ts glob. Update the shared-node tsconfig include array to
remove those duplicate paths and keep the broader glob plus the other unique
entries, so the config stays minimal and maintainable.

🤖 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.

Nitpick comments:
In `@tsconfig.shared-node.json`:
- Around line 15-22: The include list in tsconfig.shared-node.json has redundant
explicit entries because electron/db/schema.ts and electron/db/queries.ts are
already covered by the electron/db/**/*.ts glob. Update the shared-node tsconfig
include array to remove those duplicate paths and keep the broader glob plus the
other unique entries, so the config stays minimal and maintainable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e5e72eb3-9f20-4f03-8e3b-fceae531b645

📥 Commits

Reviewing files that changed from the base of the PR and between ca7692c and f27bf2f.

⛔ Files ignored due to path filters (1)

• mobile/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (55)

• .github/workflows/release-mobile.yml
• docs/plans/mobile-app-viability.md
• electron/db/queries.ts
• electron/db/schema.ts
• electron/main.ts
• electron/sync/desktop-sync.ts
• mobile/README.md
• mobile/app.json
• mobile/app/(tabs)/chat/index.tsx
• mobile/app/(tabs)/search/index.tsx
• mobile/app/_layout.tsx
• mobile/app/card/[id].tsx
• mobile/app/card/new.tsx
• mobile/app/conflicts.tsx
• mobile/app/note/[id].tsx
• mobile/babel.config.js
• mobile/eas.json
• mobile/metro.config.js
• mobile/package.json
• mobile/src/chat/ai-config.ts
• mobile/src/chat/providers/rork.ts
• mobile/src/components/AiSettingsSheet.tsx
• mobile/src/components/DueDatePickerSheet.tsx
• mobile/src/components/MathView.tsx
• mobile/src/components/MermaidView.tsx
• mobile/src/components/NoteEditorToolbar.tsx
• mobile/src/components/TagPickerSheet.tsx
• mobile/src/components/UpdateBanner.tsx
• mobile/src/components/WebViewRenderer.tsx
• mobile/src/components/WikilinkPickerSheet.tsx
• mobile/src/db/chat-store.ts
• mobile/src/db/index.ts
• mobile/src/db/migrations.ts
• mobile/src/db/queries.ts
• mobile/src/sync/controller.ts
• mobile/src/sync/sync.ts
• mobile/src/updates/useAppUpdates.ts
• package.json
• scripts/publishupdate.sh
• scripts/releasemobile.sh
• shared/README.md
• shared/format/date.ts
• shared/notes/folder-tree.ts
• shared/notes/format.ts
• shared/notes/text.ts
• shared/shared.test.ts
• shared/sync/engine.ts
• shared/sync/hlc.test.ts
• shared/sync/hlc.ts
• shared/sync/transport.test.ts
• shared/sync/transport.ts
• src/components/settings/SyncSettings.tsx
• src/lib/markdown/pipeline.tsx
• tsconfig.shared-node.json
• tsconfig.shared.json

💤 Files with no reviewable changes (1)

• mobile/metro.config.js

✅ Files skipped from review due to trivial changes (3)

• shared/sync/hlc.test.ts
• scripts/publishupdate.sh
• electron/db/queries.ts

🚧 Files skipped from review as they are similar to previous changes (36)

• mobile/babel.config.js
• mobile/src/components/DueDatePickerSheet.tsx
• electron/db/schema.ts
• shared/notes/text.ts
• mobile/src/components/WikilinkPickerSheet.tsx
• shared/notes/folder-tree.ts
• mobile/src/db/index.ts
• mobile/src/components/TagPickerSheet.tsx
• mobile/app.json
• mobile/package.json
• src/components/settings/SyncSettings.tsx
• mobile/app/conflicts.tsx
• package.json
• mobile/src/components/MathView.tsx
• electron/main.ts
• mobile/src/sync/sync.ts
• mobile/app/card/new.tsx
• mobile/app/(tabs)/search/index.tsx
• mobile/src/chat/providers/rork.ts
• src/lib/markdown/pipeline.tsx
• mobile/app/card/[id].tsx
• shared/sync/hlc.ts
• electron/sync/desktop-sync.ts
• mobile/app/_layout.tsx
• shared/notes/format.ts
• docs/plans/mobile-app-viability.md
• mobile/app/note/[id].tsx
• mobile/src/components/MermaidView.tsx
• mobile/app/(tabs)/chat/index.tsx
• mobile/src/components/NoteEditorToolbar.tsx
• mobile/src/components/WebViewRenderer.tsx
• shared/shared.test.ts
• mobile/src/sync/controller.ts
• shared/sync/engine.ts
• mobile/src/components/AiSettingsSheet.tsx
• mobile/src/db/queries.ts