TOCTOU: plan-task, plan-slice, reassess-roadmap, replan-slice guards outside transaction

[igouss]

Summary

Four tool handlers run state machine guards outside the transaction() callback, then perform writes inside a separate transaction. This creates a time-of-check-to-time-of-use (TOCTOU) race where two agents can both pass the guard simultaneously and both write successfully.

Impact

Critical — In multi-agent deployments (the exact threat model the Single-Writer Engine v3 addresses), concurrent agents can:

• Both re-plan the same completed slice/task
• Both reassess the same milestone simultaneously
• Both replan the same blocked slice

SQLite's single-writer lock prevents corruption but does not prevent both operations from succeeding — the second write wins silently.

Affected Files

plan-task.ts

• Line 80: getSlice() guard — outside txn
• Line 88: getTask() guard — outside txn
• Line 94: transaction(() => { insertTask/upsertTaskPlanning }) — separate txn

plan-slice.ts

• Line 149: getMilestone() guard — outside txn
• Line 157: getSlice() guard — outside txn
• Line 166: transaction(() => { ... }) — separate txn

reassess-roadmap.ts

• Line 108: getMilestone() check — outside txn
• Line 117: getSlice() for completedSliceId — outside txn
• Lines 126-146: getMilestoneSlices() + enforcement — outside txn
• Line 158: transaction() — separate txn

replan-slice.ts

• Line 94: getSlice() slice-status check — outside txn
• Line 103: getTask() blocker check — outside txn
• Lines 112-132: completed-task enforcement — outside txn
• Line 138: transaction() — separate txn

Correct Pattern (for reference)

complete-task.ts:155-208, complete-slice.ts:223-257, reopen-task.ts:60-92, and reopen-slice.ts:59-88 all correctly colocate guards and writes inside a single transaction() callback.

Fix

Move guard checks into the transaction() callback in all four files, following the same pattern as complete-task.ts. Use a guardError variable set inside the transaction, checked after it returns.

Confidence

85% — structural pattern verified across all tool handlers.

[igouss]

Investigation Results — CONFIRMED (5 files, not 4)

Guard vs Transaction Placement

CORRECT pattern (for reference):

• complete-task.ts:155-208 — guards AND writes inside single transaction()
• complete-slice.ts:223-257 — same
• reopen-task.ts:60-92 — same
• reopen-slice.ts:59-88 — same

INCORRECT — guards outside transaction:

File	Guard lines	Transaction starts	Gap
plan-task.ts	80-91 (getSlice, getTask)	94	3 lines
plan-slice.ts	149-163 (getMilestone, getSlice)	166	3 lines
reassess-roadmap.ts	108-146 (getMilestone, getSlice, getMilestoneSlices loop)	158	12 lines
replan-slice.ts	94-132 (getSlice, getTask, getSliceTasks loop)	138	6 lines
plan-milestone.ts	193-209 (getMilestone, depends_on loop)	212	3 lines

Additional finding: plan-milestone.ts also affected

The original issue listed 4 files. Investigation confirms 5 files have this pattern — plan-milestone.ts:193-209 has the same guards-outside-txn issue.

Race scenario (concrete)

1. Agent A calls plan-task, reads slice status = "in_progress" at line 80 — guard passes
2. Agent B calls complete-slice, marks slice "complete" inside its transaction
3. Agent A enters transaction at line 94, plans a task inside a now-complete slice

SQLite serializes the writes so no corruption, but the structural invariant ("cannot plan inside a complete slice") is violated.

Fix

Move guard checks into the transaction() callback in all 5 files. Use the guardError variable pattern from complete-task.ts.