Skip to content

Commit 1d654e4

Browse files
committed
fix(mirror,relations): persist edges into entity markdown frontmatter
Two related bugs that meant relations created via UI/REST/MCP never made it into the markdown mirror, plus a silent data-loss path on subsequent entity updates. 1. Mirror was never told about relations The phase4 SQLite migration (c8997ee) wired StoreManager to call mirrorNoteCreate/Update, mirrorTaskCreate/Update, mirrorSkillCreate/ Update, mirrorEpicCreate/Update, but hardcoded the `relations` parameter as `[]` everywhere, and dropped all calls to the dedicated mirrorNoteRelation / mirrorTaskRelation / mirrorSkillRelation / mirrorEpicRelation event functions. Those functions stayed in file-mirror.ts but became dead code, and their tests were deleted in the same commit. Effects: - Edges created through createEdge() never appeared in the mirror file frontmatter — they lived in SQLite only. - Every updateNote/updateTask/updateSkill regenerated the snapshot with `relations: []`, wiping any frontmatter relations that had been put there by an external file edit. - If a user then edited the markdown file in their IDE and saved it, the reverse-import path (syncEdgesFromFile) would diff the empty desired list against current SQLite edges and *delete every edge for that entity*. Silent data loss. Fix: - Add private StoreManager.buildOutgoingForMirror(graph, id, slug) that walks findOutgoingEdges + resolveIdToSlug to construct the RelationLike[] the file-mirror layer expects. Indexed-graph targets (docs/code/files) are skipped because they have no slug. - Replace all 9 hardcoded `[]` arguments at the entity create/update/ move/reorder/bulk call sites with this helper. - Add private StoreManager.mirrorRelationEvent(action, edge) that dispatches to mirrorNoteRelation / mirrorTaskRelation / mirrorSkillRelation / mirrorEpicRelation depending on edge.fromGraph, re-uses buildMirrorTaskAttrs and the inline note/skill/epic attr builders, and records the mirror write through the existing tracker so the watcher does not feedback-loop the change back into SQLite. - Wire createEdge() and deleteEdge() to call mirrorRelationEvent after the SQLite mutation succeeds. 2. REST DELETE /relations and /links never deleted anything The REST handlers for knowledge/tasks/skills delete-edge endpoints passed `kind: ''` to deleteEdge() unconditionally. The underlying SQL DELETE matches edges by (fromGraph, fromId, toGraph, toId, kind), so the empty kind never matched any real row — the SQL affected 0 rows but the handler returned 204 OK. Pre-existing bug, masked until now because the mirror file was empty too, so the UI saw "delete worked". Fix: - .pick() the validation schemas to include `kind` (already required by createRelationSchema/createTaskLinkSchema/createSkillLinkSchema). - Read kind from req.body and pass it through to deleteEdge. - Update the frontend deleteRelation / deleteTaskLink / deleteSkillLink type signatures to require kind, and update RelationManager.handleDeleteConfirmed to pass rel.kind from the original edge. Tests - Restored 3 unit tests in src/tests/file-mirror.test.ts covering mirrorNoteRelation / mirrorTaskRelation / mirrorSkillRelation, so these dedicated event functions never silently rot again. - 8 round-trip tests in src/tests/store/store-manager.test.ts that drive StoreManager.createEdge/deleteEdge and assert the markdown file frontmatter, including the silent-data-loss case (createEdge followed by updateNote must preserve the relation), cross-graph edges (graph: tag), and tasks/skills mirror coverage. - New "2.3.1 Forward mirror" group in functional test 02-knowledge.ts that POSTs a relation, PUTs the note body, and DELETEs the relation, asserting the mirror file frontmatter at each step through the actual REST API + dist build. - Updated existing tests in 02-knowledge.ts, 03-tasks.ts, 13-websocket.ts, 19-coverage-gaps.ts and rest-api-gaps.test.ts to pass the now-required `kind` field. Several of those tests had been silently exercising the broken code path. Verified: 1597 jest tests pass, 502 functional sandbox tests pass.
1 parent d864adf commit 1d654e4

15 files changed

Lines changed: 476 additions & 39 deletions

File tree

demo-projects/test-sandbox/tests/02-knowledge.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ let restNoteId = '';
2020
let mcpNoteId = '';
2121
let noteA_Id = '';
2222
let noteB_Id = '';
23+
let noteA_Slug = '';
24+
let noteB_Slug = '';
2325
let codeSymbolId = '';
2426

2527
// ─── 2.1 CRUD ────────────────────────────────────────────────────
@@ -129,6 +131,8 @@ test('Create note for search tests', async () => {
129131
});
130132
assertOk(res);
131133
noteA_Id = res.data.noteId ?? res.data.id;
134+
noteA_Slug = res.data.slug ?? '';
135+
assertExists(noteA_Slug, 'noteA slug');
132136
});
133137

134138
test('REST GET /knowledge/search?q=quantum — finds note', async () => {
@@ -158,6 +162,8 @@ test('Create second note for relations', async () => {
158162
});
159163
assertOk(res);
160164
noteB_Id = res.data.noteId ?? res.data.id;
165+
noteB_Slug = res.data.slug ?? '';
166+
assertExists(noteB_Slug, 'noteB slug');
161167
});
162168

163169
test('REST POST /knowledge/relations — create relation', async () => {
@@ -181,6 +187,7 @@ test('REST DELETE /knowledge/relations — remove relation', async () => {
181187
const res = await del('/knowledge/relations', {
182188
fromId: noteA_Id,
183189
toId: noteB_Id,
190+
kind: 'related_to',
184191
});
185192
assertOk(res);
186193
});
@@ -210,6 +217,60 @@ test('MCP notes_delete_link — remove relation', async () => {
210217
assertMcpOk(res);
211218
});
212219

220+
// ─── 2.3.1 Forward mirror (DB → file relations) ─────────────────
221+
//
222+
// Regression for the silent data-loss bug: relations created via REST/MCP
223+
// were never written to the markdown mirror, and any subsequent entity
224+
// update wiped existing relations from the file frontmatter.
225+
226+
group('2.3.1 Forward mirror (DB → file relations)');
227+
228+
function readNoteMd(slug: string): string {
229+
return readFile(projectPath('.notes', slug, 'note.md'));
230+
}
231+
232+
test('POST /knowledge/relations writes outgoing relation to note.md', async () => {
233+
const res = await post('/knowledge/relations', {
234+
fromId: noteA_Id,
235+
toId: noteB_Id,
236+
kind: 'related_to',
237+
});
238+
assertOk(res);
239+
await wait(200);
240+
241+
const md = readNoteMd(noteA_Slug);
242+
assert(md.includes('relations:'), `note.md should contain relations: frontmatter, got:\n${md.substring(0, 300)}`);
243+
assert(md.includes(`to: ${noteB_Slug}`), `note.md should reference target slug ${noteB_Slug}`);
244+
assert(md.includes('kind: related_to'), 'note.md should have the relation kind');
245+
});
246+
247+
test('PUT /knowledge/notes/{id} preserves existing relations', async () => {
248+
// The previous test left the noteA → noteB edge in place. Updating the note
249+
// body must NOT wipe it from the mirror frontmatter.
250+
const res = await put(`/knowledge/notes/${noteA_Id}`, {
251+
content: 'Quantum computing uses qubits — updated body.',
252+
});
253+
assertOk(res);
254+
await wait(200);
255+
256+
const md = readNoteMd(noteA_Slug);
257+
assert(md.includes(`to: ${noteB_Slug}`), `relation must survive entity update, got:\n${md.substring(0, 400)}`);
258+
assert(md.includes('kind: related_to'), 'kind must survive update');
259+
});
260+
261+
test('DELETE /knowledge/relations removes the relation from note.md', async () => {
262+
const res = await del('/knowledge/relations', {
263+
fromId: noteA_Id,
264+
toId: noteB_Id,
265+
kind: 'related_to',
266+
});
267+
assertOk(res);
268+
await wait(200);
269+
270+
const md = readNoteMd(noteA_Slug);
271+
assert(!md.includes(`to: ${noteB_Slug}`), `relation should be removed, got:\n${md.substring(0, 300)}`);
272+
});
273+
213274
// ─── 2.4 Cross-graph links ──────────────────────────────────────
214275

215276
group('2.4 Cross-graph links');

demo-projects/test-sandbox/tests/03-tasks.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ test('REST DELETE /tasks/links — delete link', async () => {
252252
const res = await del('/tasks/links', {
253253
fromId: taskB_Id,
254254
toId: taskC_Id,
255+
kind: 'related_to',
255256
});
256257
assertOk(res);
257258
});

demo-projects/test-sandbox/tests/13-websocket.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ test('Create note→note relation → receives note:relation:added', async () =>
234234
// Delete relation → note:relation:deleted
235235
clearReceived();
236236
await restWith(BASE, 'DELETE', '/api/projects/sandbox/knowledge/relations',
237-
{ fromId: n1.data.id, toId: n2.data.id });
237+
{ fromId: n1.data.id, toId: n2.data.id, kind: 'related_to' });
238238
const delEvt = await findEvent('note:relation:deleted');
239239
assertExists(delEvt, 'note:relation:deleted event');
240240

@@ -258,7 +258,7 @@ test('Create task→note relation → receives task:relation:added', async () =>
258258

259259
clearReceived();
260260
await restWith(BASE, 'DELETE', '/api/projects/sandbox/tasks/links',
261-
{ fromId: task.data.id, toId: note.data.id, targetGraph: 'knowledge' });
261+
{ fromId: task.data.id, toId: note.data.id, kind: 'references', targetGraph: 'knowledge' });
262262
const delEvt = await findEvent('task:relation:deleted');
263263
assertExists(delEvt, 'task:relation:deleted event');
264264

@@ -282,7 +282,7 @@ test('Create skill→skill relation → receives skill:relation:added', async ()
282282

283283
clearReceived();
284284
await restWith(BASE, 'DELETE', '/api/projects/sandbox/skills/links',
285-
{ fromId: s1.data.id, toId: s2.data.id });
285+
{ fromId: s1.data.id, toId: s2.data.id, kind: 'depends_on' });
286286
const delEvt = await findEvent('skill:relation:deleted');
287287
assertExists(delEvt, 'skill:relation:deleted event');
288288

demo-projects/test-sandbox/tests/19-coverage-gaps.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ test('REST DELETE /skills/links — remove link', async () => {
183183
const res = await del('/skills/links', {
184184
fromId: skillId,
185185
toId: skillB_Id,
186+
kind: 'depends_on',
186187
});
187188
assertOk(res);
188189
});
@@ -290,6 +291,7 @@ test('REST DELETE /knowledge/relations with targetGraph=code', async () => {
290291
const res = await del('/knowledge/relations', {
291292
fromId: noteId,
292293
toId: skillId,
294+
kind: 'references',
293295
targetGraph: 'code',
294296
});
295297
assert(res.status < 500, `should not 500, got ${res.status}`);
@@ -323,6 +325,7 @@ test('REST DELETE /tasks/links with targetGraph=code', async () => {
323325
const res = await del('/tasks/links', {
324326
fromId: taskId,
325327
toId: skillId,
328+
kind: 'references',
326329
targetGraph: 'code',
327330
});
328331
assert(res.status < 500, `should not 500, got ${res.status}`);
@@ -669,11 +672,13 @@ test('Cleanup cross-graph file-link fixtures', async () => {
669672
await del('/knowledge/relations', {
670673
fromId: linkedNoteId,
671674
toId: fileNodeId,
675+
kind: 'references',
672676
targetGraph: 'files',
673677
});
674678
await del('/tasks/links', {
675679
fromId: linkedTaskId,
676680
toId: fileNodeId,
681+
kind: 'references',
677682
targetGraph: 'files',
678683
});
679684
await del(`/knowledge/notes/${linkedNoteId}`);

src/api/rest/knowledge.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,17 @@ export function createKnowledgeRouter(_users?: Record<string, UserConfig>): Rout
133133
} catch (err) { next(err); }
134134
});
135135

136-
// Delete edge (relation)
137-
router.delete('/relations', requireWriteAccess, validateBody(createRelationSchema.pick({ fromId: true, toId: true, targetGraph: true, projectId: true })), async (req, res, next) => {
136+
// Delete edge (relation). `kind` is required because the underlying SQL
137+
// delete matches edges by (fromId, toId, fromGraph, toGraph, kind) — without
138+
// it we'd silently affect 0 rows.
139+
router.delete('/relations', requireWriteAccess, validateBody(createRelationSchema.pick({ fromId: true, toId: true, kind: true, targetGraph: true, projectId: true })), async (req, res, next) => {
138140
try {
139141
const { storeManager: mgr, mutationQueue } = getProject(req);
140-
const { fromId, toId, targetGraph } = req.body;
142+
const { fromId, toId, kind, targetGraph } = req.body;
141143
const fromGraph: GraphName = 'knowledge';
142144
const toGraph: GraphName = targetGraph || 'knowledge';
143145
await mutationQueue.enqueue(async () => {
144-
mgr.deleteEdge({ fromGraph, fromId, toGraph, toId, kind: '' });
146+
mgr.deleteEdge({ fromGraph, fromId, toGraph, toId, kind });
145147
});
146148
res.status(204).end();
147149
} catch (err) { next(err); }

src/api/rest/skills.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,17 @@ export function createSkillsRouter(_users?: Record<string, UserConfig>): Router
159159

160160
// Delete skill link
161161
// Must be registered before DELETE /:skillId to avoid 'links' being parsed as a skillId
162-
router.delete('/links', requireWriteAccess, validateBody(createSkillLinkSchema.pick({ fromId: true, toId: true, targetGraph: true, projectId: true })), async (req, res, next) => {
162+
router.delete('/links', requireWriteAccess, validateBody(createSkillLinkSchema.pick({ fromId: true, toId: true, kind: true, targetGraph: true, projectId: true })), async (req, res, next) => {
163163
try {
164164
const p = getProject(req);
165-
const { fromId, toId, targetGraph } = req.body;
165+
const { fromId, toId, kind, targetGraph } = req.body;
166166
await p.mutationQueue.enqueue(async () => {
167167
p.storeManager.deleteEdge({
168168
fromGraph: 'skills',
169169
fromId,
170170
toGraph: targetGraph ?? 'skills',
171171
toId,
172-
kind: '',
172+
kind,
173173
});
174174
});
175175
res.status(204).end();

src/api/rest/tasks.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,16 +203,16 @@ export function createTasksRouter(_users?: Record<string, unknown>): Router {
203203

204204
// Delete edge
205205
// Must be registered before DELETE /:taskId to avoid 'links' being parsed as a taskId
206-
router.delete('/links', requireWriteAccess, validateBody(createTaskLinkSchema.pick({ fromId: true, toId: true, targetGraph: true, projectId: true })), async (req, res, next) => {
206+
router.delete('/links', requireWriteAccess, validateBody(createTaskLinkSchema.pick({ fromId: true, toId: true, kind: true, targetGraph: true, projectId: true })), async (req, res, next) => {
207207
try {
208208
const p = getProject(req);
209-
const { fromId, toId, targetGraph } = req.body;
209+
const { fromId, toId, kind, targetGraph } = req.body;
210210
const edge: Edge = {
211211
fromGraph: 'tasks' as GraphName,
212212
fromId,
213213
toGraph: (targetGraph ?? 'tasks') as GraphName,
214214
toId,
215-
kind: '', // kind not required for delete lookup
215+
kind,
216216
};
217217
await p.mutationQueue.enqueue(async () => {
218218
p.storeManager.deleteEdge(edge);

0 commit comments

Comments
 (0)