-
-
Notifications
You must be signed in to change notification settings - Fork 0
release: merge develop into main #238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9a48ab2
3d49703
b6c0d96
403642e
628b16b
a9fde10
26401e3
db02671
535d53c
4055595
305bcb5
fcd26b9
a88f19d
488359e
f34ab5e
4d2c4ac
fff8cd8
0782ea8
1375fcd
51e8112
2e60230
26f2757
043148c
e8250b8
c913465
26bccb5
31a880e
63610e4
73f9938
cd01862
e1eb092
cb9a82e
aa2bbf6
46ae6de
fbb5448
4a087e7
3902345
47fec79
614b26f
06a147d
f5f8f25
5fe153e
56ba2a5
1f61ed7
bf58f4a
7bda4c1
b44fc75
9e16eec
78a20cd
3d16297
d8f8ca6
589fe17
9f6f229
48195ab
8ea9877
016a524
0cd1214
308c229
d1c3cb7
2cb58bd
fcc9d96
9f137d3
36e1a5b
d104dd1
a6b3558
0e435e2
080cb68
3646cb2
57d4303
8153c78
fc9ab96
99b3f56
f4e904c
1e1a44c
5af6419
71bf473
249a2ad
801a738
17c9ff2
3dabece
38a5170
c099dbd
b8a4e81
e013638
c87de4e
2c753b3
6948a26
ff0e19b
35c346e
35f3804
09cf396
d773955
bc424cb
c8d8833
d8846d9
188c7d8
d86c2ec
d096658
65abbd6
d6783af
df64ff5
f9766d3
452f3bd
3f7b49d
5d112ff
3d41f25
5e3af25
4b08f07
7344281
b429ab3
3ed08b4
cdd2db0
177091d
9f2ba75
cbe39c8
38b8128
592d889
09ac09a
5cd5520
2ef5a9a
a8bf258
0999107
40c5216
d65eb5c
8dc7079
024e46c
193bbce
d5cd8ed
cfe513d
21be0a5
b959a25
00e25bf
04e4e18
21af479
4ccab57
e908280
8a1f5c8
a24a231
64d7010
f686ab6
9d64ea9
a472580
6e05279
b2f65aa
01b51cb
f6ddcad
41c811c
078c8f4
7030149
ad325d8
1680e01
31c5474
7fd0ee1
2bf9c1e
20e101c
e80caf8
e3b2ef8
1e20308
fb0dafc
3b01d4b
3f0bc7f
578ad70
ab4a486
dc5bc73
075995d
0e85b24
472bc4e
4e36788
d2d763f
52a2bbe
68c56b6
2672cdc
4ace564
9bccc62
a1f8422
ef6c615
549339d
19bf2ca
920cad9
321da87
ebb4c17
b0700b4
b5d2733
0dfe6db
6328c4b
e366f10
a1a6679
8deb73a
1ab1400
d58dbf8
4f17e78
807a8e4
f0e2715
49f5662
a63fad9
120ce15
67a0831
d4ce973
bb88da2
550f1c2
272c85f
1836b02
0514398
32f522a
3c7fab8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -438,12 +438,44 @@ function NotesApp() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Plugin runtime: init once, React observes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const discoveredPlugins = useStore(pluginRuntimeStore, s => s.plugins); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const pluginErrors = useStore(pluginRuntimeStore, s => s.errors); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [builtInEnabledMap, setBuiltInEnabledMap] = useState<Record<string, boolean> | null>(null); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, []); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
443
to
469
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Refactor: deduplicate the built-in state loader. The same ♻️ 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const allPlugins = useMemo(() => [...builtInPlugins, ...discoveredPlugins], [discoveredPlugins]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const allPlugins = useMemo(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Don't mount built-in plugins until the enabled state is loaded | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // to avoid activating disabled plugins on the initial render | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const enabledBuiltIn = builtInEnabledMap | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? builtInPlugins.filter(p => builtInEnabledMap[p.id] !== false) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return [...enabledBuiltIn, ...discoveredPlugins]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [discoveredPlugins, builtInEnabledMap]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const configBridge = useMemo( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| () => ({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| import { describe, it, expect, beforeEach, afterEach } from 'vitest'; | ||
| import Database from 'better-sqlite3'; | ||
| import { openDb } from '../db.js'; | ||
|
|
||
| /** | ||
| * Minimal schema that mirrors the real Readied database enough to | ||
| * exercise the FTS5 triggers that caused "no such module: fts5" | ||
| * when the MCP server used sql.js (which lacks FTS5). | ||
| */ | ||
| const SCHEMA = ` | ||
| CREATE TABLE notes ( | ||
| id TEXT PRIMARY KEY, | ||
| content TEXT NOT NULL, | ||
| title TEXT NOT NULL, | ||
| created_at TEXT NOT NULL, | ||
| updated_at TEXT NOT NULL, | ||
| word_count INTEGER NOT NULL DEFAULT 0, | ||
| notebook_id TEXT DEFAULT 'inbox', | ||
| is_pinned INTEGER DEFAULT 0, | ||
| is_deleted INTEGER DEFAULT 0, | ||
| status TEXT DEFAULT 'active', | ||
| needs_sync INTEGER DEFAULT 0, | ||
| local_version INTEGER DEFAULT 1, | ||
| sync_version INTEGER DEFAULT 0 | ||
| ); | ||
|
|
||
| CREATE TABLE notebooks ( | ||
| id TEXT PRIMARY KEY, | ||
| name TEXT NOT NULL, | ||
| parent_id TEXT, | ||
| depth INTEGER NOT NULL DEFAULT 0, | ||
| "order" INTEGER NOT NULL DEFAULT 0, | ||
| created_at TEXT NOT NULL, | ||
| updated_at TEXT NOT NULL | ||
| ); | ||
|
|
||
| INSERT INTO notebooks (id, name, created_at, updated_at) | ||
| VALUES ('inbox', 'Inbox', datetime('now'), datetime('now')); | ||
|
|
||
| -- FTS5 virtual table (migration 008) | ||
| CREATE VIRTUAL TABLE notes_fts USING fts5( | ||
| id UNINDEXED, | ||
| title, | ||
| content, | ||
| tokenize='porter unicode61' | ||
| ); | ||
|
|
||
| -- Trigger: sync FTS on INSERT | ||
| CREATE TRIGGER notes_fts_insert AFTER INSERT ON notes | ||
| WHEN NEW.is_deleted = 0 OR NEW.is_deleted IS NULL | ||
| BEGIN | ||
| INSERT INTO notes_fts(id, title, content) | ||
| VALUES (NEW.id, NEW.title, NEW.content); | ||
| END; | ||
|
|
||
| -- Trigger: sync FTS on UPDATE (delete + re-insert) | ||
| CREATE TRIGGER notes_fts_update AFTER UPDATE ON notes | ||
| BEGIN | ||
| DELETE FROM notes_fts WHERE id = OLD.id; | ||
| INSERT INTO notes_fts(id, title, content) | ||
| SELECT NEW.id, NEW.title, NEW.content | ||
| WHERE NEW.is_deleted = 0 OR NEW.is_deleted IS NULL; | ||
| END; | ||
|
|
||
| -- Trigger: sync FTS on DELETE | ||
| CREATE TRIGGER notes_fts_delete AFTER DELETE ON notes | ||
| BEGIN | ||
| DELETE FROM notes_fts WHERE id = OLD.id; | ||
| END; | ||
| `; | ||
|
|
||
| describe('FTS5 trigger execution', () => { | ||
| let db: Database.Database; | ||
|
|
||
| beforeEach(() => { | ||
| db = new Database(':memory:'); | ||
| db.exec(SCHEMA); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| db.close(); | ||
| }); | ||
|
|
||
| it('INSERT fires notes_fts_insert trigger without error', () => { | ||
| const now = new Date().toISOString(); | ||
|
|
||
| expect(() => { | ||
| db.prepare( | ||
| `INSERT INTO notes (id, content, title, created_at, updated_at, word_count, notebook_id, status, needs_sync, local_version, sync_version) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, 'active', 1, 1, 0)` | ||
| ).run('note-1', '# Test Note\n\nHello world', 'Test Note', now, now, 2, 'inbox'); | ||
| }).not.toThrow(); | ||
|
|
||
| const ftsRow = db.prepare('SELECT * FROM notes_fts WHERE notes_fts MATCH ?').get('hello'); | ||
| expect(ftsRow).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('UPDATE fires notes_fts_update trigger without error', () => { | ||
| const now = new Date().toISOString(); | ||
| db.prepare( | ||
| `INSERT INTO notes (id, content, title, created_at, updated_at, word_count) | ||
| VALUES (?, ?, ?, ?, ?, ?)` | ||
| ).run('note-1', '# Original\n\nOriginal content', 'Original', now, now, 2); | ||
|
|
||
| expect(() => { | ||
| db.prepare( | ||
| 'UPDATE notes SET content = ?, title = ?, updated_at = ?, word_count = ?, needs_sync = 1, local_version = local_version + 1 WHERE id = ?' | ||
| ).run('# Updated\n\nBrand new content', 'Updated', now, 3, 'note-1'); | ||
| }).not.toThrow(); | ||
|
|
||
| const oldMatch = db.prepare('SELECT * FROM notes_fts WHERE notes_fts MATCH ?').get('original'); | ||
| expect(oldMatch).toBeUndefined(); | ||
|
|
||
| const newMatch = db.prepare('SELECT * FROM notes_fts WHERE notes_fts MATCH ?').get('brand'); | ||
| expect(newMatch).toBeTruthy(); | ||
| }); | ||
|
|
||
| it('soft-delete UPDATE removes entry from FTS index', () => { | ||
| const now = new Date().toISOString(); | ||
| db.prepare( | ||
| `INSERT INTO notes (id, content, title, created_at, updated_at, word_count) | ||
| VALUES (?, ?, ?, ?, ?, ?)` | ||
| ).run('note-1', '# Trashable\n\nGoing away', 'Trashable', now, now, 2); | ||
|
|
||
| expect(() => { | ||
| db.prepare( | ||
| 'UPDATE notes SET is_deleted = 1, updated_at = ?, needs_sync = 1 WHERE id = ?' | ||
| ).run(now, 'note-1'); | ||
| }).not.toThrow(); | ||
|
|
||
| const match = db.prepare('SELECT * FROM notes_fts WHERE notes_fts MATCH ?').get('trashable'); | ||
| expect(match).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('DELETE fires notes_fts_delete trigger without error', () => { | ||
| const now = new Date().toISOString(); | ||
| db.prepare( | ||
| `INSERT INTO notes (id, content, title, created_at, updated_at, word_count) | ||
| VALUES (?, ?, ?, ?, ?, ?)` | ||
| ).run('note-1', '# Deletable\n\nWill be removed', 'Deletable', now, now, 3); | ||
|
|
||
| expect(() => { | ||
| db.prepare('DELETE FROM notes WHERE id = ?').run('note-1'); | ||
| }).not.toThrow(); | ||
|
|
||
| const match = db.prepare('SELECT * FROM notes_fts WHERE notes_fts MATCH ?').get('deletable'); | ||
| expect(match).toBeUndefined(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('FTS5 runtime check', () => { | ||
| it('openDb succeeds with an in-memory database (FTS5 available)', () => { | ||
| const db = openDb(':memory:'); | ||
| expect(db).toBeTruthy(); | ||
| // Verify FTS5 actually works on the returned connection | ||
| db.prepare('CREATE VIRTUAL TABLE _test_fts USING fts5(x)').run(); | ||
| db.prepare('DROP TABLE _test_fts').run(); | ||
| db.close(); | ||
| }); | ||
| }); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Guard against setState after unmount.
The async IIFE in the mount effect has no cancellation guard. If
NotesAppunmounts beforelistState()resolves (e.g., during fast test teardown or HMR),setBuiltInEnabledMapwill 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:reloadhandler in the second effect.📝 Committable suggestion
🤖 Prompt for AI Agents