diff --git a/docs/implementation/runner-lifecycle.md b/docs/implementation/runner-lifecycle.md index c7d65cb7..27ed903d 100644 --- a/docs/implementation/runner-lifecycle.md +++ b/docs/implementation/runner-lifecycle.md @@ -155,6 +155,8 @@ In every case, a failed terminal write leaves an unresolved marker (see "Slots a 1. **Stop requests do not end the attempt.** A cancel, hard stop, shutdown, time limit or detected staleness sets the first reason on the in-memory job, writes it onto the row, and calls `handle.cancel(...)`. The row stays `running`, and the UI shows "Stopping". **The in-memory job is the source of the first reason until the terminal write.** The terminal write stores the job's first reason and the terminal state together in one transaction. So if the earlier reason write fails, nothing is lost while the process lives, and the UI shows "Stopping (not saved yet)". If that terminal write also fails, the unresolved marker keeps the reason in memory. The one case that loses it is a crash after a failed reason write and before a successful terminal write. Startup recovery then records `failed` "Interrupted" and adds "a stop may have been requested" to the diagnostic. It cannot recover a reason that was never saved. + + **A stop inside a user action waits for its commit** (#79). Cancel attempt and cancel task run inside `Store.userAction`, so their reason write is part of the caller's transaction. The in-memory effect cannot be undone: the job's first reason, the aborted preparation and `handle.cancel(...)`. So the coordinator applies it only after that transaction commits, through `Store.afterCommit`. If the transaction rolls back, including when COMMIT itself fails, the stop is dropped: D keeps running, the row has no first reason, and the task stays open. Until the commit, the stop already wins over any later stop in the same transaction. A stop outside a transaction (timers, shutdown, the launch check) applies at once, because `afterCommit` then runs its callback immediately. If the reason write itself throws, the stop also applies at once, shown as unsaved, and the in-memory reason is the source of truth as described above. 2. **The terminal state comes from the first reason.** When `settled` resolves, the terminal state is chosen in this order: | First reason recorded by F | D result | Terminal state | @@ -552,6 +554,8 @@ Each case needs a test that fails before the fix and passes after it. Each test | Human-gated change during settlement (review round 35) | Attempt running → a request to move the task to `needs amendment` → refused while the attempt is active; with the status forced by raw SQL → a valid result ends `cancelled`, never `completed` | | Stop after storage allocation, before launch (review round 36) | Task storage allocated → cancel before the D start call → host-side files removed → storage still present → terminal write → `removeTaskFilesystems` → slot freed; with the terminal write failing, the storage remains for recovery | | Old attempt settles after a retry (D/F contract) | Attempt A cancelled and settled → retry B admitted → a late publish from A is refused → B's row and the visible status are unchanged | +| Stop rolled back with its user action (#79) | Attempt running → cancel task, or a plain stop, inside `userAction` → the action throws and rolls back → `handle.cancel` was never called and preparation was not aborted → D returns a valid result → row `completed`, task `running` with no `cancel_requested` | +| Stop applied only after its commit (#79) | Attempt running → cancel task inside `userAction` → no `handle.cancel` before the commit → after it, `handle.cancel('cancelled')`, then row `cancelled` and task `cancelled`. Two stops in one action: the first (`stale`) wins | ## Decisions (approved 2026-09-25) diff --git a/runner/coordinator.ts b/runner/coordinator.ts index 1e801ec2..f1f048e9 100644 --- a/runner/coordinator.ts +++ b/runner/coordinator.ts @@ -91,6 +91,11 @@ interface Job { decided?: boolean; /** A cancel task the Store recorded on a job that no longer takes stops; shown in status only. */ cancelShown?: boolean; + /** + * A stop whose reason write is waiting for the caller's transaction (userAction) to commit. It already wins over a + * later stop, but the job's reason, its abort signal and D's handle change only on commit; a rollback drops it (#79). + */ + pendingReason?: FirstReason; controller: AbortController; handle?: InvocationHandle; timers: ReturnType[]; done?: Promise; } interface Marker { group: Group; attemptId: string; reason: UnresolvedReason } @@ -171,34 +176,29 @@ export class RunnerCoordinator { stop(identity: PlanIdentity, attemptId: string, reason: 'cancelled' | 'stale', cause?: string): boolean { const job = this.#jobs.get(identityKey(identity)); if (!job || job.attemptId !== attemptId) return false; - const won = this.#requestStop(job, reason); - if (won && reason === 'stale' && cause !== undefined) job.staleCause = bounded(cause); - return won; + return this.#requestStop(job, reason, reason === 'stale' && cause !== undefined ? bounded(cause) : undefined); } /** Cancel task: the Store records the reason and the pending close; the coordinator stops the running work. */ cancelTask(identity: PlanIdentity, expectedStateVersion: number, actionId: string): 'closed' | 'stopping' { const job = this.#jobs.get(identityKey(identity)); // The Store writes `cancelled` wherever the row has no reason yet, so save an earlier unsaved reason first. That write // bumps the state version, so it is made only when the caller's version is current, and the cancel then uses the new one. + // It counts as saved only once the caller's transaction commits. if (job?.firstReason && !job.reasonSaved) { try { if (this.#store.getTask(identity).stateVersion === expectedStateVersion && this.#store.recordFirstReason(job.identity, job.attemptId, job.firstReason)) { - job.reasonSaved = true; this.#confirmSaved(job); + this.#store.afterCommit(() => { job.reasonSaved = true; }); expectedStateVersion = this.#store.getTask(identity).stateVersion; } } catch { /* still unsaved; the Store's own write below is likely to fail the same way */ } } const outcome = this.#store.cancelTask(identity, expectedStateVersion, actionId); - if (outcome === 'stopping' && job && !this.#requestStop(job, 'cancelled') && !job.firstReason) { + if (outcome === 'stopping' && job && !this.#requestStop(job, 'cancelled') && !job.firstReason && !job.pendingReason) { // The Store already wrote `cancelled` onto the row (a pending cancel task wins, even over a preparation timeout). - // Status shows it, but it never becomes the job's reason: the write may roll back with the caller's transaction, - // and the terminal write reads the row's own reason anyway. - job.cancelShown = true; - queueMicrotask(() => { - try { if (this.#store.getAttempt(job.identity, job.attemptId).firstReason !== 'cancelled') job.cancelShown = false; } - catch { /* unreadable: keep showing what the Store reported */ } - }); + // Status shows it once the write commits, but it never becomes the job's reason: the terminal write reads the + // row's own reason. + this.#store.afterCommit(() => { job.cancelShown = true; }); } return outcome; } @@ -225,34 +225,39 @@ export class RunnerCoordinator { await Promise.all(jobs.map(job => job.done)); } - #requestStop(job: Job, reason: FirstReason): boolean { + /** + * Record the first reason, then set it on the job, abort preparation and cancel D. The write may be part of the + * caller's transaction (userAction), so the in-memory effect, which cannot be undone, waits for it to commit and is + * dropped if it rolls back, including when COMMIT fails (#79). A reason write that throws applies the stop at once, + * unsaved: the in-memory reason is then the source of truth ("Rules for the running state", rule 1). + */ + #requestStop(job: Job, reason: FirstReason, staleCause?: string): boolean { // The attempt deadline passed before launch: it ends `failed` with no first reason, so a later stop cannot claim it. if (job.decided || (job.preparationTimedOut && !job.firstReason)) return false; if (job.firstReason) { job.handle?.cancel(D_REASON[job.firstReason]); return false; } - job.firstReason = reason; - try { - if (job.attemptId && !this.#write(() => this.#store.recordFirstReason(job.identity, job.attemptId, reason))) { - // Another writer (for example cancel task) recorded a reason first; adopt the durable one. - const durable = this.#store.getAttempt(job.identity, job.attemptId).firstReason; - if (durable) job.firstReason = durable; - } - job.reasonSaved = true; this.#confirmSaved(job); - } catch { job.reasonSaved = false; } - job.controller.abort(new Error(`Stopped: ${job.firstReason}`)); - job.handle?.cancel(D_REASON[job.firstReason]); + // An earlier stop in the same transaction wins; its commit applies it. + if (job.pendingReason) return false; + const apply = (first: FirstReason, saved: boolean) => { + job.firstReason = first; job.reasonSaved = saved; + if (staleCause !== undefined) job.staleCause = staleCause; + job.controller.abort(new Error(`Stopped: ${first}`)); + job.handle?.cancel(D_REASON[first]); + }; + let changed: boolean; + try { changed = this.#write(() => this.#store.recordFirstReason(job.identity, job.attemptId, reason)); } + catch { apply(reason, false); return true; } + job.pendingReason = reason; + this.#store.afterCommit(() => { + job.pendingReason = undefined; + if (changed) return apply(reason, true); + // Another writer (for example cancel task) recorded a reason first; adopt the durable one. If it cannot be read, + // keep this stop's reason, shown as unsaved, as before #79. + let durable: FirstReason | null; + try { durable = this.#store.getAttempt(job.identity, job.attemptId).firstReason; } catch { return apply(reason, false); } + apply(durable ?? reason, true); + }, () => { job.pendingReason = undefined; }); return true; } - /** - * A reason write may be part of the caller's transaction (userAction) and roll back with it. Once that transaction - * has ended, check the row, so an undone write shows as unsaved and the next cancel task saves it again. - */ - #confirmSaved(job: Job): void { - queueMicrotask(() => { - if (!job.firstReason || !job.reasonSaved) return; - try { if (this.#store.getAttempt(job.identity, job.attemptId).firstReason !== job.firstReason) job.reasonSaved = false; } - catch { /* unreadable: keep what the write reported */ } - }); - } /** Task budget and, before launch, the attempt deadline. D enforces the deadline once it runs. */ #arm(job: Job, attempt: AttemptRecord): void { const at = (when: number, fire: () => void) => { diff --git a/runner/store.ts b/runner/store.ts index 604fdc8f..6d85e4b3 100644 --- a/runner/store.ts +++ b/runner/store.ts @@ -147,7 +147,14 @@ export class Store { } close(): void { this.#db.close(); } #get(sql: string, ...args: SQLInputValue[]) { return this.#db.prepare(sql).get(...args); } - #run(sql: string, ...args: SQLInputValue[]) { this.#checkWrite(); return this.#db.prepare(sql).run(...args); } + #run(sql: string, ...args: SQLInputValue[]) { + this.#checkWrite(); + // Some errors (for example a full disk) make SQLite roll back on its own. If the caller caught one, a later write must + // not autocommit on its own while the rest of the transaction is lost. + if (this.#depth > 0 && !this.#db.isTransaction) + throw Object.assign(new Error('SQLite rolled back the transaction after an earlier error.'), { code: 'ERR_SQLITE_ERROR' }); + return this.#db.prepare(sql).run(...args); + } // Shutdown write gate (runner-lifecycle.md, "Shutdown" step 1). #gateClosed = false; #privileged = 0; #capabilityIssued = false; #checkWrite(): void { if (this.#gateClosed && this.#privileged === 0) throw new ShuttingDownError(); } @@ -163,14 +170,40 @@ export class Store { #depth = 0; /** The user action whose transaction is open, so its events can prove they belong to it. */ #action: { key: string; actionId: string } | null = null; + /** Callbacks waiting for the outermost transaction to end, in the order they were registered. */ + #pending: { commit: () => void; rollback?: () => void }[] = []; /** Nested calls join the outer transaction, so a user action can wrap existing Store methods atomically. */ #transaction(fn: () => T): T { if (this.#depth > 0) { this.#depth++; try { return fn(); } finally { this.#depth--; } } this.#checkWrite(); this.#db.exec('BEGIN IMMEDIATE'); this.#depth = 1; - try { const result = fn(); this.#db.exec('COMMIT'); return result; } - catch (error) { this.#db.exec('ROLLBACK'); throw error; } - finally { this.#depth = 0; } + let result: T, committed = false; + try { result = fn(); this.#db.exec('COMMIT'); committed = true; } + // A failed COMMIT may already have rolled back (for example a full disk); ROLLBACK would then hide the real error. + catch (error) { if (this.#db.isTransaction) this.#db.exec('ROLLBACK'); throw error; } + finally { + this.#depth = 0; + if (this.#pending.length) { + const pending = this.#pending; this.#pending = []; + for (const entry of pending) Store.#callback(committed ? entry.commit : entry.rollback); + } + } + return result; + } + /** A callback runs after the transaction has ended; a throw must not make a committed write look failed. */ + static #callback(fn: (() => void) | undefined): void { + if (!fn) return; + try { fn(); } catch (error) { console.error(`A transaction callback failed: ${JSON.stringify(bounded(error instanceof Error ? error.message : String(error)))}`); } + } + /** + * Run `commit` once the writes made so far are durable: at once outside a transaction (a throw then reaches the + * caller), otherwise after the outermost transaction commits (a throw is logged). If it rolls back instead, including + * when COMMIT itself fails, `commit` is dropped and `rollback` runs. Lets a caller apply an in-memory effect that cannot + * be undone only once the write that records it has committed (#79). + */ + afterCommit(commit: () => void, rollback?: () => void): void { + if (this.#depth === 0) commit(); + else this.#pending.push({ commit, rollback }); } #current(key: string) { const row = this.#get('SELECT * FROM plans WHERE key=?', key); diff --git a/test/runner-coordinator.test.ts b/test/runner-coordinator.test.ts index cf9d9431..e8e6f3de 100644 --- a/test/runner-coordinator.test.ts +++ b/test/runner-coordinator.test.ts @@ -2,6 +2,7 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { randomUUID } from 'node:crypto'; +import { DatabaseSync } from 'node:sqlite'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { Store } from '../runner/store.ts'; import { RunnerCoordinator, type PreparedAttempt, type RunnerDeps, type StartRequest } from '../runner/coordinator.ts'; @@ -670,3 +671,123 @@ describe('cancel task, limits and shutdown', () => { expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'stale', firstReason: 'stale' }); }); }); + +describe('stops inside the caller\'s transaction (#79)', () => { + async function running() { + const s = setup(); + const attempt = s.runner.start(A, request(s.store, A)); + await until(() => s.preparations.length === 1, 'preparation'); s.preparations[0]!.resolve(); + await until(() => s.launches.length === 1, 'launch'); + return { ...s, attempt }; + } + const rolledBack = (store: Store, kind: string, fn: () => void) => expect(() => store.userAction(A, { actionId: randomUUID(), kind, request: {} }, () => { + fn(); throw new Error('commit failed'); + })).toThrow(/commit failed/); + + it('leaves D running and the task open when a cancel task rolls back', async () => { + const { store, runner, launches, attempt } = await running(); + rolledBack(store, 'cancel-task', () => expect(runner.cancelTask(A, store.getTask(A).stateVersion, randomUUID())).toBe('stopping')); + await tick(); + expect(launches[0]!.cancels).toEqual([]); + expect(runner.status(A).stopRequested).toBeNull(); + launches[0]!.settle(); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'completed', firstReason: null, result: { text: 'done' } }); + expect(store.getTask(A)).toMatchObject({ status: 'running', cancelRequested: null }); + }); + it('leaves D running when a plain stop rolls back, and a later stop still works', async () => { + const { store, runner, launches, attempt } = await running(); + rolledBack(store, 'cancel-attempt', () => expect(runner.stop(A, attempt.id, 'stale', 'plan revision 2 replaced 1')).toBe(true)); + await tick(); + expect(launches[0]!.cancels).toEqual([]); + expect(runner.status(A).stopRequested).toBeNull(); + expect(store.getAttempt(A, attempt.id).firstReason).toBeNull(); + expect(runner.stop(A, attempt.id, 'cancelled')).toBe(true); + expect(launches[0]!.cancels).toEqual(['cancelled']); + launches[0]!.settle({ exitCode: 1, stopReason: 'cancelled' }); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'cancelled', firstReason: 'cancelled' }); + }); + it('leaves D running when the cancel task\'s COMMIT fails', async () => { + const { store, runner, launches, attempt } = await running(); + const exec = DatabaseSync.prototype.exec; + const spy = vi.spyOn(DatabaseSync.prototype, 'exec').mockImplementation(function (this: DatabaseSync, sql: string) { + if (sql === 'COMMIT') throw Object.assign(new Error('disk I/O error'), { code: 'ERR_SQLITE_ERROR' }); + return exec.call(this, sql); + }); + try { + expect(() => store.userAction(A, { actionId: randomUUID(), kind: 'cancel-task', request: {} }, () => { + expect(runner.cancelTask(A, store.getTask(A).stateVersion, randomUUID())).toBe('stopping'); + })).toThrow(/disk I\/O error/); + } finally { spy.mockRestore(); } + expect(launches[0]!.cancels).toEqual([]); + expect(runner.status(A).stopRequested).toBeNull(); + launches[0]!.settle(); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'completed', firstReason: null }); + expect(store.getTask(A)).toMatchObject({ status: 'running', cancelRequested: null }); + }); + it('does not abort preparation when a stop during preparation rolls back', async () => { + const { store, runner, preparations, launches } = setup(); + const attempt = runner.start(A, request(store, A)); + await until(() => preparations.length === 1, 'preparation'); + rolledBack(store, 'cancel-task', () => runner.cancelTask(A, store.getTask(A).stateVersion, randomUUID())); + expect(preparations[0]!.signal.aborted).toBe(false); + preparations[0]!.resolve(); + await until(() => launches.length === 1, 'launch'); + launches[0]!.settle(); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id).state).toBe('completed'); + }); + it('cancels D only after a committed cancel task commits', async () => { + const { store, runner, launches, attempt } = await running(); + store.userAction(A, { actionId: randomUUID(), kind: 'cancel-task', request: {} }, () => { + expect(runner.cancelTask(A, store.getTask(A).stateVersion, randomUUID())).toBe('stopping'); + expect(launches[0]!.cancels).toEqual([]); + expect(runner.status(A).stopRequested).toBeNull(); + }); + expect(launches[0]!.cancels).toEqual(['cancelled']); + expect(runner.status(A).stopRequested).toEqual({ attemptId: attempt.id, reason: 'cancelled', saved: true }); + launches[0]!.settle({ exitCode: 1, stopReason: 'cancelled' }); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'cancelled', firstReason: 'cancelled' }); + expect(store.getTask(A).status).toBe('cancelled'); + }); + it('lets the first stop in a transaction win while its commit is pending', async () => { + const { store, runner, launches, attempt } = await running(); + store.userAction(A, { actionId: randomUUID(), kind: 'cancel-task', request: {} }, () => { + expect(runner.stop(A, attempt.id, 'stale', 'plan revision 2 replaced 1')).toBe(true); + expect(runner.stop(A, attempt.id, 'cancelled')).toBe(false); + expect(runner.cancelTask(A, store.getTask(A).stateVersion, randomUUID())).toBe('stopping'); + expect(launches[0]!.cancels).toEqual([]); + }); + expect(launches[0]!.cancels).toEqual(['cancelled']); + expect(runner.status(A).stopRequested).toEqual({ attemptId: attempt.id, reason: 'stale', saved: true }); + launches[0]!.settle({ exitCode: 1, stopReason: 'cancelled' }); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'stale', firstReason: 'stale', diagnostic: 'plan revision 2 replaced 1' }); + expect(store.getTask(A).status).toBe('cancelled'); + }); + it('shows the stop as unsaved when the durable reason cannot be read after the commit', async () => { + const { store, runner, launches, attempt } = await running(); + store.userAction(A, { actionId: randomUUID(), kind: 'cancel-task', request: {} }, () => { + runner.cancelTask(A, store.getTask(A).stateVersion, randomUUID()); + vi.spyOn(store, 'getAttempt').mockImplementationOnce(() => { throw Object.assign(new Error('disk'), { code: 'ERR_SQLITE_ERROR' }); }); + }); + expect(launches[0]!.cancels).toEqual(['cancelled']); + expect(runner.status(A).stopRequested).toEqual({ attemptId: attempt.id, reason: 'cancelled', saved: false }); + launches[0]!.settle({ exitCode: 1, stopReason: 'cancelled' }); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'cancelled', firstReason: 'cancelled' }); + }); + it('still stops D at once when the reason write fails with a storage error, even if the action then rolls back', async () => { + const { store, runner, launches, attempt } = await running(); + vi.spyOn(store, 'recordFirstReason').mockImplementationOnce(() => { throw Object.assign(new Error('disk'), { code: 'ERR_SQLITE_ERROR' }); }); + rolledBack(store, 'cancel-attempt', () => expect(runner.stop(A, attempt.id, 'cancelled')).toBe(true)); + expect(launches[0]!.cancels).toEqual(['cancelled']); + expect(runner.status(A).stopRequested).toEqual({ attemptId: attempt.id, reason: 'cancelled', saved: false }); + launches[0]!.settle({ exitCode: 1, stopReason: 'cancelled' }); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'cancelled', firstReason: 'cancelled' }); + }); +}); diff --git a/test/runner-lifecycle-store.test.ts b/test/runner-lifecycle-store.test.ts index 8ceda1d6..840d03e1 100644 --- a/test/runner-lifecycle-store.test.ts +++ b/test/runner-lifecycle-store.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { randomUUID } from 'node:crypto'; import { DatabaseSync } from 'node:sqlite'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { Store, mergeActionResponse } from '../runner/store.ts'; import { ActionIdReused, GuardRefusal, classifySettlement, requestHash } from '../runner/lifecycle.ts'; import type { Plan, PlanContext } from '../core/plan.ts'; @@ -13,7 +13,7 @@ const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a', ki const plan = (summary = 'Example'): Plan => ({ schema_version: 1, revision: 1, issue: 1, summary, questions: [], items: [{ id: 'P1', title: 'Change', intent: 'Improve', files: [{ path: 'a', kind: 'edit', renamed_from: null, change: 'Change' }], acceptance: [{ type: 'check', text: 'Works' }], depends_on: [] }] }); const oid = (n: number) => n.toString(16).padStart(40, '0'); const dirs: string[] = [], stores: Store[] = []; -afterEach(() => { for (const store of stores.splice(0)) store.close(); for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); +afterEach(() => { vi.restoreAllMocks(); for (const store of stores.splice(0)) store.close(); for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); function open(path: string) { const store = new Store(path); stores.push(store); return store; } function fixture() { const dir = mkdtempSync(join(tmpdir(), 'codeboost-lifecycle-')); dirs.push(dir); @@ -532,6 +532,94 @@ describe('runner commits and preparation groups (#87)', () => { }); }); +describe('after commit (#79)', () => { + it('runs at once outside a transaction, after the outermost commit inside one, and drops the callback on rollback', () => { + const { store } = queued(); + const calls: string[] = []; + store.afterCommit(() => calls.push('outside')); + expect(calls).toEqual(['outside']); + store.userAction(identity, { actionId: randomUUID(), kind: 'note', request: {} }, () => { + store.afterCommit(() => calls.push('first'), () => calls.push('first rolled back')); + store.afterCommit(() => calls.push('second')); + expect(calls).toEqual(['outside']); + }); + expect(calls).toEqual(['outside', 'first', 'second']); + expect(() => store.userAction(identity, { actionId: randomUUID(), kind: 'note', request: {} }, () => { + store.afterCommit(() => calls.push('dropped'), () => calls.push('rolled back')); + throw new Error('commit failed'); + })).toThrow(/commit failed/); + expect(calls).toEqual(['outside', 'first', 'second', 'rolled back']); + }); + it('keeps a committed action committed when a callback throws, and still runs the later callbacks', () => { + const { store } = queued(); + const errors = vi.spyOn(console, 'error').mockImplementation(() => undefined); + let ran = false; + const actionId = randomUUID(); + const outcome = store.userAction(identity, { actionId, kind: 'note', request: {} }, () => { + store.afterCommit(() => { throw new Error('callback failed'); }); + store.afterCommit(() => { ran = true; }); + return 'done'; + }); + expect(outcome).toEqual({ response: 'done', replayed: false }); + expect(ran).toBe(true); + expect(errors).toHaveBeenCalledTimes(1); + expect(store.savedAction(identity, { actionId, kind: 'note', request: {} })).toEqual({ response: 'done', replayed: true }); + }); + it('drops the callback and runs the rollback one when COMMIT itself fails', () => { + const { store } = queued(); + const exec = DatabaseSync.prototype.exec; + vi.spyOn(DatabaseSync.prototype, 'exec').mockImplementation(function (this: DatabaseSync, sql: string) { + if (sql === 'COMMIT') throw Object.assign(new Error('disk I/O error'), { code: 'ERR_SQLITE_ERROR' }); + return exec.call(this, sql); + }); + const calls: string[] = [], actionId = randomUUID(); + expect(() => store.userAction(identity, { actionId, kind: 'note', request: {} }, () => { + store.afterCommit(() => calls.push('committed'), () => calls.push('rolled back')); + })).toThrow(/disk I\/O error/); + expect(calls).toEqual(['rolled back']); + expect(store.savedAction(identity, { actionId, kind: 'note', request: {} })).toBeUndefined(); + }); + it('reports the COMMIT error, not a failed ROLLBACK, when SQLite already rolled back', () => { + const { store } = queued(); + const exec = DatabaseSync.prototype.exec; + vi.spyOn(DatabaseSync.prototype, 'exec').mockImplementation(function (this: DatabaseSync, sql: string) { + if (sql !== 'COMMIT') return exec.call(this, sql); + exec.call(this, 'ROLLBACK'); + throw Object.assign(new Error('database or disk is full'), { code: 'ERR_SQLITE_ERROR' }); + }); + expect(() => store.userAction(identity, { actionId: randomUUID(), kind: 'note', request: {} }, () => 'done')).toThrow(/disk is full/); + }); + it('refuses later writes once SQLite rolled back the transaction on its own, so nothing autocommits', () => { + const { store } = queued(); const attempt = admit(store); + const prepare = DatabaseSync.prototype.prepare; + let failed = false; + vi.spyOn(DatabaseSync.prototype, 'prepare').mockImplementation(function (this: DatabaseSync, sql: string) { + if (failed || !sql.startsWith('UPDATE attempts SET first_reason')) return prepare.call(this, sql); + failed = true; + return { run: () => { this.exec('ROLLBACK'); throw Object.assign(new Error('database or disk is full'), { code: 'ERR_SQLITE_ERROR' }); } } as unknown as ReturnType; + }); + const actionId = randomUUID(), committed: string[] = []; + expect(() => store.userAction(identity, { actionId, kind: 'cancel-task', request: {} }, () => { + try { store.recordFirstReason(identity, attempt.id, 'cancelled'); } catch { /* the caller carries on, as the coordinator does */ } + store.afterCommit(() => committed.push('stop')); + return store.cancelTask(identity, store.getTask(identity).stateVersion, randomUUID()); + })).toThrow(/rolled back the transaction/); + expect(committed).toEqual([]); + expect(store.savedAction(identity, { actionId, kind: 'cancel-task', request: {} })).toBeUndefined(); + expect(store.getAttempt(identity, attempt.id).firstReason).toBeNull(); + expect(store.getTask(identity).cancelRequested).toBeNull(); + }); + it('lets a throw outside a transaction reach the caller, and runs a callback registered by a commit callback', () => { + const { store } = queued(); + expect(() => store.afterCommit(() => { throw new Error('cancel failed'); })).toThrow(/cancel failed/); + const calls: string[] = []; + store.userAction(identity, { actionId: randomUUID(), kind: 'note', request: {} }, () => { + store.afterCommit(() => { calls.push('outer'); store.afterCommit(() => calls.push('inner')); }); + }); + expect(calls).toEqual(['outer', 'inner']); + }); +}); + describe('durable safety findings (#87 item 3)', () => { it('keeps the first finding of an active attempt, and acts on it in the terminal write, whatever the outcome', () => { const { store } = queued(); const attempt = admit(store); store.markRunning(identity, attempt.id);