F2b: durable safety findings, failed-run audit and partial output (#87, part 2)

docs/implementation/runner-lifecycle.md

@@ -230,6 +230,19 @@ codeboost never writes the user's repository. The commits it makes live in a **r
 | Review | Once a task has runner commits (a completed execute or fix attempt that made a commit), the review screen and Ask read it from the runner-owned repository at the head the Store recorded. The user's HEAD is no longer observed for it: runner commits move the head only through the Store, with their ledger entries, and observing the user's older HEAD would roll the task back. A task without runner commits still observes the user's HEAD, owned ledger entries or not: a reviewed branch in the user's repository (the demo, a planted experiment) carries them too. |
 
 
+## Safety findings and partial output (#87 part 2)
+
+| What | How it works |
+|---|---|
+| A safety finding | The runner's own audit records it on the attempt (`recordSafetyFinding`), through the shutdown capability, before the terminal write. The terminal write then sends the task to needs human in the same transaction, whatever outcome wins: a stop or a stale context does not undo what the agent did. A pending cancel still closes the task. A task cannot change status while it has an active attempt, so no human gate can hold the finding back. The text stays on the row (`safetyFinding`). |
+| A finding a crash left | Startup recovery acts on it the same way, and does not requeue the task. |
+| Who started the attempt | It does not matter: the Store acts on the finding, so an attempt started through `/api/runner` start or retry goes to needs human too, and admission refuses new work from needs human. |
+| A finding whose save failed | Held in memory (`SafetyFindings`) and acted on by `ItemExecutor`, as before: owed at a human gate until the task leaves it. |
+| A failed run | When the agent ends badly on its own (a non-zero exit, a D stop reason, or a result that is not this attempt's, with no first reason), `auditFailed` inspects and audits what it left before the terminal write. A violation, or an inspection that refuses, is a finding. Nothing is committed. A run a person stopped, that went stale, hit the time limit or was shut down is not audited. |
+| A `through-link` declared link | Never launched: preparation records a finding and fails, so the task goes to needs human. |
+| Partial output | For a writable attempt that did not complete, `exportPartial` saves D's bounded diff (`exportTaskDiff`, 60 s deadline, never cut short by a stop) as `<diagnostics>/<attemptId>.diff`, owner-only, written through an exclusive temporary file. The terminal write stores it as `diagnostic_ref`. A failed export adds "Partial output could not be exported: <reason>" (quoted) to the diagnostic instead. |
+| Retention | Over the cap (256 MiB by default), unreferenced files go first, oldest first. Then the oldest referenced file: its row's `diagnostic_ref` is cleared and "(partial output removed by retention)" is appended, in one transaction, and only after that commit is the file deleted. No row ever points at a missing file. |
+
 ## Shutdown
 
 `web/server.ts` `close()` and the runner coordinator follow this order. Part of step 1 and all of steps 2 and 3 already exist. F1 adds the coordinator barrier in step 1, steps 4 and 5, and step 8.

runner/coordinator.ts

@@ -49,6 +49,18 @@ export interface RunnerDeps {
    * saved with `completed` in one transaction. Throw FinishFailure with an actionable diagnostic to fail the attempt.
    */
   finish?(attempt: AttemptRecord, result: InvocationResult, prepared: PreparedAttempt, signal: AbortSignal): Promise<{ value: unknown; history?: HistoryRecord }>;
+  /**
+   * Optional, for writable attempts whose agent ended badly on its own (a non-zero exit, a D stop reason, or a result
+   * that is not this attempt's, with no first reason): check what it left before the terminal write, recording any safety finding durably, so a failed run that
+   * did something unsafe goes to a person instead of being retried (#87 item 2). It commits nothing and never throws
+   * past its own failures: the attempt fails either way.
+   */
+  auditFailed?(attempt: AttemptRecord, prepared: PreparedAttempt, signal: AbortSignal): Promise<void>;
+  /**
+   * Optional, for writable attempts that did not complete: save their partial output before the terminal write, which
+   * stores the reference (`diagnostic_ref`). It bounds itself and reports a failure instead of throwing.
+   */
+  exportPartial?(attempt: AttemptRecord, prepared: PreparedAttempt): Promise<{ diagnosticRef?: string; failure?: string }>;
   /** Optional: remove task storage after the terminal write and before the slot is freed. A failure keeps the slot under a marker. */
   release?(attempt: AttemptRecord, prepared: PreparedAttempt): Promise<void>;
   now?(): number;
@@ -317,8 +329,9 @@ export class RunnerCoordinator {
       // Accept only the result of this exact invocation, as the question path does. Anything else is never validated
       // or saved: the attempt fails closed.
       if (result.attemptId !== attempt.id || !result.context || !sameContext(result.context, attempt.context)) {
-        const foreignSaved = this.#settle(job, { stopReason: 'capture-failure', exitCode: null, signal: null, valid: false,
-          detail: job.firstReason === 'stale' ? job.staleCause : FOREIGN_RESULT });
+        // The result is not this attempt's, but the storage is: what the agent left is audited and kept all the same.
+        const evidence = await this.#keepEvidence(job, attempt, prepared, job.firstReason === 'stale' ? job.staleCause : FOREIGN_RESULT, true);
+        const foreignSaved = this.#settle(job, { stopReason: 'capture-failure', exitCode: null, signal: null, valid: false, ...evidence });
         job.decided = true;
         // Task storage and host-side preparation files wait for the terminal write, as on every other path.
         if (foreignSaved) await this.#release(job, attempt, prepared);
@@ -336,7 +349,10 @@ export class RunnerCoordinator {
       }
       // A stale stop keeps its own cause; the agent's stderr is not a reason the attempt went stale.
       if (job.firstReason === 'stale') detail = job.staleCause;
-      const saved = this.#settle(job, { stopReason: result.stopReason, exitCode: result.exitCode, signal: result.signal, valid, result: value, detail, history });
+      // A clean exit was audited by finish already; only a run that ended badly on its own is audited here.
+      const clean = !job.firstReason && result.exitCode === 0 && !result.stopReason;
+      const evidence = valid ? { detail } : await this.#keepEvidence(job, attempt, prepared, detail, !clean);
+      const saved = this.#settle(job, { stopReason: result.stopReason, exitCode: result.exitCode, signal: result.signal, valid, result: value, history, ...evidence });
       job.decided = true;
       // Task storage goes after the terminal write too; a failed removal holds the slot under a marker.
       if (saved) await this.#release(job, attempt, prepared);
@@ -350,6 +366,26 @@ export class RunnerCoordinator {
       if (this.#jobs.get(job.key) === job) this.#jobs.delete(job.key);
     }
   }
+  /**
+   * For an attempt that ran and did not complete, before its terminal write: audit what the agent left when it ended
+   * badly on its own (no first reason), recording any safety finding, and keep its partial output for diagnosis. Returns
+   * the terminal write's detail and `diagnosticRef`.
+   */
+  async #keepEvidence(job: Job, attempt: AttemptRecord, prepared: PreparedAttempt, detail: string | undefined, audit: boolean)
+    : Promise<{ detail?: string; diagnosticRef?: string }> {
+    if (audit && !job.firstReason && this.#deps.auditFailed) {
+      // The attempt fails whatever this finds.
+      try { await this.#deps.auditFailed(attempt, prepared, job.controller.signal); }
+      catch (error) { console.error(`Runner job ${job.attemptId} could not audit its failed run: ${JSON.stringify(message(error))}`); }
+    }
+    if (!this.#deps.exportPartial) return { detail };
+    let exported: { diagnosticRef?: string; failure?: string };
+    try { exported = await this.#deps.exportPartial(attempt, prepared); }
+    catch (error) { exported = { failure: message(error) }; }
+    // The reason can quote agent-chosen paths: quoted, like the agent's own text.
+    return { diagnosticRef: exported.diagnosticRef, detail: exported.failure
+      ? bounded(`${detail ?? ''} Partial output could not be exported: ${JSON.stringify(exported.failure)}`.trim()) : detail };
+  }
   /** D gave up on cleanup (presence, not length, is the signal): fail closed until restart, as Ask does. */
   #noteUnreleased(result: InvocationResult): void {
     if (result.unreleased === undefined) return;
@@ -396,7 +432,7 @@ export class RunnerCoordinator {
       if (!this.#markers.has(job.key)) this.#markers.set(job.key, { group: job.group, attemptId: job.attemptId, reason: 'storage-not-removed' });
     }
   }
-  #settle(job: Job, s: { stopReason?: StopReason; exitCode: number | null; signal: string | null; valid: boolean; result?: unknown; detail?: string; history?: HistoryRecord }): Classification | undefined {
+  #settle(job: Job, s: { stopReason?: StopReason; exitCode: number | null; signal: string | null; valid: boolean; result?: unknown; detail?: string; history?: HistoryRecord; diagnosticRef?: string }): Classification | undefined {
     try {
       return this.#write(() => this.#store.settleAttempt(job.identity, job.attemptId, { ...s, firstReason: job.firstReason }));
     } catch {

runner/diagnostics.ts

@@ -0,0 +1,55 @@
+import { randomUUID } from 'node:crypto';
+import { lstatSync, readdirSync, renameSync, rmSync, writeFileSync } from 'node:fs';
+import { join } from 'node:path';
+import { isUuidV4 } from './lifecycle.ts';
+import type { Store } from './store.ts';
+
+/** The total the diagnostics directory may hold before retention deletes files. */
+export const DEFAULT_DIAGNOSTICS_CAP_BYTES = 256 * 1024 * 1024;
+const NAME = /^([0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})\.diff$/;
+
+/**
+ * Save a stopped attempt's partial output as `<directory>/<attemptId>.diff` (runner-lifecycle.md, the task storage
+ * row), then apply retention. The file is written through a unique temporary file opened exclusively, owner-only, and
+ * renamed into place; a failed write leaves nothing. Returns the path the attempt row references as `diagnostic_ref`.
+ */
+export function saveDiagnostic(store: Store, directory: string, attemptId: string, bytes: Buffer,
+  capBytes = DEFAULT_DIAGNOSTICS_CAP_BYTES): string {
+  if (!isUuidV4(attemptId)) throw new Error('Attempt ID must be a UUID v4.');
+  if (!Number.isSafeInteger(capBytes) || capBytes < 1) throw new Error('The diagnostics cap must be a positive integer.');
+  const path = join(directory, `${attemptId}.diff`), temporary = join(directory, `.${attemptId}.${randomUUID()}.tmp`);
+  try {
+    writeFileSync(temporary, bytes, { mode: 0o600, flag: 'wx' });
+    renameSync(temporary, path);
+  } catch (error) { rmSync(temporary, { force: true }); throw error; }
+  // The saved file stands even if retention cannot run now (the write gate closed): the next save retains again.
+  try { retain(store, directory, capBytes, path); }
+  catch (error) { console.error(`Diagnostics retention did not run: ${JSON.stringify(error instanceof Error ? error.message : String(error))}`); }
+  return path;
+}
+
+/**
+ * Retention never deletes a file an attempt row still references. Over the cap, it deletes unreferenced files oldest
+ * first; if that is not enough, it takes the oldest referenced file and, in one transaction, clears that row's
+ * `diagnostic_ref` and notes the removal in its diagnostic, and only after that commit deletes the file. The file just
+ * saved (`keep`) is never removed: it is not referenced until the terminal write.
+ */
+export function retain(store: Store, directory: string, capBytes: number, keep?: string): void {
+  const files = readdirSync(directory).flatMap(name => {
+    const match = NAME.exec(name), path = join(directory, name);
+    if (!match) return [];
+    const stat = lstatSync(path, { throwIfNoEntry: false });
+    return stat?.isFile() ? [{ path, attemptId: match[1]!, size: stat.size, mtime: stat.mtimeMs }] : [];
+  }).sort((a, b) => a.mtime - b.mtime);
+  let total = files.reduce((sum, file) => sum + file.size, 0);
+  const referenced = new Set(store.referencedDiagnostics());
+  for (const unreferenced of [true, false]) {
+    for (const file of files) {
+      if (total <= capBytes) return;
+      if (file.path === keep || referenced.has(file.path) === unreferenced) continue;
+      if (!unreferenced && !store.forgetDiagnostic(file.path)) continue;
+      rmSync(file.path, { force: true });
+      total -= file.size;
+    }
+  }
+}