F1: apply a stop only after the caller's transaction commits (#79)

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)
 

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<typeof setTimeout>[]; done?: Promise<void>;
 }
 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) => {

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<T>(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);