Add a revision/CAS protocol for task mutations

[groeneai]

Merge after #404 and #422, which add v050 and v051.

task_update reads a task row, rewrites the markdown, then writes the row back from that read, and upsert_task replaces it unconditionally. A completion landing in between is undone: status reverts and file_path points back into active/ for a file already in done/. Nothing on the row changes while that window is open, so no caller can fence against it (#453). TaskManager.mark_done also returned True without writing the row when the file was already gone.

The change

• v052 adds tasks.revision INTEGER NOT NULL DEFAULT 1; every write to a row advances it.
• transition_task(task_id, to_status, expect=..., expect_revision=..., **fields): one guarded UPDATE, True only when rowcount == 1, shaped like transition_review_loop, claim_wakeup and transition_workflow_run. update_task_status wraps it.
• expect_revision= on upsert_task and update_task_tags. The conditional upsert is a plain guarded UPDATE: an ON CONFLICT ... WHERE guard would leave the INSERT arm free to re-create a deleted row.
• task_done, task_reopen, mark_done write status, file_path and FTS text in one transition_task call.
• _atomic() begins with BEGIN IMMEDIATE, so a body's first SELECT and the write based on it are one transaction against other connections too. The account store's own seven BEGIN IMMEDIATE lines go.
• Tools: task_read returns the token (as structured and as a trailing HTML comment that task_write strips) and the REST task rows carry revision. task_update and task_done take an optional expect_revision; on a mismatch nothing is written and the conflict is reported. Without it writes stay last-write-wins, so the undo above is prevented only for writers that pass a token.

task_update forwards the token when it delegates to task_done or to the reopen, and fences edits after a reopen on the revision the reopen wrote. A reopen now claims the row before moving the file, so its "is it still done?" check is atomic and a second concurrent reopen is refused instead of moving the file again.

The fence covers the row, not the markdown body: two writers that both pass a current token can still lose each other's markdown edits, and since the row and the file are still two writes, a writer landing between them or a failure after the first can leave them disagreeing. Making them atomic is the open question on #453; this PR is its option A.

Validation

With a task_done injected inside task_update's window, main reverts the completion; with expect_revision the update is refused and the completion stands. Of the 46 new revision tests, 43 fail on main and 3 pin unchanged behaviour, and each of 27 mutations fails its tests. Two of the three new _atomic() tests fail on main. Full suite: 3950 passed vs 3901 on main.

Every writer of tasks

Writer	Revision
upsert_task, unconditional	inserts 1; DO UPDATE adds revision = tasks.revision + 1
upsert_task(expect_revision=)	guarded plain UPDATE, no INSERT arm
transition_task (and update_task_status)	bumped in the guarded UPDATE
move_task	bumped
update_task_tags	bumped; optional expect_revision
update_task_escalation	bumped (no callers today)
_renormalize_lane	bumped per row, so a rare lane re-space invalidates that lane's tokens

Closes #453

🤖 Generated with Claude Code

[groeneai]

cc @alex-clickhouse, could you review this? It adds a tasks.revision counter that every write to a task row advances, and one guarded transition_task write, so a caller that passes the revision it read is refused instead of undoing a concurrent completion (#453).

Internal second-model review: adjudication log (click to expand)

Pre-publication review by an independent model (engine: codex; 4 findings on the first pass, 1 on the recheck) plus my own cold read of the code on both.

#	Sev	Finding	Verdict	Evidence / action
1	❌	After a fenced reopen, task_update fenced its remaining edits on whatever revision it re-read, so a completion landing in between could be undone	AGREE, fixed	The continuation fences on the revision the reopen wrote, and a refused continuation says the reopen landed and the other edits did not
2	❌	An untokened reopen moved the file, then ignored a refused expect=(done,) write and reported success	AGREE, fixed (also found in my own read)	Every reopen claims the row before any file work, and a refusal is reported
3	⚠️	With the task file missing, a fenced note/title/deadline edit skipped the revision check and reported success	AGREE, fixed	With a token, a file-backed edit that has no file to write refuses the whole call
4	❌	A call that writes the row first leaves row and file split when the file write or move then fails. Raised again on the recheck with an injected OSError: a fenced task_done leaves the row pointing into done/ while the file stays in active/	DISAGREE on where the fix lands	True, and the task tools do not move the file back: a retried task_done takes the no-file branch, and a reopen refuses the missing file. The same applies to a reopen without a token, since every reopen now claims first. Row-first is what lets a refused call touch no file, and what stops two racing reopens moving one file. Making row and file atomic is option B of the open question on #453

Severity: ❌ blocker / ⚠️ major / 💡 nit. DISAGREE verdicts carry recorded evidence and are terminal per finding.

Also noted, not blocking (💡): a task_read -> task_write round trip adds a trailing newline to a task file that had none, and "nothing is written" in the tool descriptions does not cover a reopen that lands before its remaining edits are refused (the returned error says so).

Session id: cron:clickhouse-publish-slot-35:20260925-221300

[alex-clickhouse]

@groeneai please rebase on main now that we merged #452, I believe there are going to be some issues afterwards that will need to be fixed.

[groeneai]

Rebased on main @ a5ec79f; still one commit, a2d66c0.

• Conflict: one hunk, the task_update write site. This PR defers that write into a callable that runs before the row write without a token and after it with one, so the resolution makes that callable write_task_file and both orders use the atomic writer. No task-file write in the PR bypasses write_task_file or move_task_file now.
• Broke afterwards: only test_write_order[task_update-*], which spied on Path.write_text; it now spies on write_task_file. Resolving the conflict the other way (keeping Path.write_text) also fails two of fix(tasks): write task files atomically so a failed write cannot empty them #452's tests, test_task_update_with_an_unencodable_note_keeps_the_file and test_carriers_keep_the_file_when_the_write_fails_late[task_update], so that site stays covered.
• Full suite: 3464 passed (3418 on main). I updated the description's merge-order line and test counts to match.

[alex-clickhouse]

1. The migration number is problematic atm as main has already reached v049. Fix it,
2. As a follow-up PR we need a check in the CI that newly added migrations are > the biggest number on main.
3. _atomic() serializes writers on one connection, but does not begin a database transaction before the initial SELECT, the transaction should begin before reading.

[groeneai]

Rebased on main @ f453e5b and pushed.

1. Renumbered to v052 (ad4ed0b). v050 and v051 are already claimed by the open Shared chat-channel foundations #404 and Route channel traffic into source inbox #422. On a database already at v049, v045 was skipped and tasks.revision never appeared; v052 adds it. If this lands before Shared chat-channel foundations #404 and Route channel traffic into source inbox #422, those two have to move above 52, which the check in item 2 will enforce.
2. Acknowledged. I'll open the CI check as a separate PR and link it here.
3. _atomic() now starts with BEGIN IMMEDIATE (484120e), so the body's first SELECT and the write based on it are one transaction against other connections too. I used IMMEDIATE rather than a plain BEGIN because a deferred transaction pins a read snapshot at that SELECT, and another connection's commit then fails the first write with SQLITE_BUSY_SNAPSHOT instead of waiting (test_stale_snapshot_write_fails_without_heal shows it). The seven explicit BEGIN IMMEDIATE lines in nerve/db/accounts.py are removed, since a second BEGIN raises. The cost: every _atomic() body holds the database write lock from its start, including one that reads and then writes nothing.

Tests: three new _atomic() tests in tests/test_db_atomicity.py, two of which fail on main. Without the BEGIN IMMEDIATE they fail again, and so does test_two_connections_racing_to_claim_leave_one_winner in tests/test_accounts.py. Full suite: 3950 passed (3901 on main).

[alex-clickhouse]

Ok, let's make sure this gets merged after 404/422 then.

[groeneai]

Noted. The description now opens with "Merge after #404 and #422". In that order nothing needs renumbering: I merged #462 onto #422 (which contains #404) locally, it has no conflicts, the migrations sit in order v050, v051, v052, and the full suite passes on the result (4478 passed).

[groeneai]

2. I opened CI: check that new migrations are numbered above main's highest #476 for this: it adds a step to backend-tests that fails a PR whose newly added migration is numbered at or below the highest one on main.