Feat/generic rca agent plugin v3 - #2
Open
Dave3130 wants to merge 86 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>
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>
… drifting The last two runs got slower and the cause was self-inflicted. On vrt_740, 92 of 407 tool calls (23%, 5.1 per test) were agents re-deriving the plugin's own API at runtime — `grep -n "^export function" lib/…`, `cat config/…`, repeated `ls .claude/skills/`. The previous run spent 13 (5%, 0.9 per test). That is ~79 of the ~120 extra calls between the runs, and most of the wall-clock regression from 18.2 to 36.3 minutes. Nothing was wrong with the helpers. Five were added this session to fix five silent correctness bugs, each justified — but the SKILL named them without signatures, so the only way for an agent to learn a call shape was to read the source. The plugin was taxing every agent to learn itself. Adds one "API reference" section before Step 0 with every signature a run needs, grouped by module, plus the bin/ commands, the three load-bearing constants (COLUMNS, RESUMABLE, TEST_LOGS) and the config keys. Fully generic: build ids, repos, branches, workloads and paths are all inputs supplied by the gate and the connector skills — verified no product or machine literals. Documenting it fixes today; drift is what actually caused this, so a test now asserts every exported lib helper appears in that section, with an explicit INTERNAL allowlist for module internals agents reach through bin/ instead. Verified it fires: adding an undocumented export fails the suite by name. For the record, this run also confirmed the fixes that preceded it: the base file's partial-view markers moved evidence reads from 16% to 74% folded (4:21 to 43:15), clustering persisted 18/18, the sibling barrier held with 0 duplicate dispatches, and gh calls per test fell 4.4 to 4.1 — so the hygiene fix displaced live gathering rather than adding to it. 164 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ng it Swept for the two bug classes this session kept producing — mutate-and-return footguns, and hard rules that live only in the prompt. Class 1 (mutate-and-return) came back clean: clusterRows is the only one, and clusterAndPersist already covers it. A pendingRows hit was a false positive in my own grep, not a defect. Class 2 turned up "A PRODUCT_BUG RCA without a culprit PR is incomplete" — load-bearing, and enforced by nothing. Checked it against four real builds before changing anything: it has held. Of 9 product-bug rows, 3 carried a PR link and 6 carried an explicit "none — searched <what>", which the rule permits. Zero were blank. So this is a guard, not a bug fix. Worth adding anyway: a blank related_prs is indistinguishable in the CSV from a genuine dead end, so the failure would be a silent product-bug attribution with no evidence trail — the same shape as every other bug found this session. flip() now warns when failure_type is a product bug and related_prs is EMPTY. A stated "none, searched X" is compliant and deliberately not nagged. 165 tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y with Step 4, and gather NEEDS_INFO asks concurrently Step 4b submits each cluster representative's turn 1 directly (tfaRcaTurn), in the same tool-call batch as Step 4's evidence pre-fetch, instead of waiting for Step 5's coordinator dispatch to submit it. A RESOLVED turn 1 flips the CSV row straight to terminal and dispatches its siblings immediately, with no Step 5 dispatch at all for that representative; a NEEDS_INFO/PENDING outcome is handed to the eventual coordinator via the new turn1_result/resume inputs (lib/turn1-registry.mjs) so turn 1 is never resubmitted. Wired through all three execution paths: lib/loop.mjs (sequential harness), workflows/rca-batch.mjs (Workflow path, plus a resume/turn1_result mutual-exclusivity guard so a prior-run pending-resume thread always wins over a same-run Step 4b entry), and agents/ai-tfa-coordinator.md (direct-dispatch prose contract). Also parallelizes a NEEDS_INFO turn's independent gather() calls (lib/routing.mjs's routeAsk/routeAsks classify each ask with no cross-ask state — priority only orders the assembled message, never the fetching) via Promise.all instead of one round-trip at a time, mirrored in the coordinator's own prose contract. Coordinator paths (references/evidence-routing.md, references/github-evidence.md) are also now consistently pluginRoot-qualified rather than bare, so a dispatched coordinator never resolves them against the wrong cwd.
…l report cleanupBuildArtifacts(buildId, stateDir) removes this build's own CSV, evidence file + .contrib shards, tool cache dir, and turn1 registry — called only after triggerRcaReport succeeds (Step 6), never a periodic sweep. Safe specifically because Step 6 only runs once every row is terminal: there is nothing left to resume for this build at that point. Deliberately separate from lib/state-dir.mjs's pruneStateDir, which remains the age-based safety net for a build that crashes before ever reaching Step 6.
…ver client-side signatures
clustersFromThemes(rows, themesResult, testsByThemeId) builds { rows, clusters }
from getBuildFailureThemes + listTestsInFailureTheme (server-computed themes),
mirroring lib/signature.mjs's clusterRows() shape so downstream fan-out code
doesn't care which path produced it. Falls back to lib/signature.mjs's
client-side text-signature clustering when getBuildFailureThemes reports
ready: false. A row already claimed by an earlier theme is skipped
(first-theme-wins) rather than landing in two clusters; any failed test the
server didn't assign to a theme becomes its own singleton cluster, never
silently dropped.
…h, and Step 5/6 wiring Two bodies of work landed together in this file (interleaved at the hunk level, so split by content below rather than by commit): - Step 3: document the server-side clustersFromThemes path (getBuildFailureThemes + listTestsInFailureTheme) as preferred over lib/signature.mjs's client-side clustering, with the fallback condition and verification step. - Step 4b (new): turn-1 pre-dispatch for cluster representatives, concurrent with Step 4's evidence pre-fetch — mechanic, the three-way branch on RESOLVED/NEEDS_INFO/PENDING, and the skip condition for a representative already in pending-resume state. - Step 5: wires the turn1 registry into each representative's dispatch (resume for PENDING, turn1_result for NEEDS_INFO); rewrites the "per cluster, not global" sibling-ordering guidance into an explicit rolling work-queue, distinguishing the Workflow path's true per-cluster streaming from the direct-Agent-tool-dispatch path's per-batch limit; adds an explicit warning that the direct-dispatch path has no code enforcing the Step 4b handoff, unlike the other two execution paths. - Step 6: calls cleanupBuildArtifacts only after triggerRcaReport succeeds. - Hard rules: carves out Step 4b's direct tfaRcaTurn call as the sole, narrow exception to "always dispatch via ai-tfa-coordinator". - API reference: documents lib/turn1-registry.mjs and lib/build-cleanup.mjs's exports (enforced by tests/wiring.test.mjs's doc-drift guard).
…ize Step 1 connector probes Step 4b was previously documented as running "concurrently with Step 4" via a same-batch trick, but a real run executed it sequentially anyway because the wording didn't specify which of Step 4's several batches to merge into. Rewrite it around the Agent tool's genuine fire-and-forget semantics instead: dispatch one lightweight subagent per cluster representative for turn 1, proceed to Step 4 in the very next turn without waiting, and handle each result as pure bookkeeping as notifications interleave with Step 4's own turns. Adds: - TURN1_OUTPUT fixed-shape contract so concurrent dispatches stay attributable - concurrency-capped dispatch (reusing config's existing `concurrency` value) - fail-open documentation for a subagent that never reports back - immediate sibling pre-dispatch when a representative resolves in one turn - explicit "narrate as one combined phase" guidance to stop the same bug recurring via mis-narration Also applies the same independent/parallel-batching fix to Step 1 Gate Part A's connector base probes and scope probes, which had the same ambiguous sequential-looking wording. Pressure-tested via superpowers:writing-skills (fresh subagents given only the wording, no other context): Step 1 and two of three Step 4b scenarios executed correctly end to end. One gap found — an agent could still insert an unnecessary Step-4b-only turn before Step 4's own tool calls began, since nothing said Step 4b's prep (registry init, resume skip-list) has zero dependency on Step 4. Closed with an explicit sentence permitting/encouraging both in the same first turn.
Pressure-tested every pre-today section of SKILL.md the same way as this
session's Step 4b/Step 1 rewrite (superpowers:writing-skills, fresh subagents,
verbatim section only). Step 3 (clustering path selection), Step 5
(rolling-refill work-queue, sibling guard), and Step 6 (cleanup-after-report
gating) all executed correctly across every branch tested.
One gap in Step 5: its own bullet only cross-referenced "resume/turn1_result
per agents/ai-tfa-coordinator.md" without restating which registry status maps
to which field. A fresh agent given only Step 5's text folded a NEEDS_INFO
result into `resume: true` instead of the documented `turn1_result:
{threadId, asks}` shape — the exact mapping only lived in Step 4b's own
closing paragraph, which didn't survive being read in isolation. Restate the
mapping explicitly in Step 5's own bullet so it doesn't depend on carrying
detail forward from an earlier section.
…tative rule Every "measured: X of Y (Z%)" / "A vs B" citation was a one-time measurement from a specific past run, not a guarantee about the next one — the digits added token cost and false precision without changing the instruction. Replace each with a plain statement of the failure mode and, where it matters, a brief "this has happened on a real run" flag. The rule and its consequence are the load-bearing part; the exact arithmetic was decoration.
…ce pre-fetch, and coordinator probes Adds <use_parallel_tool_calls> guidance (per Anthropic's parallel-tool-use docs) to SKILL.md and ai-tfa-coordinator.md, bulletproofs Step 1's gate-probe batching rule (a real run violated the existing rule anyway - 4+ min cost), adds the same batching contract to Step 4's per-repo/per-workload pre-fetch loops and to github-evidence.md's per-probe github hunt, and adds tables of contents to evidence-routing.md/github-evidence.md (both >100 lines) per the agent-skills best-practices doc.
A forensic pass on a real run (build hazbi3bs...) found four documented steps were skipped entirely, not just executed out of order: - Step 1A: only one of three present connector families was ever opened (a11y-regression-context, the one matching the build's actual failures, was never read) - adds a required enumerate-then-match check. - Step 1B: two separate AskUserQuestion calls instead of one, for fields the selected connector's own intake-defaults section already answered outright - adds a check-the-connector-first rule and bulletproofs "never a second question" with a STOP condition. - Step 1A scope probes: zero scope-probe calls ran despite the connector declaring seven - adds a required pre-Step-2 checkpoint. - Step 3: clusterAndPersist fallback ran without ever attempting the preferred getBuildFailureThemes call first - adds a required-order gate. - Step 5: fired the full fan-out with Step 4b (turn-1 pre-dispatch) never having run at all, so every cluster paid full coordinator cost with none of the one-turn resolve-and-skip fast path attempted - adds a required Step-4b-happened gate before Step 5's first dispatch.
The prior wording ("Step 4b must have already happened", "go back and do
Step 4b first") could be misread as waiting for Step 4b's subagents to
finish before Step 5 starts - reintroducing the sequential-latency bug Step
4b exists to remove. Reworded to be explicit: the gate only checks that the
dispatch batch was issued (fire-and-forget), same contract Step 4b already
documents, not that it completed.
…a stale variable Traced a real "test id mismatch -> falls back to client-side clustering" report to actual data: getBuildFailureThemes/listTestsInFailureTheme both returned correct, matching numeric testRunIds - no format bug. But the run's final CSV carried clusterAndPersist's c-xxxxx signature-hash IDs, not clustersFromThemes' theme-<id>/solo-<id> format, meaning the preferred path was silently abandoned. Root cause: an earlier listTestIds(status=failed) call had errored once and a later call used a different status filter in the same session - clustersFromThemes was fed whatever `rows` variable was still in scope, so every theme member came back unmatched (looks exactly like an ID mismatch, isn't one). Pins rows to readRows(csvPath) - the one row set guaranteed fresh and from a successful seed.
…not turnCap
Confirmed via real production logs (test-failure-agent-chat, namespace
testcasegeneration): a thread that gets "TFA agent run failed" twice in a
row is hitting the backend's openai.BadRequestError context_length_exceeded,
not a transient wedge - the thread's accumulated history exceeded the
model's context window, which a resubmit cannot fix since history doesn't
shrink. Rule 4b previously advised resubmitting all the way to turnCap,
wasting every remaining turn on a thread that structurally cannot recover.
Now: one retry for a first failure (still handles genuine transient wedges),
but two consecutive same-thread failures end the test PENDING immediately
with a new distinct note ("likely-context-exceeded"), added as a 4th
documented status alongside turn-cap/soft-pending/blocked.
…thread restart, not just a faster PENDING The prior fix (end PENDING after 2 consecutive same-thread wedges) only failed cleaner - it didn't get the test to an actual resolution, which is the real goal. Since the dead thread's oversized accumulated history is the only broken part (not the test itself), add a real recovery: on confirmed context_length_exceeded (2 consecutive same-thread failures), distill everything gathered so far into one condensed hypothesis message and submit it as turn 1 of a brand new thread - a single, narrow, justified exception to "never start a second thread" in step 4, since the first thread is provably dead rather than merely inconvenient. Bounded to exactly one restart per test; a restarted thread that also overflows twice ends PENDING for real rather than restarting indefinitely.
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.