[mchwang]

Closes #79.

A stop made inside Store.userAction (cancel attempt, cancel task) used to apply its in-memory effect at once: the job's first reason, the aborted preparation and handle.cancel. If the action then threw, or its COMMIT failed, the write rolled back but D stopped anyway. The attempt then ended cancelled from the job's in-memory reason, and the task stayed open.

What changes

Piece	What it does
Store.afterCommit(commit, rollback?)	Outside a transaction, it runs commit at once (a throw reaches the caller). Inside one, it queues commit until the outermost transaction commits (a throw is logged, so a committed write never looks failed). On rollback, including a failed COMMIT, it drops commit and runs rollback.
RunnerCoordinator.#requestStop	It writes the reason first. On commit, it sets the job's reason, aborts preparation and cancels D. While the commit is pending, pendingReason makes this first stop win over later ones; a rollback clears it. A reason write that throws still applies the stop at once, unsaved ("Rules for the running state", rule 1). If the durable reason cannot be read back, the stop shows as unsaved, as before.
RunnerCoordinator.cancelTask	The re-save of an unsaved reason, and cancelShown, are now set on commit. This replaces the two microtask checks (#confirmSaved and the cancelShown recheck).
Store.#transaction	ROLLBACK runs only while SQLite still has a transaction open (isTransaction). Before, a COMMIT that SQLite had already rolled back (for example on a full disk) reported "cannot rollback" instead of its own error.
Store.#run	Inside a transaction, a write throws a storage error once SQLite has rolled the transaction back on its own. Before, after a caught error of that kind, the later writes autocommitted one by one: the row and the saved action said "cancelled", while the stop callback was dropped. That is the reverse of #79.
runner-lifecycle.md	Rule 1 gains "A stop inside a user action waits for its commit". The race table gains two rows for #79.

Validation (local, head 7b4de20)

• npm run typecheck passes.

• Runner and Store files (runner-coordinator, runner-lifecycle-store, runner-shutdown, runner-execution, runner-recovery, store): 263 of 263 pass.

• Full suite without the two real-Docker files: 1247 pass, and 2 failed under load: question-leftovers (a question-leftovers test) and review (one test took 36 s). Both files pass alone, 61 of 61. This change does not touch them.

• 16 new tests. 14 were checked to fail before their fix: the 7 coordinator behaviour tests, the 4 Store afterCommit tests, the two COMMIT-failure tests (with committed set before COMMIT), the auto-rollback ROLLBACK test, the autocommit guard test and the read-failure test. The two remaining tests guard behaviour the issue says to keep: a stop whose write fails still stops D at once.

• Merge of main (F2b: durable safety findings, failed-run audit and partial output (#87, part 2) #94, Ask: recover only this review's Docker leftovers by owner (#65) #95) at 7f890fc: one test-file conflict, where both sides added test blocks, resolved by keeping both. The typecheck passes, and the runner and Store files pass 277 of 277. F2b: durable safety findings, failed-run audit and partial output (#87, part 2) #94's new stop paths go through #requestStop. Its new error handling (the failed-run audit and partial export) runs outside Store transactions.

Review rounds

Round	Fixed	Declined
Independent subagent review	• Outside a transaction, a callback's throw reaches the caller again, as before. • A failed read of the durable reason shows the stop as unsaved, not saved. • Comments and docs no longer say "storage error" where any throw counts, or "nothing in memory changes". • Docs say a failed COMMIT drops the stop.	• Dropping the stop when COMMIT fails: the issue asks for this.
/code-review high 1	• A test for COMMIT itself failing (Store and coordinator). • Removed a dead !job.attemptId guard. • Tests use vi.spyOn. • No callback list is allocated for a transaction without callbacks.	• A throwing handle.cancel in a commit callback: D's cancel records its reason in its first statement and does not throw (agents/adapters/supervisor.ts).
2	• ROLLBACK only while a transaction is open. • Mocks are restored in finally and in afterEach. • The two afterCommit describe blocks are merged.	None
3	• #run refuses writes after SQLite's own rollback.	None
4	Nothing new.	None
Copilot (3 reviews: twice on 7b4de20, once on 7f890fc)	No findings.	None

Review lessons (merge gate)

• Stop applied before its commit (the issue itself): covered by AGENTS.md "Never apply a background response without proving it is still current". The new rule 1 paragraph in runner-lifecycle.md states it for the coordinator.
• ROLLBACK masking a COMMIT error, and autocommit after SQLite's own rollback: one-off. Both are Store transaction mechanics. They are now enforced in #transaction and #run, so every caller gets the fix, and a rule would only repeat the code.
• Callback throw at depth 0, and read failure shown as saved: covered by "Preserve the original timeout, cancellation, and shutdown reason through every layer" and "Review readiness" (trace each claim through error paths).
• Mock hygiene in tests: one-off test cleanup.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟢 Approval recommended

The transaction and coordinator changes consistently address #79 with focused regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Defers runner stop effects until the enclosing Store transaction commits, preventing rolled-back cancellations from stopping active work.

Changes:

• Adds commit/rollback callbacks and guards against SQLite auto-rollback.
• Defers coordinator cancellation effects while preserving first-reason semantics.
• Adds lifecycle documentation and regression coverage.

File	Description
runner/​store.ts	Adds transaction callbacks and rollback safeguards.
runner/​coordinator.ts	Applies stop effects only after commit.
test/​runner-lifecycle-store.test.ts	Tests transaction callback behavior.
test/​runner-coordinator.test.ts	Tests cancellation commit and rollback races.
docs/​implementation/​runner-lifecycle.md	Documents the revised lifecycle guarantees.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟢 Approval recommended

The transactional behavior is consistent, documented, and covered by targeted race and failure-path tests.

Review effort: Balanced
Findings: None

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation matches issue #79 and covers commit, rollback, storage-failure, and ordering paths.

Review effort: Balanced
Findings: None