release: merge develop into main#238
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Notebooks now sync before notes in syncNow() to ensure note-notebook dependencies are satisfied. Adds pullNotebooks/pushNotebooks methods and applyRemoteNotebookChange for bidirectional notebook sync. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move validateNotebookTree from inline test definition to a shared module so it can be reused by the API route and other consumers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add conflict state to SyncStatusIndicator with amber warning icon and count. Conflicts now take priority over idle state so users discover them without navigating to Settings. Also export ConflictResolver from sync components barrel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
DatabaseConnection.transaction() already calls the inner fn — no need for extra () at call site. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix pullNotebooks() to only advance cursor to last successfully applied change (prevents skipping failed changes on retry) - Fix tree validation snapshot to properly exclude deleted notebooks (prevents ghost parent references in validation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
feat: add bidirectional notebook sync
test: add sync-core unit tests (62 tests)
feat: surface sync conflicts in status indicator
# Conflicts: # apps/desktop/src/main/services/apiClient.ts # apps/desktop/src/main/services/syncService.ts # packages/api/src/db/schema.ts # packages/api/src/routes/sync.ts # packages/storage-sqlite/src/migrations/index.ts
feat: add bidirectional tag sync
Configure automated code review with path-specific instructions for core, storage, desktop, and API packages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ration Add optional metadata (name, version, priority) to registerRemarkPlugin and registerRehypePlugin signatures for debugging and execution ordering. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lucide-react v1.0 removed brand icons. Local SVG replacements. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…231) ## Summary ### New Features - **Local HTTP API** (port 29168): REST API with bearer auth for Alfred/Raycast/curl integration - **Quick Capture** (Cmd+Shift+N): frameless floating window, always-on-top, auto-focus - **Mermaid diagrams**: code block renderer with "Open in Mermaid Live" button - **Math/LaTeX**: styled code block renderer with copy button - **Vim mode**: plugin stub with toggle command (ready for @codemirror/vim) ### Bug Fix - **ASAR crash fix**: `ERR_PACKAGE_PATH_NOT_EXPORTED` — workspace packages now bundled by electron-vite instead of externalized to node_modules ### Verified - Note window already shows only editor (no sidebar) — confirmed correct ## Test plan - [x] `pnpm typecheck` — 17/17 pass - [x] `pnpm build` (desktop) — builds successfully 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Quick Capture window with Cmd/Ctrl+Shift+N for rapid note entry (floating, frameless). * Local HTTP server controls (start/stop/status/get token) accessible from the app. * Math & LaTeX plugin: render math blocks and copy LaTeX. * Mermaid plugin: render Mermaid blocks and copy source. * Vim Mode plugin: toggleable Vim Mode command and status indicator. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes ERR_PACKAGE_PATH_NOT_EXPORTED crash. Workspace packages already bundled by electron-vite. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
) ## Summary - **Allow disabling built-in plugins** — users can now toggle built-in plugins on/off from settings - **Fix MCP server FTS5 crash** — replaced `sql.js` (WASM, no FTS5) with `better-sqlite3` (native, FTS5 included). Write operations (create/update/trash notes) were failing with `no such module: fts5` because the FTS5 triggers in the database fired against a runtime that didn't support FTS5 - **FTS5 runtime check** — MCP server now verifies FTS5 availability at startup and fails with a clear error message instead of a cryptic trigger failure - **WAL mode** — enables safe concurrent DB access between the Electron app and MCP server (the old `sql.js` approach loaded the entire DB into memory, risking data loss on concurrent writes) ## Test plan - [x] `pnpm test` — all 17 suites pass - [x] `pnpm typecheck` — clean across all packages - [x] `pnpm build` — builds successfully - [x] FTS5 regression tests: INSERT/UPDATE/DELETE triggers execute without error - [ ] Manual: run MCP server and verify `readied_create_note` / `readied_update_note` work 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Built-in plugins can now be individually enabled or disabled, with state persistence across reloads. * Improved search functionality with full-text search indexing support. * **Tests** * Added comprehensive test coverage for search index synchronization and database operations. * **Chores** * Upgraded database layer for enhanced performance and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe pull request migrates the MCP server from sql.js to better-sqlite3, implements built-in plugin toggle functionality in the desktop settings UI, and improves error handling and behavior in the preload API and markdown editor. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…er cleanup (#239) ## Summary This commit was pushed to #237 after the squash-merge, so it didn't make it into develop. Cherry-picked here. - **P1: gate built-in plugins until enabled state loads** — `builtInEnabledMap` started as `{}` so `!== false` passed for every plugin, briefly activating disabled plugins on launch. Now starts as `null` and built-in plugins are excluded from `PluginHost` until `listState()` resolves - **P2: fix IPC listener cleanup** — `ipc.on()` cleanup used `removeAllListeners(channel)` which nuked other listeners on the same channel (e.g. `pluginRuntimeStore`). Changed to `removeListener` with the specific handler reference ## Test plan - [x] `pnpm typecheck` — clean - [x] `pnpm test` — all pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
# Conflicts: # apps/desktop/src/preload/api/settings.ts # apps/desktop/src/renderer/pages/settings/sections/plugins/PluginCard.tsx # apps/desktop/src/renderer/pages/settings/sections/plugins/index.tsx # packages/mcp-server/package.json # pnpm-lock.yaml
💡 Codex Reviewhttps://github.com/tomymaritano/readide/blob/272c85fd69c1fea84b52af473df7cf999566b858/apps/desktop/src/preload/api/localServer.ts#L13
https://github.com/tomymaritano/readide/blob/272c85fd69c1fea84b52af473df7cf999566b858/apps/desktop/src/renderer/components/MarkdownEditor.tsx#L585 The auto-link flow finds the just-pasted URL with ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
3 tasks
…#240) ## Summary Fixes two P2 findings from Codex review on PR #238. - **`localServer.getToken()` type mismatch** — IPC handler returns `{ ok, value }` but preload declared `Promise<string>` and passed the raw object through. Any caller would send `Bearer [object Object]`. Now unwraps `value` in preload and throws on error. - **Auto-link paste rewrites wrong URL** — `indexOf()` could match an earlier duplicate URL in the document. Now tracks the exact insert position (`from`/`to`) and verifies the text at that range still matches before replacing with the markdown link. ## Test plan - [x] `pnpm typecheck` — clean - [x] `pnpm test` — all pass - [ ] Manual: paste a URL in the editor, verify auto-link replaces the correct occurrence 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
CHANGELOG.md (1)
1-109: 🧹 Nitpick | 🔵 TrivialConsider consolidating duplicate version sections.
The changelog contains duplicate entries for several versions:
v0.14.0appears at lines 1-11 and 18-22v0.13.0appears at lines 24-34 and 35-44v0.12.0appears at lines 51-62 and 63-73v0.9.1appears at lines 99-104 and 105-109This duplication reduces changelog readability. Consider consolidating each version into a single canonical section.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 1 - 109, The CHANGELOG contains duplicated release sections (e.g., headings "## [0.14.0]", "## [0.13.0]", "## [0.12.0]", "## [0.9.1]"); remove the redundant duplicate blocks and consolidate each version into a single canonical section preserving all unique entries and links (commit hashes, PR/issue references) so information is not lost—locate the repeated headings ("## [0.14.0]", "## [0.13.0]", "## [0.12.0]", "## [0.9.1]") and merge their content into one section per version, deleting the duplicates.apps/desktop/src/renderer/pages/settings/sections/plugins/index.tsx (1)
123-138: 🧹 Nitpick | 🔵 TrivialMinor: shared
handleTogglepollutesbuiltInEnabledwith community plugin IDs.Because this callback fires for both built-in and community toggles, line 129 inserts community plugin IDs into the
builtInEnabledmap (and conversely, line 127 is a no-op for built-in IDs). It's currently harmless since built-in cards only readbuiltInEnabled[plugin.id]by their own id, but it's an invariant leak that makes the map misleading.Consider narrowing the update to the correct bucket:
♻️ Proposed refactor
- const handleToggle = useCallback(async (pluginId: string, enabled: boolean) => { + const builtInIds = useMemo(() => new Set(builtInPlugins.map(p => p.id)), []); + const handleToggle = useCallback(async (pluginId: string, enabled: boolean) => { try { await window.readied.plugins.setEnabled(pluginId, enabled); - // Update community plugins state - setPlugins(prev => prev.map(p => (p.id === pluginId ? { ...p, enabled } : p))); - // Update built-in plugins state - setBuiltInEnabled(prev => ({ ...prev, [pluginId]: enabled })); + if (builtInIds.has(pluginId)) { + setBuiltInEnabled(prev => ({ ...prev, [pluginId]: enabled })); + } else { + setPlugins(prev => prev.map(p => (p.id === pluginId ? { ...p, enabled } : p))); + } // Trigger reload in main window so preview updates immediately window.readied.plugins.requestReload(); toast.success(`Plugin ${enabled ? 'enabled' : 'disabled'}`); } catch (error) { ... } - }, []); + }, [builtInIds]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/pages/settings/sections/plugins/index.tsx` around lines 123 - 138, The shared handleToggle callback updates both community state (setPlugins) and the built-in map (setBuiltInEnabled), causing community plugin IDs to be inserted into builtInEnabled; change handleToggle to detect whether pluginId is a built-in or community plugin before updating each bucket: locate handleToggle, setPlugins, setBuiltInEnabled and builtInEnabled and add a conditional (e.g., check existence in builtInEnabled keys or a provided isBuiltIn predicate) so only built-in IDs update setBuiltInEnabled and only community IDs update setPlugins accordingly, leaving the other state untouched; keep the existing window.readied plugins calls and toast behavior.packages/mcp-server/src/index.ts (1)
286-298:⚠️ Potential issue | 🟡 MinorTop-level
main()invocation runs on any import of this module.Now that
createServeris exported (line 298), anything importing@readied/mcp-server(e.g., tests, an embedding host) will also triggermain()at module load — which callsopenDb()against the user's real SQLite file and attaches aStdioServerTransport. That's a nasty side effect for a library export.Gate the side-effectful entry point on
import.meta.url === pathToFileURL(process.argv[1]).href(or equivalent) somain()only runs when the file is invoked as the CLI binary.🔧 Proposed fix
+import { pathToFileURL } from 'url'; + ... -main().catch(err => { - console.error('MCP server error:', err); - process.exit(1); -}); +if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { + main().catch(err => { + console.error('MCP server error:', err); + process.exit(1); + }); +} export { createServer };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp-server/src/index.ts` around lines 286 - 298, The module currently calls main() at import which triggers openDb() and StdioServerTransport; change it so main() only runs when the module is executed as the CLI by gating the call with an entry-check (compare import.meta.url to pathToFileURL(process.argv[1]).href or equivalent). Specifically, keep export { createServer } as-is but move the main().catch(...) invocation behind a conditional that verifies import.meta.url === pathToFileURL(process.argv[1]).href (import pathToFileURL from 'url' if needed), so imports of the module do not run openDb(), createServer(...).connect(...) or attach a StdioServerTransport.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/App.tsx`:
- Around line 443-469: The built-in plugin state mapping
(window.readied.plugins.listState → Record<string, boolean> →
setBuiltInEnabledMap) is duplicated in the initial effect and the plugins:reload
handler; extract a single async helper e.g. loadBuiltInEnabledMap that calls
window.readied.plugins.listState, builds the map from s.pluginId/s.enabled and
calls setBuiltInEnabledMap, then replace the inline IIFEs in the effect that
calls pluginRuntimeStore.getState().init() and in the handler passed to
window.readied.ipc.on('plugins:reload') to both invoke this helper so the logic
is centralized and stays in sync.
- Around line 443-454: The async IIFE in the first useEffect can call
setBuiltInEnabledMap after NotesApp unmounts; add a cancellation guard (e.g., a
local "mounted" boolean or AbortController) inside the effect and check it
before calling setBuiltInEnabledMap after awaiting
window.readied.plugins.listState(); also apply the same guard to the
"plugins:reload" handler in the second effect by ensuring the handler is removed
or checks the mounted flag before calling setState, and clean up the flag/abort
in the effect's return cleanup so no setState runs after unmount.
In `@packages/mcp-server/src/__tests__/fts5-triggers.test.ts`:
- Around line 151-157: The test uses expect(db).toBeTruthy() which only checks a
handle exists; instead verify FTS5 actually works by executing a simple FTS5
operation on the returned connection: use the openDb(':memory:') result and run
a short FTS5 statement (for example create a virtual FTS5 table, insert a row,
and run a query) via the same connection to assert the module is usable, then
close the DB; reference the openDb helper and the test block for where to add
the FTS5 create/insert/query assertions.
In `@packages/mcp-server/src/db.ts`:
- Around line 58-71: The current assertFts5Available function can leave a
persistent _fts5_check table if interrupted and also discards the original
error; update assertFts5Available to perform an idempotent, non-destructive
check and preserve the original error as the cause—either 1) wrap the
create/drop in a safe transaction and use CREATE VIRTUAL TABLE IF NOT EXISTS
_fts5_check ... then DROP TABLE IF EXISTS _fts5_check (so an interrupted run
cannot leave a fatal leftover), or 2) replace the schema-touching check with a
compile-time check using db.prepare("SELECT
sqlite_compileoption_used('ENABLE_FTS5') AS v").get() and throw if v is falsy;
in either case construct the thrown Error with the original caught error as the
cause (e.g., new Error(..., { cause: err })) so the original exception is
preserved.
- Around line 73-79: openDb currently sets db.pragma('journal_mode = WAL')
unconditionally which is a no-op for in-memory DBs; change the function (openDb)
to detect when resolvedPath equals ':memory:' (or when dbPath indicates an
in-memory DB) and skip calling db.pragma for that case so tests and health
checks don't misinterpret the journal mode; keep the pragma for all other paths
and reference resolvedPath and the db instance when implementing the
conditional.
In `@packages/mcp-server/src/index.ts`:
- Line 14: The docstring and query for readied_search_notes are incorrect:
instead of substring LIKE against notes, change the reader to query the FTS5
virtual table notes_fts using MATCH (join notes ON notes.rowid =
notes_fts.rowid) and filter notes.is_deleted = 0; return snippet(notes_fts, ...)
for hit excerpts and use bm25(notes_fts) as the ranking score, ordering by that
rank (ascending) and keeping the LIMIT parameter. Update the SQL in
readied_search_notes to bind the search term to the MATCH clause and adjust
returned columns to include snippet and rank so callers get FTS5
tokenization/ranking instead of plain LIKE results.
---
Outside diff comments:
In `@apps/desktop/src/renderer/pages/settings/sections/plugins/index.tsx`:
- Around line 123-138: The shared handleToggle callback updates both community
state (setPlugins) and the built-in map (setBuiltInEnabled), causing community
plugin IDs to be inserted into builtInEnabled; change handleToggle to detect
whether pluginId is a built-in or community plugin before updating each bucket:
locate handleToggle, setPlugins, setBuiltInEnabled and builtInEnabled and add a
conditional (e.g., check existence in builtInEnabled keys or a provided
isBuiltIn predicate) so only built-in IDs update setBuiltInEnabled and only
community IDs update setPlugins accordingly, leaving the other state untouched;
keep the existing window.readied plugins calls and toast behavior.
In `@CHANGELOG.md`:
- Around line 1-109: The CHANGELOG contains duplicated release sections (e.g.,
headings "## [0.14.0]", "## [0.13.0]", "## [0.12.0]", "## [0.9.1]"); remove the
redundant duplicate blocks and consolidate each version into a single canonical
section preserving all unique entries and links (commit hashes, PR/issue
references) so information is not lost—locate the repeated headings ("##
[0.14.0]", "## [0.13.0]", "## [0.12.0]", "## [0.9.1]") and merge their content
into one section per version, deleting the duplicates.
In `@packages/mcp-server/src/index.ts`:
- Around line 286-298: The module currently calls main() at import which
triggers openDb() and StdioServerTransport; change it so main() only runs when
the module is executed as the CLI by gating the call with an entry-check
(compare import.meta.url to pathToFileURL(process.argv[1]).href or equivalent).
Specifically, keep export { createServer } as-is but move the main().catch(...)
invocation behind a conditional that verifies import.meta.url ===
pathToFileURL(process.argv[1]).href (import pathToFileURL from 'url' if needed),
so imports of the module do not run openDb(), createServer(...).connect(...) or
attach a StdioServerTransport.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8da60b6d-2968-4797-a646-fedf36d578f0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
CHANGELOG.mdapps/desktop/src/preload/api/localServer.tsapps/desktop/src/preload/api/settings.tsapps/desktop/src/renderer/App.tsxapps/desktop/src/renderer/components/MarkdownEditor.tsxapps/desktop/src/renderer/pages/settings/sections/plugins/PluginCard.tsxapps/desktop/src/renderer/pages/settings/sections/plugins/index.tsxpackages/mcp-server/package.jsonpackages/mcp-server/src/__tests__/fts5-triggers.test.tspackages/mcp-server/src/db.tspackages/mcp-server/src/index.tspackages/mcp-server/src/sql.js.d.tspackages/mcp-server/tsconfig.json
💤 Files with no reviewable changes (2)
- apps/desktop/src/renderer/pages/settings/sections/plugins/PluginCard.tsx
- packages/mcp-server/src/sql.js.d.ts
Comment on lines
443
to
469
| useEffect(() => { | ||
| void pluginRuntimeStore.getState().init(); | ||
| // Load built-in plugin enabled states | ||
| void (async () => { | ||
| const stateList = await window.readied.plugins.listState(); | ||
| const map: Record<string, boolean> = {}; | ||
| for (const s of stateList) { | ||
| map[s.pluginId] = s.enabled; | ||
| } | ||
| setBuiltInEnabledMap(map); | ||
| })(); | ||
| }, []); | ||
|
|
||
| // Re-check built-in enabled state when plugins reload | ||
| useEffect(() => { | ||
| const handler = () => { | ||
| void (async () => { | ||
| const stateList = await window.readied.plugins.listState(); | ||
| const map: Record<string, boolean> = {}; | ||
| for (const s of stateList) { | ||
| map[s.pluginId] = s.enabled; | ||
| } | ||
| setBuiltInEnabledMap(map); | ||
| })(); | ||
| }; | ||
| return window.readied.ipc.on('plugins:reload', handler); | ||
| }, []); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Refactor: deduplicate the built-in state loader.
The same listState → Record<string, boolean> mapping is implemented twice (initial load and plugins:reload handler). Extract a small helper to keep them in sync if the shape ever changes.
♻️ Proposed refactor
+ const loadBuiltInEnabled = useCallback(async () => {
+ const stateList = await window.readied.plugins.listState();
+ const map: Record<string, boolean> = {};
+ for (const s of stateList) {
+ map[s.pluginId] = s.enabled;
+ }
+ setBuiltInEnabledMap(map);
+ }, []);
+
useEffect(() => {
void pluginRuntimeStore.getState().init();
- // Load built-in plugin enabled states
- void (async () => {
- const stateList = await window.readied.plugins.listState();
- const map: Record<string, boolean> = {};
- for (const s of stateList) {
- map[s.pluginId] = s.enabled;
- }
- setBuiltInEnabledMap(map);
- })();
- }, []);
+ void loadBuiltInEnabled();
+ }, [loadBuiltInEnabled]);
// Re-check built-in enabled state when plugins reload
useEffect(() => {
- const handler = () => {
- void (async () => {
- const stateList = await window.readied.plugins.listState();
- const map: Record<string, boolean> = {};
- for (const s of stateList) {
- map[s.pluginId] = s.enabled;
- }
- setBuiltInEnabledMap(map);
- })();
- };
- return window.readied.ipc.on('plugins:reload', handler);
- }, []);
+ return window.readied.ipc.on('plugins:reload', () => {
+ void loadBuiltInEnabled();
+ });
+ }, [loadBuiltInEnabled]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| void pluginRuntimeStore.getState().init(); | |
| // Load built-in plugin enabled states | |
| void (async () => { | |
| const stateList = await window.readied.plugins.listState(); | |
| const map: Record<string, boolean> = {}; | |
| for (const s of stateList) { | |
| map[s.pluginId] = s.enabled; | |
| } | |
| setBuiltInEnabledMap(map); | |
| })(); | |
| }, []); | |
| // Re-check built-in enabled state when plugins reload | |
| useEffect(() => { | |
| const handler = () => { | |
| void (async () => { | |
| const stateList = await window.readied.plugins.listState(); | |
| const map: Record<string, boolean> = {}; | |
| for (const s of stateList) { | |
| map[s.pluginId] = s.enabled; | |
| } | |
| setBuiltInEnabledMap(map); | |
| })(); | |
| }; | |
| return window.readied.ipc.on('plugins:reload', handler); | |
| }, []); | |
| const loadBuiltInEnabled = useCallback(async () => { | |
| const stateList = await window.readied.plugins.listState(); | |
| const map: Record<string, boolean> = {}; | |
| for (const s of stateList) { | |
| map[s.pluginId] = s.enabled; | |
| } | |
| setBuiltInEnabledMap(map); | |
| }, []); | |
| useEffect(() => { | |
| void pluginRuntimeStore.getState().init(); | |
| void loadBuiltInEnabled(); | |
| }, [loadBuiltInEnabled]); | |
| // Re-check built-in enabled state when plugins reload | |
| useEffect(() => { | |
| return window.readied.ipc.on('plugins:reload', () => { | |
| void loadBuiltInEnabled(); | |
| }); | |
| }, [loadBuiltInEnabled]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/App.tsx` around lines 443 - 469, The built-in
plugin state mapping (window.readied.plugins.listState → Record<string, boolean>
→ setBuiltInEnabledMap) is duplicated in the initial effect and the
plugins:reload handler; extract a single async helper e.g. loadBuiltInEnabledMap
that calls window.readied.plugins.listState, builds the map from
s.pluginId/s.enabled and calls setBuiltInEnabledMap, then replace the inline
IIFEs in the effect that calls pluginRuntimeStore.getState().init() and in the
handler passed to window.readied.ipc.on('plugins:reload') to both invoke this
helper so the logic is centralized and stays in sync.
Comment on lines
443
to
+454
| useEffect(() => { | ||
| void pluginRuntimeStore.getState().init(); | ||
| // Load built-in plugin enabled states | ||
| void (async () => { | ||
| const stateList = await window.readied.plugins.listState(); | ||
| const map: Record<string, boolean> = {}; | ||
| for (const s of stateList) { | ||
| map[s.pluginId] = s.enabled; | ||
| } | ||
| setBuiltInEnabledMap(map); | ||
| })(); | ||
| }, []); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Guard against setState after unmount.
The async IIFE in the mount effect has no cancellation guard. If NotesApp unmounts before listState() resolves (e.g., during fast test teardown or HMR), setBuiltInEnabledMap will fire on an unmounted component. Low risk in production, but trivial to harden:
🛡️ Proposed fix
useEffect(() => {
void pluginRuntimeStore.getState().init();
- // Load built-in plugin enabled states
- void (async () => {
+ let cancelled = false;
+ void (async () => {
const stateList = await window.readied.plugins.listState();
+ if (cancelled) return;
const map: Record<string, boolean> = {};
for (const s of stateList) {
map[s.pluginId] = s.enabled;
}
setBuiltInEnabledMap(map);
})();
+ return () => {
+ cancelled = true;
+ };
}, []);The same guard is worth applying to the plugins:reload handler in the second effect.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| void pluginRuntimeStore.getState().init(); | |
| // Load built-in plugin enabled states | |
| void (async () => { | |
| const stateList = await window.readied.plugins.listState(); | |
| const map: Record<string, boolean> = {}; | |
| for (const s of stateList) { | |
| map[s.pluginId] = s.enabled; | |
| } | |
| setBuiltInEnabledMap(map); | |
| })(); | |
| }, []); | |
| useEffect(() => { | |
| void pluginRuntimeStore.getState().init(); | |
| let cancelled = false; | |
| void (async () => { | |
| const stateList = await window.readied.plugins.listState(); | |
| if (cancelled) return; | |
| const map: Record<string, boolean> = {}; | |
| for (const s of stateList) { | |
| map[s.pluginId] = s.enabled; | |
| } | |
| setBuiltInEnabledMap(map); | |
| })(); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/App.tsx` around lines 443 - 454, The async IIFE in
the first useEffect can call setBuiltInEnabledMap after NotesApp unmounts; add a
cancellation guard (e.g., a local "mounted" boolean or AbortController) inside
the effect and check it before calling setBuiltInEnabledMap after awaiting
window.readied.plugins.listState(); also apply the same guard to the
"plugins:reload" handler in the second effect by ensuring the handler is removed
or checks the mounted flag before calling setState, and clean up the flag/abort
in the effect's return cleanup so no setState runs after unmount.
Comment on lines
+58
to
+71
| function assertFts5Available(db: Database.Database): void { | ||
| try { | ||
| db.prepare('CREATE VIRTUAL TABLE _fts5_check USING fts5(x)').run(); | ||
| db.prepare('DROP TABLE _fts5_check').run(); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : String(err); | ||
| throw new Error( | ||
| `FTS5 module is not available in this SQLite build.\n` + | ||
| `The Readied database uses FTS5 for full-text search triggers.\n` + | ||
| `Without FTS5, write operations (create/update/delete notes) will fail.\n` + | ||
| `Original error: ${message}` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
assertFts5Available can brick the DB on a crashed run.
CREATE VIRTUAL TABLE _fts5_check USING fts5(x) (line 60) is not wrapped in IF NOT EXISTS and is not in a transaction. If the process is killed between lines 60 and 61 (SIGKILL, OOM, power loss), _fts5_check will persist in the user's real readied.db. On the next startup:
- Line 60 throws
table _fts5_check already exists. - The catch block re-throws as
FTS5 module is not available in this SQLite build.— a false negative that prevents the server from ever starting until the user manually drops the table.
On top of that, the thrown Error drops the original exception (ESLint preserve-caught-error is valid here). Both issues share a fix:
🛡️ Proposed fix — idempotent check + preserved cause
function assertFts5Available(db: Database.Database): void {
try {
- db.prepare('CREATE VIRTUAL TABLE _fts5_check USING fts5(x)').run();
- db.prepare('DROP TABLE _fts5_check').run();
+ db.prepare('CREATE VIRTUAL TABLE IF NOT EXISTS _fts5_check USING fts5(x)').run();
+ db.prepare('DROP TABLE IF EXISTS _fts5_check').run();
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
throw new Error(
`FTS5 module is not available in this SQLite build.\n` +
`The Readied database uses FTS5 for full-text search triggers.\n` +
`Without FTS5, write operations (create/update/delete notes) will fail.\n` +
`Original error: ${message}`,
+ { cause: err }
);
}
}A non-destructive alternative worth considering is:
const [{ v }] = db.prepare("SELECT sqlite_compileoption_used('ENABLE_FTS5') AS v").all() as { v: number }[];
if (!v) throw new Error('FTS5 not compiled in');which avoids touching the user's schema entirely.
🧰 Tools
🪛 ESLint
[error] 64-69: There is no cause attached to the symptom error being thrown.
(preserve-caught-error)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mcp-server/src/db.ts` around lines 58 - 71, The current
assertFts5Available function can leave a persistent _fts5_check table if
interrupted and also discards the original error; update assertFts5Available to
perform an idempotent, non-destructive check and preserve the original error as
the cause—either 1) wrap the create/drop in a safe transaction and use CREATE
VIRTUAL TABLE IF NOT EXISTS _fts5_check ... then DROP TABLE IF EXISTS
_fts5_check (so an interrupted run cannot leave a fatal leftover), or 2) replace
the schema-touching check with a compile-time check using db.prepare("SELECT
sqlite_compileoption_used('ENABLE_FTS5') AS v").get() and throw if v is falsy;
in either case construct the thrown Error with the original caught error as the
cause (e.g., new Error(..., { cause: err })) so the original exception is
preserved.
2 tasks
## Summary Addresses two major CodeRabbit findings on PR #238. ### 1. Non-destructive FTS5 check (was Major) `assertFts5Available` used `CREATE VIRTUAL TABLE` + `DROP TABLE` to probe FTS5. If the process crashed between those two statements, a stale `_fts5_check` table would persist and block all future startups with a false "FTS5 not available" error. **Fix**: Use `sqlite_compileoption_used('ENABLE_FTS5')` — a read-only query that never touches the schema. ### 2. Search now actually uses FTS5 (was Major) The docstring and PR description said search used FTS5, but the query was still `LIKE '%term%'`. The FTS5 triggers were maintaining the `notes_fts` index on every write, but no reader ever queried it. **Fix**: `readied_search_notes` now uses `notes_fts MATCH` with `bm25()` ranking and `snippet()` highlights. ### Also - Skip WAL pragma for `:memory:` databases (no-op but avoids confusion in tests/health checks) ## Test plan - [x] `pnpm --filter @readied/mcp-server build` — clean - [x] `pnpm --filter @readied/mcp-server test` — 5/5 pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Jun 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release PR
Merges
developintomainto trigger a new release via semantic-release.Highlights since last release
MCP server: sql.js → better-sqlite3
no such module: fts5)Plugin system fixes (from code review)
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores