PR #62 plugged this FK violation at the extraction-layer insertEdges site (empty-named nodes whose containment edges had no target), but the same violation kept reappearing on v0.9.5 during the daemon's *watch sync* once an agent's daemon had been running long enough. The resolution-layer insertEdges (and the callback-synthesizer pass) wasn't guarded the same way: a per-resolver name cache or a framework resolver's WeakMap-keyed lookup map could hand back a Node whose row had been removed by a recent file rewrite, and the FK check then aborted the entire resolution batch, leaving the daemon log filling with `Watch sync failed { error: 'FOREIGN KEY constraint failed' }`. The resolution layer now mirrors the #62 defense — one cache-aware getNodesByIds per pass drops any edge whose source or target is no longer in the nodes table, so the rest of the resolved batch still lands. Regression test seeds the resolver's nameCache with a stale Node and calls resolveAndPersist directly; verified to throw FOREIGN KEY constraint failed without the fix and pass with it. Full suite: 984/984 pass. Closes #455. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
e76cc547b0
commit
1dfaf30a8b
@@ -812,6 +812,19 @@ export function synthesizeCallbackEdges(queries: QueryBuilder, ctx: ResolutionCo
|
||||
seen.add(key);
|
||||
merged.push(e);
|
||||
}
|
||||
if (merged.length > 0) queries.insertEdges(merged);
|
||||
return merged.length;
|
||||
if (merged.length > 0) {
|
||||
// Defense-in-depth (issues #42, #455): drop edges whose source/target
|
||||
// no longer resolves to a real node. Some channel maps cache native
|
||||
// Node refs across a resolver lifetime (WeakMap-keyed by context), so
|
||||
// a file rewrite between map build and synthesis can leave stale IDs
|
||||
// here. One FK violation aborts the whole batch — better to skip the
|
||||
// dead edges and emit the rest than lose every synthesized edge.
|
||||
const allIds = new Set<string>();
|
||||
for (const e of merged) { allIds.add(e.source); allIds.add(e.target); }
|
||||
const existing = queries.getNodesByIds([...allIds]);
|
||||
const validEdges = merged.filter((e) => existing.has(e.source) && existing.has(e.target));
|
||||
if (validEdges.length > 0) queries.insertEdges(validEdges);
|
||||
return validEdges.length;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
+28
-4
@@ -607,6 +607,28 @@ export class ReferenceResolver {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Defense-in-depth: drop edges whose source or target is no longer in
|
||||
* the nodes table. PR #62 (issue #42) applied this filter at the
|
||||
* extraction-layer `insertEdges` site; #455 reports the same
|
||||
* `FOREIGN KEY constraint failed` reappearing here at the
|
||||
* resolution-layer site during watch sync, where a resolver lookup that
|
||||
* crosses a framework-specific cache can hand us a target whose node
|
||||
* was removed by a concurrent file rewrite. One batched, cache-aware
|
||||
* `getNodesByIds` query is enough to skip those edges quietly instead
|
||||
* of aborting the whole sync.
|
||||
*/
|
||||
private filterEdgesByExistingNodes(edges: Edge[]): Edge[] {
|
||||
if (edges.length === 0) return edges;
|
||||
const allIds = new Set<string>();
|
||||
for (const e of edges) {
|
||||
allIds.add(e.source);
|
||||
allIds.add(e.target);
|
||||
}
|
||||
const existing = this.queries.getNodesByIds([...allIds]);
|
||||
return edges.filter((e) => existing.has(e.source) && existing.has(e.target));
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve and persist edges to database
|
||||
*/
|
||||
@@ -620,8 +642,9 @@ export class ReferenceResolver {
|
||||
const edges = this.createEdges(result.resolved);
|
||||
|
||||
// Insert edges into database
|
||||
if (edges.length > 0) {
|
||||
this.queries.insertEdges(edges);
|
||||
const validEdges = this.filterEdgesByExistingNodes(edges);
|
||||
if (validEdges.length > 0) {
|
||||
this.queries.insertEdges(validEdges);
|
||||
}
|
||||
|
||||
// Clean up resolved refs from unresolved_refs table so metrics are accurate
|
||||
@@ -668,8 +691,9 @@ export class ReferenceResolver {
|
||||
|
||||
// Persist edges immediately
|
||||
const edges = this.createEdges(result.resolved);
|
||||
if (edges.length > 0) {
|
||||
this.queries.insertEdges(edges);
|
||||
const validEdges = this.filterEdgesByExistingNodes(edges);
|
||||
if (validEdges.length > 0) {
|
||||
this.queries.insertEdges(validEdges);
|
||||
}
|
||||
|
||||
// Clean up resolved refs so they don't appear in the next batch
|
||||
|
||||
Reference in New Issue
Block a user