chore(rca-build): prune + genericize the plugin for customer release

bin/cached-exec.mjs

@@ -1,54 +1,30 @@
 #!/usr/bin/env node
-// Run a READ-ONLY command through the build's tool cache, in ONE tool call.
+// Run a command through the build's tool cache, in ONE tool call.
 //
-// Why a wrapper: a "check cache / run / store" sequence done by hand costs
-// three tool calls to save one, which is worse than not caching. This collapses
-// it to a single call that behaves exactly like the underlying command —
-// same stdout, same exit code — but only actually executes on a miss.
+// Cached: IMMUTABLE reads (sha-pinned gh api, git show/cat-file/ls-tree/log with
+// a sha) AND run-stable repo reads (gh pr view/diff/list, gh api repo reads, gh
+// search, read-only git) — the latter don't change within a single minutes-long
+// build RCA and are fetched identically by every sibling confirming the same
+// suspect PRs. Live state (kubectl/curl/logs) passes through uncached. Mutations
+// are refused.
 //
-// Usage (command is ONE argument, so the caller's own quoting survives):
+// Usage:
 //   node bin/cached-exec.mjs <buildId> <writerId> '<command>'
 //   node bin/cached-exec.mjs <buildId> <writerId> -        # command on STDIN
 //   node bin/cached-exec.mjs <buildId> --stats
-//
-// Wrap only the expensive fetch and leave filtering to the outer shell:
-//   node bin/cached-exec.mjs "$B" 3895581484 'gh api repos/o/r/contents/f' | jq -r .content | head -40
-// Two coordinators piping the same fetch through different greps then share
-// one cache entry, instead of each paying for the fetch.
-//
-// TWO GOTCHAS, both hit in real use:
-//
-//  1. Hit/miss banners go to STDERR, so stdout stays byte-identical to the raw
-//     command and `| jq` works. But `2>&1 | jq` merges the banner back into
-//     the pipe and jq dies on it ("Invalid literal at line 1, column 12").
-//     Don't redirect stderr into a pipe. If you silence it with `2>/dev/null`
-//     you also lose the hit/miss signal — so set `TOOLCACHE_LOG=<path>` and
-//     the banners are teed there too: `grep -c HIT <path>` still works.
-//
-//  2. Nested single quotes. A command containing its own `'…'` (typically
-//     `--jq '.[] | "\(.number)"'`) cannot be passed inside a single-quoted
-//     argument — the outer shell terminates the string early and the argument
-//     arrives mangled. Use `-` and pipe the command in on stdin instead:
-//       printf '%s' 'gh pr list -R o/r --json number --jq ".[].number"' \
-//         | node bin/cached-exec.mjs "$B" 3895 -
 
-import { execFileSync } from "node:child_process";
-import { readFileSync, appendFileSync } from "node:fs";
+import { execSync } from "node:child_process";
+import { readFileSync } from "node:fs";
 import {
-  toolCacheDirFor, cacheKey, cacheGet, cachePut, cacheStats, isRunnable, tokenize,
+  toolCacheDirFor, cacheKey, cacheGet, cachePut, cacheStats,
+  isCacheable, isImmutableRead, isRunStableRead, banner,
 } from "../lib/tool-cache.mjs";
 
 const [, , buildId, writerOrFlag, commandArg] = process.argv;
 
-// `-` means the command arrives on stdin, which sidesteps the nested-quoting
-// problem entirely (see gotcha 2 above).
 let command = commandArg;
 if (command === "-") {
-  try {
-    command = readFileSync(0, "utf8").trim();
-  } catch {
-    command = "";
-  }
+  try { command = readFileSync(0, "utf8").trim(); } catch { command = ""; }
   if (!command) {
     console.error("[tool-cache] '-' given but stdin was empty");
     process.exit(2);
@@ -62,102 +38,73 @@ if (!buildId || (writerOrFlag !== "--stats" && !command)) {
 }
 
 const dir = toolCacheDirFor(buildId, process.env.RCA_STATE_DIR ?? "");
-
-// Where hit/miss banners go. Default stderr keeps stdout byte-identical to the
-// wrapped command. But callers pipe stdout into jq/sed and silence stderr with
-// `2>/dev/null` to keep the tool chatter out — which also throws away the
-// banner, so the run's own hit-rate becomes unmeasurable. Setting
-// TOOLCACHE_LOG=<path> tees banners to a file, letting a caller suppress
-// stderr and still count hits afterwards (`grep -c HIT <path>`).
 const logPath = process.env.TOOLCACHE_LOG ?? "";
-function banner(line) {
-  console.error(line);
-  if (logPath) {
-    try {
-      appendFileSync(logPath, line + "\n", { encoding: "utf8", mode: 0o600 });
-    } catch {
-      /* logging must never break the fetch */
-    }
-  }
-}
 
 if (writerOrFlag === "--stats") {
-  const s = cacheStats(dir);
-  console.log(JSON.stringify({ cacheDir: dir, ...s }, null, 2));
+  console.log(JSON.stringify({ cacheDir: dir, ...cacheStats(dir) }, null, 2));
   process.exit(0);
 }
 
-// Parse into a fetch + filter chain before anything runs.
-const gate = isRunnable(command);
-if (!gate.ok) {
-  console.error(`[tool-cache REFUSED] ${gate.reason}`);
+// Refuse mutations.
+if (!isCacheable(command)) {
+  console.error(`[tool-cache REFUSED] command looks mutating`);
   console.error(`  command: ${command}`);
   process.exit(2);
 }
 
-// Key on the FETCH ONLY. Downstream filters are pure text transforms, so two
-// agents filtering the same fetch differently share one cached network call.
-const key = cacheKey(gate.fetchText);
-
-// Run one argv with `input` on stdin, no shell. Returns { stdout, exitCode }.
-function run(argv, input) {
+// Run a command via the shell and return { stdout, exitCode }.
+function run(cmd) {
   try {
     return {
-      stdout: execFileSync(argv[0], argv.slice(1), {
+      stdout: execSync(cmd, {
         encoding: "utf8",
+        shell: true,
         maxBuffer: 64 * 1024 * 1024,
-        // Capture stderr rather than let it inherit: execFileSync otherwise
-        // BOTH inherits and captures, so relaying it ourselves printed
-        // failures three times.
-        stdio: [input === undefined ? "ignore" : "pipe", "pipe", "pipe"],
-        ...(input === undefined ? {} : { input }),
+        stdio: ["ignore", "pipe", "pipe"],
       }),
       exitCode: 0,
     };
   } catch (err) {
-    if (err.stderr) process.stderr.write(err.stderr.toString()); // the only copy
+    if (err.stderr) process.stderr.write(err.stderr.toString());
     return {
       stdout: (err.stdout ?? "").toString(),
       exitCode: typeof err.status === "number" ? err.status : 1,
     };
   }
 }
 
-let fetched;
-const hit = cacheGet(dir, key);
-if (hit) {
-  banner(`[tool-cache HIT ${key} — captured by ${hit.writerId ?? "?"}, ${hit.bytes}B]`);
-  fetched = hit.stdout;
-} else {
-  const res = run(gate.fetch, undefined);
-  fetched = res.stdout;
-  if (res.exitCode !== 0) {
-    // Preserve the real behaviour. Deliberately NOT cached — a transient
-    // failure (rate limit, expired token) must not become a permanent answer.
-    banner(`[tool-cache MISS ${key} — fetch exited ${res.exitCode}, NOT cached]`);
-    process.stdout.write(fetched);
-    process.exit(res.exitCode);
-  }
-  if (fetched.trim() === "") {
-    // An empty result is usually a wrong selector or a silently failed lookup;
-    // caching it creates a sticky, invisible negative for every later reader.
-    banner(`[tool-cache MISS ${key} — empty result, NOT cached]`);
-  } else {
-    // nowMs is read here, at the process edge — lib/ keeps its no-clock
-    // discipline so it stays sandbox-safe.
-    cachePut(dir, key, { command: gate.fetchText, writerId: writerOrFlag, stdout: fetched, exitCode: 0 }, Date.now());
-    banner(`[tool-cache MISS ${key} — stored ${fetched.length}B]`);
+const shouldCache = isImmutableRead(command) || isRunStableRead(command);
+const key = cacheKey(command);
+
+// Try cache only for cacheable reads.
+if (shouldCache) {
+  const hit = cacheGet(dir, key);
+  if (hit) {
+    banner(`[tool-cache HIT ${key} — captured by ${hit.writerId ?? "?"}, ${hit.bytes}B]`, logPath);
+    process.stdout.write(hit.stdout);
+    process.exit(0);
   }
 }
 
-// Apply the filter chain to whatever the fetch produced (cached or fresh).
-let out = fetched;
-let finalExit = 0;
-for (const f of gate.filters) {
-  const res = run(f, out);
-  out = res.stdout;
-  if (res.exitCode !== 0) { finalExit = res.exitCode; break; }
+// Execute the command (cached or pass-through).
+const res = run(command);
+
+if (res.exitCode !== 0) {
+  banner(`[tool-cache MISS ${key} — exited ${res.exitCode}, NOT cached]`, logPath);
+  process.stdout.write(res.stdout);
+  process.exit(res.exitCode);
+}
+
+if (shouldCache) {
+  if (res.stdout.trim() === "") {
+    banner(`[tool-cache MISS ${key} — empty result, NOT cached]`, logPath);
+  } else {
+    cachePut(dir, key, { command, writerId: writerOrFlag, stdout: res.stdout, exitCode: 0 }, Date.now());
+    banner(`[tool-cache MISS ${key} — stored ${res.stdout.length}B]`, logPath);
+  }
+} else {
+  banner(`[tool-cache PASS-THROUGH — not a cacheable read]`, logPath);
 }
 
-process.stdout.write(out);
-process.exit(finalExit);
+process.stdout.write(res.stdout);
+process.exit(0);

bin/cached-mcp.mjs

@@ -2,53 +2,28 @@
 // Memo cache for READ-ONLY **MCP** tool calls, sharing the same per-build
 // store as `cached-exec.mjs`.
 //
-// Shell calls can be wrapped transparently (`cached-exec.mjs` runs the command
-// for you). MCP calls cannot — only the agent can invoke an MCP tool — so the
-// contract here is check-then-call:
-//
+// Usage:
 //   1. get   → node bin/cached-mcp.mjs <buildId> get <tool> '<argsJson>'
-//              exit 0 + result on stdout  = HIT, skip the MCP call entirely
-//              exit 1, empty stdout       = MISS, make the MCP call yourself
-//   2. put   → node bin/cached-mcp.mjs <buildId> put <tool> '<argsJson>' <writerId>
-//              (payload on STDIN — pipe the digest you want shared)
-//
-// WHEN THIS PAYS OFF, and when it does not. A hit replaces one MCP call with
-// one cheap local read, so it wins on latency and on tokens whenever the
-// cached payload is a digest smaller than the raw response. A miss costs two
-// extra calls (the probe + the store), so this is worth it for **expensive,
-// broadly-reusable, build-level queries** — a VictoriaLogs sweep, a
-// `listTestIds`, a `getFailureLogs` several coordinators would each re-run —
-// and NOT worth it for a one-off lookup only this test will ever need.
+//   2. put   → node bin/cached-mcp.mjs <buildId> put <tool> '<argsJson>' <writerId>   # payload on stdin
+//   3. list  → node bin/cached-mcp.mjs <buildId> list
+//   4. stats → node bin/cached-mcp.mjs <buildId> stats
 //
 // Never cacheable (refused): `tfaRcaTurn`, `getTfaTurnResult`,
-// `triggerRcaReport`. Those are stateful — a turn's status is *expected* to
-// change between reads, so serving one from cache is wrong, not just stale.
-// Prefer storing a DIGEST rather than a raw payload: the point is to spare the
-// next reader the raw rows, not to relay them.
+// `triggerRcaReport`. Those are stateful.
 
-import { readFileSync, readdirSync, existsSync, appendFileSync } from "node:fs";
+import { readFileSync, readdirSync, existsSync } from "node:fs";
 import { join } from "node:path";
 import {
-  toolCacheDirFor, mcpCacheKey, cacheGet, cachePut, cacheStats, isCacheableMcp,
+  toolCacheDirFor, mcpCacheKey, cacheGet, cachePut, cacheStats, isCacheableMcp, banner,
 } from "../lib/tool-cache.mjs";
 
-// Same TOOLCACHE_LOG tee as cached-exec, so shell and MCP hits can be counted
-// from one file. Previously only shell banners were logged, which made a run's
-// combined hit rate impossible to total.
 const logPath = process.env.TOOLCACHE_LOG ?? "";
-function banner(line) {
-  console.error(line);
-  if (logPath) {
-    try { appendFileSync(logPath, line + "\n", { encoding: "utf8", mode: 0o600 }); } catch { /* never break the call */ }
-  }
-}
-
 const [, , buildId, verb, tool, argsJson, writerId] = process.argv;
 
 if (!buildId || !verb) {
   console.error("usage: cached-mcp.mjs <buildId> get <tool> '<argsJson>'");
   console.error("       cached-mcp.mjs <buildId> put <tool> '<argsJson>' <writerId>   # payload on stdin");
-  console.error("       cached-mcp.mjs <buildId> list    # what is cached, with exact args to copy");
+  console.error("       cached-mcp.mjs <buildId> list");
   console.error("       cached-mcp.mjs <buildId> stats");
   process.exit(2);
 }
@@ -60,25 +35,19 @@ if (verb === "stats") {
   process.exit(0);
 }
 
-// `list` exists because a HIT requires reproducing the args EXACTLY, and
-// canonicalization only normalizes key ORDER, not content. A coordinator that
-// guesses the logql/window/limit triple misses — one real run burned four
-// probe calls guessing, to save two. Listing what is actually cached turns
-// that into a single call: read the available queries, then `get` the one you
-// want with its args copied verbatim.
 if (verb === "list") {
   if (!existsSync(dir)) { console.log("(no cache yet)"); process.exit(0); }
   let n = 0;
   for (const f of readdirSync(dir).filter((x) => x.endsWith(".json"))) {
     let e; try { e = JSON.parse(readFileSync(join(dir, f), "utf8")); } catch { continue; }
-    if (!/^mcp__/.test(e.command ?? "")) continue; // shell entries live here too
+    if (!/^mcp__/.test(e.command ?? "")) continue;
     n++;
     const sp = e.command.indexOf(" ");
     console.log(`\n[${e.key}] ${e.command.slice(0, sp)}  (by ${e.writerId ?? "?"}, ${e.bytes}B)`);
     console.log(`  args: ${e.command.slice(sp + 1)}`);
     console.log(`  digest: ${String(e.stdout).replace(/\s+/g, " ").slice(0, 150)}…`);
   }
-  if (!n) console.log("(no MCP entries cached — the orchestrator should pre-seed Step 4's queries)");
+  if (!n) console.log("(no MCP entries cached)");
   process.exit(0);
 }
 
@@ -93,9 +62,7 @@ if (!isCacheableMcp(tool)) {
 }
 
 let args;
-try {
-  args = JSON.parse(argsJson);
-} catch (err) {
+try { args = JSON.parse(argsJson); } catch (err) {
   console.error(`[mcp-cache] argsJson is not valid JSON: ${err.message}`);
   process.exit(2);
 }
@@ -105,27 +72,23 @@ const key = mcpCacheKey(tool, args);
 if (verb === "get") {
   const hit = cacheGet(dir, key);
   if (!hit) {
-    banner(`[mcp-cache MISS ${key} ${tool}] — make the MCP call, then 'put' the digest`);
+    banner(`[mcp-cache MISS ${key} ${tool}] — make the MCP call, then 'put' the digest`, logPath);
     process.exit(1);
   }
-  banner(`[mcp-cache HIT ${key} ${tool} — captured by ${hit.writerId ?? "?"}, ${hit.bytes}B]`);
+  banner(`[mcp-cache HIT ${key} ${tool} — captured by ${hit.writerId ?? "?"}, ${hit.bytes}B]`, logPath);
   process.stdout.write(hit.stdout);
   process.exit(0);
 }
 
 if (verb === "put") {
   let payload = "";
-  try {
-    payload = readFileSync(0, "utf8"); // stdin
-  } catch {
-    payload = "";
-  }
+  try { payload = readFileSync(0, "utf8"); } catch { payload = ""; }
   if (!payload.trim()) {
     console.error("[mcp-cache] refusing to store an empty payload");
     process.exit(2);
   }
   const rec = cachePut(dir, key, { command: `${tool} ${argsJson}`, writerId, stdout: payload }, Date.now());
-  banner(`[mcp-cache STORED ${key} ${tool} — ${rec.bytes}B]`);
+  banner(`[mcp-cache STORED ${key} ${tool} — ${rec.bytes}B]`, logPath);
   process.exit(0);
 }