Skip to content

fix(cfg): avoid dual-connection WAL conflict in native bulkInsertCfg #719

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.

Sign up for GitHub

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

Merged
carlos-alm merged 4 commits into from
Mar 31, 2026
Merged

fix(cfg): avoid dual-connection WAL conflict in native bulkInsertCfg #719

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3c37675
fix(cfg): move delete-before-insert into native bulkInsertCfg to avoi…
carlos-alm Mar 30, 2026
c2c85a1
fix(cfg): disable native bulk CFG path to avoid dual-connection WAL c…
carlos-alm Mar 30, 2026
9e3c3da
fix(cfg): prefix unused engineOpts param with underscore
carlos-alm Mar 30, 2026
5b9db18
fix: propagate CFG delete errors instead of silently swallowing (#719)
carlos-alm Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions crates/codegraph-core/src/native_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,22 @@ impl NativeDatabase {
)
.map_err(|e| napi::Error::from_reason(format!("cfg_edges prepare failed: {e}")))?;

let mut del_edges = tx.prepare(
"DELETE FROM cfg_edges WHERE function_node_id = ?1",
)
.map_err(|e| napi::Error::from_reason(format!("cfg_edges del prepare failed: {e}")))?;
let mut del_blocks = tx.prepare(
"DELETE FROM cfg_blocks WHERE function_node_id = ?1",
)
.map_err(|e| napi::Error::from_reason(format!("cfg_blocks del prepare failed: {e}")))?;

for entry in &entries {
// Delete existing CFG data for this node so the caller doesn't
// need to perform deletes on a separate (JS) connection, which
// would cause a WAL conflict with the native connection.
let _ = del_edges.execute(params![entry.node_id]);
let _ = del_blocks.execute(params![entry.node_id]);

@greptile-apps greptile-apps Bot Mar 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent delete errors may mask partial state

Both DELETE statements discard their Result with let _ = ..., silently swallowing any error. If either delete fails inside the transaction (e.g., a locked auxiliary index or a statement constraint), the subsequent INSERTs still proceed, potentially leaving duplicate rows for the same function_node_id.

The existing INSERT code uses the same pattern (if let Ok(_) = block_stmt.execute(...)) so this is consistent, but the deletes are "load-bearing" here — a silent failure means old rows survive alongside new ones. Consider propagating delete errors the same way the tx.commit() error is propagated:

Suggested change
let _ = del_edges.execute(params![entry.node_id]);
let _ = del_blocks.execute(params![entry.node_id]);
del_edges.execute(params![entry.node_id])
.map_err(|e| napi::Error::from_reason(format!("cfg_edges delete failed: {e}")))?;
del_blocks.execute(params![entry.node_id])
.map_err(|e| napi::Error::from_reason(format!("cfg_blocks delete failed: {e}")))?;

This keeps the whole function consistent with how tx.commit() failures are surfaced, and ensures the caller gets a clear error rather than silently storing duplicate blocks.

@carlos-alm carlos-alm Mar 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Changed both del_edges.execute() and del_blocks.execute() to propagate errors via .map_err(|e| napi::Error::from_reason(...))?; — consistent with how tx.commit() errors are surfaced. A failed delete now aborts the transaction instead of silently proceeding with inserts.


let mut block_db_ids: std::collections::HashMap<u32, i64> =
std::collections::HashMap::new();
for block in &entry.blocks {
Expand Down
9 changes: 7 additions & 2 deletions src/features/cfg.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,13 @@ export async function buildCFGData(
const nodeId = getFunctionNodeId(db, def.name, relPath, def.line);
if (!nodeId) continue;

deleteCfgForNode(db, nodeId);
if (!def.cfg?.blocks?.length) continue;
// Deletion is handled inside nativeDb.bulkInsertCfg to avoid
// dual-connection WAL conflicts between JS and native connections.
if (!def.cfg?.blocks?.length) {
// Still send an entry so the native side deletes stale CFG data.
entries.push({ nodeId, blocks: [], edges: [] });
continue;
}

const cfg = def.cfg as unknown as { blocks: CfgBuildBlock[]; edges: CfgBuildEdge[] };
entries.push({
Expand Down
Loading