fix(test): keep the microcompact scratch when a case FAILS, not when it passes

test/proxy-microcompact-stability.test.mjs

@@ -1,8 +1,11 @@
 import { test } from "node:test";
 import assert from "node:assert/strict";
-import { mkdtemp, readFile, rm, stat } from "node:fs/promises";
+import { readFile, stat, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
+import { existsSync, rmSync } from "node:fs";
+import { spawnSync } from "node:child_process";
 import { join } from "node:path";
+import { scratchDir } from "./scratch-registry.mjs";
 
 import ext, {
   matchesSentinelPattern,
@@ -12,6 +15,12 @@ import ext, {
   runMicrocompactStability,
 } from "../proxy/extensions/microcompact-stability.mjs";
 
+// Registration lives in scratch-registry.mjs, which gates removal on the FILE's
+// outcome: a green run cleans up, a red one keeps the dirs and names them. A
+// test body that throws never reaches its own cleanup, which is the whole point.
+const SCRATCH_PREFIX = "mc-";
+const mcTemp = () => scratchDir(SCRATCH_PREFIX);
+
 // --- Fixture helpers ---
 
 const SENTINEL_BARE = "[Old tool result content cleared]";
@@ -149,7 +158,7 @@ test("4. unrelated truncation message → no match in either mode (rejected from
 // --- Mode B ---
 
 test("4a. sentinel + trailing text → Mode B match, body NOT mutated, prefix_64 only in dump", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -177,12 +186,11 @@ test("4a. sentinel + trailing text → Mode B match, body NOT mutated, prefix_64
   assert.equal(rec.partial_matches[0].sentinel_text, undefined);
   assert.ok(rec.partial_matches[0].prefix_64.length <= 64);
   assert.ok(rec.partial_matches[0].prefix_64.startsWith("[Old tool result content cleared"));
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("4b. long trailing → prefix_64 captures only first 64 chars, byte_length reports full size", async () => {
   const long = SENTINEL_TS + " " + "x".repeat(200);
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -197,11 +205,10 @@ test("4b. long trailing → prefix_64 captures only first 64 chars, byte_length
   assert.equal(rec.partial_matches.length, 1);
   assert.equal(rec.partial_matches[0].prefix_64.length, 64);
   assert.equal(rec.partial_matches[0].byte_length, Buffer.byteLength(long, "utf8"));
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("4c. CACHE_FIX_MICROCOMPACT_REDACT_LEN=0 → prefix_64 absent", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -222,7 +229,6 @@ test("4c. CACHE_FIX_MICROCOMPACT_REDACT_LEN=0 → prefix_64 absent", async () =>
   assert.equal(rec.partial_matches.length, 1);
   assert.equal(rec.partial_matches[0].prefix_64, undefined);
   assert.equal(typeof rec.partial_matches[0].byte_length, "number");
-  await rm(dir, { recursive: true, force: true });
 });
 
 // --- Custom patterns ---
@@ -237,7 +243,7 @@ test("5a. CACHE_FIX_MICROCOMPACT_SENTINEL_PATTERN_1 adds custom Mode A pattern",
 });
 
 test("5b. custom Mode A regex + custom Mode B prefix → exact match goes to exact_matches", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   // Exact match against a custom regex from a sentinel family that does NOT
   // share the built-in prefix. Verifies exact-match path for custom families.
@@ -261,15 +267,14 @@ test("5b. custom Mode A regex + custom Mode B prefix → exact match goes to exa
   assert.equal(rec.exact_matches.length, 1);
   assert.equal(rec.partial_matches.length, 0);
   assert.equal(rec.exact_matches[0].sentinel_text, "[CC microcompact rev2]");
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("5c. custom Mode B prefix → variant-of-custom-family captured redacted in partial_matches", async () => {
   // The blocker Codex flagged: a custom sentinel family that matches
   // EXACTLY normalizes, but its prefix-only variant must also be captured
   // in Mode B (redacted) — not silently dropped just because the variant
   // doesn't share the built-in `[Old tool result content cleared` prefix.
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const text = "[CC microcompact rev2 at 2026-04-30T17:00:00Z] (with trailing content)";
   const body = makeBody([
@@ -299,7 +304,6 @@ test("5c. custom Mode B prefix → variant-of-custom-family captured redacted in
   assert.equal(rec.partial_matches.length, 1);
   assert.equal(rec.partial_matches[0].sentinel_text, undefined); // never full text
   assert.ok(rec.partial_matches[0].prefix_64.startsWith("[CC microcompact"));
-  await rm(dir, { recursive: true, force: true });
 });
 
 // --- Tool_result content shapes ---
@@ -352,7 +356,7 @@ test("8. mixed array (text + image) — only the text matches → image untouche
 // --- Diagnostic dump ---
 
 test("9. dump set + sentinel match → JSONL line; session_id is hashed (no plaintext)", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -369,11 +373,10 @@ test("9. dump set + sentinel match → JSONL line; session_id is hashed (no plai
   assert.equal(rec.session_id_hash.length, 8);
   assert.ok(!JSON.stringify(rec).includes("secret-session-12345"));
   assert.equal(rec.model, "claude-opus-4-7-20260101");
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("10. dump unset → no fs activity", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -391,11 +394,10 @@ test("10. dump unset → no fs activity", async () => {
     );
   });
   await assert.rejects(() => stat(dumpPath), /ENOENT/);
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("11. multiple matches in one request → ONE JSONL line with arrays split A/B", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -455,7 +457,7 @@ test("13. normalize on + custom canonical → matched sentinel becomes the custo
 });
 
 test("14. normalize disabled, dump enabled → matches recorded in dump but body NOT mutated", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -476,7 +478,6 @@ test("14. normalize disabled, dump enabled → matches recorded in dump but body
   assert.equal(JSON.stringify(body), before);
   const rec = JSON.parse((await readFile(dumpPath, "utf8")).trim());
   assert.equal(rec.exact_matches.length, 1);
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("15. two requests with different timestamps → byte-identical bodies after normalization", async () => {
@@ -500,7 +501,7 @@ test("15. two requests with different timestamps → byte-identical bodies after
 // --- Activation ---
 
 test("16. both gates unset → extension fires but exits early; no telemetry, no mutation, no fs", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -522,11 +523,10 @@ test("16. both gates unset → extension fires but exits early; no telemetry, no
   assert.equal(JSON.stringify(body), before);
   assert.equal(ctx.meta.microcompactStats, undefined);
   await assert.rejects(() => stat(dumpPath), /ENOENT/);
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("17. only diagnostic enabled → telemetry present, JSONL written, no mutation", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -544,11 +544,10 @@ test("17. only diagnostic enabled → telemetry present, JSONL written, no mutat
   assert.equal(ctx.meta.microcompactStats.diagnostic_enabled, true);
   assert.equal(ctx.meta.microcompactStats.normalization_enabled, false);
   assert.equal(ctx.meta.microcompactStats.diagnostic_records_written, 1);
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("18. only normalize enabled → telemetry present, mutation happens, no JSONL", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -564,11 +563,10 @@ test("18. only normalize enabled → telemetry present, mutation happens, no JSO
   assert.equal(ctx.meta.microcompactStats.normalization_enabled, true);
   assert.equal(ctx.meta.microcompactStats.sentinels_normalized, 1);
   await assert.rejects(() => stat(dumpPath), /ENOENT/);
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("19. both enabled → telemetry, mutation, AND JSONL all happen; raw text captured pre-normalization", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -594,11 +592,10 @@ test("19. both enabled → telemetry, mutation, AND JSONL all happen; raw text c
   assert.equal(rec.exact_matches[0].normalized_text, undefined);
   // Telemetry attached.
   assert.equal(ctx.meta.microcompactStats.sentinels_normalized, 1);
-  await rm(dir, { recursive: true, force: true });
 });
 
 test("19a. CACHE_FIX_DUMP_MICROCOMPACT_INCLUDE_NORMALIZED=1 → adds normalized_text alongside raw sentinel_text", async () => {
-  const dir = await mkdtemp(join(tmpdir(), "mc-"));
+  const dir = await mcTemp();
   const dumpPath = join(dir, "dump.jsonl");
   const body = makeBody([
     assistantMsg("t1"),
@@ -619,7 +616,6 @@ test("19a. CACHE_FIX_DUMP_MICROCOMPACT_INCLUDE_NORMALIZED=1 → adds normalized_
   const rec = JSON.parse((await readFile(dumpPath, "utf8")).trim());
   assert.equal(rec.exact_matches[0].sentinel_text, SENTINEL_TS);
   assert.equal(rec.exact_matches[0].normalized_text, SENTINEL_BARE);
-  await rm(dir, { recursive: true, force: true });
 });
 
 // --- Telemetry shape ---
@@ -772,3 +768,81 @@ test("15. all sources missing → null", () => {
   const rec = buildDiagnosticRecord(reqCtx, [], [], 0, { ts: "t" });
   assert.equal(rec.session_id_hash, null);
 });
+
+// A dir minted outside mcTemp() is unregistered, so `after()` cannot remove it
+// and a throwing case strands it, with the suite green either way.
+// The lifecycle cannot be asserted in-process: the decision is taken on the way
+// out, after every hook this file could run. So it is measured through a real
+// child, both ways -- the passing arm is the control, without which "the dir is
+// there" would also be what a registry that never cleans anything looks like.
+test("scratch: a failing run keeps its scratch, a passing one does not", async () => {
+  const registry = new URL("./scratch-registry.mjs", import.meta.url).href;
+  const run = async (outcome) => {
+    const dir = await mcTemp();
+    const file = join(dir, `probe-${outcome}.test.mjs`);
+    await writeFile(file, `
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { scratchDir } from ${JSON.stringify(registry)};
+test("probe", async () => {
+  const d = await scratchDir("mcprobe-");
+  console.error("PROBE_DIR=" + d);
+  ${outcome === "fail" ? 'assert.fail("deliberate");' : "assert.ok(true);"}
+});
+`);
+    // NODE_TEST_CONTEXT is inherited, and the runner refuses to run files when it
+    // sees it ("run() is being called recursively"), so the child would report
+    // nothing and the case would fail on its own plumbing.
+    const env = { ...process.env };
+    delete env.NODE_TEST_CONTEXT;
+    const r = spawnSync(process.execPath, ["--test", file],
+                        { env, encoding: "utf8", timeout: 60_000 });
+    // STDOUT, not stderr: node:test captures a case's output and re-emits it on
+    // its own stream, so a child's `process.stderr.write` arrives here.
+    const out = `${r.stdout}${r.stderr}`;
+    const probe = /PROBE_DIR=(.*)/.exec(out)?.[1]?.trim();
+    assert.ok(probe, `the probe never reported its dir: ${out}`);
+    return { probe, status: r.status, out };
+  };
+
+  const failed = await run("fail");
+  assert.notEqual(failed.status, 0, "premise: the failing probe exited 0");
+  assert.ok(existsSync(failed.probe),
+    "a failing run deleted the scratch that is the only record of what it was looking at");
+  assert.match(failed.out, /\[scratch kept\]/,
+    "the dir was kept and never named — the caller cannot find it");
+  rmSync(failed.probe, { recursive: true, force: true });
+
+  const passed = await run("pass");
+  assert.equal(passed.status, 0, "premise: the passing probe did not pass");
+  assert.ok(!existsSync(passed.probe),
+    "a passing run left its scratch behind — the registry cleans nothing");
+});
+
+test("scratch: no test body mints an unregistered temp dir", async () => {
+  const src = await readFile(new URL(import.meta.url), "utf8");
+  // Matches any quoted prefix, which the registrar (passing a const) does not
+  // have. Line comments are skipped and this line's own escaping keeps it from
+  // matching itself, so neither the prose above nor the assertion self-flags.
+  // Only "//": nothing can follow it on the line, while a block comment closes
+  // mid-line, so skipping "/*" and "*" would hide a real mint after the close
+  // and behind any generator method. Ceiling: a single-line textual match on
+  // one exact call form, so any other spelling passes.
+  const call = /mkdtemp(?:Sync)?\(join\(tmpdir\(\),\s*["'`][^"'`]+["'`]\s*\)\)/;
+  const raw = src.split("\n")
+    .map((line, i) => [i + 1, line])
+    .filter(([, l]) => !l.trim().startsWith("//"))
+    .filter(([, l]) => call.test(l))
+    .map(([n]) => n);
+  assert.deepEqual(raw, [],
+    `every temp dir must go through mcTemp(); raw mkdtemp at line(s): ${raw.join(", ")}`);
+  // POSITIVE CONTROL. Without it the pattern can be neutered -- a typo, a
+  // rename, anything -- and this case stays green over a file it no longer
+  // matches. Assembled from pieces so the sample itself is not a mint on this
+  // line for the pattern above to find.
+  const sample = ["mkdtemp", "(join(tmpdir(), ", '"x-"))'].join("");
+  assert.ok(call.test(sample), "the pattern no longer matches a raw mint");
+  // The premise is a SOURCE count, not a runtime array: the registration moved
+  // out of this file, so a runtime one would now be empty here and read green.
+  assert.ok(src.split("mcTemp(").length - 1 > 1, "premise: this file does mint temp dirs");
+});

test/scratch-registry.mjs

@@ -0,0 +1,35 @@
+// Scratch dirs that survive the failure they exist to explain.
+//
+// A test body that throws never reaches its own cleanup, so every dir is
+// registered here. Removal is gated on the FILE's outcome: after a green run the
+// scratch is litter, after a red one it is the only record of what the case was
+// looking at when it died.
+//
+// `process.on("exit")` and not `after()`, because the hook runs too early to
+// know: measured on node 24, `process.exitCode` is `undefined` inside a
+// file-level `after()` on a file that HAS a failure, and `1` by exit on the same
+// file (and `undefined` at both points on a file without one).
+//
+// Sync removal for the same reason -- an exit handler cannot await, so a
+// promise-based rm would be registered and never run.
+import { mkdtemp } from "node:fs/promises";
+import { rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+const registered = [];
+
+export async function scratchDir(prefix) {
+  const d = await mkdtemp(join(tmpdir(), prefix));
+  registered.push(d);
+  return d;
+}
+
+process.on("exit", () => {
+  for (const d of registered) {
+    // Named on the way out, or keeping them helps nobody: the runner's output is
+    // the only place the caller will look for the path.
+    if (process.exitCode) { process.stderr.write(`[scratch kept] ${d}\n`); continue; }
+    try { rmSync(d, { recursive: true, force: true }); } catch { /* best effort */ }
+  }
+});