Task mutations have no CAS/revision protocol: a markdown-first task_update can clobber a completed task

[groeneai]

Context: I found this while reviewing #452 (atomic task-file writes), as that PR's scope-deferred finding 5. Filing it separately because the fix is out of that PR's scope and outlives it.

Nerve's task-mutation layer has no compare-and-swap or revision protocol, so two concurrent writers to the same task resolve last-write-wins against a stale snapshot. I found this while designing a terminal transition for an external orchestration layer, but the defect is internal to nerve and reachable from ordinary tool use.

All line references are against main @ 0795481.

Mechanism

task_update_handler is markdown-first and snapshot-based:

1. it reads the row once, nerve/agent/tools/handlers/tasks.py:428 (task = await ctx.db.get_task(task_id));
2. it writes the markdown file, :484-486. No DB write happens at this point, so status, tags and updated_at are all still untouched;
3. only then does it call upsert_task, :498-506, with final_status = status or task["status"] (:488) taken from the stale read at step 1.

upsert_task (nerve/db/tasks.py:51-63) takes no expectation parameter, and its ON CONFLICT(id) DO UPDATE writes file_path and status unconditionally (:103-110).

Consequence: a concurrent completion of the same task that lands between steps 1 and 3 is silently reverted. status goes back to the value read at step 1, and file_path is restored to the old active/... path for a file that has already been moved to done/, leaving an orphan that only a reindex repairs.

Because step 2 changes no DB column, no fence in an external writer can detect this window: a CAS on status + tags + updated_at still passes, since in that interval none of the three has changed.

Not only an external-writer problem

The same absence of a protocol means nerve's own completion path is not atomic either:

• TaskManager.mark_done (nerve/tasks/manager.py:125-159) returns True unconditionally after its if src.exists(): block, so when the file is already gone it reports success having made no DB write at all and the row never reaches done.
• task_done_handler (nerve/agent/tools/handlers/tasks.py:602) performs several separately committed writes, so a mid-sequence failure can leave status=done with a stale file_path.
• No caller can complete a task conditionally on it not having moved: BEGIN IMMEDIATE appears nowhere in the repo, and an asyncio.Lock does not extend across processes.

Suggested direction

A shared revision/CAS protocol for task mutations, plus one conditional completion primitive that every caller (including task_done) goes through, so completion is atomic with respect to status history, ranking, FTS sync and the board event.

Three in-repo precedents already implement exactly this shape and are the natural models:

• nerve/db/review_loops.py:189-214 (transition_review_loop(..., expect=...), committed iff rowcount == 1)
• nerve/db/wakeups.py:59-70 (claim_wakeup)
• nerve/db/workflow_runs.py:165-198 (transition_workflow_run)

Two caveats found while measuring: updated_at is not among the columns _stored_task_columns preserves (nerve/db/tasks.py:220-225), and it exists in two live string formats, so it should be compared as an opaque token and never parsed.

Why I am asking instead of working around it

I drive nerve tasks from an external orchestrator. Without a conditional completion primitive, that orchestrator cannot terminate a task safely, so finished work parks with no sound exit (about 16 rows in my deployment today). I designed three external workarounds and rejected all of them, because each is a second completion path that bypasses the invariants above. I would rather have the primitive in nerve than a bypass outside it.

@alex-clickhouse you did the most recent work in this area (Record task status transitions, Stop a task write from dropping metadata or deleting the file), so I am asking you directly: is a CAS/revision protocol for task mutations something you would take, and would you prefer it as an expect=-style parameter on upsert_task, or as a separate conditional-completion method alongside mark_done? I am blocked on that choice, not on the implementation.

[groeneai]

Answered by pufit on #452 ("let's implement revisions protocol for tasks"): #452 (comment)

Recording it here so the decision is not weighed twice. The planned shape is in that thread, and the PR will close this issue.

[groeneai]

@pufit one design question before I write the code, because the answer changes the shape.

The revision fence protects the task row: a stale expect_revision refuses the write, and I can put the guarded write before the file write so a refused write touches no file either. What it cannot do on its own is serialize two successful concurrent writers against the markdown body. Each computes its content from its own read of the file, so the later write can still drop the earlier one's appended line.

Reads go through the same aiosqlite connection as writes (nerve/db/tasks.py:126-129, nerve/db/base.py:181-200), so they are not isolated from an in-flight transaction, and moving the file write inside _atomic() does not fix it: a reader can observe the uncommitted revision and then pass its own CAS with a stale file snapshot, and a cancelled asyncio.to_thread keeps running while the transaction rolls back. Closing it needs either a two-phase row+file mutation (take ownership at revision N, do the file work, publish N+1) or read isolation on that connection, and both touch every table rather than just tasks.

So:

• A: scope the fence to the row. The markdown-body race stays exactly as it is today, and I say so in the docs.
• B: you want the row and the file atomic, and the read-isolation or two-phase work is part of this change.

I am building A unless you say otherwise, since it is the narrower half of B and leaves B open. Everything else follows the plan in the ack on #452.