Feat/generic rca agent plugin v3 - #2
Open
Dave3130 wants to merge 71 commits into
Open
Conversation
…EADME Identity-only .claude-plugin/plugin.json; root .mcp.json wires the bstack MCP server (stdio); config/rca.config.json centralizes all formerly-hardcoded product/infra values (no kubectl/chitragupta/bifrost literals); /rca-build command parses build id + mode and hands off to the skill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the obs-tfa-rca loop decoupled: ai-tfa-coordinator drives tfaRcaTurn to a terminal RCA (turn-cap, one-thread, soft-PENDING, digest-not-dump) with the gather mechanism routed by capability (no kubectl/chitragupta/bifrost literals). lib/routing.mjs classifies each ask skip/gather/gap against the config registry + capability manifest; the gap action is the only mode fork (auto=unavailable, interactive=ask-user). references/evidence-routing.md carries the digest format and size caps verbatim. Adds sibling pre-seed one-turn-confirm hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
SKILL.md orchestrator spec: mandatory GitHub intake ('I don't have one' → RCA-only;
headless missing-input fail-fast), discovery via listTestIds(failed,
includeFailureDetail), then cluster/pre-compute/fan-out/report steps.
lib/csv-state.mjs is the resumable WAL spine — seed (idempotent, terminal-
preserving), claim/heartbeat/flip, reaper, pendingRows — with timestamps injected
(workflow-sandbox-safe) and an RFC4180 codec for multiline RCA fields.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…otocol lib/signature.mjs computes signature = normalize(category|error|file) off the U1 discovery payload (folds timestamps/uuids/hex/line:col/numbers), groups rows by signature, picks a deterministic representative (non-flaky, then smallest id), and leaves signal-less rows as their own singletons. references/clustering.md documents the O(causes) protocol: representative runs the full loop; siblings pre-seed a one-turn confirm against their own logs with a fall-back-to-own-loop safeguard (never blindly inherit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buildManifest enumerates the client's discovered capabilities once into
capability→{available,via}, declared to the user + TFA so no evidence is asked
for that the client provably can't get. lib/evidence-cache.mjs computes the
last-green→this-build delta once and caches by (repo,range,evidenceType) — fresh
per-run Map, no module globals (multi-tenant-safe) — with resolveBaseline for the
never-green fallback. Routes the same grounded window into every coordinator.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
workflows/rca-batch.mjs orchestrates the batch in auto mode: a pipeline over clusters dispatches ai-tfa-coordinator agents — representative full loop → siblings one-turn-confirm, no barrier between stages — with a structured RCA schema. Sandbox-correct: does no state I/O itself (orchestrator passes the clustered work-list + manifest + pre-computed build evidence via args; each coordinator agent persists its own CSV row eagerly). Gap → 'unavailable' back to TFA, no user prompt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
references/interactive-mode.md specifies the orchestrator loop: spawn ai-tfa-coordinator subagents 5 at a time; a subagent cannot pause to prompt the user, so on an evidence gap it ends early with a GAP_OUTPUT carrying resume handles (threadId+turnId); the orchestrator asks A1, then re-dispatches with resume= and the answer. Same coordinator as auto — only the gap action differs. Compact blocks not transcripts (lean main context); partial-first; auto-first/ escalate-the-residue noted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
references/github-evidence.md specifies exactly what each github ask needs (diff-since-baseline, PRs-in-window touching the failing path, blame, deploy timing) and the discovery order GitHub MCP → gh → degrade — no shipped forensics harness. Adds the adversarial falsification protocol (path overlap / deploy-state guard / direction) so only verdict:supported suspects enter related_prs; ruled-out suspects stay as disconfirming evidence. Coordinator runs it for product_code/ deploy/ci asks, reusing the pre-computed build evidence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… in U4) lib/coverage.mjs derives a per-row evidence-coverage band — TFA confidence capped by coverage (full keeps it, partial→medium, thin→low) so a RESOLVED built with evidence unavailable reads as lower confidence BECAUSE of the gap. lib/report.mjs renders the CSV to markdown: status counts + per-test table + coverage caveats, degrading missing fields to 'not available' and never crashing on an empty/partial batch. report-format.md documents the stamp, layout, and the startup reaper resume path. Blast-radius digest explicitly deferred. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…harness lib/loop.mjs (runRcaLoop) is an executable mirror of the coordinator loop — status branching, ask routing, gap resolution, turn-cap, one-thread, soft-PENDING — driven by an injected submit(). It doubles as the D5 sequential thin-client harness. tests/conformance.test.mjs replays recorded tfaRcaTurn transcripts (resolved/blocked/pending/turn-cap fixtures) and proves: rca capture, test_logs skip, soft-PENDING no-re-poll, turn-cap never submits a 7th turn, and the degraded (no-capability auto) path still reaches a valid terminal RCA — same loop, same result. 48 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, skip turn-cap gather Code-review fixes (suggested, non-blocking): - pending-resume removed from TERMINAL_STATES → soft-PENDING rows are now re-claimable, listed by pendingRows, and skipped by the reaper (they cleared in_flight), so the retained threadId/turnId actually drive an in-session resume instead of being stranded as a permanent non-terminal terminal. - flip() now rejects a missing/non-terminal rca_done without mutating, so a partial flip can't clear the claim yet leave the row pending (duplicate-RCA clobber). - loop checks the turn-cap BEFORE gathering, so evidence on the never-submitted final turn isn't gathered for nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mirror the slack-mcp-plugin parallel-manifest pattern over one shared skills/ tree: add .cursor-plugin/plugin.json + .cursor-mcp.json (stdio bstack, Cursor dialect) and codex-mcp.example.toml (~/.codex/config.toml block). INTEGRATION.md documents per-host wiring — MCP config, Agent-Skills skill/agent discovery (.cursor/skills, .agents/skills, .codex/agents), the Add-to-Cursor deeplink and codex mcp add — and the one Claude-only piece (auto-mode dynamic workflow), which Cursor/Codex fill with interactive subagents or the sequential lib/loop.mjs harness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
automation/: failing API cases for the rcaChat / is_mcp_driven feature (pins the jsonb 42601, setTestRca data-gate, and ownership-lockout regressions), the JUnit report (build-rca-failures.xml), and upload.sh. Uploaded as project 'RCA Feature Fencing' / build 'VRT Build' (build awswxm0t5ve7vbjnspfna4xbvjwxn92u2lwv5fw2). skills/k8s-rengg-tfa/ + bin/k8s-context.sh: the discovered k8s evidence capability — read-only obs-api context (pod health, deployed image, error-level logs, events) from the rengg-tfa namespace, error-level grep + secret redaction. Wired as the k8s discoveryHint in rca.config.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both commands/rca-build.md (/tfa-rca:rca-build) and skills/rca-build/SKILL.md (name: rca-build) existed in the plugin — the slash invocation resolved ambiguously and returned a bare 'loaded' stub, forcing the agent to hunt for and re-read SKILL.md. Remove the redundant command (the skill is already slash-invocable and parses build id/mode/PRs in Step 0) and tighten the skill description (393 -> ~270 chars) so it registers cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…is present Auto mode is autonomous (no mid-run user input), but Step 1 said the intake was 'mandatory, both modes', so the agent prompted and STALLED waiting for intake answers even with mode=auto. Rewrite Step 1 per mode: auto gathers intake from invocation args + cheap inference (gh repo view, current branch), records any missing field as 'I don't have one' (RCA-only), shows a one-line FYI, and continues immediately — never prompting. Only interactive mode asks A1, once, upfront. Headless unchanged (build id required, rest default to none). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Plugin agents register under <plugin>:<name>, so the auto workflow's
agent({agentType:'ai-tfa-coordinator'}) failed with 'agent type not found'.
Use 'tfa-rca:ai-tfa-coordinator' in rca-batch.mjs (both representative + sibling
dispatch) and fix the interactive-mode / agent-doc dispatch examples to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e_all) The prior fix only updated the sibling call (different indentation); the representative agent() at the top of the pipeline still used the bare 'ai-tfa-coordinator' and would fail identically. Both now use the namespaced type. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- skills/rca-build/ -> skills/factory/ (frontmatter name: factory); no command file with the same name (collision lesson, see 2af22ef) - delete the mode concept: no mode arg, no auto|interactive fork, drop references/interactive-mode.md - restructure to ONE GATE before execution: Part A = connector discovery + cheap probe validation into a connector -> valid|invalid|absent manifest (gaps recorded, never blockers); Part B = intake resolved by assumption, at most ONE consolidated question at gate close (headless never asks). After gate close the run never asks the user again. - no local report: drop references/report-format.md and config paths.reportFile; finish = glimpse table + triggerRcaReport + Test Observability UI link - PRODUCT_BUG mandate in github-evidence.md: application-bug RCA must carry culprit PR link(s) or explicitly state what was searched - server emits only NEEDS_INFO|RESOLVED: drop BLOCKED from references Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… local report
- lib/loop.mjs: RESOLVED now carries {glimpse:{root_cause,failure_type,
related_prs}, viewRca} — no full rca payload; drop BLOCKED (server
emits only NEEDS_INFO|RESOLVED, plus soft-PENDING); drop the
resolveGap injection — gaps always degrade to an 'unavailable' block
(auto-only, no prompt path)
- delete lib/report.mjs and .rca/rca-report*.md — the plugin never
renders a local RCA report; new lib/glimpse.mjs renders the terse
end-of-run glimpse table (testRunId → cluster → status → confidence
one-liner) from the CSV spine
- fixtures: resolved.json/pending.json to the trimmed shapes; drop
blocked.json; conformance + coverage tests updated (49 green)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- ai-tfa-coordinator: drop the mode input and the interactive gap-return
(GAP_OUTPUT); gaps always degrade to 'unavailable' — never a prompt.
Classify only RESOLVED|PENDING|NEEDS_INFO (BLOCKED gone); consume the
trimmed RESOLVED glimpse {root_cause, failure_type, related_prs} +
viewRca and pass it through verbatim; RCA_OUTPUT gains failure_type +
view_rca, drops possible_fix/BLOCKED
- mandatory culprit-PR hunt for PRODUCT_BUG / application bugs: feed PR
link(s) to TFA so dashboard related_prs populates; no PR by turn cap →
explicit 'no culprit PR identified after <searched>' + recorded gap
- workflows/rca-batch.mjs: single autonomous path (no mode arg), schema
matches the new RCA_OUTPUT, PRODUCT_BUG mandate in the shared prompt
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…CKED residue - README: /factory usage, the one-gate two-part flow (validated connector manifest + assumption-first intake, at most one consolidated question), autonomous execution, glimpse + triggerRcaReport + Test Observability UI link as the only outputs - INTEGRATION: factory skill naming, triggerRcaReport in the MCP core row, no-local-report finish on every host - automation/README: demo invocation is now /factory <build-id> - lib/routing.mjs comments: gaps degrade to 'unavailable', no resolveGap Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…c-generator skill Revert only the rename from the single-gate rework (name collision with the user-level /factory skill); all auto-only, single-gate, glimpse, and PRODUCT_BUG-mandate behavior stays. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eDir) Two fixes to the state spine's location: 1. The build id is now IN the filename (rca-state.<buildId>.csv) — previously a fixed .rca/rca-state.csv meant a run on build B would collide with and 'resume' into build A's state. 2. Default directory is OS temp (<tmpdir>/bstack-rca/), not the invoking workspace — a background harness must not pollute the repo it runs from. paths.stateDir remains an override for retained artifacts (e.g. CI). Resume-safety is preserved per build (same id -> same path). Hostile ids are sanitized. 4 new tests; 53/53 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…les/, scripts/ - k8s-rengg-tfa is self-contained: bin/k8s-context.sh -> skills/k8s-rengg-tfa/ scripts/ (carries the pending redaction hardening for auth headers) - rca-build gains templates/ (suspect-packet, evidence-block, gate-summary) as the canonical fillable formats — references now point at them instead of embedding drift-prone copies — and examples/sample-run.md (fictional, matches the recorded-turn fixtures): gate summary, answered NEEDS_INFO turn with supported + ruled-out suspects, terminal glimpse output Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ai_report/aitfa tabs The viewReport/viewRca destination is the build's AI-TFA report: <base>/builds/<buildUuid>?tab=ai_report&subTab=aitfa on automation.browserstack.com (designer-confirmed; was an observability.browserstack.com assumption). Fixture now carries the guidance string the bridge actually emits on RESOLVED turns — closes the fixture-drift review finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Environment-specific harness removed; k8s stays a generic gate-probed capability (discoveryHints now empty, like logs/metrics). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…runtime the user has The harness no longer assumes Kubernetes. 'infra' is the capability (runtime state of the service); k8s/EKS, ECS, docker, Nomad, VMs, PM2 are all just possible connectors behind it — the gate probes what actually exists and records the kind in the manifest (via). The k8s evidenceType remains a routing alias into infra so TFA asks spelled 'k8s' keep working. kubectl is one probe on a menu, not the definition of the capability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…; note prod default base URL Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…beta.1 Pinned across all three client wirings (.mcp.json, .cursor-mcp.json, codex example) + INTEGRATION.md. Codex example env aligned with the prod-default base URL (staging override optional). Version verified present on the public npm registry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Self-imposed plugin budget, well under the tfaRcaTurn tool's actual 5000-char hard cap. Retunes the per-block digest caps (SUMMARY, SNIPPET) proportionally so real turn-1 messages fit the new budget instead of relying on ad-hoc truncation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d file The pre-fetch file was read-only from a coordinator's side — when a live gather filled a gap or went deeper than the pre-fetch had (a full diff instead of a summary, a PR the pre-fetch never named), that extra work died with the coordinator instead of benefiting its own siblings or other clusters sharing the same repo/workload. Add mergeGithubEvidence/mergeLogsEvidence (read-modify-write, dedupe PRs by number, union clusterIds, never drops what a patch doesn't mention) and wire Operating Principle 0 + the culprit-PR mandate + the routing step in ai-tfa-coordinator.md to call them after any live gather. Confirmed on a real prior run (vrt_345): a representative's own deep PR-diff investigation was exactly the kind of work its sibling had no way to reuse under the read-only version of this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The write-back added in 70ef859 had every coordinator read-modify-writing one shared JSON file. Measured under 8 concurrent writers with a realistic read -> work -> write window, that design lost 28 of 40 updates (70%). Replace it with a layout where contention is structurally impossible instead of merely unlikely: the orchestrator solely owns the base file, and each coordinator writes only its own shard at rca-evidence.<buildId>.contrib/<testRunId>.json. No two processes ever open the same file for writing. readEvidenceFile folds base + all shards into one view (sorted order; real evidence beats a recorded gap; PRs unioned by number), and readBaseFile keeps the orchestrator's own write path from absorbing shard content back into base. Same 8-writer test against the new layout: 0 of 40 lost. A 12-process run hammering one repo key kept all 480 writes. A corrupt/half-written shard is skipped rather than breaking every subsequent read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two measured sources of duplicate work on a real 10-test build: 1. `gh` was 37% of all coordinator tool calls (151), and 46 were byte-identical commands re-run by different coordinators — one BStackAutomation spec fetched 12 times, a frontend component 5 times. None of these fit the evidence file's schema (deployState / prsInWindow), so the write-back added earlier could not share them. Add a build-scoped memo cache keyed by the CALL rather than by evidence shape, so duplication is caught regardless of what the call was for. `bin/cached-exec.mjs` wraps a shell fetch transparently (same stdout, same exit code, executes only on a miss); `bin/cached-mcp.mjs` does check-then- store for read-only MCP queries. One file per call key + atomic rename, so concurrent writers cannot collide or produce a torn read. Failures are never cached (a transient rate-limit must not become a permanent answer), stateful tools (tfaRcaTurn/getTfaTurnResult/triggerRcaReport) are refused, secrets are redacted before anything reaches disk, and the runner uses execFile with a hand-rolled tokenizer so no shell ever interprets an argument. 2. Drain reads plus their sleeps were 23% of all coordinator tool calls. The drain treated "TFA is still thinking" and "the TFA run hard-failed" identically, spending the full 40-read/10-min budget on turns that had already died — the four tests that wedged this way were the four slowest in the batch. Distinguish the two: stop after `maxErrorReads` (default 3) CONSECUTIVE hard failures, note it as `tfa-error`, keep the row resumable. A single good read clears the streak, so flaky-but-recovering reads still land. Measured: cross-writer cache hit served in 0.185s vs 1.131s for the live fetch. 108 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three on-disk artifacts live under a world-readable OS temp dir and carry sensitive content: the tool cache holds raw gh/kubectl output (private repo source, internal hostnames, log bodies), the evidence file holds PR detail and app-log digests, and the state CSV holds root causes and culprit PRs. Default umask left them 0644 — readable by any local user. Create directories 0700 and write files 0600. The file mode is the load-bearing control: a pre-existing directory keeps its own permissions (silently chmod'ing a caller-supplied stateDir would be presumptuous), but entries are 0600 either way, and a traversable cache dir only exposes opaque hash filenames. Note this hardens storage, not the redaction: redact() is best-effort pattern matching over token-shaped strings and was never sufficient on its own to make these files safe to leave world-readable. Verified on disk for both a fresh dir (0700/0600) and a pre-existing 0755 dir (files still 0600). 111 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dispatched three coordinators concurrently on a live build. They surfaced issues no unit test had, worst first: 1. CORRECTNESS: an empty `prsInWindow` with `gap: null` is byte-identical whether the PR search ran and found nothing or was never populated. Seen live — the file asserted 0 PRs for a repo that actually had 21 (the loader's search had silently returned empty), which would let a coordinator report "no culprit PR identified" with false confidence. Add an explicit `prsSearched` flag (sticky across contributors), a `hasTrustworthyPrList` helper, and a `coverage.reposWithUntrustedPrList` signal. Repo-level coverage semantics are unchanged — trustworthiness is reported alongside, not folded into, covered/gapped. 2. The runnable-guard scanned the raw command string, so it refused legitimate read-only calls: `;` inside a jq expression, `&` inside a quoted URL. Check whole argv TOKENS instead — post-tokenization a quoted metacharacter is inside an argument (harmless, we execFile) while a real operator is its own token. Verified both previously-refused commands now run. 3. execFileSync BOTH inherits and captures stderr, so relaying err.stderr printed failures three times. Capture only, relay once — wrapper stderr is now byte-identical to the command's own, plus one banner line. 4. Empty results were cached, making a sticky invisible negative. Not stored. 5. Nested single quotes made `--jq '...'` unpassable inside a quoted command argument. Accept `-` to read the command from stdin. 6. File mode applies on create only, so files left by a pre-hardening run kept 0644 forever. chmod explicitly on overwrite too. Also documented two traps that are usage, not bugs: `2>&1 | jq` merges the stderr banner into the pipe (the banner is correctly on stderr — one agent misreported this as a stdout bug; measured to confirm), and `cached-mcp get`'s exit code is lost behind a pipe. 118 tests pass, including regression tests for each fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaying 223 recorded gh/kubectl calls from prior coordinator runs through the wrapper showed it was nearly useless in practice: only 25 calls were accepted and ZERO were hits. The cache was fine; the guard was wrong about what real traffic looks like. Three causes, each measured: - 89% of real calls embed the fetch inside a pipeline (`gh api X | jq .y`), which the guard refused outright. Now pipelines are parsed into a fetch plus a chain of pure text filters (jq/grep/head/... allowlisted), each run via execFile with no shell anywhere. Only the FETCH is keyed, so several agents filtering one fetch differently share a single network call. - `2>&1` appeared on 134 of 223 calls — agents add it reflexively because gh is chatty. It says nothing about what to fetch, and the wrapper captures stderr separately anyway, so it is normalized away instead of refused. Refusing it alone drove acceptance to zero. - `>/dev/null` was classified as "looks mutating", which is both wrong and misleading. Redirection is now separate from mutation, with an accurate message; genuine file redirects are still refused. Result on the same recorded traffic: 123/223 accepted, 32 duplicate fetches eliminated, 26% hit rate. The rest are `for repo in ...` shell loops, which legitimately need decomposing into one fetch per call — that is also better for cache granularity. Verified end-to-end that `gh api X 2>&1 | jq .a` and `... | jq .b` now share one cached fetch, and that chaining, non-filter binaries and file redirects are still refused. 123 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dispatched six coordinators concurrently against a warm cache. Five of six resolved; the swarm found three real defects, one of them data-destroying. 1. DATA LOSS: redaction consumed 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 214,074-byte response was cached as 816 bytes with the content field silently gone. Every private-repo file fetch through the cache was corrupted, and a coordinator hit exactly the predicted failure: it could not verify a line-number-specific hypothesis and nearly falsified a claim that was in fact TRUE (TestRunService.java:1747 really is an unguarded `.get(0)`). Redaction is now bounded by the first structural delimiter. Two already-corrupted entries were purged from the live cache. 2. Neither tokenize nor splitPipeline handled backslash escapes, so `\"` read as a closing quote. Two coordinators hit the two symptoms: jq string equality arrived mangled (`select(.filename==\a/b.json\)`), and a `|` in a jq regex alternation was split as a shell pipe, refusing the command. One fix, both symptoms — escapes are now POSIX-style (literal everywhere except inside single quotes). 3. Two findings corroborated independently by multiple coordinators, now in the contract: a drain error kills the TURN, not the THREAD — resubmitting on the same threadId resolves immediately, whereas the old reading ended PENDING and discarded a resolvable test; and the turn-2 wedge correlates with message size (~1400/~1350-char submits failed, a ~940-char retry landed) against a configured 1000-char cap. Cache went 33 -> 63 entries over the swarm with 7 cross-agent hits, all seven writer shards present and no collisions. 128 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…abulary `flip` accepted only the lowercase CSV states and returned a bare `false` for anything else — including `RESOLVED`, which is the exact value the RCA_OUTPUT contract tells every coordinator to emit. Callers that didn't check the return saw no difference between success and a no-op, so their rows stayed `pending` and looked un-run. This was not theoretical: a six-coordinator swarm appeared to "skip" the CSV flip entirely. A later coordinator diagnosed the real cause — they had almost certainly called flip with `RESOLVED` and been silently refused. Field names had the same trap: `thread_id`/`status` were dropped without comment by the COLUMNS guard, since the columns are `threadId`/`rca_done`. Accept and normalize the output-block vocabulary (RESOLVED->resolved, PENDING->pending-resume, case-insensitive) plus the field aliases, and make genuine rejections and dropped keys warn loudly instead of failing silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ce and hit counts Three findings from a warm-cache verification run. 1. RETRACTION. The contract told coordinators to keep turn messages under 1000 chars to avoid the TFA wedge, on the strength of a two-point correlation (~1400/~1350-char submits failing where a ~940-char retry landed). A later run refuted it: a 240-char message wedged exactly as a 1500-char one did. The cap stays — a tight digest is the contract — but it is no longer presented as a wedge cure, and a wedge must not be read as evidence the message was too long. The reliable response remains resubmit on the same thread. 2. Contributions were invisible to the obvious read. Coordinators are handed the base path and naturally `cat` it, which shows base only — one agent reported "2 repos" when the folded view had 5, including an 11-PR observability-api entry a sibling had contributed. Shards are what make concurrent write-back safe, so add `bin/evidence-show.mjs` to print the merged view (with --summary flagging repos whose PR list was never actually searched) rather than give up the layout. 3. Hit-rate was unmeasurable in normal use: callers silence tool chatter with `2>/dev/null`, which also discards the banner the metric depends on. Banners now tee to TOOLCACHE_LOG when set, so stderr can be suppressed and hits still counted. Warm-cache result that motivated this: cross-agent hits confirmed — one coordinator re-filtered a sibling's cached 76KB PR diff three ways for free, another reused two cached diffs it never fetched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The MCP cache had stored zero entries across every live run. The cause is structural: an agent's get -> call -> put costs three tool calls on a miss to save one later, so skipping it is correct for a one-off query, and every coordinator did. Pre-seeding inverts the economics. The orchestrator already runs these log sweeps in Step 4, so depositing each digest under the key a coordinator would compute makes the agent's `get` a single call that usually hits. Verified: after seeding four VictoriaLogs digests, a coordinator get returns HIT, and arg-order canonicalization means a differently-ordered query still matches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1. `flip` silently dropped `view_rca` and `turns_used` — both are in the RCA_OUTPUT contract but had no CSV column, and `view_rca` is the dashboard link the entire run exists to produce. Added as columns. 2. An MCP cache HIT requires reproducing the args exactly, and canonicalization normalizes key ORDER, not content — so a coordinator that guesses the logql/window/limit triple misses. One run burned four probe calls guessing to save two, making the cache a net loss on that pass. Added `cached-mcp <build> list`, which prints every cached query with its exact args to copy: one call instead of guessing. 3. MCP banners were not tee'd to TOOLCACHE_LOG, so shell and MCP hits could not be totalled from one file. Both now log there. Verification-run result for the record: 22 tool calls vs 81 on the same test cold (73% fewer), but the saving came overwhelmingly from the shared evidence file, not the call-level caches (1 shell hit of 8; MCP roughly a wash after the failed probes). The caches only pay on repeated identical lookups, and that run's second turn demanded a code path nobody had walked — a low hit rate there is the correct outcome, not a cache failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sely Both from a live run on a second build (ObservabilityApiLaneSuite, 189 failures across 9 clusters). - `evidence-show --prs [repo]` prints mergedAt | #num | title plus the build's started_at. That table is the highest-value falsification per byte: on this run it disqualified 11 of 22 candidate PRs on the window guard alone, before a single diff was fetched. Previously a coordinator had to pipe --repo's raw JSON through an ad-hoc node one-liner to build it. - `flip(testRunId, fields)` — dropping the leading csvPath — used to bind an object to testRunId, read a nonexistent CSV, and return a bare `false` that a caller mistook for success. It now says exactly what went wrong and shows the correct signature. Cache behaviour on this build, worth recording: the shell cache was COLD, yet the representative still logged 4 hits of 7 fetches — concurrently dispatched siblings populated it for each other, including the single most expensive call (`gh pr diff 16450`). And `cached-mcp list` alone avoided two grafana calls: the seeded digests were self-sufficient, so no `get` was needed at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ve absence A live run on a second build found the pre-fetch structurally blind to its own build's cause. An upstream outage ran 06:12:00-06:15:51 while the build's finished_at was 06:12:21, so a sweep scoped strictly to started_at..finished_at saw 21 seconds of a 4-minute outage. Step 4 now pads to started_at-2m .. finished_at+10m and labels findings inside/outside the strict window — without licensing an arbitrary window, which is the opposite failure (matching a coincidence in unrelated traffic). Two query mechanics added, both of which cost real calls in that run: - `direction` defaults to newest-first, so a limited query returns the END of the range. Verifying this finding required direction:"forward" — my own backward queries kept returning traffic clustered at each window's tail, which looked like the gap was absent when it was simply out of view. - Absence needs a control: a zero-result query is indistinguishable from a bad selector. Prove the logger was alive in the same window with a query you expect to be non-empty before treating silence as evidence. Verification note on the finding itself: the gap is real (zero /ext/v1 06:12:00->06:15:51, resuming with a 500 on /ext/v1/badge/build). Two secondary details in the coordinator's report did not hold — traffic resumes at 06:15:51 not 06:16:30, and badge traffic was not absent until 06:48. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ng a negative Both from a live run on a second build. - `TFA agent run failed` (the wedge, size-independent, fix = resubmit on the same thread) and `turn expired or not found` (observed on a ~2000-char over-cap submit) are different failures. The latter's text names a thread/turn problem, so a coordinator reads it as a wedge and resubmits unchanged instead of shortening. Named both explicitly. - A coordinator nearly concluded a manifest lacked an entry when the fetch had been cut at ~64KB; its own `wc -l` control caught it (1042 lines vs 1518). Investigated and the tool cache is NOT the cause — verified 104KB and 214KB files round-trip intact, and the only truncation is past 256KB with an explicit marker — so this was surrounding tool plumbing. The hazard is real regardless of layer, and a silent truncation converts "grep found nothing" into a false negative, so: size-check a large fetch before treating an absent match as evidence of absence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Measured across three real runs (407 gh calls): per-PR `gh pr view <n> --json files` was 44 calls (10.8%), and every one is avoidable. `gh pr list` accepts `--json files`, so ONE request per repo returns every PR's changed paths — the Step 4 call we already make, just asking for one more field. Verified live on build fiehz…: 2 calls covered 22 PRs / 104 changed paths, replacing up to 22 per-PR fetches across coordinators. Path-overlap — the first falsification test in github-evidence.md — is now answerable from the evidence file with zero calls. It independently surfaces testhub#16450 -> project.js, the culprit a coordinator previously spent calls hunting. Two other measured wastes named in the same step: connector re-probes (`gh auth status` / `kubectl version`) were 17 calls (4.2%) purely because the manifest wasn't trusted, and file CONTENTS were 31% but only partly predictable — so those are deliberately NOT bulk-fetched. The `files` lists tell a coordinator which files matter; the tool cache dedupes the ones two coordinators both open. Diffs stay on-demand: large, and few PRs need one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
File CONTENTS were 126 of 407 gh calls (31%) across three real runs — the largest remaining slice. Six of the eight repos this workspace needs are already cloned locally, so most of those round-trips are avoidable. Measured: `gh api .../contents/<path>?ref=<sha>` ~1022ms vs `git show <sha>:<path>` ~37ms — 27x faster, byte-identical. A stale clone needs one targeted `git fetch` (~5.4s), which pays for itself after ~6 reads of that repo. SHA-PINNED ONLY, and this is the whole reason the module is careful rather than a one-liner. This workspace's clones were 12 commits behind: reading testPlan.js from the local BRANCH returned 281,061 bytes where the real head had 282,315. RCA reasons about what changed in a window, so silently reading different code produces a confident wrong answer. Pinned to the build-time sha — which Step 4 already records as deployState — content is byte-identical and staleness stops mattering, because a commit either exists locally or it provably doesn't. Two deliberate non-behaviours: the library never falls back to the network itself (it reports remote-needed and lets the edge decide, so a local answer can't be silently substituted), and a path genuinely absent at that commit is returned as an ANSWER rather than a fallback trigger. Remote reads still go through the tool cache, so a repo without a local clone behaves exactly as before. 137 tests pass, including a real two-commit git fixture that exercises the stale-branch-vs-pinned-sha distinction rather than mocking it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…literals The first cut of bin/repo-read.mjs defaulted RCA_WORKSPACE_ROOT to a specific developer's path and RCA_SHIPPING_BRANCH to observability_pre_prod. That is the exact coupling the capability-manifest design exists to prevent: which repos exist, where they are checked out, and what branch ships are facts the product CONNECTOR SKILL owns and the gate resolves, not something the plugin may assume. As written it would have worked for one product on one machine and silently misbehaved elsewhere. Both are now required inputs. Missing RCA_WORKSPACE_ROOT fails loudly with an explanation instead of guessing; the shipping branch is optional and only widens a fetch on a miss. lib/repo-source.mjs names no repo, branch or path — callers pass everything. Audited lib/, bin/, workflows/ and config/: zero product literals remain. 137 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…evidence file Two follow-ups to the local-repo reader. GENERIC DISCOVERY. discoverWorkspaceRoot() finds the clone directory without any hardcoded path: it tries the caller's dir, its parent, then grandparent, and accepts one only if it actually contains a repo from THIS run's validated list. That check is what makes it generic — a different product with a different layout resolves by the same rule, and nothing about repos, branches or paths is assumed. It is deliberately bounded to ~3 tries: climbing further risks matching an unrelated checkout, which is silently wrong rather than merely slow, and "not found" is a perfectly good answer since the caller falls back to the network. An explicit RCA_WORKSPACE_ROOT is honoured but still verified, so a stale override fails loudly instead of quietly. RESOLVED ONCE, NOT PER-AGENT. resolveLocalRepos() + setLocalRepos() record, in the evidence file, which repos are readable locally at their pinned shas. A coordinator reads that map and knows immediately whether to go local or remote, with no filesystem probing of its own — the same duplication the evidence file already removes for PRs and logs. Resolution order in bin/repo-read.mjs is now: evidence file, then RCA_WORKSPACE_ROOT, then bounded discovery, then network. Verified end to end on this workspace: gate discovered the root in 2 tries, resolved 3 of 4 repos as locally readable (the fourth has no clone and falls through to the cached gh path), and a coordinator then read a 145KB file with no network and no probing, sourced "via evidence-file (resolved at gate)". 144 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rompt The reader worked but nothing invoked it. bin/repo-read.mjs was referenced in zero skill/agent files, and discoverWorkspaceRoot/resolveLocalRepos/ setLocalRepos were called from nowhere outside tests — the whole local-clone path was unreachable at runtime, so every coordinator still went to the network for file contents (31% of gh traffic). Three changes close that: - SKILL.md Step 4 gains a step 6 that resolves the workspace once against the run's validated repo list and persists the per-repo map via setLocalRepos, pinned to deployState commit shas rather than branch names. - ai-tfa-coordinator.md documents repo-read alongside cached-exec/cached-mcp, with the sha-not-branch rule stated as the reason rather than a rule. - repo-read derives the evidence path from the buildId it already receives, so there is no RCA_EVIDENCE_FILE for a dispatch prompt to forget; an explicit override still wins. Verified with no env set: read teststack@41cc211b from the local clone with no network, sourced "via evidence-file (resolved at gate)"; a branch name is still refused. 144 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three fixes, one of which prevents the other two from recurring. REACHABILITY TEST (tests/wiring.test.mjs). Twice a helper shipped fully unit-tested and invoked by nothing. Unit tests cannot catch it — the module works perfectly alone, which is why it survives review. The prompt layer is the real call graph, so the test asserts every bin/*.mjs is named by some skill/agent markdown, plus that gate-critical lib exports have a call site outside tests. Verified it actually fires: adding an orphan file fails the suite with the helper named. FOLDED READS. Operating Principle 0 told coordinators to `Read` the evidence path — which shows the orchestrator's base file only and hides every contribution shard, the exact bug bin/evidence-show.mjs exists to fix and which cost a real run a re-gather of an 11-PR entry. It now directs --summary/--prs/--repo, and explains why reading the raw JSON is both wrong and more expensive. STALENESS. The file is keyed by buildId alone, so a resumed run silently reuses deployState and a PR window that have both moved on — the same silent-wrong-answer risk we refuse branch names over, with no guard at all. stalenessOf() reports age and advice; it deliberately does not expire anything, since stale build-level context still beats none and the failure window itself never moves. evidence-show warns on stderr on every view, so piped JSON stays clean. Caught while testing on the live build: a FUTURE timestamp clamped to age 0 and reported "fresh" — failing in the reassuring direction on clock skew. Now returns known:false, stale:true. 148 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ing columns readRows keyed rows by whatever header the file had, while writeRows only ever emits COLUMNS — so any header name not in COLUMNS was silently discarded on the next write. Found on a real legacy 10-column state file (test_id,test_name,error_summary,is_flaky,...): flipping one row through the current code returned "resolved" and reported success, having dropped test_id and test_name entirely and blanked cluster_id. Losing which test a row describes is worse than any error we could raise. readRows now normalises the header through COLUMN_ALIASES (previously applied only to flip()'s field names, not to the header) and throws on anything still unrecognised, naming the offending columns and saying to re-seed. Known legacy spellings — test_run_id, status, thread_id, turn_id — keep working. This is why build fiehz... cannot simply be resumed: its CSV predates the current schema and must be re-seeded. 150 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mkdirSync's `mode` applies only when it creates the directory — the same trap already fixed for the files themselves. Found on this machine: <tmpdir>/bstack-rca was drwxr-xr-x, created before the hardening landed, with correctly-0600 files inside it. The directory listing alone leaks which builds were analysed, and pre-hardening files inside it are still 0644. These artifacts hold root causes, culprit PRs and log excerpts and live in a shared OS temp dir, so ensureOwnerOnlyDir() now chmods an existing directory to 0700 as well as creating new ones that way. Shared by csv-state, evidence-file and tool-cache, which all had the identical pattern. 151 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Per-write hardening only tightens the file being written, so artifacts from a build analysed before that landed keep their old permissions forever — a completed build is never rewritten again. On this machine that left 6 files at 0644 holding root causes, culprit PRs and log excerpts in a shared OS temp dir. Fixing it needs a sweep, not another per-write guard. hardenStateDir() walks the tree and makes it owner-only (0700 dirs / 0600 files). Idempotent and cheap, so Step 2 now runs it unconditionally. It never throws: a file owned by another user is skipped and counted, because failing an RCA run over one un-chmod-able leftover is worse than the leak it closes. pruneStateDir() addresses growth (2.2MB / 143 files / 6 builds accumulated here, nothing ever deleting them). It is deliberately NOT automatic and defaults to 7 days: these files ARE the resume state, keyed by buildId, so anything that deletes them can silently turn a resumable build into a lost one. dryRun reports without touching. Uses mtime, not atime, so reading a file during a resume doesn't make an abandoned build look fresh. Applied to the live directory: 5 dirs / 143 files swept, 6 leftovers repaired, 0 skipped; prune dry-run confirms nothing is old enough to remove. Two test bugs found and fixed while writing this: a hardcoded future nowMs made the whole fixture look ancient (passing for the wrong reason), and `new Date(seconds)` was read as milliseconds, landing every stamp in 1970. 155 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both only surface when the gate runs for real; neither is visible to unit tests.
CONNECTOR DISCOVERY MISSED EVERY CONNECTOR. Step 0 ran `ls .claude/skills/
~/.claude/skills/`. When the plugin is itself a repo inside the workspace, cwd
is the plugin and the product's connector skills sit one or two levels UP — so
the listing found nothing and the run would have degraded to raw MCP tools with
best-effort repo guesses, exactly the failure mode Step 0 exists to prevent. It
missed all three real connectors here (tra-regression-context, nl2steps-github,
nl2steps-infra). Now also checks ../ and ../../.
PINS EXISTED ONLY AS PROSE. resolveLocalRepos needs {repo: sha}, but Step 4
wrote the build-time sha into deployState.summary as English ("Branch tip on
<branch> at build start = cd88535b"). There was no structured field, so the
gate got an empty pin map and every repo fell back to the network while
appearing to work — 0 of 6 local instead of 3 of 6. deployShas() prefers an
explicit deployState.sha and falls back to parsing the summary, reporting which
happened. The fallback anchors on the build-start phrase: a bare hex word would
also match the deploy timestamp 260731135020Z sitting in the same sentence.
Verified end to end on build fiehz…: workspace found in 2 tries, 3 of 6 repos
resolved local, and a coordinator read a 280KB file with no network.
156 tests pass. Not pushed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both fold paths deduped prsInWindow with `String(p.pr)` as the key. When a contributor writes back PRs without a `pr` number, every one of them keys to the literal string "undefined" and the list silently collapses to whichever arrived last. Caught during a live dry run, not by a test: a coordinator reported writing a 6-PR BStackAutomation window for its 21 siblings; the file kept 1, with `pr: undefined`, and still flagged the entry `prsSearched: true` — so the gap read as closed while five PRs had vanished. Siblings would have inherited a confidently-wrong window. prKey() now falls back to url, then title, then a per-entry anonymous key, so unnumbered PRs stay distinct and two unknowns are never treated as the same PR. Numbered PRs still merge across writers, and '#10' and 10 normalise to the same key. 158 tests pass. Not pushed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…uster_id
Root cause of the vrt_742 run's 30-minute wall clock, found by reading its
transcript rather than inferring it.
clusterRows() assigns cluster_id by MUTATING its input and returns
{rows, clusters}. The orchestrator ran:
const { clusters } = clusterRows(rows); // never writeRows(csvPath, rows)
so the clusters printed correctly, the run looked clustered, and every
cluster_id went to the CSV empty. Fan-out then had no representative/sibling
structure and dispatched one coordinator per test: 12 tests became 26
subagents and 30.5 minutes, even though per-agent cost was healthy at 12.2
calls (vs a 45.4 baseline). The savings were real but spent on agent count
instead of banked as time.
Two independent callers made this exact mistake on the same day — including me,
in the session that found it. That makes it an API footgun, not a user error,
so the fix is at the API: clusterAndPersist(csvPath, csvState) reads, clusters,
writes back, and throws if the persisted count doesn't match the row count.
Step 3 now mandates it and tells the reader to verify cluster_id is non-empty
before fan-out, since an unclustered run degrades silently to O(tests).
159 tests pass. Not pushed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…_seed
The sibling path DID run on the vrt_742 build — and inverted. Siblings averaged
22.7 tool calls and 2.2 turns against 8.0 and 2.0 for the representative they
were meant to be a cheap fraction of; one burned 60 calls over 17 minutes.
Cause: the coordinator has always accepted a `pre_seed` input, but Step 5 never
said siblings must be dispatched AFTER their representative lands, and never
said the dispatch prompt must carry the rep's finding. A "one-turn confirm"
with nothing to confirm degenerates into a full independent investigation with
the sibling framing on top — hence costing more, not less. Nothing detected it
because the RCAs were still correct; only the cost was wrong.
siblingPreSeed(csvPath, csvState, clusterId, representativeId) builds the seed
from the representative's landed row and returns {ok:false, reason} when the rep
is not resolved or recorded no root_cause — those siblings must not be
dispatched yet. The seed carries an explicit "confirm against your own evidence,
do not adopt this" instruction so the independence rule travels with it.
Step 5 now states the per-cluster barrier: rep first, wait, then siblings.
Clusters still run concurrently with each other; the barrier is per cluster.
160 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Telling coordinators in the prompt to use evidence-show was not enough. Measured on the vrt_741 run: 21 of 25 evidence-file reads were raw cat/grep/Read against the base path and only 4 went through the folded view. A raw read shows the orchestrator's base ONLY and silently hides every contribution shard — precisely the representative-to-sibling context the file exists to carry, and the objective we were about to report as done. Three different agents each cat-ed the same file and each saw a partial picture. The file now says so in the first bytes anyone sees. JSON has no comments, so three `_`-prefixed marker keys are stamped first on every base write: what is missing, the exact evidence-show command to run instead, and why. They are stripped on read, so nothing downstream sees them as data, and re-stamping is idempotent rather than accumulating. Also checked what prompted this: the tool cache is barely used now because the redundancy it was built for is gone. vrt_741 made 169 shell calls of which only 6 were cross-agent repeats (4%), against the original "46 exact gh repeats for 10 tests". The MCP repeats are all same-agent getTfaTurnResult polling, which the cache correctly refuses as stateful. Low cache usage is the evidence file working, not the cache failing. 162 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…wRca semantics
Investigated two things I had reported as open bugs. Both were wrong, and the
investigation surfaced a real one underneath.
NOT A BUG — turnId empty on resolved rows. TFA returns turnId ONLY on a soft-
PENDING turn ({status, threadId, turnId}) and omits it on RESOLVED/NEEDS_INFO.
Verified across two runs: 8 of 41 and 12 of 38 tool results carried a turnId,
all of them PENDING. An empty turnId on a resolved row is correct.
NOT A BUG — viewRca being a generic hostname. TFA itself returns
"https://automation.browserstack.com — open the build's AI report (...)";
coordinators passed it through faithfully. The real per-build URL comes from
triggerRcaReport at Step 6, once per run, not per test.
THE REAL GAP. Because turnId exists only on PENDING — exactly the case that
produces pending-resume — a row can land resumable with no turnId, and then the
resume path cannot drain the in-flight turn: it submits blind onto a thread that
still has a turn running. Such a row is indistinguishable from a healthy one in
the CSV. flip() now warns loudly, naming the testRunId and what to do. A warning
rather than a rejection: losing the row entirely is worse than resuming
imperfectly.
Coordinator doc now states both semantics so the next agent doesn't re-derive
them or invent a more specific link than the data supports.
Also removed four stray log files left in the working tree.
163 tests pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.