resolved · pending · failed`). **Nothing per-test.**
2. Call **`triggerRcaReport(buildUuid=)`** (add `force=true` only to
re-run over an existing completed report).
-3. Print the link line, verbatim shape:
+3. **Only once that call succeeds**, call
+ `cleanupBuildArtifacts(buildId, config.paths.stateDir)`
+ (`lib/build-cleanup.mjs`) to delete THIS build's own CSV, evidence file +
+ `.contrib/` shards, tool cache, and turn1 registry. Never call this before
+ `triggerRcaReport` succeeds, and never on a run that ends with any row still
+ non-terminal — at that point resume still needs these files. This is safe
+ specifically because Step 6 only runs "when every row is terminal": there is
+ nothing left to resume for THIS build once its report has generated. It is
+ deliberately not `lib/state-dir.mjs`'s `pruneStateDir` (a separate, manual,
+ age-based sweep across every build in the shared temp dir) — that remains
+ the safety net for a build that crashes before ever reaching Step 6.
+4. Print the link line, verbatim shape:
```
Full report on the Test Observability UI:
@@ -235,7 +1141,12 @@ thread.
the gate closes, never ask the user anything.**
- An invalid/absent connector is a recorded gap, never a blocker.
- Headless + missing build id → end immediately. Headless never asks.
-- Never call `tfaRcaTurn` from this skill — always via the `ai-tfa-coordinator`.
+- Never call `tfaRcaTurn` from this skill — always via the `ai-tfa-coordinator` —
+ **except Step 4b's turn-1 pre-dispatch**, which is a deliberate, narrow carve-out
+ (one direct call per cluster representative, concurrent with Step 4, never a
+ follow-up turn) documented there. Every OTHER `tfaRcaTurn` call — every turn
+ past 1, and every sibling's turn 1 — still goes exclusively through a
+ dispatched coordinator.
- A soft-`PENDING` is never an answer: it must be drained with
`getTfaTurnResult(testRunId, turnId)` before any further submit on that thread.
Only a spent drain budget may end a test `PENDING`.
@@ -245,3 +1156,10 @@ thread.
the Test Observability UI link only.
- A PRODUCT_BUG RCA without a GitHub PR link is incomplete — dig until the turn
cap, else state what was searched and record the gap.
+- Step 4's first `gh pr list` call per repo MUST include `files` in `--json` —
+ never split into a plain list followed by a per-PR `gh pr view --json files`
+ backfill loop.
+- Every reference-doc / `lib/` path handed to a coordinator (in the dispatch
+ prompt or in `agents/ai-tfa-coordinator.md`) MUST be `pluginRoot`-qualified
+ (`/skills/rca-build/references/.md`) — never a bare
+ `references/.md`, which resolves against an unknown coordinator cwd.
diff --git a/skills/rca-build/references/clustering.md b/skills/rca-build/references/clustering.md
index 1ac5866..689434d 100644
--- a/skills/rca-build/references/clustering.md
+++ b/skills/rca-build/references/clustering.md
@@ -1,4 +1,4 @@
-# Failure-signature clustering
+# Clustering
Why: a red build's N failures usually trace to a handful of causes (one bad
PR/deploy/shared helper). Running the full collaborative loop once per *cause*
@@ -7,9 +7,33 @@ causes)** — the only thing that makes "RCA for ALL failed tests, even thousand
feasible. But **every failed test must still show a per-test RCA in the TRA
dashboard**, so clustering collapses the *evidence hunt*, not the *output*.
-The logic lives in `lib/signature.mjs`; this file is the protocol.
-
-## The signature
+## Two sources, one contract
+
+Both paths produce the identical `{ cluster_id, signature, members,
+representative, siblings }` shape, so nothing downstream (the fan-out
+workflow, the sequential harness) needs to know which one ran:
+
+- **Preferred — server-computed themes.** `lib/theme-clustering.mjs` →
+ `clustersFromThemes(rows, themesResult, testsByThemeId)`, fed from the
+ `getBuildFailureThemes` / `listTestsInFailureTheme` MCP tools (SKILL.md Step
+ 3). `getBuildFailureThemes` is responsible for making themes exist, not just
+ reading them — if nothing has ever been computed for this build it triggers
+ computation (one POST, same call) and polls for `buildThemeWorkflow.status`
+ to reach `SUCCESS`, up to its own budget. The grouping reflects the
+ server's own root-cause clustering instead of a text-signature guess — two
+ failures with an identical error string but unrelated causes are not
+ conflated the way a client-side "signature" would be.
+- **Fallback — client-side failure signature.** `lib/signature.mjs` →
+ `clusterAndPersist(csvPath, csvStateModule)`, the original text-normalization
+ approach described below. Used whenever `getBuildFailureThemes` comes back
+ `ready: false` — still computing past its poll budget, a failure status, or
+ `status: "trigger-unavailable"` (the trigger call didn't succeed). This
+ makes the flow independent of whether the trigger call currently succeeds:
+ whenever it doesn't, every un-computed build degrades straight to this
+ fallback; whenever it does, the same call reaches `ready: true` on its own
+ and this fallback simply isn't exercised.
+
+## The signature (fallback path only)
Computed from the trimmed failure detail `listTestIds(includeFailureDetail=true)`
already returns on each row — **no extra probe turns**:
diff --git a/skills/rca-build/references/evidence-routing.md b/skills/rca-build/references/evidence-routing.md
index 05b4435..2c3ba5b 100644
--- a/skills/rca-build/references/evidence-routing.md
+++ b/skills/rca-build/references/evidence-routing.md
@@ -13,6 +13,13 @@ capability manifest — see `SKILL.md` § Gate Part A). There are **no `kubectl`
`chitragupta` /
`bifrost` literals here** — that is the whole point of going generic.
+**Contents:** [How asks are processed](#how-a-turns-asks-are-processed) ·
+[Routing table](#routing-table-capability-not-tool) ·
+[Digest format](#digest-format) ·
+[Unfulfillable asks](#unfulfillable-asks--report-dont-drop) ·
+[Capability manifest](#capability-manifest-built-once-at-the-gate) ·
+[Build-level evidence cache](#build-level-evidence-cache-compute-once)
+
The registry logic lives in `lib/routing.mjs` (`routeAsk` / `routeAsks`); this
file is the human/agent-facing contract for the digest and the size caps.
@@ -89,16 +96,21 @@ and unfulfillable variants) — copy it, don't retype it. Shape:
| Field / scope | Soft target | Hard ceiling | On exceed |
|---|---|---|---|
-| `SUMMARY` | ≤ 300 chars | 400 chars | Tighten to the finding; drop restatement of the ask |
-| `SNIPPET` per ask | ≤ 20 lines | 40 lines | Keep the load-bearing lines; replace the rest with `… (N lines elided — see LINK)` |
-| Code diff in a `product_code` snippet | ≤ 1 hunk | 3 hunks | Show changed lines + 3 lines context; link the full PR |
-| Whole next-turn `message` | ≤ 200 lines | 400 lines (and ≤ `turnMessageMaxChars`) | Drop `low`-priority asks first; keep every `high` ask's block |
+| `SUMMARY` | ≤ 60 chars | 80 chars | Tighten to the finding; drop restatement of the ask |
+| `SNIPPET` per ask | ≤ 4 lines | 8 lines | Keep the load-bearing lines; replace the rest with `… (N lines elided — see LINK)` |
+| Code diff in a `product_code` snippet | ≤ 1 hunk | 2 hunks | Show changed lines only, no context lines; link the full PR |
+| Whole next-turn `message` | ≤ 40 lines | 80 lines (and ≤ `turnMessageMaxChars`) | Drop `low`-priority asks first; keep every `high` ask's block |
| Asks fulfilled per turn | all `high` + `medium` | — | Defer `low` asks to a later turn rather than truncating a `high` ask |
Truncation rule of thumb: **never truncate a `high`-priority ask's block to fit a
`low`-priority one.** Drop the low block whole; keep the high block intact. The
-whole-message ceiling also honors `turnMessageMaxChars` from
-`config/rca.config.json` (the tool caps `message` at 5000 chars).
+whole-message ceiling honors `turnMessageMaxChars` from
+`config/rca.config.json`, now set to **1000 chars** — a plugin-configured
+self-limit, tighter than the underlying tool's actual hard cap (the
+`tfaRcaTurn` MCP tool itself allows up to 5000 chars per `message`; the plugin
+just chooses not to use all of it). At this budget, expect at most 2-3 ask
+blocks per turn before hitting the ceiling — defer lower-priority asks to a
+follow-up turn rather than cramming everything into one.
### What never goes in a digest
diff --git a/skills/rca-build/references/github-evidence.md b/skills/rca-build/references/github-evidence.md
index 4b2c445..2960a9c 100644
--- a/skills/rca-build/references/github-evidence.md
+++ b/skills/rca-build/references/github-evidence.md
@@ -9,6 +9,15 @@ that tries to *disprove* each suspect before it enters `related_prs`.
> needed and use whatever the client already has — **GitHub MCP if available,
> else `gh`, else degrade** to an `unavailable` block.
+**Contents:** [Capability discovery](#capability-discovery-in-order) ·
+[Culprit-PR hunt](#application-bugs-require-a-culprit-pr-hunt-mandatory) ·
+[Batching probes](#batch-every-independent-probe-into-one-message--never-one-call-per-turn) ·
+[Evidence per ask](#evidence-each-ask-needs-be-specific--no-fishing) ·
+[Field-filtering](#field-filtering--project-before-you-pull-every-call) ·
+[Falsification protocol](#falsification-protocol--rule-out-dont-just-rule-in) ·
+[Suspect packet](#the-suspect-packet-structured-not-free-text) ·
+[Digest discipline](#digest-discipline)
+
## Capability discovery (in order)
1. **GitHub MCP** (`mcp__github__*`) — preferred for structured PR/diff/blame queries.
@@ -41,6 +50,30 @@ the CSV row records the gap. Never fabricate a PR; if the github connector is
invalid/absent, the same explicit statement plus an `unavailable` block goes to
TFA (a gate-recorded gap).
+## Batch every independent probe into one message — never one call per turn
+
+This hunt routinely needs several `gh` calls that don't depend on each
+other's output: a commit-history check per candidate file in "changed paths
+vs failure signature," each row of the "Evidence each ask needs" table
+below, and each candidate PR's falsification check. **None of these need to
+see a prior result before running** — the only exception is when one call's
+output supplies a literal input to the next (e.g., you need a PR number
+back from a search before you can `gh pr view` it).
+
+Issue every independent probe as its own tool call **within the same
+message** — the same discipline `ai-tfa-coordinator.md`'s NEEDS_INFO step
+already requires across multiple asks (`Promise.all` / concurrent gather)
+applies here too, one level down, across multiple probes inside a single
+ask. One call per file path, fired one message at a time, waiting for each
+result before issuing the next, spends a full turn's think-time on every
+individual `gh api` round trip even though the call itself finishes in
+under two seconds — for a five-file changed-paths check that is the
+difference between one batched message and five serialized ones. Plan the
+full probe list first (every candidate file, every table row, every
+falsification check that has no dependency on another probe's result), then
+fire all of them together; only serialize the ones with a genuine
+input-from-output dependency.
+
## Evidence each ask needs (be specific — no fishing)
| Ask intent | Gather exactly |
@@ -55,6 +88,35 @@ Scope everything by the failing test's `file_path` + the error summary. The
build-level evidence (diff-since-last-green, PR window) is **pre-computed once**
and passed in — reuse it; do not re-fetch per test.
+## Field-filtering — project before you pull, every call
+
+The single most common way a gather call wastes context: pulling a full
+object when the ask only needs one or two fields from it. This applies to
+whichever connector resolved for `github` (most commonly the `gh` CLI today,
+or a GitHub MCP tool) — every call should already be filtered to the field(s)
+the ask needs, not filtered after the fact by reading past the noise. The
+same discipline applies to `infra` gather calls (`kubectl` or whatever the
+manifest resolved to), since the failure mode is identical.
+
+| Need | Don't — pulls the whole object | Do — projects to the field(s) the ask needs |
+|---|---|---|
+| Repo exists / default branch | `gh api repos/OWNER/REPO` | `gh api repos/OWNER/REPO --jq '.default_branch'` |
+| Branch exists on the shipping branch | `gh api repos/OWNER/REPO/branches/BRANCH` | `gh api repos/OWNER/REPO/branches/BRANCH --jq '.name'` |
+| Commit history / PR-window search | `gh api "repos/OWNER/REPO/commits?sha=BRANCH&per_page=100"` | add `--jq '[.[] | {sha: .sha[0:8], date: .commit.committer.date, msg: (.commit.message | split("\n")[0])}]'` |
+| PR metadata | `gh pr view N --repo OWNER/REPO` (full payload) | `gh pr view N --repo OWNER/REPO --json state,mergedAt,baseRefName,headRefOid,files,author` — `--json` is itself a field allowlist; list only the fields this ask uses |
+| Pod / workload listing | `kubectl get pods -n NS -o wide` | `kubectl get pods -n NS -o custom-columns='NAME:.metadata.name,STATUS:.status.phase'` |
+| Deploy / image state | `kubectl get deploy -n NS -o yaml` | `kubectl get deploy -n NS -o custom-columns='NAME:.metadata.name,IMAGE:.spec.template.spec.containers[0].image'` |
+| Log sweep | a raw `--tail` dump | `kubectl logs POD --since= --tail=2000 \| grep -E '\|ERROR\|Exception'` — filter by the correlation token, never a raw tail |
+
+**Never run the unfiltered form "to see the shape first."** An exploratory
+raw call costs the same context whether or not its output ends up in the
+digest — a bare repo or commit object routinely carries license/URL metadata
+and a multi-hundred-character signature block that no evidence ask ever
+consults. If the exact field path is genuinely unknown, learn the shape from
+one throwaway call against a cheap target, then filter every real call from
+that point on — never repeat the unfiltered form per repo, per PR, or per
+test.
+
## Falsification protocol — rule out, don't just rule in
For **each** candidate suspect PR, try to **break** the hypothesis:
diff --git a/skills/rca-build/templates/evidence-block.md b/skills/rca-build/templates/evidence-block.md
index fa0bf49..97b41f7 100644
--- a/skills/rca-build/templates/evidence-block.md
+++ b/skills/rca-build/templates/evidence-block.md
@@ -6,12 +6,12 @@ the forbidden list: `../references/evidence-routing.md`.
Fulfilled ask:
```
-ASK:
+ASK:
TYPE:
FOUND:
-SUMMARY: <1–3 sentences — the finding, in the agent's words. ≤ 400 chars>
+SUMMARY: <1 sentence — the finding, in the agent's words. ≤ 80 chars>
SNIPPET:
-
+
LINK:
```
diff --git a/tests/build-cleanup.test.mjs b/tests/build-cleanup.test.mjs
new file mode 100644
index 0000000..e903052
--- /dev/null
+++ b/tests/build-cleanup.test.mjs
@@ -0,0 +1,88 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, rmSync, existsSync, mkdirSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { cleanupBuildArtifacts } from "../lib/build-cleanup.mjs";
+import { csvPathFor } from "../lib/csv-state.mjs";
+import { evidencePathFor, contribDirFor } from "../lib/evidence-file.mjs";
+import { toolCacheDirFor } from "../lib/tool-cache.mjs";
+import { turn1PathFor } from "../lib/turn1-registry.mjs";
+
+function fixture() {
+ return mkdtempSync(join(tmpdir(), "rca-cleanup-"));
+}
+
+// Writes every artifact family for one build, so a test can assert cleanup
+// removed exactly (and only) what that build produced.
+function seedBuild(buildId, dir) {
+ writeFileSync(csvPathFor(buildId, dir), "buildId,testRunId\n");
+ writeFileSync(evidencePathFor(buildId, dir), "{}");
+ const contrib = contribDirFor(evidencePathFor(buildId, dir));
+ mkdirSync(contrib, { recursive: true });
+ writeFileSync(join(contrib, "3900000001.json"), "{}");
+ const cache = toolCacheDirFor(buildId, dir);
+ mkdirSync(cache, { recursive: true });
+ writeFileSync(join(cache, "entry.json"), "{}");
+ writeFileSync(turn1PathFor(buildId, dir), "{}");
+}
+
+test("cleanupBuildArtifacts deletes every artifact family for the given build", () => {
+ const dir = fixture();
+ seedBuild("b1", dir);
+
+ const r = cleanupBuildArtifacts("b1", dir);
+
+ assert.equal(existsSync(csvPathFor("b1", dir)), false);
+ assert.equal(existsSync(evidencePathFor("b1", dir)), false);
+ assert.equal(existsSync(contribDirFor(evidencePathFor("b1", dir))), false);
+ assert.equal(existsSync(toolCacheDirFor("b1", dir)), false);
+ assert.equal(existsSync(turn1PathFor("b1", dir)), false);
+ assert.equal(r.deleted.length, 5);
+ assert.deepEqual(r.errors, []);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// The load-bearing test: a concurrent run over a DIFFERENT build in the same
+// stateDir must survive this build's cleanup untouched — nothing here should
+// ever glob/sweep the shared directory.
+test("cleanupBuildArtifacts never touches a different build's artifacts in the same stateDir", () => {
+ const dir = fixture();
+ seedBuild("b1", dir);
+ seedBuild("b2", dir);
+
+ cleanupBuildArtifacts("b1", dir);
+
+ assert.equal(existsSync(csvPathFor("b1", dir)), false);
+ assert.equal(existsSync(csvPathFor("b2", dir)), true, "other build's CSV must survive");
+ assert.equal(existsSync(evidencePathFor("b2", dir)), true, "other build's evidence file must survive");
+ assert.equal(existsSync(contribDirFor(evidencePathFor("b2", dir))), true, "other build's shards must survive");
+ assert.equal(existsSync(toolCacheDirFor("b2", dir)), true, "other build's tool cache must survive");
+ assert.equal(existsSync(turn1PathFor("b2", dir)), true, "other build's turn1 registry must survive");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("cleanupBuildArtifacts is a no-op, not a throw, when nothing was ever written for this build", () => {
+ const dir = fixture();
+ const r = cleanupBuildArtifacts("never-ran", dir);
+ assert.deepEqual(r, { deleted: [], errors: [] });
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("cleanupBuildArtifacts tolerates a build with only some artifact families present", () => {
+ const dir = fixture();
+ // Only the CSV and turn1 registry exist — no evidence file, no tool cache
+ // (e.g. an unclustered rerun that never hit Step 4b's NEEDS_INFO/PENDING path).
+ writeFileSync(csvPathFor("b1", dir), "buildId,testRunId\n");
+ writeFileSync(turn1PathFor("b1", dir), "{}");
+
+ const r = cleanupBuildArtifacts("b1", dir);
+
+ assert.equal(r.deleted.length, 2);
+ assert.equal(existsSync(csvPathFor("b1", dir)), false);
+ assert.equal(existsSync(turn1PathFor("b1", dir)), false);
+
+ rmSync(dir, { recursive: true, force: true });
+});
diff --git a/tests/conformance.test.mjs b/tests/conformance.test.mjs
index 33ccbbb..d9af539 100644
--- a/tests/conformance.test.mjs
+++ b/tests/conformance.test.mjs
@@ -174,6 +174,67 @@ test("a failed read is not a verdict — the drain keeps reading and still lands
assert.equal(reads, 2);
});
+test("a PERSISTENT hard error stops the drain early instead of burning the budget", async () => {
+ const fx = load("soft-pending-drain.json");
+ let reads = 0;
+ const result = await runRcaLoop({
+ testRunId: fx.testRunId,
+ submit: replaySubmit([fx.turns[0]]), // always soft-PENDING
+ readTurn: async () => {
+ reads++;
+ throw new Error("Failed to get tfa turn result: TFA agent run failed");
+ },
+ // Budget allows 40 reads; the error cap must cut it off long before that.
+ config: { ...CONFIG, softPendingDrain: { maxWaitMs: 600_000, intervalMs: 1, maxReads: 40, maxErrorReads: 3 } },
+ sleep: noSleep,
+ });
+ assert.equal(result.status, "PENDING");
+ assert.equal(reads, 3, "stopped at maxErrorReads, not the 40-read budget");
+ assert.match(result.root_cause, /tfa-error/);
+ assert.equal(result.turnId, "c2e1a6fd-2243-4f93-bc69-62f298db062c"); // still resumable
+});
+
+test("an error-shaped RESULT (not thrown) also trips the fast-fail", async () => {
+ const fx = load("soft-pending-drain.json");
+ let reads = 0;
+ const result = await runRcaLoop({
+ testRunId: fx.testRunId,
+ submit: replaySubmit([fx.turns[0]]),
+ // The MCP tool reports the wedge as a returned payload, not an exception.
+ readTurn: async () => {
+ reads++;
+ return { status: "ERROR", message: "TFA agent run failed" };
+ },
+ config: { ...CONFIG, softPendingDrain: { maxWaitMs: 600_000, intervalMs: 1, maxReads: 40, maxErrorReads: 2 } },
+ sleep: noSleep,
+ });
+ assert.equal(result.status, "PENDING");
+ assert.equal(reads, 2);
+ assert.match(result.root_cause, /tfa-error/);
+});
+
+test("INTERMITTENT errors do not trip the fast-fail — a good read clears the streak", async () => {
+ const fx = load("soft-pending-drain.json");
+ const landed = fx.reads[2];
+ let reads = 0;
+ const result = await runRcaLoop({
+ testRunId: fx.testRunId,
+ submit: replaySubmit(fx.turns),
+ readTurn: async () => {
+ reads++;
+ // fail, ok, fail, ok, ... never 2 consecutive failures
+ if (reads % 2 === 1) throw new Error("transient 502");
+ return reads < 6 ? { status: "PENDING" } : landed;
+ },
+ config: { ...CONFIG, softPendingDrain: { maxWaitMs: 600_000, intervalMs: 1, maxReads: 40, maxErrorReads: 2 } },
+ manifest: GITHUB_AVAILABLE,
+ gather,
+ sleep: noSleep,
+ });
+ assert.equal(result.status, "RESOLVED", "flaky-but-recovering reads must still land");
+ assert.equal(reads, 6);
+});
+
test("BLOCKED surfaced by a drain is terminal — no empty resubmits to the turn cap", async () => {
const fx = load("soft-pending-drain.json");
let submits = 0;
diff --git a/tests/csv-state.test.mjs b/tests/csv-state.test.mjs
index 3cbc83a..6070ef4 100644
--- a/tests/csv-state.test.mjs
+++ b/tests/csv-state.test.mjs
@@ -1,12 +1,13 @@
import { test, beforeEach, afterEach } from "node:test";
import assert from "node:assert/strict";
-import { mkdtempSync, rmSync } from "node:fs";
+import { mkdtempSync, rmSync, writeFileSync, mkdirSync, chmodSync, statSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import {
csvPathFor,
seed,
readRows,
+ writeRows,
claim,
heartbeat,
flip,
@@ -121,6 +122,28 @@ test("pendingRows returns only pending work", () => {
assert.equal(pend[0].testRunId, "102");
});
+// Regression: `flip` used to accept ONLY the lowercase CSV vocabulary and
+// return a bare `false` for anything else — including `RESOLVED`, the exact
+// value the RCA_OUTPUT contract mandates. A whole batch of coordinator results
+// was lost that way: they called flip, got a silent no-op, and the rows stayed
+// `pending` looking un-run.
+test("flip accepts the RCA_OUTPUT vocabulary and normalizes it", () => {
+ seed(csv, "build-1", TESTS);
+ assert.equal(flip(csv, 101, { rca_done: "RESOLVED", root_cause: "x" }, 1000), true);
+ assert.equal(readRows(csv).find((r) => r.testRunId === "101").rca_done, "resolved");
+
+ assert.equal(flip(csv, 102, { status: "PENDING" }, 1000), true);
+ assert.equal(readRows(csv).find((r) => r.testRunId === "102").rca_done, "pending-resume");
+});
+
+test("flip maps the output block's field names onto real columns", () => {
+ seed(csv, "build-1", TESTS);
+ flip(csv, 101, { rca_done: "resolved", thread_id: "chat:101", turn_id: "t-7" }, 1000);
+ const row = readRows(csv).find((r) => r.testRunId === "101");
+ assert.equal(row.threadId, "chat:101");
+ assert.equal(row.turnId, "t-7");
+});
+
test("flip rejects a missing/non-terminal rca_done without mutating the row", () => {
seed(csv, "build-1", TESTS);
claim(csv, 101, "w1", 1000);
@@ -185,3 +208,108 @@ test("csvPathFor: stateDir override wins over temp", () => {
const p = csvPathFor("b1", "/ci/artifacts");
assert.equal(p, join("/ci/artifacts", "rca-state.b1.csv"));
});
+
+// A foreign header must fail loudly, because writeRows only emits COLUMNS and
+// would silently drop anything it didn't recognise. A real legacy 10-column
+// file lost test_id and test_name this way while reporting success.
+test("readRows refuses a foreign schema instead of silently dropping columns", () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-legacy-"));
+ const p = join(dir, "legacy.csv");
+ writeFileSync(p, "test_id,test_name,rca_done\nt1,login spec,pending\n", "utf8");
+
+ assert.throws(() => readRows(p), /unrecognised column/i,
+ "must name the problem rather than mangle the file");
+ assert.throws(() => readRows(p), /test_id/, "must say WHICH columns");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// Known legacy spellings are still accepted — the guard is for genuinely
+// foreign schemas, not for every older name.
+test("readRows maps aliased header names rather than rejecting them", () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-alias-"));
+ const p = join(dir, "aliased.csv");
+ writeFileSync(p, "test_run_id,status,thread_id\n42,pending,th-1\n", "utf8");
+
+ const rows = readRows(p);
+ assert.equal(rows[0].testRunId, "42");
+ assert.equal(rows[0].rca_done, "pending");
+ assert.equal(rows[0].threadId, "th-1");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// mkdirSync's `mode` applies on CREATE only, so a directory made before the
+// hardening landed keeps 0755 forever — with root causes and culprit PRs in it.
+test("writeRows tightens a pre-existing world-readable state dir", () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-perm-"));
+ const loose = join(dir, "loose");
+ mkdirSync(loose, { mode: 0o755 });
+ chmodSync(loose, 0o755); // as an older version would have left it
+
+ const csv = join(loose, "rca-state.b.csv");
+ writeRows(csv, []);
+
+ assert.equal(statSync(loose).mode & 0o777, 0o700, "existing dir must be tightened, not left open");
+ assert.equal(statSync(csv).mode & 0o777, 0o600);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// turnId only exists on a soft-PENDING turn, which is exactly the case that
+// produces pending-resume. Without it the resume path submits blind onto a
+// thread that still has a turn in flight — and the row looks healthy in the CSV.
+test("flipping to pending-resume without a turnId warns loudly", () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-resume-"));
+ const csv = join(dir, "s.csv");
+ seed(csv, "b", [{ test_id: 1, test_name: "t" }, { test_id: 2, test_name: "u" }]);
+
+ const warnings = [];
+ const orig = console.warn;
+ console.warn = (m) => warnings.push(String(m));
+ try {
+ flip(csv, 1, { rca_done: "pending-resume" }, 1000);
+ flip(csv, 2, { rca_done: "pending-resume", turnId: "abc-123" }, 1000);
+ } finally {
+ console.warn = orig;
+ }
+
+ const noTurn = warnings.filter((w) => /NO turnId/.test(w));
+ assert.equal(noTurn.length, 1, "exactly the seedless row must warn");
+ assert.match(noTurn[0], /submit blind/);
+ assert.equal(readRows(csv).find((r) => r.testRunId === "2").turnId, "abc-123");
+
+ // Still resumable either way — warning, not rejection.
+ assert.equal(readRows(csv).find((r) => r.testRunId === "1").rca_done, "pending-resume");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// "A PRODUCT_BUG RCA without a culprit PR is incomplete" was a prompt-only rule.
+// A stated "none — searched X" satisfies it; a blank field does not, and the two
+// are indistinguishable in the CSV.
+test("flip warns on a product bug with no PR evidence trail", () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-pb-"));
+ const csv = join(dir, "s.csv");
+ seed(csv, "b", [{ test_id: 1 }, { test_id: 2 }, { test_id: 3 }]);
+
+ const warnings = [];
+ const orig = console.warn;
+ console.warn = (m) => warnings.push(String(m));
+ try {
+ flip(csv, 1, { rca_done: "resolved", failure_type: "PRODUCT_BUG" }, 1);
+ flip(csv, 2, { rca_done: "resolved", failure_type: "PRODUCT_BUG", related_prs: ["https://x/pull/1"] }, 1);
+ flip(csv, 3, { rca_done: "resolved", failure_type: "PRODUCT_BUG", related_prs: "none — searched repo-a, repo-b in window" }, 1);
+ } finally {
+ console.warn = orig;
+ }
+
+ const pb = warnings.filter((w) => /EMPTY related_prs/.test(w));
+ assert.equal(pb.length, 1, "only the blank one warns");
+ assert.match(pb[0], /testRunId=1/);
+
+ // An honest dead end is compliant — must not be nagged.
+ assert.ok(!pb.some((w) => /testRunId=3/.test(w)), "a stated 'none, searched X' satisfies the rule");
+
+ rmSync(dir, { recursive: true, force: true });
+});
diff --git a/tests/evidence-file.test.mjs b/tests/evidence-file.test.mjs
new file mode 100644
index 0000000..b9a84d8
--- /dev/null
+++ b/tests/evidence-file.test.mjs
@@ -0,0 +1,450 @@
+import { test, beforeEach, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, rmSync, writeFileSync, statSync, chmodSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import {
+ evidencePathFor,
+ emptyEvidenceFile,
+ initEvidenceFile,
+ readEvidenceFile,
+ writeEvidenceFile,
+ setBaseline,
+ setGithubEvidence,
+ setLogsEvidence,
+ contributeGithubEvidence,
+ contributeLogsEvidence,
+ contribDirFor,
+ contribPathFor,
+ readBaseFile,
+ hasTrustworthyPrList,
+ recomputeCoverage,
+} from "../lib/evidence-file.mjs";
+
+let dir;
+let file;
+
+beforeEach(() => {
+ dir = mkdtempSync(join(tmpdir(), "rca-evidence-"));
+ file = join(dir, "evidence.json");
+});
+afterEach(() => rmSync(dir, { recursive: true, force: true }));
+
+test("evidencePathFor: build id is in the filename, default dir is OS temp", () => {
+ const p = evidencePathFor("abc123XYZ");
+ assert.ok(p.startsWith(join(tmpdir(), "bstack-rca")));
+ assert.ok(p.endsWith("rca-evidence.abc123XYZ.json"));
+});
+
+test("evidencePathFor: different builds never share a path", () => {
+ assert.notEqual(evidencePathFor("build-A"), evidencePathFor("build-B"));
+});
+
+test("evidencePathFor: sanitizes hostile ids and handles empty", () => {
+ assert.ok(evidencePathFor("../../etc/passwd").endsWith("rca-evidence..._.._etc_passwd.json"));
+ assert.ok(evidencePathFor("").endsWith("rca-evidence.unknown-build.json"));
+});
+
+test("evidencePathFor: stateDir override wins over temp", () => {
+ const p = evidencePathFor("b1", "/ci/artifacts");
+ assert.equal(p, join("/ci/artifacts", "rca-evidence.b1.json"));
+});
+
+test("readEvidenceFile on a missing path returns the empty shape, never throws", () => {
+ const doc = readEvidenceFile(file);
+ assert.deepEqual(doc, emptyEvidenceFile("unknown-build", 0));
+});
+
+test("initEvidenceFile creates the file with the given buildId", () => {
+ const doc = initEvidenceFile(file, "build-1", 1000);
+ assert.equal(doc.buildId, "build-1");
+ assert.equal(doc.generatedAtMs, 1000);
+ assert.deepEqual(readEvidenceFile(file), doc);
+});
+
+test("initEvidenceFile is idempotent — does not clobber an existing file", () => {
+ initEvidenceFile(file, "build-1", 1000);
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "x" } }, 2000);
+ const before = readEvidenceFile(file);
+ const again = initEvidenceFile(file, "build-1", 9999);
+ assert.deepEqual(again, before);
+});
+
+test("setGithubEvidence and setLogsEvidence coexist without clobbering each other", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "a-deploy" } }, 1000);
+ setLogsEvidence(file, "workload-1", { gap: null, kubectlSweep: { block: "w1-logs" } }, 1000);
+ const doc = readEvidenceFile(file);
+ assert.equal(doc.github["org/a"].deployState.block, "a-deploy");
+ assert.equal(doc.logs["workload-1"].kubectlSweep.block, "w1-logs");
+});
+
+test("setGithubEvidence for a second repo does not disturb the first", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "a" } }, 1000);
+ setGithubEvidence(file, "org/b", { gap: null, deployState: { block: "b" } }, 1000);
+ const doc = readEvidenceFile(file);
+ assert.equal(doc.github["org/a"].deployState.block, "a");
+ assert.equal(doc.github["org/b"].deployState.block, "b");
+});
+
+test("setGithubEvidence twice for the SAME repo overwrites only that repo", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "old" } }, 1000);
+ setGithubEvidence(file, "org/b", { gap: null, deployState: { block: "b" } }, 1000);
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "new" } }, 2000);
+ const doc = readEvidenceFile(file);
+ assert.equal(doc.github["org/a"].deployState.block, "new");
+ assert.equal(doc.github["org/b"].deployState.block, "b"); // untouched
+});
+
+test("setBaseline records baseline and suspectWindow without touching github/logs", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "a" } }, 1000);
+ setBaseline(file, { ref: "sha123", isFallback: false }, { reposRequested: ["org/a"] }, 2000);
+ const doc = readEvidenceFile(file);
+ assert.deepEqual(doc.baseline, { ref: "sha123", isFallback: false });
+ assert.deepEqual(doc.suspectWindow, { reposRequested: ["org/a"] });
+ assert.equal(doc.github["org/a"].deployState.block, "a"); // untouched
+});
+
+test("recomputeCoverage: a covered repo/workload has no gap; a missing one is gapped", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "a" } }, 1000);
+ setLogsEvidence(file, "w1", { gap: null, kubectlSweep: { block: "w1" } }, 1000);
+ const coverage = recomputeCoverage(
+ file,
+ { repos: ["org/a", "org/b"], workloads: ["w1", "w2"] },
+ 2000,
+ );
+ assert.deepEqual(coverage.reposCovered, ["org/a"]);
+ assert.deepEqual(coverage.reposGapped, ["org/b"]);
+ assert.deepEqual(coverage.workloadsCovered, ["w1"]);
+ assert.deepEqual(coverage.workloadsGapped, ["w2"]);
+});
+
+test("recomputeCoverage: a present entry with a non-null gap is NOT covered", () => {
+ setGithubEvidence(file, "org/a", { gap: "gh auth failed for this repo" }, 1000);
+ const coverage = recomputeCoverage(file, { repos: ["org/a"], workloads: [] }, 2000);
+ assert.deepEqual(coverage.reposCovered, []);
+ assert.deepEqual(coverage.reposGapped, ["org/a"]);
+});
+
+test("recomputeCoverage persists onto the file (readable afterwards)", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "a" } }, 1000);
+ recomputeCoverage(file, { repos: ["org/a"], workloads: [] }, 2000);
+ const doc = readEvidenceFile(file);
+ assert.deepEqual(doc.coverage.reposCovered, ["org/a"]);
+});
+
+test("a block string with newlines and quotes round-trips through JSON unchanged", () => {
+ const block = 'ASK: did X change?\nTYPE: product_code\nFOUND: yes\nSUMMARY: "quoted" finding\nSNIPPET: line1\nline2';
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block } }, 1000);
+ const doc = readEvidenceFile(file);
+ assert.equal(doc.github["org/a"].deployState.block, block);
+});
+
+test("contribute writes a shard, never the base file", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "base" } }, 1000);
+ contributeGithubEvidence(file, "3895581484", "org/a", {
+ deployState: { block: "coordinator's full diff" },
+ }, 2000);
+ // base is untouched...
+ assert.equal(readBaseFile(file).github["org/a"].deployState.block, "base");
+ // ...but the folded view shows the contribution
+ assert.equal(readEvidenceFile(file).github["org/a"].deployState.block, "coordinator's full diff");
+});
+
+test("contribPathFor: one file per writer, under the build's contrib dir", () => {
+ const p = contribPathFor(file, "3895581484");
+ assert.ok(p.startsWith(contribDirFor(file)));
+ assert.ok(p.endsWith("3895581484.json"));
+ assert.notEqual(contribPathFor(file, "w1"), contribPathFor(file, "w2"));
+});
+
+test("contribPathFor sanitizes a hostile writerId", () => {
+ assert.ok(contribPathFor(file, "../../etc/passwd").endsWith("_.._etc_passwd.json"));
+});
+
+test("CONCURRENCY: two writers on the same repo both survive (no lost update)", () => {
+ setGithubEvidence(file, "org/a", {
+ gap: null, deployState: { block: "base" }, prsInWindow: [{ pr: "#1" }],
+ }, 1000);
+ // Interleave the two writers the way real concurrent coordinators would:
+ // each reads, then each writes — under a single shared file this is exactly
+ // the sequence that drops the first writer's update.
+ contributeGithubEvidence(file, "writerA", "org/a", { prsInWindow: [{ pr: "#2", by: "A" }] }, 2000);
+ contributeGithubEvidence(file, "writerB", "org/a", { prsInWindow: [{ pr: "#3", by: "B" }] }, 2000);
+ const prs = readEvidenceFile(file).github["org/a"].prsInWindow.map((p) => p.pr).sort();
+ assert.deepEqual(prs, ["#1", "#2", "#3"]); // base + BOTH contributions
+});
+
+test("CONCURRENCY: two writers on the same workload both survive", () => {
+ contributeLogsEvidence(file, "writerA", "w1", { kubectlSweep: { block: "A found 3 lines" } }, 1000);
+ contributeLogsEvidence(file, "writerB", "w1", { victorialogs: { block: "B found 5xx" } }, 1000);
+ const w = readEvidenceFile(file).logs["w1"];
+ assert.equal(w.kubectlSweep.block, "A found 3 lines");
+ assert.equal(w.victorialogs.block, "B found 5xx");
+});
+
+test("fold: real contributed evidence beats a base-recorded gap", () => {
+ setGithubEvidence(file, "org/a", { gap: "gh auth failed" }, 1000);
+ contributeGithubEvidence(file, "w1", "org/a", {
+ gap: null, deployState: { block: "reachable after all" },
+ }, 2000);
+ const entry = readEvidenceFile(file).github["org/a"];
+ assert.equal(entry.gap, null);
+ assert.equal(entry.deployState.block, "reachable after all");
+});
+
+test("fold: a contributed gap does NOT overwrite real base evidence", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "real base evidence" } }, 1000);
+ contributeGithubEvidence(file, "w1", "org/a", { deployState: { gap: "my call failed" } }, 2000);
+ assert.equal(readEvidenceFile(file).github["org/a"].deployState.block, "real base evidence");
+});
+
+test("fold: same PR number contributed later wins (deeper finding replaces placeholder)", () => {
+ setGithubEvidence(file, "org/a", {
+ gap: null, prsInWindow: [{ pr: "#9011", verdict: "unassessed", files: null }],
+ }, 1000);
+ contributeGithubEvidence(file, "w1", "org/a", {
+ prsInWindow: [{ pr: "#9011", verdict: "supported", files: ["Foo.java"] }],
+ }, 2000);
+ const prs = readEvidenceFile(file).github["org/a"].prsInWindow;
+ assert.equal(prs.length, 1);
+ assert.equal(prs[0].verdict, "supported");
+});
+
+test("fold: contributing a repo the pre-fetch never named", () => {
+ contributeGithubEvidence(file, "w1", "org/brand-new", {
+ prsInWindow: [{ pr: "#8912", verdict: "supported" }],
+ }, 1000);
+ assert.equal(readEvidenceFile(file).github["org/brand-new"].prsInWindow[0].pr, "#8912");
+});
+
+test("fold: clusterIds union across base and multiple shards", () => {
+ setLogsEvidence(file, "w1", { gap: null, clusterIds: ["c-A"], kubectlSweep: { block: "x" } }, 1000);
+ contributeLogsEvidence(file, "w1writer", "w1", { clusterIds: ["c-B"] }, 2000);
+ contributeLogsEvidence(file, "w2writer", "w1", { clusterIds: ["c-C"] }, 2000);
+ assert.deepEqual(readEvidenceFile(file).logs["w1"].clusterIds.sort(), ["c-A", "c-B", "c-C"]);
+});
+
+test("fold: a corrupt shard is skipped, not fatal", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "base" } }, 1000);
+ contributeGithubEvidence(file, "good", "org/a", { prsInWindow: [{ pr: "#2" }] }, 2000);
+ writeFileSync(contribPathFor(file, "corrupt"), "{not json", "utf8");
+ const doc = readEvidenceFile(file); // must not throw
+ assert.equal(doc.github["org/a"].prsInWindow[0].pr, "#2");
+});
+
+test("recomputeCoverage counts a coordinator-filled gap as covered", () => {
+ setGithubEvidence(file, "org/a", { gap: "unreachable at pre-fetch time" }, 1000);
+ let cov = recomputeCoverage(file, { repos: ["org/a"], workloads: [] }, 2000);
+ assert.deepEqual(cov.reposGapped, ["org/a"]);
+ contributeGithubEvidence(file, "w1", "org/a", { gap: null, deployState: { block: "got it" } }, 3000);
+ cov = recomputeCoverage(file, { repos: ["org/a"], workloads: [] }, 4000);
+ assert.deepEqual(cov.reposCovered, ["org/a"]);
+ assert.deepEqual(cov.reposGapped, []);
+});
+
+// Regression: an empty prsInWindow with gap:null used to read as "searched,
+// found none" when it may simply never have been populated. Observed live —
+// a file asserted 0 PRs for a repo that actually had 21, which would have let
+// a coordinator conclude "no culprit PR" with false confidence.
+test("empty prsInWindow is NOT coverage unless the search is recorded", () => {
+ setGithubEvidence(file, "org/never-searched", { gap: null, deployState: { block: "d" }, prsInWindow: [] }, 1000);
+ setGithubEvidence(file, "org/searched-empty", { gap: null, deployState: { block: "d" }, prsInWindow: [], prsSearched: true }, 1000);
+ const cov = recomputeCoverage(file, { repos: ["org/never-searched", "org/searched-empty"], workloads: [] }, 2000);
+ // Both repos ARE covered (each has deploy state) — but only one has a PR
+ // list safe to read as "no PRs in window".
+ assert.deepEqual(cov.reposCovered.sort(), ["org/never-searched", "org/searched-empty"]);
+ assert.deepEqual(cov.reposWithUntrustedPrList, ["org/never-searched"]);
+});
+
+test("hasTrustworthyPrList distinguishes searched-empty from never-populated", () => {
+ setGithubEvidence(file, "org/a", { gap: null, prsInWindow: [] }, 1000);
+ setGithubEvidence(file, "org/b", { gap: null, prsInWindow: [], prsSearched: true }, 1000);
+ setGithubEvidence(file, "org/c", { gap: null, prsInWindow: [{ pr: "#1" }] }, 1000);
+ const doc = readEvidenceFile(file);
+ assert.equal(hasTrustworthyPrList(doc, "org/a"), false);
+ assert.equal(hasTrustworthyPrList(doc, "org/b"), true);
+ assert.equal(hasTrustworthyPrList(doc, "org/c"), true);
+});
+
+test("contributing a PR list records that the search actually ran", () => {
+ contributeGithubEvidence(file, "w1", "org/a", { prsInWindow: [] }, 1000);
+ assert.equal(hasTrustworthyPrList(readEvidenceFile(file), "org/a"), true);
+});
+
+test("prsSearched is sticky — a later non-searching contributor cannot downgrade it", () => {
+ setGithubEvidence(file, "org/a", { gap: null, prsInWindow: [{ pr: "#1" }], prsSearched: true }, 1000);
+ contributeGithubEvidence(file, "w1", "org/a", { deployState: { block: "just deploy info" } }, 2000);
+ assert.equal(readEvidenceFile(file).github["org/a"].prsSearched, true);
+});
+
+test("a pre-existing loose-mode file is tightened to 0600 on the next write", () => {
+ writeEvidenceFile(file, emptyEvidenceFile("b", 0));
+ chmodSync(file, 0o644); // simulate a file left by a pre-hardening run
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "x" } }, 1000);
+ assert.equal(statSync(file).mode & 0o777, 0o600);
+});
+
+test("evidence file and contribution shards are owner-only (0600)", () => {
+ setGithubEvidence(file, "org/a", { gap: null, deployState: { block: "private PR detail" } }, 1000);
+ contributeGithubEvidence(file, "w1", "org/a", { prsInWindow: [{ pr: "#1" }] }, 2000);
+ assert.equal(statSync(file).mode & 0o777, 0o600);
+ assert.equal(statSync(contribPathFor(file, "w1")).mode & 0o777, 0o600);
+});
+
+test("writeEvidenceFile creates the parent directory if missing", () => {
+ const nested = join(dir, "nested", "sub", "evidence.json");
+ writeEvidenceFile(nested, emptyEvidenceFile("build-1", 0));
+ assert.deepEqual(readEvidenceFile(nested).buildId, "build-1");
+});
+
+// Staleness: the resume-path analogue of refusing a branch name.
+test("stalenessOf flags an old pre-fetch but never invalidates it", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-stale-"));
+ const { evidencePathFor, initEvidenceFile, stalenessOf, readEvidenceFile } =
+ await import("../lib/evidence-file.mjs");
+ const t0 = 1_700_000_000_000;
+ const p = evidencePathFor("b-stale", dir);
+ initEvidenceFile(p, "b-stale", t0);
+
+ const fresh = stalenessOf(p, t0 + 5 * 60 * 1000);
+ assert.equal(fresh.stale, false, "5m into a run is fresh");
+ assert.equal(fresh.known, true);
+
+ const old = stalenessOf(p, t0 + 20 * 60 * 60 * 1000);
+ assert.equal(old.stale, true, "an overnight resume must be flagged");
+ assert.match(old.note, /re-verify/, "must say what to do, not just that it is old");
+
+ // Crucially it is a SIGNAL, not an expiry — the data is still there, because
+ // stale build-level context still beats none and the failure window is fixed.
+ assert.ok(readEvidenceFile(p), "file must remain readable when stale");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// The clamp-to-zero trap: a future timestamp must not read as "fresh".
+test("stalenessOf refuses to call a future timestamp fresh", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-skew-"));
+ const { evidencePathFor, initEvidenceFile, stalenessOf } = await import("../lib/evidence-file.mjs");
+ const t0 = 1_700_000_000_000;
+ const p = evidencePathFor("b-skew", dir);
+ initEvidenceFile(p, "b-skew", t0);
+
+ // Coordinator's clock is behind the gate's, or the stamp was seeded by hand.
+ const s = stalenessOf(p, t0 - 11 * 60 * 60 * 1000);
+ assert.equal(s.stale, true, "unknown age must fail closed, not report fresh");
+ assert.equal(s.known, false, "we genuinely cannot compute an age here");
+ assert.match(s.note, /future/);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// The sha lived only in prose, so the one consumer that needs it structurally
+// got an empty map — silently downgrading every local read to a network call.
+test("deployShas prefers the explicit field and falls back to the summary", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-pins-"));
+ const { evidencePathFor, initEvidenceFile, setGithubEvidence, deployShas } =
+ await import("../lib/evidence-file.mjs");
+ const p = evidencePathFor("b-pins", dir);
+ initEvidenceFile(p, "b-pins", 1);
+
+ setGithubEvidence(p, "org/explicit", { deployState: { sha: "abc1234", summary: "" } }, 2);
+ setGithubEvidence(p, "org/prose", {
+ deployState: { summary: "Branch tip on main at build start = cd88535b (deploy proxy). Redeploy stamped 260731135020Z." },
+ }, 3);
+ setGithubEvidence(p, "org/none", { deployState: { summary: "no sha here" } }, 4);
+
+ const { pins, source } = deployShas(p);
+ assert.equal(pins["org/explicit"], "abc1234");
+ assert.equal(source["org/explicit"], "field");
+ assert.equal(pins["org/prose"], "cd88535b", "must recover the sha from prose");
+ assert.equal(source["org/prose"], "parsed-from-summary");
+ assert.equal(pins["org/none"], undefined, "absent must stay absent, not guess");
+
+ // The timestamp 260731135020Z is hex-ish and long — anchoring on the
+ // build-start phrase is what stops it being mistaken for a commit.
+ assert.notEqual(pins["org/prose"], "260731135020");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// Observed live: a coordinator wrote back a 6-PR window and the file kept ONE,
+// with `pr: undefined`, while still flagging the search trustworthy. Cause:
+// String(undefined) is the constant "undefined", so every numberless PR
+// collided on a single dedupe key.
+test("numberless PRs do not collapse into one another", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-prkey-"));
+ const { evidencePathFor, initEvidenceFile, contributeGithubEvidence, readEvidenceFile } =
+ await import("../lib/evidence-file.mjs");
+ const p = evidencePathFor("b-prkey", dir);
+ initEvidenceFile(p, "b-prkey", 1);
+
+ contributeGithubEvidence(p, "w1", "org/r", {
+ prsSearched: true,
+ prsInWindow: [{ title: "first" }, { title: "second" }, { title: "third" }],
+ }, 2);
+ const got = readEvidenceFile(p).github["org/r"].prsInWindow;
+ assert.equal(got.length, 3, "three distinct unnumbered PRs must all survive");
+ assert.deepEqual(got.map((x) => x.title), ["first", "second", "third"]);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("numbered PRs still merge across writers, string or numeric", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-prnum-"));
+ const { evidencePathFor, initEvidenceFile, contributeGithubEvidence, readEvidenceFile } =
+ await import("../lib/evidence-file.mjs");
+ const p = evidencePathFor("b-prnum", dir);
+ initEvidenceFile(p, "b-prnum", 1);
+
+ contributeGithubEvidence(p, "w1", "org/r", { prsInWindow: [{ pr: "#10", title: "a" }] }, 2);
+ contributeGithubEvidence(p, "w2", "org/r", { prsInWindow: [{ pr: 10, title: "a-updated" }] }, 3);
+
+ const got = readEvidenceFile(p).github["org/r"].prsInWindow;
+ assert.equal(got.length, 1, "'#10' and 10 are the same PR");
+ assert.equal(got[0].title, "a-updated", "later writer wins");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// Prompting agents to use evidence-show wasn't enough: 21 of 25 reads on a real
+// run were raw cat/grep/Read against the base path, each silently missing every
+// contribution shard. The file now announces that in its own first bytes.
+test("a raw read of the base file announces that it is partial", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-warn-"));
+ const { evidencePathFor, initEvidenceFile, setGithubEvidence, readEvidenceFile, readBaseFile } =
+ await import("../lib/evidence-file.mjs");
+ const { readFileSync } = await import("node:fs");
+ const p = evidencePathFor("b-warn", dir);
+ initEvidenceFile(p, "b-warn", 1);
+ setGithubEvidence(p, "org/r", { deployState: { sha: "abc1234" } }, 2);
+
+ const raw = readFileSync(p, "utf8");
+ const head = raw.slice(0, 400);
+ assert.match(head, /PARTIAL VIEW/, "warning must be in the first bytes a cat/head shows");
+ assert.match(raw, /evidence-show\.mjs/, "must name the command that gives the real view");
+
+ // Markers are documentation, never data — nothing downstream should see them.
+ for (const doc of [readBaseFile(p), readEvidenceFile(p)]) {
+ assert.equal(doc._READ_ME_FIRST, undefined);
+ assert.equal(doc._USE_INSTEAD, undefined);
+ assert.equal(doc._WHY, undefined);
+ }
+ // And the real content still round-trips.
+ assert.equal(readEvidenceFile(p).github["org/r"].deployState.sha, "abc1234");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("markers survive repeated writes without accumulating", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-warn2-"));
+ const { evidencePathFor, initEvidenceFile, setGithubEvidence } = await import("../lib/evidence-file.mjs");
+ const { readFileSync } = await import("node:fs");
+ const p = evidencePathFor("b-w2", dir);
+ initEvidenceFile(p, "b-w2", 1);
+ for (let i = 0; i < 3; i++) setGithubEvidence(p, `org/r${i}`, { deployState: { sha: "abc1234" } }, i + 2);
+
+ const raw = readFileSync(p, "utf8");
+ assert.equal(raw.split("_READ_ME_FIRST").length - 1, 1, "exactly one marker, not one per write");
+
+ rmSync(dir, { recursive: true, force: true });
+});
diff --git a/tests/loop-parallel-gather.test.mjs b/tests/loop-parallel-gather.test.mjs
new file mode 100644
index 0000000..a05f56f
--- /dev/null
+++ b/tests/loop-parallel-gather.test.mjs
@@ -0,0 +1,154 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { runRcaLoop } from "../lib/loop.mjs";
+
+// A NEEDS_INFO turn's independent asks (lib/routing.mjs's routeAsk/routeAsks
+// have no cross-ask state) should be gathered concurrently, not one round-trip
+// at a time. This file proves both properties of that fix: the calls actually
+// overlap, and the final message still assembles blocks in priority order
+// regardless of which one finishes first.
+
+const CONFIG = {
+ turnCap: 6,
+ evidenceRouting: {
+ test_logs: { owner: "tfa", skip: true },
+ product_code: { capability: "github" },
+ infra: { capability: "infra" },
+ other: { capability: "other" },
+ },
+};
+const MANIFEST = {
+ github: { available: true, via: "gh" },
+ infra: { available: true, via: "kubectl" },
+};
+const resolved = (threadId) => ({
+ status: "RESOLVED",
+ threadId,
+ confidence: "high",
+ glimpse: { root_cause: "r", failure_type: "f", related_prs: [] },
+ viewRca: "https://automation.browserstack.com/x",
+});
+const delay = (ms) => new Promise((r) => setTimeout(r, ms));
+
+test("independent NEEDS_INFO gathers run concurrently, not sequentially", async () => {
+ const events = [];
+ const gather = async (g) => {
+ events.push(`start:${g.evidenceType}`);
+ // The slower ask (product_code) is listed FIRST but finishes LAST. If
+ // gathers were sequential, infra's "start" could never appear before
+ // product_code's "end".
+ await delay(g.evidenceType === "product_code" ? 30 : 5);
+ events.push(`end:${g.evidenceType}`);
+ return `BLOCK:${g.evidenceType}`;
+ };
+
+ let calls = 0;
+ const submit = async () => {
+ calls++;
+ if (calls === 1) {
+ return {
+ status: "NEEDS_INFO",
+ threadId: "chat:1",
+ asks: [
+ { evidenceType: "product_code", priority: "high", ask: { what: "diff" } },
+ { evidenceType: "infra", priority: "medium", ask: { what: "pod status" } },
+ ],
+ };
+ }
+ return resolved("chat:1");
+ };
+
+ await runRcaLoop({
+ testRunId: "1",
+ firstMessage: "start",
+ submit,
+ config: CONFIG,
+ manifest: MANIFEST,
+ gather,
+ });
+
+ const firstEnd = events.findIndex((e) => e.startsWith("end:"));
+ const startsBeforeFirstEnd = events.slice(0, firstEnd).filter((e) => e.startsWith("start:"));
+ assert.equal(
+ startsBeforeFirstEnd.length,
+ 2,
+ `expected both gathers to start before either finished, got: ${events.join(", ")}`,
+ );
+});
+
+test("gathered blocks preserve priority order in the message even when the slower ask finishes first", async () => {
+ const gather = async (g) => {
+ await delay(g.evidenceType === "product_code" ? 30 : 5);
+ return `BLOCK:${g.evidenceType}`;
+ };
+
+ let calls = 0;
+ const submits = [];
+ const submit = async (args) => {
+ calls++;
+ submits.push(args);
+ if (calls === 1) {
+ return {
+ status: "NEEDS_INFO",
+ threadId: "chat:2",
+ // Listed low-priority-first on purpose — the message must still put
+ // high-priority product_code ahead of low-priority infra.
+ asks: [
+ { evidenceType: "infra", priority: "low", ask: { what: "pod status" } },
+ { evidenceType: "product_code", priority: "high", ask: { what: "diff" } },
+ ],
+ };
+ }
+ return resolved("chat:2");
+ };
+
+ await runRcaLoop({
+ testRunId: "2",
+ firstMessage: "start",
+ submit,
+ config: CONFIG,
+ manifest: MANIFEST,
+ gather,
+ });
+
+ assert.equal(
+ submits[1].message,
+ "BLOCK:product_code\n\nBLOCK:infra",
+ "high-priority product_code must precede low-priority infra regardless of which gather resolved first",
+ );
+});
+
+test("gap blocks still follow every gathered block, unaffected by concurrency", async () => {
+ const gather = async (g) => `BLOCK:${g.evidenceType}`;
+
+ let calls = 0;
+ const submits = [];
+ const submit = async (args) => {
+ calls++;
+ submits.push(args);
+ if (calls === 1) {
+ return {
+ status: "NEEDS_INFO",
+ threadId: "chat:3",
+ asks: [
+ { evidenceType: "product_code", priority: "high", ask: { what: "diff" } },
+ { evidenceType: "metrics", priority: "low", ask: { what: "latency" } }, // no capability -> gap
+ ],
+ };
+ }
+ return resolved("chat:3");
+ };
+
+ const result = await runRcaLoop({
+ testRunId: "3",
+ firstMessage: "start",
+ submit,
+ config: CONFIG,
+ manifest: MANIFEST,
+ gather,
+ });
+
+ assert.match(submits[1].message, /^BLOCK:product_code\n\nASK:/);
+ assert.deepEqual(result.asks_fulfilled, ["product_code"]);
+ assert.deepEqual(result.asks_unavailable, ["metrics"]);
+});
diff --git a/tests/loop-turn1-result.test.mjs b/tests/loop-turn1-result.test.mjs
new file mode 100644
index 0000000..a7d286f
--- /dev/null
+++ b/tests/loop-turn1-result.test.mjs
@@ -0,0 +1,110 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { runRcaLoop } from "../lib/loop.mjs";
+
+// Step 4b (SKILL.md Step 4b) pre-submits turn 1 for a cluster representative,
+// concurrently with Step 4's evidence pre-fetch. When it lands NEEDS_INFO,
+// `turn1Result` carries that thread + those asks in so the loop never
+// resubmits turn 1 — this file is the conformance coverage for that skip-ahead
+// path, mirroring tests/conformance.test.mjs's fixture style but inline since
+// there's nothing to replay: turn 1 already happened before runRcaLoop starts.
+
+const CONFIG = {
+ turnCap: 6,
+ evidenceRouting: {
+ test_logs: { owner: "tfa", skip: true },
+ product_code: { capability: "github" },
+ other: { capability: "other" },
+ },
+};
+const GITHUB_AVAILABLE = { github: { available: true, via: "gh" } };
+const gather = async (g) => `ASK: ${g.ask.what}\nTYPE: ${g.evidenceType}\nFOUND: yes\nSUMMARY: stub`;
+
+test("turn1Result: never resubmits turn 1, starts at ROUTE, turns_used counts the pre-dispatched turn", async () => {
+ const submits = [];
+ // The only submit() call the loop makes is the FOLLOW-UP — turn 1 already
+ // happened in Step 4b and is represented purely by `turn1Result`.
+ const submit = async (args) => {
+ submits.push(args);
+ return {
+ status: "RESOLVED",
+ threadId: "chat:99",
+ confidence: "high",
+ glimpse: { root_cause: "root cause found", failure_type: "product_regression", related_prs: ["#1"] },
+ viewRca: "https://automation.browserstack.com/x",
+ };
+ };
+
+ const result = await runRcaLoop({
+ testRunId: "99",
+ submit,
+ config: CONFIG,
+ manifest: GITHUB_AVAILABLE,
+ gather,
+ turn1Result: { threadId: "chat:99", asks: [{ evidenceType: "product_code", ask: { what: "diff" } }] },
+ });
+
+ assert.equal(submits.length, 1, "turn 1 must never be submitted — only the follow-up");
+ assert.equal(submits[0].threadId, "chat:99", "the follow-up reuses Step 4b's thread");
+ assert.equal(submits[0].turnId, undefined, "no turnId — NEEDS_INFO never carries one");
+ assert.equal(result.status, "RESOLVED");
+ assert.equal(result.turns_used, 2, "1 = Step 4b's pre-dispatched turn, 2 = this follow-up");
+ assert.equal(result.threadId, "chat:99");
+});
+
+test("turn1Result only short-circuits the FIRST pass — later iterations submit normally", async () => {
+ let calls = 0;
+ const submit = async () => {
+ calls++;
+ if (calls === 1) {
+ return { status: "NEEDS_INFO", threadId: "chat:99", asks: [{ evidenceType: "other", ask: { what: "logs excerpt" } }] };
+ }
+ return {
+ status: "RESOLVED",
+ threadId: "chat:99",
+ confidence: "medium",
+ glimpse: { root_cause: "resolved on turn 3", failure_type: "infra", related_prs: [] },
+ viewRca: "https://automation.browserstack.com/y",
+ };
+ };
+
+ const result = await runRcaLoop({
+ testRunId: "99",
+ submit,
+ config: CONFIG,
+ manifest: GITHUB_AVAILABLE,
+ gather,
+ turn1Result: { threadId: "chat:99", asks: [{ evidenceType: "product_code", ask: { what: "diff" } }] },
+ });
+
+ assert.equal(calls, 2, "one real submit for the ROUTE follow-up, one more to resolve");
+ assert.equal(result.status, "RESOLVED");
+ assert.equal(result.turns_used, 3, "1 pre-dispatched + 2 real submits");
+});
+
+test("without turn1Result, behaviour is unchanged: turn 1 IS submitted normally", async () => {
+ const submits = [];
+ const submit = async (args) => {
+ submits.push(args);
+ return {
+ status: "RESOLVED",
+ threadId: "chat:1",
+ confidence: "high",
+ glimpse: { root_cause: "root", failure_type: "product_regression", related_prs: [] },
+ viewRca: "https://automation.browserstack.com/z",
+ };
+ };
+
+ const result = await runRcaLoop({
+ testRunId: "1",
+ firstMessage: "Initiating collaborative RCA for test run 1.",
+ submit,
+ config: CONFIG,
+ manifest: GITHUB_AVAILABLE,
+ gather,
+ });
+
+ assert.equal(submits.length, 1);
+ assert.equal(submits[0].message, "Initiating collaborative RCA for test run 1.");
+ assert.equal(result.turns_used, 1);
+});
diff --git a/tests/repo-source.test.mjs b/tests/repo-source.test.mjs
new file mode 100644
index 0000000..08cfb40
--- /dev/null
+++ b/tests/repo-source.test.mjs
@@ -0,0 +1,148 @@
+import { test, beforeEach, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, rmSync, mkdirSync, writeFileSync } from "node:fs";
+import { execFileSync } from "node:child_process";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { localCloneFor, hasCommit, readFileAt, discoverWorkspaceRoot, resolveLocalRepos } from "../lib/repo-source.mjs";
+
+let ws, repoDir, sha1, sha2;
+
+// Build a real throwaway git repo with two commits, so the staleness scenario
+// is exercised for real rather than mocked.
+beforeEach(() => {
+ ws = mkdtempSync(join(tmpdir(), "rca-ws-"));
+ repoDir = join(ws, "testrepo");
+ mkdirSync(repoDir);
+ const g = (...a) => execFileSync("git", ["-C", repoDir, ...a], { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] });
+ g("init", "-q");
+ g("config", "user.email", "t@t.t");
+ g("config", "user.name", "t");
+ writeFileSync(join(repoDir, "app.js"), "VERSION_ONE\n");
+ g("add", "."); g("commit", "-qm", "one");
+ sha1 = g("rev-parse", "HEAD").trim();
+ writeFileSync(join(repoDir, "app.js"), "VERSION_TWO\n");
+ g("add", "."); g("commit", "-qm", "two");
+ sha2 = g("rev-parse", "HEAD").trim();
+});
+afterEach(() => rmSync(ws, { recursive: true, force: true }));
+
+test("localCloneFor finds a clone by bare repo name", () => {
+ assert.equal(localCloneFor("browserstack/testrepo", ws), repoDir);
+ assert.equal(localCloneFor("browserstack/not-cloned", ws), null);
+});
+
+test("hasCommit distinguishes present from absent commits", () => {
+ assert.equal(hasCommit(repoDir, sha1), true);
+ assert.equal(hasCommit(repoDir, "0".repeat(40)), false);
+});
+
+// The core safety property. A branch name resolves to whatever the clone
+// happens to have, which on a real machine was 12 commits stale and returned
+// different bytes than the true head — a silently wrong RCA input.
+test("a branch name is REFUSED; only a commit sha is accepted", () => {
+ const r = readFileAt({ repo: "browserstack/testrepo", sha: "main", path: "app.js", workspaceRoot: ws });
+ assert.equal(r.ok, false);
+ assert.equal(r.source, "remote-needed");
+ assert.match(r.reason, /must be a commit sha/);
+});
+
+test("a pinned sha reads the content AT THAT COMMIT, not the tip", () => {
+ const older = readFileAt({ repo: "browserstack/testrepo", sha: sha1, path: "app.js", workspaceRoot: ws });
+ assert.equal(older.ok, true);
+ assert.equal(older.source, "local");
+ assert.equal(older.content.trim(), "VERSION_ONE", "must read the old commit, not HEAD");
+
+ const newer = readFileAt({ repo: "browserstack/testrepo", sha: sha2, path: "app.js", workspaceRoot: ws });
+ assert.equal(newer.content.trim(), "VERSION_TWO");
+});
+
+test("no local clone -> defers to the caller for a remote read", () => {
+ const r = readFileAt({ repo: "browserstack/absent", sha: sha1, path: "app.js", workspaceRoot: ws });
+ assert.equal(r.ok, false);
+ assert.equal(r.source, "remote-needed");
+ assert.match(r.reason, /no local clone/);
+});
+
+test("commit absent locally -> remote-needed, and does NOT fetch unless asked", () => {
+ const r = readFileAt({ repo: "browserstack/testrepo", sha: "0".repeat(40), path: "app.js", workspaceRoot: ws });
+ assert.equal(r.ok, false);
+ assert.equal(r.source, "remote-needed");
+ assert.match(r.reason, /not present|allowFetch/);
+});
+
+// A path that genuinely didn't exist at that commit is an ANSWER. Treating it
+// as a fallback trigger would send the caller to the network to be told the
+// same thing, and risks a tip-of-branch read papering over the real history.
+// The real shape: the plugin lives one level inside the workspace, alongside
+// the clones, so the root is found on the second try.
+test("discoverWorkspaceRoot walks up to the dir holding the clones", () => {
+ const pluginDir = join(ws, "some-plugin");
+ mkdirSync(pluginDir, { recursive: true });
+ const d = discoverWorkspaceRoot({ repos: ["browserstack/testrepo"], from: pluginDir });
+ assert.equal(d.root, ws);
+ assert.equal(d.matched, "browserstack/testrepo");
+ assert.equal(d.tried.length, 2, "found on the second candidate");
+});
+
+// The bound is a feature: from deep inside a repo the root is out of reach,
+// and the correct answer is to stop rather than climb toward `/` and risk
+// matching an unrelated checkout.
+test("discoverWorkspaceRoot stops at maxTries instead of climbing far", () => {
+ const deep = join(repoDir, "a", "b", "c");
+ mkdirSync(deep, { recursive: true });
+ const d = discoverWorkspaceRoot({ repos: ["browserstack/testrepo"], from: deep, maxTries: 3 });
+ assert.equal(d.root, null, "workspace is 4 levels up — out of the bounded range");
+ assert.equal(d.tried.length, 3);
+});
+
+// Genericity: a candidate wins only if it holds a repo THIS run validated.
+// Nothing about the product or layout is assumed.
+test("discoverWorkspaceRoot verifies against the run's own repo list", () => {
+ const d = discoverWorkspaceRoot({ repos: ["browserstack/some-other-product"], from: repoDir });
+ assert.equal(d.root, null, "must not accept a dir that lacks the requested repo");
+ assert.match(d.reason, /some-other-product/);
+});
+
+test("discoverWorkspaceRoot is bounded — it gives up rather than hunting", () => {
+ const d = discoverWorkspaceRoot({ repos: ["browserstack/nope"], from: repoDir, maxTries: 3 });
+ assert.equal(d.root, null);
+ assert.ok(d.tried.length <= 3, `tried ${d.tried.length}, expected <= 3`);
+});
+
+test("an explicit root is still VERIFIED, so a stale override fails loudly", () => {
+ const ok = discoverWorkspaceRoot({ repos: ["browserstack/testrepo"], explicit: ws });
+ assert.equal(ok.root, ws);
+ const bad = discoverWorkspaceRoot({ repos: ["browserstack/testrepo"], explicit: join(ws, "nowhere") });
+ assert.equal(bad.root, null, "a wrong explicit path must not be trusted blindly");
+});
+
+// This is the context-saving payload: resolved once, read by every coordinator.
+test("resolveLocalRepos reports per-repo usability at the pinned sha", () => {
+ const r = resolveLocalRepos({
+ repos: ["browserstack/testrepo", "browserstack/absent"],
+ pins: { "browserstack/testrepo": sha1 },
+ workspaceRoot: ws,
+ });
+ assert.equal(r["browserstack/testrepo"].usable, true);
+ assert.equal(r["browserstack/testrepo"].sha, sha1);
+ assert.equal(r["browserstack/absent"].usable, false);
+ assert.match(r["browserstack/absent"].reason, /no local clone/);
+});
+
+test("resolveLocalRepos marks a repo unusable when its sha is absent", () => {
+ const r = resolveLocalRepos({
+ repos: ["browserstack/testrepo"],
+ pins: { "browserstack/testrepo": "0".repeat(40) },
+ workspaceRoot: ws,
+ });
+ assert.equal(r["browserstack/testrepo"].usable, false);
+ assert.match(r["browserstack/testrepo"].reason, /not present locally/);
+});
+
+test("path missing at that commit is a local answer, not a remote fallback", () => {
+ const r = readFileAt({ repo: "browserstack/testrepo", sha: sha1, path: "nope.js", workspaceRoot: ws });
+ assert.equal(r.ok, false);
+ assert.equal(r.source, "local");
+ assert.match(r.reason, /path not present/);
+});
diff --git a/tests/signature.test.mjs b/tests/signature.test.mjs
index f721167..02dbfd0 100644
--- a/tests/signature.test.mjs
+++ b/tests/signature.test.mjs
@@ -1,5 +1,8 @@
import { test } from "node:test";
import assert from "node:assert/strict";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
import {
normalize,
computeSignature,
@@ -77,3 +80,62 @@ test("clusterRows stamps cluster_id onto every row", () => {
assert.ok(rows.every((r) => r.cluster_id));
assert.notEqual(rows[0].cluster_id, rows[1].cluster_id);
});
+
+// clusterRows mutates its input; a caller that destructures only `clusters`
+// silently loses every cluster_id. Two independent callers did exactly that on
+// the same day, collapsing a clustered run into one coordinator per test.
+test("clusterAndPersist writes cluster_id back to the CSV", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-cap-"));
+ const csvState = await import("../lib/csv-state.mjs");
+ const { clusterAndPersist } = await import("../lib/signature.mjs");
+ const csv = join(dir, "s.csv");
+ csvState.seed(csv, "b", [
+ { test_id: 1, test_name: "a", failure: { error_summary: "boom" } },
+ { test_id: 2, test_name: "b", failure: { error_summary: "boom" } },
+ { test_id: 3, test_name: "c", failure: { error_summary: "other" } },
+ ]);
+
+ const clusters = clusterAndPersist(csv, csvState);
+ assert.equal(clusters.length, 2, "two distinct signatures");
+
+ // The whole point: re-READ from disk, don't trust the in-memory rows.
+ const reread = csvState.readRows(csv);
+ assert.ok(reread.every((r) => r.cluster_id), "every row must have a persisted cluster_id");
+ assert.equal(reread[0].cluster_id, reread[1].cluster_id, "same signature → same cluster");
+ assert.notEqual(reread[0].cluster_id, reread[2].cluster_id);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+// Siblings are only cheap because they confirm someone else's hypothesis.
+// Dispatched without one they re-investigate from scratch — measured at 22.7
+// calls vs 8.0 for the representative they were meant to be a fraction of.
+test("siblingPreSeed refuses to seed from an unfinished representative", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "rca-seed-"));
+ const csvState = await import("../lib/csv-state.mjs");
+ const { siblingPreSeed } = await import("../lib/signature.mjs");
+ const csv = join(dir, "s.csv");
+ csvState.seed(csv, "b", [
+ { test_id: 1, test_name: "rep", failure: { error_summary: "boom" } },
+ { test_id: 2, test_name: "sib", failure: { error_summary: "boom" } },
+ ]);
+
+ const early = siblingPreSeed(csv, csvState, "c-1", 1);
+ assert.equal(early.ok, false, "rep is still pending — must block");
+ assert.match(early.reason, /not resolved/);
+
+ // Resolved but with no root_cause is equally useless to a sibling.
+ csvState.flip(csv, 1, { rca_done: "resolved" }, 1000);
+ const empty = siblingPreSeed(csv, csvState, "c-1", 1);
+ assert.equal(empty.ok, false);
+ assert.match(empty.reason, /no root_cause/);
+
+ csvState.flip(csv, 1, { rca_done: "resolved", root_cause: "PR #42 broke seeding", failure_type: "PRODUCT_BUG" }, 2000);
+ const ok = siblingPreSeed(csv, csvState, "c-1", 1);
+ assert.equal(ok.ok, true);
+ assert.equal(ok.pre_seed.cause, "PR #42 broke seeding");
+ assert.equal(ok.pre_seed.failure_type, "PRODUCT_BUG");
+ assert.match(ok.pre_seed.instruction, /Do not adopt it/, "independence must travel with the seed");
+
+ rmSync(dir, { recursive: true, force: true });
+});
diff --git a/tests/state-dir.test.mjs b/tests/state-dir.test.mjs
new file mode 100644
index 0000000..1613785
--- /dev/null
+++ b/tests/state-dir.test.mjs
@@ -0,0 +1,102 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, rmSync, mkdirSync, writeFileSync, chmodSync, statSync, existsSync, utimesSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { hardenStateDir, pruneStateDir } from "../lib/state-dir.mjs";
+
+const mode = (p) => statSync(p).mode & 0o777;
+
+function fixture() {
+ const root = mkdtempSync(join(tmpdir(), "rca-sd-"));
+ const dir = join(root, "bstack-rca");
+ mkdirSync(dir, { mode: 0o755 });
+ writeFileSync(join(dir, "rca-state.b1.csv"), "a\n", { mode: 0o644 });
+ const cache = join(dir, "rca-toolcache.b1");
+ mkdirSync(cache, { mode: 0o755 });
+ writeFileSync(join(cache, "entry.json"), "{}", { mode: 0o644 });
+ chmodSync(dir, 0o755);
+ chmodSync(cache, 0o755);
+ return { root, dir, cache };
+}
+
+// Per-write hardening only fixes the file being written, so a build analysed
+// before the hardening landed keeps 0644 forever — a completed build is never
+// rewritten. This is the sweep that repairs them.
+test("hardenStateDir tightens leftovers recursively", () => {
+ const { root, dir, cache } = fixture();
+ assert.equal(mode(dir), 0o755, "fixture must start open, else the test proves nothing");
+
+ const r = hardenStateDir(dir);
+
+ assert.equal(mode(dir), 0o700);
+ assert.equal(mode(join(dir, "rca-state.b1.csv")), 0o600);
+ assert.equal(mode(cache), 0o700, "nested cache dir too");
+ assert.equal(mode(join(cache, "entry.json")), 0o600, "files inside nested dirs too");
+ assert.equal(r.files, 2);
+ assert.equal(r.dirs, 2);
+
+ rmSync(root, { recursive: true, force: true });
+});
+
+test("hardenStateDir is idempotent and safe on a missing dir", () => {
+ const { root, dir } = fixture();
+ hardenStateDir(dir);
+ const second = hardenStateDir(dir);
+ assert.equal(mode(dir), 0o700);
+ assert.equal(second.files, 2, "still walks, just has nothing to change");
+
+ assert.deepEqual(hardenStateDir(join(root, "nope")), { dirs: 0, files: 0, skipped: [] });
+ rmSync(root, { recursive: true, force: true });
+});
+
+// These files ARE the resume state, so the default must not be able to eat a
+// build someone is about to resume.
+test("pruneStateDir keeps recent artifacts and removes only old ones", () => {
+ const { root, dir } = fixture();
+ // Pin every fixture file's mtime relative to the test's own clock — the
+ // files are created at real "now", so a hardcoded future nowMs would make
+ // the whole fixture look ancient and the test would pass for the wrong
+ // reason (it did, first time round).
+ const now = 1_800_000_000_000;
+ // utimesSync takes SECONDS as a plain number; `new Date(seconds)` would be
+ // read as milliseconds and land every stamp in 1970.
+ const stamp = (p, ageSec) => { const s = now / 1000 - ageSec; utimesSync(p, s, s); };
+ stamp(join(dir, "rca-state.b1.csv"), 60);
+ stamp(join(dir, "rca-toolcache.b1"), 60);
+ const old = join(dir, "rca-state.ancient.csv");
+ writeFileSync(old, "x\n");
+ stamp(old, 8 * 24 * 60 * 60);
+
+ const r = pruneStateDir(dir, now);
+
+ assert.deepEqual(r.removed, ["rca-state.ancient.csv"]);
+ assert.equal(existsSync(old), false);
+ assert.ok(existsSync(join(dir, "rca-state.b1.csv")), "a fresh build must survive");
+ assert.ok(r.kept >= 1);
+
+ rmSync(root, { recursive: true, force: true });
+});
+
+test("pruneStateDir dryRun reports without deleting", () => {
+ const { root, dir } = fixture();
+ // Pin every fixture file's mtime relative to the test's own clock — the
+ // files are created at real "now", so a hardcoded future nowMs would make
+ // the whole fixture look ancient and the test would pass for the wrong
+ // reason (it did, first time round).
+ const now = 1_800_000_000_000;
+ // utimesSync takes SECONDS as a plain number; `new Date(seconds)` would be
+ // read as milliseconds and land every stamp in 1970.
+ const stamp = (p, ageSec) => { const s = now / 1000 - ageSec; utimesSync(p, s, s); };
+ stamp(join(dir, "rca-state.b1.csv"), 60);
+ stamp(join(dir, "rca-toolcache.b1"), 60);
+ const old = join(dir, "rca-state.ancient.csv");
+ writeFileSync(old, "x\n");
+ stamp(old, 8 * 24 * 60 * 60);
+
+ const r = pruneStateDir(dir, now, { dryRun: true });
+ assert.deepEqual(r.removed, ["rca-state.ancient.csv"]);
+ assert.equal(existsSync(old), true, "dryRun must not delete");
+
+ rmSync(root, { recursive: true, force: true });
+});
diff --git a/tests/theme-clustering.test.mjs b/tests/theme-clustering.test.mjs
new file mode 100644
index 0000000..eb1004b
--- /dev/null
+++ b/tests/theme-clustering.test.mjs
@@ -0,0 +1,168 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { clustersFromThemes } from "../lib/theme-clustering.mjs";
+
+function row(id, extra = {}) {
+ return {
+ testRunId: String(id),
+ failure_category: "Assertion",
+ error_summary: "expected 200 but got 500",
+ file_path: "spec/login.rb",
+ is_flaky: "false",
+ ...extra,
+ };
+}
+
+function theme(buildFailureThemeId, name = "Some Theme") {
+ return {
+ themeId: `uuid-${buildFailureThemeId}`,
+ buildFailureThemeId,
+ themeData: { name, description: "..." },
+ affectedWorkflows: [],
+ };
+}
+
+test("one theme with two members → one cluster, representative + one sibling", () => {
+ const rows = [row(1), row(2)];
+ const themesResult = { buildThemes: [theme(10, "Data Assertion Mismatch")] };
+ const testsByThemeId = { 10: [{ testRunId: "1" }, { testRunId: "2" }] };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 1);
+ assert.equal(clusters[0].cluster_id, "theme-10");
+ assert.equal(clusters[0].signature, "Data Assertion Mismatch");
+ assert.equal(clusters[0].members.length, 2);
+ assert.equal(clusters[0].siblings.length, 1);
+ assert.ok(clusters[0].representative);
+});
+
+test("multiple themes → one cluster each", () => {
+ const rows = [row(1), row(2), row(3)];
+ const themesResult = {
+ buildThemes: [theme(10, "Theme A"), theme(20, "Theme B")],
+ };
+ const testsByThemeId = {
+ 10: [{ testRunId: "1" }],
+ 20: [{ testRunId: "2" }, { testRunId: "3" }],
+ };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 2);
+ const a = clusters.find((c) => c.cluster_id === "theme-10");
+ const b = clusters.find((c) => c.cluster_id === "theme-20");
+ assert.equal(a.members.length, 1);
+ assert.equal(a.siblings.length, 0);
+ assert.equal(b.members.length, 2);
+ assert.equal(b.siblings.length, 1);
+});
+
+test("a failed test not assigned to any theme becomes its own singleton (never dropped)", () => {
+ const rows = [row(1), row(2)];
+ const themesResult = { buildThemes: [theme(10, "Theme A")] };
+ const testsByThemeId = { 10: [{ testRunId: "1" }] };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 2);
+ const solo = clusters.find((c) => c.cluster_id === "solo-2");
+ assert.ok(solo, "uncovered test must still get a cluster");
+ assert.equal(solo.members.length, 1);
+ assert.equal(solo.siblings.length, 0);
+ assert.equal(solo.representative.testRunId, "2");
+});
+
+test("a theme with no matched member rows is skipped, not an empty cluster", () => {
+ const rows = [row(1)];
+ const themesResult = { buildThemes: [theme(10, "Theme A"), theme(20, "Empty theme")] };
+ const testsByThemeId = { 10: [{ testRunId: "1" }], 20: [] };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 1);
+ assert.equal(clusters[0].cluster_id, "theme-10");
+});
+
+test("member rows not present in listTestIds rows are dropped, not fabricated", () => {
+ const rows = [row(1)];
+ const themesResult = { buildThemes: [theme(10, "Theme A")] };
+ // testsByThemeId names a testRunId ("999") that never appeared in listTestIds.
+ const testsByThemeId = { 10: [{ testRunId: "1" }, { testRunId: "999" }] };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 1);
+ assert.equal(clusters[0].members.length, 1);
+});
+
+test("representative selection matches lib/signature.mjs's rule (non-flaky, then smallest testRunId)", () => {
+ const rows = [
+ row(5, { is_flaky: "true" }),
+ row(9, { is_flaky: "false" }),
+ row(7, { is_flaky: "false" }),
+ ];
+ const themesResult = { buildThemes: [theme(10)] };
+ const testsByThemeId = {
+ 10: [{ testRunId: "5" }, { testRunId: "9" }, { testRunId: "7" }],
+ };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters[0].representative.testRunId, "7");
+});
+
+test("clustersFromThemes stamps cluster_id onto every row, theme and singleton alike", () => {
+ const rows = [row(1), row(2)];
+ const themesResult = { buildThemes: [theme(10)] };
+ const testsByThemeId = { 10: [{ testRunId: "1" }] };
+
+ clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(rows[0].cluster_id, "theme-10");
+ assert.equal(rows[1].cluster_id, "solo-2");
+});
+
+test("numeric testRunId in theme membership (as the MCP tool's JSON would send it) still matches string testRunId rows", () => {
+ const rows = [row(1), row(2)];
+ const themesResult = { buildThemes: [theme(10)] };
+ // Membership entries carry testRunId as a NUMBER, unlike listTestIds rows (strings).
+ const testsByThemeId = { 10: [{ testRunId: 1 }, { testRunId: 2 }] };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 1);
+ assert.equal(clusters[0].members.length, 2);
+});
+
+test("a testRunId claimed by an earlier theme is skipped by a later theme (first-theme-wins, no duplicate membership)", () => {
+ const rows = [row(1), row(2)];
+ const themesResult = { buildThemes: [theme(10, "Theme A"), theme(20, "Theme B")] };
+ // Both themes claim testRunId "1" — the server is expected never to do this,
+ // but the function must not let the row land in two clusters.
+ const testsByThemeId = {
+ 10: [{ testRunId: "1" }],
+ 20: [{ testRunId: "1" }, { testRunId: "2" }],
+ };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, testsByThemeId);
+
+ assert.equal(clusters.length, 2);
+ const a = clusters.find((c) => c.cluster_id === "theme-10");
+ const b = clusters.find((c) => c.cluster_id === "theme-20");
+ assert.equal(a.members.length, 1);
+ assert.equal(a.members[0].testRunId, "1");
+ assert.equal(b.members.length, 1, "testRunId 1 must not also land in theme 20");
+ assert.equal(b.members[0].testRunId, "2");
+ assert.equal(rows[0].cluster_id, "theme-10");
+});
+
+test("no themes at all → every row is its own singleton", () => {
+ const rows = [row(1), row(2)];
+ const themesResult = { buildThemes: [] };
+
+ const { clusters } = clustersFromThemes(rows, themesResult, {});
+
+ assert.equal(clusters.length, 2);
+ assert.ok(clusters.every((c) => c.cluster_id.startsWith("solo-")));
+});
diff --git a/tests/tool-cache.test.mjs b/tests/tool-cache.test.mjs
new file mode 100644
index 0000000..56e22da
--- /dev/null
+++ b/tests/tool-cache.test.mjs
@@ -0,0 +1,272 @@
+import { test, beforeEach, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, rmSync, writeFileSync, statSync, readdirSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import {
+ toolCacheDirFor, cacheKey, mcpCacheKey, cacheGet, cachePut, cacheStats,
+ isCacheable, isCacheableMcp, isRunnable, redact, tokenize, splitPipeline,
+} from "../lib/tool-cache.mjs";
+
+let dir;
+beforeEach(() => { dir = mkdtempSync(join(tmpdir(), "rca-toolcache-")); });
+afterEach(() => rmSync(dir, { recursive: true, force: true }));
+
+test("toolCacheDirFor: build id in the path, OS temp default, stateDir override", () => {
+ assert.ok(toolCacheDirFor("b1").startsWith(join(tmpdir(), "bstack-rca")));
+ assert.ok(toolCacheDirFor("b1").endsWith("rca-toolcache.b1"));
+ assert.equal(toolCacheDirFor("b1", "/ci/art"), join("/ci/art", "rca-toolcache.b1"));
+ assert.ok(toolCacheDirFor("../../etc").endsWith("rca-toolcache..._.._etc"));
+});
+
+test("cacheKey: whitespace-insensitive, but content-sensitive", () => {
+ assert.equal(cacheKey("gh api repos/a"), cacheKey("gh api repos/a"));
+ assert.notEqual(cacheKey("gh api repos/a | head -20"), cacheKey("gh api repos/a | head -200"));
+});
+
+test("put then get round-trips", () => {
+ const k = cacheKey("gh api repos/a");
+ cachePut(dir, k, { command: "gh api repos/a", writerId: "w1", stdout: "hello" }, 1000);
+ const hit = cacheGet(dir, k);
+ assert.equal(hit.stdout, "hello");
+ assert.equal(hit.writerId, "w1");
+ assert.equal(hit.capturedAtMs, 1000);
+});
+
+test("get on a miss returns null, never throws", () => {
+ assert.equal(cacheGet(dir, cacheKey("never run")), null);
+});
+
+test("a corrupt entry reads as a miss rather than throwing", () => {
+ const k = cacheKey("gh api repos/a");
+ writeFileSync(join(dir, `${k}.json`), "{not json", "utf8");
+ assert.equal(cacheGet(dir, k), null);
+});
+
+test("secrets are redacted before anything is written to disk", () => {
+ const k = cacheKey("gh api repos/a");
+ cachePut(dir, k, {
+ command: "gh api repos/a",
+ stdout: 'ok\nAuthorization: Bearer abc123SECRET\napi_key=zzz999\ndone',
+ }, 1000);
+ const raw = cacheGet(dir, k).stdout;
+ assert.ok(!raw.includes("abc123SECRET"), "bearer token must not persist");
+ assert.ok(!raw.includes("zzz999"), "api_key must not persist");
+ assert.ok(raw.includes(""));
+});
+
+test("redact leaves ordinary output untouched", () => {
+ assert.equal(redact("just some log output"), "just some log output");
+});
+
+// Regression, found by a live coordinator. Redaction used to consume the REST
+// OF THE LINE after a secret-ish key. GitHub's file API returns SINGLE-LINE
+// JSON whose download_url always carries `?token=…`, so a 214KB response was
+// silently cached as 816 bytes with the content field gone — every
+// private-repo file fetch was corrupted, with no warning.
+test("redact bounds the value and does NOT eat the rest of a single-line JSON", () => {
+ const json = '{"name":"F.java","download_url":"https://raw.example/F.java?token=BRFIJBHPIG5IILHZ",'
+ + '"type":"file","content":"' + "A".repeat(5000) + '"}';
+ const out = redact(json);
+ assert.ok(!out.includes("BRFIJBHPIG5IILHZ"), "the token itself must be redacted");
+ assert.ok(out.includes('"type":"file"'), "structure after the token must survive");
+ assert.ok(out.includes("A".repeat(5000)), "the content payload must survive");
+ assert.ok(out.length > 5000, `expected full payload, got ${out.length} bytes`);
+});
+
+test("redact still catches a bare Bearer token and a key=value secret", () => {
+ assert.equal(redact("Authorization: Bearer abc123SECRET"), "Authorization: ");
+ assert.equal(redact("api_key=zzz999"), "api_key=");
+ assert.ok(!redact("Bearer eyJhbGciOiJIUzI1NiJ9").includes("eyJhbGciOiJIUzI1NiJ9"));
+});
+
+// Regression, found by two live coordinators. Neither tokenize nor
+// splitPipeline handled backslash escapes, so `\"` read as a closing quote.
+// That mangled jq's two most common idioms: string equality and, because the
+// parser then believed it was outside quotes, regex alternation got split as
+// a shell pipe.
+test("escaped double quotes survive tokenization for jq", () => {
+ const argv = tokenize(String.raw`gh api x --jq .[]|select(.filename==\"a/b.json\")`);
+ assert.equal(argv[argv.length - 1], '.[]|select(.filename=="a/b.json")');
+});
+
+test("a pipe inside an escaped-quote jq regex is NOT a shell pipe", () => {
+ const cmd = String.raw`gh pr view 51044 --json files | jq -c "select(test(\"vite|env|s3\";\"i\"))"`;
+ assert.deepEqual(splitPipeline(cmd).length, 2, "must split into fetch + one filter only");
+ const g = isRunnable(cmd);
+ assert.equal(g.ok, true, g.reason);
+ assert.deepEqual(g.fetch, ["gh", "pr", "view", "51044", "--json", "files"]);
+ assert.equal(g.filters[0][2], 'select(test("vite|env|s3";"i"))');
+});
+
+test("single quotes suppress escape processing, POSIX-style", () => {
+ assert.deepEqual(tokenize(String.raw`gh api 'a\nb'`), ["gh", "api", String.raw`a\nb`]);
+});
+
+test("oversized payloads are truncated and flagged", () => {
+ const k = cacheKey("gh api big");
+ const rec = cachePut(dir, k, { command: "gh api big", stdout: "x".repeat(400 * 1024) }, 1000);
+ assert.equal(rec.truncated, true);
+ assert.ok(rec.stdout.includes("[truncated by tool-cache]"));
+});
+
+test("cacheStats counts entries", () => {
+ cachePut(dir, "k1", { command: "a", stdout: "12345" }, 1);
+ cachePut(dir, "k2", { command: "b", stdout: "123" }, 1);
+ const s = cacheStats(dir);
+ assert.equal(s.entries, 2);
+ assert.equal(s.bytes, 8);
+});
+
+test("isCacheable rejects mutating shell commands", () => {
+ assert.equal(isCacheable("gh api repos/a"), true);
+ assert.equal(isCacheable("kubectl get pods"), true);
+ assert.equal(isCacheable("kubectl delete pod x"), false);
+ assert.equal(isCacheable("kubectl exec pod -- sh"), false);
+ assert.equal(isCacheable("gh pr create --title x"), false);
+ assert.equal(isCacheable("gh api -X POST repos/a"), false);
+ assert.equal(isCacheable("git push origin main"), false);
+ assert.equal(isCacheable("rm -rf /tmp/x"), false);
+});
+
+test("isRunnable enforces an allowlisted read-only leader", () => {
+ assert.equal(isRunnable("gh api repos/a").ok, true);
+ assert.equal(isRunnable("kubectl get pods -n regression").ok, true);
+ assert.equal(isRunnable("python3 -c 'print(1)'").ok, false);
+ assert.equal(isRunnable("sh -c 'echo hi'").ok, false);
+});
+
+test("isRunnable rejects chaining and redirects, but ACCEPTS pipelines", () => {
+ assert.equal(isRunnable("gh api a ; rm -rf /").ok, false);
+ assert.equal(isRunnable("gh api a && kubectl delete pod x").ok, false);
+ assert.equal(isRunnable("gh api a > /etc/passwd").ok, false);
+ // `2>&1` is stderr plumbing the wrapper already owns — stripped, not refused.
+ // Refusing it rejected 134 of 223 real recorded calls and zeroed the hit rate.
+ assert.equal(isRunnable("gh api a 2>&1").ok, true, "stderr plumbing is normalized away");
+ assert.equal(isRunnable("gh api a 2>/dev/null | jq .x").ok, true);
+ // Pipelines are supported now: refusing them meant the cache applied to
+ // almost no real traffic, since most fetches are written inline with a filter.
+ assert.equal(isRunnable("gh api a | jq .x").ok, true);
+});
+
+test("a FILE redirect is reported as a redirect, not as a mutation", () => {
+ const r = isRunnable("gh api repos/x > out.json");
+ assert.equal(r.ok, false);
+ assert.match(r.reason, /redirect/i);
+ assert.doesNotMatch(r.reason, /mutating/i);
+});
+
+test("stderr plumbing does not change the cache key", () => {
+ const a = isRunnable("gh api repos/x | jq .a");
+ const b = isRunnable("gh api repos/x 2>&1 | jq .b");
+ assert.equal(cacheKey(a.fetchText), cacheKey(b.fetchText));
+});
+
+test("pipeline plan: only the FETCH is keyed, filters are separate", () => {
+ const a = isRunnable("gh api repos/x | jq -r .name");
+ const b = isRunnable("gh api repos/x | jq -r .branch | tr a-z A-Z");
+ assert.equal(a.ok && b.ok, true);
+ // Same underlying fetch -> same cache key -> one network call serves both.
+ assert.equal(cacheKey(a.fetchText), cacheKey(b.fetchText));
+ assert.deepEqual(a.fetch, ["gh", "api", "repos/x"]);
+ assert.equal(a.filters.length, 1);
+ assert.equal(b.filters.length, 2);
+});
+
+test("only pure text filters may follow the fetch", () => {
+ assert.equal(isRunnable("gh api repos/x | jq .a").ok, true);
+ assert.equal(isRunnable("gh api repos/x | grep foo").ok, true);
+ assert.equal(isRunnable("gh api repos/x | sh").ok, false);
+ assert.equal(isRunnable("gh api repos/x | bash -c 'x'").ok, false);
+ assert.equal(isRunnable("gh api repos/x | kubectl delete pod y").ok, false);
+});
+
+test("splitPipeline ignores a pipe inside quotes", () => {
+ assert.deepEqual(splitPipeline(`gh pr list --jq '.[] | .number' | head -5`),
+ ["gh pr list --jq '.[] | .number'", "head -5"]);
+});
+
+// Regression: the old raw-string guard refused these legitimate read-only
+// calls, which is what pushed a coordinator into slower workarounds.
+test("isRunnable ALLOWS metacharacters inside quoted arguments", () => {
+ const jqSemicolon = `gh api repos/o/r/git/trees/main --jq '[.tree[].path|select(test("rcaThree";"i"))]'`;
+ assert.equal(isRunnable(jqSemicolon).ok, true, "; inside a jq expression is not a shell operator");
+
+ const urlAmp = "gh api 'search/code?q=foo&per_page=20'";
+ assert.equal(isRunnable(urlAmp).ok, true, "& inside a quoted URL is not a shell operator");
+
+ const jqPipe = `gh pr list -R o/r --json number --jq '.[] | .number'`;
+ assert.equal(isRunnable(jqPipe).ok, true, "| inside a quoted jq expression is not a shell pipe");
+});
+
+test("a quoted metacharacter survives tokenization as ONE literal argument", () => {
+ const argv = tokenize(`gh api repos/o/r --jq '[.tree[]|select(test("x";"i"))]'`);
+ assert.equal(argv.length, 5);
+ assert.equal(argv[4], '[.tree[]|select(test("x";"i"))]');
+});
+
+test("tokenize splits like a shell for quoted args, without a shell", () => {
+ assert.deepEqual(tokenize("gh api repos/a --jq '.items[].path'"),
+ ["gh", "api", "repos/a", "--jq", ".items[].path"]);
+ assert.deepEqual(tokenize('kubectl get pods -o "custom:.metadata.name"'),
+ ["kubectl", "get", "pods", "-o", "custom:.metadata.name"]);
+ assert.throws(() => tokenize("gh api 'unterminated"), /unterminated quote/);
+});
+
+test("tokenize keeps injection payloads as ONE literal argument", () => {
+ // With execFile + these argv, no shell ever sees the metacharacters.
+ const argv = tokenize(`gh api "repos/a;rm -rf /"`);
+ assert.deepEqual(argv, ["gh", "api", "repos/a;rm -rf /"]);
+});
+
+test("MCP: stateful tools are never cacheable", () => {
+ assert.equal(isCacheableMcp("mcp__grafana__query_loki_logs"), true);
+ assert.equal(isCacheableMcp("mcp__browserstack__listTestIds"), true);
+ assert.equal(isCacheableMcp("mcp__browserstack__tfaRcaTurn"), false);
+ assert.equal(isCacheableMcp("mcp__browserstack__getTfaTurnResult"), false);
+ assert.equal(isCacheableMcp("mcp__browserstack__triggerRcaReport"), false);
+});
+
+test("mcpCacheKey is argument-order independent but value sensitive", () => {
+ const a = mcpCacheKey("t", { b: 2, a: 1 });
+ const b = mcpCacheKey("t", { a: 1, b: 2 });
+ assert.equal(a, b);
+ assert.notEqual(a, mcpCacheKey("t", { a: 1, b: 3 }));
+ assert.notEqual(a, mcpCacheKey("other", { a: 1, b: 2 }));
+});
+
+test("mcpCacheKey canonicalizes nested objects and arrays", () => {
+ assert.equal(
+ mcpCacheKey("t", { q: { z: 1, y: [{ n: 1, m: 2 }] } }),
+ mcpCacheKey("t", { q: { y: [{ m: 2, n: 1 }], z: 1 } }),
+ );
+});
+
+test("an MCP result round-trips through the shared store", () => {
+ const k = mcpCacheKey("mcp__grafana__query_loki_logs", { ns: "regression", limit: 50 });
+ cachePut(dir, k, { command: "grafana query", writerId: "3889074893", stdout: "0 rows, clean" }, 1000);
+ assert.equal(cacheGet(dir, k).stdout, "0 rows, clean");
+});
+
+test("cache files are owner-only (0600) and the dir owner-only (0700)", () => {
+ const sub = join(dir, "nested-cache");
+ const k = cacheKey("gh api repos/a");
+ cachePut(sub, k, { command: "gh api repos/a", stdout: "private repo source" }, 1000);
+ // The cache sits in a world-readable OS temp dir and holds raw gh/kubectl
+ // output; redaction is best-effort, so the mode is the real control.
+ assert.equal(statSync(join(sub, `${k}.json`)).mode & 0o777, 0o600);
+ assert.equal(statSync(sub).mode & 0o777, 0o700);
+});
+
+test("no temp file is left behind after an atomic put", () => {
+ const k = cacheKey("gh api repos/a");
+ cachePut(dir, k, { command: "gh api repos/a", stdout: "x" }, 1000);
+ assert.deepEqual(readdirSync(dir).filter((f) => f.endsWith(".tmp")), []);
+});
+
+test("CONCURRENCY: same key written twice stays readable and consistent", () => {
+ const k = cacheKey("gh api repos/a");
+ cachePut(dir, k, { command: "gh api repos/a", writerId: "w1", stdout: "same-bytes" }, 1000);
+ cachePut(dir, k, { command: "gh api repos/a", writerId: "w2", stdout: "same-bytes" }, 2000);
+ assert.equal(cacheGet(dir, k).stdout, "same-bytes");
+});
diff --git a/tests/turn1-registry.test.mjs b/tests/turn1-registry.test.mjs
new file mode 100644
index 0000000..ab053f9
--- /dev/null
+++ b/tests/turn1-registry.test.mjs
@@ -0,0 +1,128 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, rmSync, statSync, existsSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import {
+ turn1PathFor,
+ initTurn1Registry,
+ recordTurn1,
+ readTurn1,
+ readAllTurn1,
+ deleteTurn1Registry,
+} from "../lib/turn1-registry.mjs";
+
+const mode = (p) => statSync(p).mode & 0o777;
+
+function fixture() {
+ return mkdtempSync(join(tmpdir(), "rca-t1-"));
+}
+
+test("turn1PathFor keys the file by buildId under the given stateDir", () => {
+ const dir = fixture();
+ const p = turn1PathFor("build-123", dir);
+ assert.equal(p, join(dir, "rca-turn1.build-123.json"));
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("readTurn1 on a non-existent registry returns null, not a throw", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ assert.equal(readTurn1(p, "3900000001"), null);
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("initTurn1Registry creates the file and is idempotent (never clobbers prior entries)", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ initTurn1Registry(p, "b1", 1000);
+ assert.ok(existsSync(p));
+
+ recordTurn1(p, "3900000001", { status: "NEEDS_INFO", threadId: "chat:1", asks: ["a"] }, 2000);
+ // Re-init after entries exist must leave them untouched.
+ initTurn1Registry(p, "b1", 3000);
+ assert.deepEqual(readTurn1(p, "3900000001").asks, ["a"]);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("recordTurn1 + readTurn1 round-trip a PENDING entry", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ recordTurn1(p, "3900000002", { status: "PENDING", threadId: "chat:2", turnId: "t-2" }, 5000);
+
+ const entry = readTurn1(p, "3900000002");
+ assert.equal(entry.status, "PENDING");
+ assert.equal(entry.threadId, "chat:2");
+ assert.equal(entry.turnId, "t-2");
+ assert.equal(entry.submittedAtMs, 5000);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("recordTurn1 + readTurn1 round-trip a NEEDS_INFO entry", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ recordTurn1(
+ p,
+ "3900000003",
+ { status: "NEEDS_INFO", threadId: "chat:3", asks: [{ evidenceType: "product_code" }] },
+ 6000,
+ );
+
+ const entry = readTurn1(p, "3900000003");
+ assert.equal(entry.status, "NEEDS_INFO");
+ assert.equal(entry.threadId, "chat:3");
+ assert.deepEqual(entry.asks, [{ evidenceType: "product_code" }]);
+ assert.equal(entry.turnId, undefined, "NEEDS_INFO never carries a turnId");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("readAllTurn1 returns every recorded entry keyed by testRunId", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ recordTurn1(p, "1", { status: "PENDING", threadId: "chat:1", turnId: "t-1" }, 1000);
+ recordTurn1(p, "2", { status: "NEEDS_INFO", threadId: "chat:2", asks: [] }, 2000);
+
+ const all = readAllTurn1(p);
+ assert.deepEqual(Object.keys(all).sort(), ["1", "2"]);
+ assert.equal(all["1"].status, "PENDING");
+ assert.equal(all["2"].status, "NEEDS_INFO");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("recordTurn1 for a second testRunId does not clobber the first", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ recordTurn1(p, "1", { status: "PENDING", threadId: "chat:1", turnId: "t-1" }, 1000);
+ recordTurn1(p, "2", { status: "PENDING", threadId: "chat:2", turnId: "t-2" }, 2000);
+
+ assert.equal(readTurn1(p, "1").threadId, "chat:1");
+ assert.equal(readTurn1(p, "2").threadId, "chat:2");
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("the registry file and its directory are owner-only (0600 / 0700)", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ recordTurn1(p, "1", { status: "PENDING", threadId: "chat:1", turnId: "t-1" }, 1000);
+
+ assert.equal(mode(p), 0o600);
+
+ rmSync(dir, { recursive: true, force: true });
+});
+
+test("deleteTurn1Registry removes the file and reports whether it existed", () => {
+ const dir = fixture();
+ const p = turn1PathFor("b1", dir);
+ assert.equal(deleteTurn1Registry(p), false, "nothing to delete yet");
+
+ recordTurn1(p, "1", { status: "PENDING", threadId: "chat:1", turnId: "t-1" }, 1000);
+ assert.equal(deleteTurn1Registry(p), true);
+ assert.equal(existsSync(p), false);
+
+ rmSync(dir, { recursive: true, force: true });
+});
diff --git a/tests/wiring.test.mjs b/tests/wiring.test.mjs
new file mode 100644
index 0000000..4138bf6
--- /dev/null
+++ b/tests/wiring.test.mjs
@@ -0,0 +1,117 @@
+// Guard against SHIPPED-BUT-UNREACHABLE code.
+//
+// Twice now a helper was built, unit-tested, and manually verified — and then
+// invoked by nothing. `bin/repo-read.mjs` and `bin/evidence-show.mjs` were both
+// referenced in zero skill/agent files, so at runtime every coordinator kept
+// doing the expensive thing the helper existed to avoid. Unit tests can't catch
+// this: the module works perfectly in isolation, which is exactly why the gap
+// survives review.
+//
+// The prompt layer IS the call graph here. An agent only runs what its skill or
+// agent markdown names, so "is this string mentioned in a prompt file" is the
+// real reachability test, crude as it looks.
+
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { readdirSync, readFileSync, statSync } from "node:fs";
+import { join } from "node:path";
+
+const ROOT = new URL("..", import.meta.url).pathname;
+
+/** Every .md under the dirs an agent actually reads. */
+function promptText() {
+ const out = [];
+ const walk = (dir) => {
+ let entries;
+ try { entries = readdirSync(dir); } catch { return; }
+ for (const e of entries) {
+ const p = join(dir, e);
+ if (statSync(p).isDirectory()) walk(p);
+ else if (e.endsWith(".md")) out.push(readFileSync(p, "utf8"));
+ }
+ };
+ for (const d of ["skills", "agents", "workflows", ".claude"]) walk(join(ROOT, d));
+ return out.join("\n");
+}
+
+test("every bin/ helper is named by at least one prompt file", () => {
+ const prompts = promptText();
+ const helpers = readdirSync(join(ROOT, "bin")).filter((f) => f.endsWith(".mjs"));
+ assert.ok(helpers.length > 0, "expected some helpers to check");
+
+ const orphans = helpers.filter((h) => !prompts.includes(h));
+ assert.deepEqual(
+ orphans,
+ [],
+ `unreachable helper(s): ${orphans.join(", ")}. A helper no skill or agent ` +
+ `names will never run — either reference it from the prompt layer or delete it.`,
+ );
+});
+
+// The exported-but-uncalled variant of the same bug: lib functions that exist
+// only because a test calls them. Checked for the few whose whole purpose is to
+// be driven by the gate, where being uncalled means the feature is off.
+test("gate-critical lib exports are actually invoked outside tests", () => {
+ const prompts = promptText();
+ const src = [];
+ const walk = (dir) => {
+ for (const e of readdirSync(dir)) {
+ const p = join(dir, e);
+ if (statSync(p).isDirectory()) walk(p);
+ else if (e.endsWith(".mjs")) src.push(readFileSync(p, "utf8"));
+ }
+ };
+ walk(join(ROOT, "lib"));
+ walk(join(ROOT, "bin"));
+ const haystack = src.join("\n") + "\n" + prompts;
+
+ // Each of these is a no-op unless something drives it: discovery that is
+ // never run means every read falls back to the network, and a map that is
+ // never written means every coordinator re-probes the filesystem.
+ for (const fn of ["discoverWorkspaceRoot", "resolveLocalRepos", "setLocalRepos", "recomputeCoverage"]) {
+ // Definition line doesn't count as a call site.
+ const uses = haystack.split(fn).length - 1;
+ assert.ok(uses >= 2, `${fn} appears ${uses}x outside tests — defined but never driven`);
+ }
+});
+
+// The root cause of the 23% discovery tax was DRIFT: helpers were added faster
+// than the docs described them, so agents grepped lib/ at runtime to learn the
+// API. Documenting it once fixes today; this test keeps it fixed.
+test("every exported lib helper appears in the SKILL's API reference", () => {
+ const skill = readFileSync(join(ROOT, "skills/rca-build/SKILL.md"), "utf8");
+
+ // Internal-by-convention: replay/test seams and trivial helpers a coordinator
+ // never calls. Anything NOT listed here must be documented.
+ const INTERNAL = new Set([
+ "emptyEvidenceFile", "writeEvidenceFile", "contribDirFor", "contribPathFor",
+ "hasTrustworthyPrList", "stalenessOf", "makeEvidenceCache",
+ "replaySubmit", "replayRead", "normalize", "computeSignature",
+ "selectRepresentative", "localCloneFor", "hasCommit", "ensureCommit",
+ "classifyCoverage", "coverageStamp", "orderAsks", "routeAsk",
+ "unavailableCapabilities", "renderGlimpse", "toolCacheDirFor", "cacheKey",
+ "isCacheable", "splitPipeline",
+ // tool-cache module internals — agents drive the cache through
+ // bin/cached-exec.mjs / bin/cached-mcp.mjs, never by importing it.
+ "isRunnable", "tokenize", "isCacheableMcp", "redact", "cacheGet",
+ "cachePut", "cacheStats", "mcpCacheKey",
+ ]);
+
+ const undocumented = [];
+ for (const f of readdirSync(join(ROOT, "lib")).filter((f) => f.endsWith(".mjs"))) {
+ const src = readFileSync(join(ROOT, "lib", f), "utf8");
+ for (const m of src.matchAll(/^export (?:function|const) ([A-Za-z0-9_]+)/gm)) {
+ const name = m[1];
+ if (INTERNAL.has(name)) continue;
+ if (!skill.includes(name)) undocumented.push(`${f}:${name}`);
+ }
+ }
+
+ assert.deepEqual(
+ undocumented,
+ [],
+ `undocumented helper(s): ${undocumented.join(", ")}. Add them to the SKILL's ` +
+ `"API reference" section — an agent that can't find a signature there greps ` +
+ `lib/ at runtime, which cost 92 of 407 tool calls on one measured run.`,
+ );
+});
diff --git a/workflows/rca-batch.mjs b/workflows/rca-batch.mjs
index f79e7a2..9d04a92 100644
--- a/workflows/rca-batch.mjs
+++ b/workflows/rca-batch.mjs
@@ -23,9 +23,25 @@ export const meta = {
// {
// csvPath, buildId,
// manifest: { capability: { available, via } },
-// buildEvidence: { baselineRef, suspectWindow, ... }, // pre-computed once
+// evidenceFilePath, // lib/evidence-file.mjs artifact for this build
+// pluginRoot, // absolute path to this plugin, so coordinators can call bin/cached-exec.mjs
+// buildEvidence: { baselineRef, isFallback, suspectWindow, reposCovered, workloadsCovered, gaps },
+// // ^ SHRUNK to a cheap summary/pointer only — the full PR list / log
+// // sweeps live in the file at evidenceFilePath, read via each
+// // coordinator's own Read tool. Repeating the full detail in every
+// // dispatch prompt (as before) is exactly the duplication this file
+// // removes.
// clusters: [
-// { cluster_id, representative: { testRunId, testName, error_summary },
+// { cluster_id,
+// representative: { testRunId, testName, error_summary,
+// // Step 4b pre-dispatch outcome (SKILL.md Step 4b, lib/turn1-registry.mjs)
+// // — at most one of these two is set, never both:
+// turn1: { status: "PENDING", threadId, turnId } |
+// { status: "NEEDS_INFO", threadId, asks },
+// // Step 4b's turn 1 already RESOLVED — no dispatch at all for this
+// // representative; `resolved` is the RCA_SCHEMA-shaped result to use
+// // directly (also already flipped into the CSV by the orchestrator).
+// resolved: | undefined },
// siblings: [ { testRunId, testName, error_summary } ] }
// ]
// }
@@ -58,23 +74,83 @@ const clusters = ctx.clusters ?? [];
const shared = [
`CSV state file: ${ctx.csvPath}`,
`Capability manifest: ${JSON.stringify(ctx.manifest ?? {})}`,
- `Build-level evidence (pre-computed once, reuse — do not re-fetch): ${JSON.stringify(ctx.buildEvidence ?? {})}`,
+ `Pre-fetched build-evidence file — READ THIS FIRST (via the Read tool) before making ANY live github/infra/logs gather call: ${ctx.evidenceFilePath}`,
+ `Build-evidence summary (full detail is in the file above; this is only a pointer — do not re-fetch what the file already covers): ${JSON.stringify(ctx.buildEvidence ?? {})}`,
+ `If the file's github/logs sections do not name a repo/workload/ask you need, or record a "gap" for it, that is a genuine gap — fall back to a live gather via the capability manifest above exactly as if no file existed. The file is an optimization, never a hard dependency.`,
+ `The file is read-write: after any live gather that fills a gap or goes deeper than what was there, write it back via contributeGithubEvidence/contributeLogsEvidence (lib/evidence-file.mjs) passing your own testRunId as writerId, before finishing this test — so a sibling dispatched after you, or another cluster sharing the same repo/workload, reads the enriched entry instead of re-fetching it. Each writer owns its own shard file, so concurrent coordinators cannot clobber each other; readers fold base + shards automatically.`,
+ `Tool cache — route read-only lookups through it so duplicate calls across coordinators become hits. Shell: node ${ctx.pluginRoot ?? ""}/bin/cached-exec.mjs '' (behaves like the raw command; pipe to jq/grep OUTSIDE the wrapper so different filters share one fetch). MCP data queries: cached-mcp.mjs get|put '' [writerId]. NEVER cache tfaRcaTurn/getTfaTurnResult/triggerRcaReport — they are stateful. Do not re-probe connectors the gate already validated.`,
`Autonomous run — on an evidence gap with no valid connector, report "unavailable" back to TFA (NEVER prompt a user). Best-effort finalize.`,
`PRODUCT_BUG / application-bug mandate: hunt the culprit PR via the github connector (deploy timeline vs last-pass window, changed paths vs failure signature) and feed the PR link(s) to TFA so related_prs populates. No PR after digging to the turn cap → state explicitly "no culprit PR identified after " so the CSV row records the gap.`,
`Soft-PENDING is NOT an answer: tfaRcaTurn abandons its in-call poll at 90s while TFA keeps working. On status PENDING, call getTfaTurnResult(testRunId, turnId) FIRST and keep reading on the softPendingDrain budget (every 5s, <=40 reads / <=10min) until the status is RESOLVED / NEEDS_INFO / BLOCKED, then continue the loop. Reads do NOT count against the turn cap. Never submit a new message onto a turn still in flight. Only a fully spent drain budget ends the test PENDING.`,
`Persist eagerly to the CSV: claim your row before turn 1, flip it on terminal (lib/csv-state.mjs).`,
].join("\n");
+function resumeLine(row) {
+ if (!row?.threadId || !row?.turnId) return null;
+ return [
+ `RESUME (do not start a new thread): this test already has an in-flight thread`,
+ `threadId=${row.threadId} turnId=${row.turnId}.`,
+ `Call getTfaTurnResult(testRunId, turnId) FIRST to read its current state`,
+ `(drain any soft-PENDING per the softPendingDrain budget) before submitting`,
+ `anything further — reuse this threadId for every follow-up on this test.`,
+ row.last_evidence_digest ? `Prior evidence already gathered (reuse, don't re-fetch): ${row.last_evidence_digest}` : null,
+ row.root_cause ? `Prior attempt note: ${row.root_cause}` : null,
+ ].filter(Boolean).join("\n");
+}
+
+// Step 4b (SKILL.md Step 4b) already submitted this representative's turn 1,
+// concurrently with Step 4's evidence pre-fetch. RESOLVED needs no coordinator
+// dispatch at all (short-circuited in the pipeline stage below); these two
+// non-terminal outcomes are handed to the coordinator instead of letting it
+// submit turn 1 again — mutually exclusive per agents/ai-tfa-coordinator.md.
+function turn1Line(r) {
+ const t = r?.turn1;
+ if (!t) return null;
+ if (t.status === "PENDING" && t.turnId) {
+ return [
+ `RESUME (turn 1 already submitted by Step 4b — do not start a new thread):`,
+ `threadId=${t.threadId} turnId=${t.turnId}.`,
+ `Call getTfaTurnResult(testRunId, turnId) FIRST to read its current state`,
+ `(drain any soft-PENDING per the softPendingDrain budget) before submitting`,
+ `anything further — reuse this threadId for every follow-up on this test.`,
+ ].join("\n");
+ }
+ if (t.status === "NEEDS_INFO") {
+ return [
+ `TURN 1 ALREADY SUBMITTED AND ANSWERED by Step 4b — do NOT submit turn 1 again.`,
+ `threadId=${t.threadId}. turns_used starts at 1.`,
+ `TFA's turn-1 response was NEEDS_INFO with these asks (verbatim): ${JSON.stringify(t.asks ?? [])}`,
+ `Start this run at the ROUTE-the-asks step using them, then submit your first`,
+ `follow-up message on this SAME thread.`,
+ ].join("\n");
+ }
+ return null;
+}
+
function repPrompt(cluster) {
const r = cluster.representative;
+ const resume = resumeLine(r);
+ // Mutual exclusivity, enforced in code, not just by convention: a
+ // representative gets AT MOST one resume-style instruction. A prior-run CSV
+ // pending-resume (`r.threadId`/`r.turnId`, an already in-flight thread from
+ // a run this build is resuming) takes precedence over a same-run Step 4b
+ // entry (`r.turn1`) — Step 4b's pre-dispatch is supposed to skip a
+ // representative already in pending-resume (SKILL.md Step 4b), but this is
+ // the backstop: presenting BOTH would hand the coordinator two different
+ // threadIds as "the" thread to resume, which is worse than picking one.
+ const t1 = resume ? null : turn1Line(r);
return [
`You are the ai-tfa-coordinator for cluster ${cluster.cluster_id}.`,
- `Run the FULL collaborative RCA loop for the representative test.`,
+ t1
+ ? `Turn 1 was pre-dispatched by Step 4b — see below for how to resume it. Otherwise run the FULL collaborative RCA loop for the representative test.`
+ : `Run the FULL collaborative RCA loop for the representative test.`,
`testRunId=${r.testRunId} testName=${r.testName ?? ""}`,
`error_digest: ${r.error_summary ?? "(none)"}`,
+ resume,
+ t1,
shared,
`Return the structured RCA_OUTPUT for this test.`,
- ].join("\n");
+ ].filter(Boolean).join("\n");
}
function siblingPrompt(sibling, repResult, cluster) {
@@ -84,29 +160,40 @@ function siblingPrompt(sibling, repResult, cluster) {
` root_cause: ${repResult?.root_cause ?? "(representative did not resolve)"}`,
` related_prs: ${JSON.stringify(repResult?.related_prs ?? [])}`,
`State this hypothesis on turn 1 and ask TFA to CONFIRM it against THIS test's own logs.`,
+ `The pre-fetched evidence file's data about your OWN workload is real evidence about YOUR OWN test — reading it is NOT blind inheritance. What must stay independent is the CONFIRMATION judgment: never adopt the representative's verdict just because the file already has the answer in it.`,
`If TFA confirms in one turn → done. If it does NOT (NEEDS_INFO), fall back to the full loop — never blindly inherit.`,
`testRunId=${sibling.testRunId} testName=${sibling.testName ?? ""}`,
`error_digest: ${sibling.error_summary ?? "(none)"}`,
+ resumeLine(sibling),
shared,
`Return the structured RCA_OUTPUT for this test.`,
- ].join("\n");
+ ].filter(Boolean).join("\n");
}
log(`Batch: ${clusters.length} cluster(s) over build ${ctx.buildId ?? "?"}`);
// Pipeline: each cluster flows representative → siblings independently (no barrier
// between stages), so a small cluster's siblings confirm while a big cluster's
-// representative is still looping. Concurrency is bounded by the workflow runtime
-// (~min(16, cores-2)); config.concurrency (5) is the intended soft target.
+// representative is still looping. Concurrency is capped by the Workflow runtime
+// at min(16, cores-2) — an architectural limit of the tool itself, not something
+// this script or config.concurrency (20, see rca.config.json) can raise. That
+// config value is an intended soft target/upper bound on THIS path only; the
+// runtime queues anything beyond its own cap regardless of what this file says.
const results = await pipeline(
clusters,
(cluster) =>
- agent(repPrompt(cluster), {
- label: `rep:${cluster.representative.testRunId}`,
- phase: "Representatives",
- agentType: "tfa-rca:ai-tfa-coordinator",
- schema: RCA_SCHEMA,
- }).then((rca) => ({ cluster, rca })),
+ // Step 4b's turn 1 already RESOLVED this representative — no dispatch at
+ // all, zero added latency. The orchestrator already flipped this row's
+ // CSV entry to terminal; `resolved` just needs to flow into the sibling
+ // stage's pre_seed the same way a dispatched rep's result would.
+ cluster.representative?.resolved
+ ? Promise.resolve({ cluster, rca: cluster.representative.resolved })
+ : agent(repPrompt(cluster), {
+ label: `rep:${cluster.representative.testRunId}`,
+ phase: "Representatives",
+ agentType: "tfa-rca:ai-tfa-coordinator",
+ schema: RCA_SCHEMA,
+ }).then((rca) => ({ cluster, rca })),
({ cluster, rca }) =>
parallel(
(cluster.siblings ?? []).map((sib) => () =>