Stop treating msbench-cli extract's exit code as proof it extracted

evals/msbench/README.md

@@ -2657,6 +2657,28 @@ with a clear message:
   because it explained anything: the artifact scare that prompted the enumeration turned
   out to be a read that predated the run's completion by about five minutes, and member
   order was not the cause. Noted here so nobody re-derives it as one.
+- **`msbench-cli extract` reports refusal on stderr and still exits 0.** Pointed at a
+  destination that already exists and is nonempty it prints
+  `ERROR <dir> exists and is nonempty. Quitting to avoid overwriting.`, writes nothing,
+  and **exits 0** — the same status as a successful extraction. Measured on the same run
+  id, same CLI, only the destination differing:
+
+  | destination | exit code | instance output written |
+  | --- | --- | --- |
+  | fresh directory | 0 | yes |
+  | existing nonempty directory | 0 | **no** |
+
+  So `result.status !== 0` cannot tell the two apart, and any consumer relying on it
+  returns a stale cache as though it were a fresh download. This is not hypothetical: it
+  made `gate-health` report a permanent `READER FAULT` on run `2026090413313337` after a
+  single audit taken while that run was still in flight cached a partial extraction, and
+  it made `regrade`'s `--refresh` a silent no-op. `--refresh` could not clear either,
+  because it re-extracts into the same nonempty directory and hits the identical refusal.
+  Both now clear their own cache before extracting and verify that something was actually
+  written; a caller-supplied `--extract-dir` is reported rather than deleted.
+
+  The general rule this is an instance of: **an exit code is a claim, not a
+  verification.** Where a tool's output is the thing you need, check for the output.
 - **Name the field that answers your question before you read one.** Every status
   surface here has a neighbouring field that looks like the answer and isn't, and
   reading the neighbour has now cost four separate investigations in two days:

evals/msbench/extraction.ts

@@ -25,7 +25,7 @@
  */
 
 import { spawnSync } from 'node:child_process';
-import { existsSync, mkdirSync, readdirSync } from 'node:fs';
+import { existsSync, mkdirSync, readdirSync, rmSync } from 'node:fs';
 import { dirname, join, resolve } from 'node:path';
 
 /**
@@ -115,6 +115,16 @@ export function resolveExtraction(request: ExtractRequest, log: Logger = console
         args.push('--instance', wanted);
     }
 
+    // `msbench-cli extract` refuses to write into a destination that already exists and is
+    // nonempty. When the destination is this module's own cache it is derived data, safe to
+    // discard, and clearing it is what makes `--refresh` mean what it says. A caller-supplied
+    // `--extract-dir` gets no such treatment — it may point somewhere that must not be deleted,
+    // so that case is detected and reported after the call instead.
+    if (!request.extractDir) {
+        rmSync(dir, { recursive: true, force: true });
+    }
+    const destinationWasNonEmpty = existsSync(dir) && readdirSync(dir).length > 0;
+
     log(`$ msbench-cli ${args.join(' ')}`);
     const result = spawnSync('msbench-cli', args, { stdio: 'inherit' });
     if (result.error && (result.error as NodeJS.ErrnoException).code === 'ENOENT') {
@@ -130,6 +140,37 @@ export function resolveExtraction(request: ExtractRequest, log: Logger = console
             'extraction reads a stored blob and needs an Azure identity.'
         );
     }
+
+    // Exit 0 does not mean anything was written. `msbench-cli extract` reports
+    // "exists and is nonempty. Quitting to avoid overwriting." on stderr and **still exits 0**,
+    // so the status check above cannot tell a real extraction from a refused one.
+    //
+    // Left undetected that makes `--refresh` a silent no-op — the caller asks to re-download,
+    // gets the stale cache back, and is told nothing — and it lets `--instance B` be answered
+    // by a cache built from `--instance A`. Both are wrong answers rather than failures, which
+    // is worse than either an error or a slow re-download.
+    //
+    // The test is deliberately "did extraction write an instance tree at all", counting
+    // `incomplete` alongside `instances`. A tree that arrived without its `session.sqlite` is a
+    // *different* failure with its own reporting downstream — notably the Windows path-length
+    // limit that truncates extractions under a repo-nested cache — and swallowing that into this
+    // message would trade one misdiagnosis for another.
+    const found = findInstances(dir);
+    const wroteNothing = found.instances.length === 0 && found.incomplete.length === 0;
+    const missingWanted = wanted !== undefined &&
+        !found.instances.some(candidate => matchesInstance(candidate.name, wanted)) &&
+        !found.incomplete.some(name => matchesInstance(name, wanted));
+    if (wroteNothing || missingWanted) {
+        throw new MsBenchToolError(
+            `msbench-cli extract exited 0 but wrote no extraction to ${dir}` +
+            `${missingWanted ? ` matching --instance ${wanted}` : ''}.\n` +
+            (destinationWasNonEmpty
+                ? 'The destination already existed and was nonempty, so extraction was almost ' +
+                'certainly refused ("Quitting to avoid overwriting") — which still exits 0.\n' +
+                `Remove ${dir} and retry, or pass --extract-dir <empty-dir>.`
+                : `Run \`msbench-cli extract --run_id ${runId} --output <dir>\` directly to see why.`)
+        );
+    }
     return dir;
 }
 

evals/msbench/gate-health.ts

@@ -61,7 +61,7 @@
  */
 
 import { spawnSync } from 'node:child_process';
-import { existsSync, readFileSync, readdirSync } from 'node:fs';
+import { existsSync, readFileSync, readdirSync, rmSync } from 'node:fs';
 import { homedir } from 'node:os';
 import { dirname, join, resolve } from 'node:path';
 import { DatabaseSync } from 'node:sqlite';
@@ -366,6 +366,23 @@ function extractRun(runId: string, refresh: boolean): string | undefined {
         return dir;
     }
 
+    // `msbench-cli extract` refuses to write into a directory that already exists and is nonempty
+    // ("Quitting to avoid overwriting") — and it reports that refusal on stderr while still exiting
+    // **0**. The status check below therefore cannot see it, and the refusal is indistinguishable
+    // from a successful extraction.
+    //
+    // That turned a transient condition into a permanent one. Auditing a run while it was still in
+    // flight cached a partial extraction — `run_metadata.json` and no instance output, because none
+    // existed yet. Every later audit then found no instances, re-invoked extract, got the silent
+    // refusal plus exit 0, and reported the run as a READER FAULT forever. `--refresh` could not
+    // clear it either: it skips the early return above but extracts into the same nonempty
+    // directory, so it hit the identical refusal.
+    //
+    // Clearing the destination first makes the extraction authoritative rather than advisory. It is
+    // safe because the cache is derived data, reconstructible from `results.zip` on demand, and it
+    // is reached only when the cache holds no instances — the case that needs re-extraction anyway.
+    rmSync(dir, { recursive: true, force: true });
+
     const result = spawnSync('msbench-cli', ['extract', '--run_id', runId, '--output', dir], {
         encoding: 'utf8',
     });