chore(rca-build): prune + genericize the plugin for customer release - #7
Open
Dave3130 wants to merge 22 commits into
Open
chore(rca-build): prune + genericize the plugin for customer release#7Dave3130 wants to merge 22 commits into
Dave3130 wants to merge 22 commits into
Conversation
…ease Remove product/pod/skill names that shouldn't ship to customers: the a11y/tm/tra 'real run' war-story + tra/a11y-regression-context skill names (also trimmed to the generic rule), the railsApp/misc-services/ai-sdk-node repo example, and two o11y mentions in comments. No behavior change.
…980) Cut ~194 lines of verbosity from the per-run skill context with zero behavior change: collapse the parallel-tool-dispatch rule (stated 4x) to once, remove ~16 'a real run hit/measured/violated' justification clauses, and de-pad over-narrated gate blocks and Step 3/4/4b/5 prose. Every imperative rule, gate check, config value, function name, path, and code block is preserved (verified: clusterAndPersist first-move guard, gh pr list --json files MUST, no-second-gate-question, scope-probes gate, rows=readRows, and all step/section headings intact; code fences balanced).
Dave3130
changed the base branch from
feat/generic-rca-agent-plugin
to
feat/generic-rca-agent-plugin-v3
August 20, 2026 13:52
Remove ~150 lines of war-story justification and re-explanation from the per-run coordinator spec with zero behavior change: drop measured-run anecdotes (37% cache calls, 64KB, 104s, '0 PRs for a repo that had 21', etc.) and duplicate re-statements across Principle 0/4b/4c/5/9 and the loop. Also genericize the MCP-cache example 'grafana/VictoriaLogs' -> 'a log or metrics server' (infra-agnostic wording). Every operating principle (0-9), gate, RCA_OUTPUT schema field, tool/function name, path, and code block preserved.
Collapse Principle 4b-i's first bullet (re-explained 4b's resubmit/restart in full) to a one-line pointer, keeping the two-failure-mode contrast and the net-new turnId semantics. Drop the viewRca note — already stated in loop step 2 (capture glimpse + viewRca), the RCA_OUTPUT view_rca schema, and Principle 8 (pass glimpse verbatim). No behavior change.
…l/curl caching (970->581) Fix a real staleness bug and de-bloat the tool cache. Previously it memoized any read-only gh/kubectl/curl/git command with no expiry, so a later coordinator could be served a stale kubectl pod list / log sweep, and kubectl dedup duplicated the pre-fetch evidence file's job. Now cache ONLY provably-immutable reads (sha-pinned gh api /git/ or ?ref=<sha>; git show/cat-file/ls-tree/log with a sha); everything else (kubectl, curl, unpinned gh/git) runs pass-through UNCACHED (never refused). Delete the ~120-line no-shell parser (self-admitted security theater; caller has shell), dedup banner() into lib. Preserved: mutation refusal, stateful-MCP exclusion, 0600/0700 perms, atomic writes, secret redaction. Tests: 181/181 pass.
Plugin runs on the user's machine and must not delete/prune their local data. Remove pruneStateDir (7-day sweep) from state-dir.mjs and the orphaned deleteTurn1Registry; strip the Step 6 cleanup call, API-ref entries, and prune/cleanup narration from SKILL.md (base skill never had this — v3-added). hardenStateDir stays (perms only). Pairs with the build-cleanup.mjs removal in the previous commit. Tests: 181/181 pass.
…aseline Revert 1eba07f across all 5 files: turnMessageMaxChars 1000->5000, and the proportionally-shrunk digest caps back to base (SUMMARY 300/400, SNIPPET 20/40, code diff 1/3 hunks + context, whole-message 200/400 lines; evidence-block + coordinator/SKILL wording). The 1000-char self-limit was too tight and drove the message-size wedge/truncation problems later commits chased; 5000 is the tool's real cap. Tests: 181/181 pass.
Remove the console.warn guard added in 9d24c86 + its test. By the commit's own measurement it never fired (0/9 rows blank across 4 builds), it only prints to stderr (unread in an autonomous run, so it prevents nothing), and it sat in the hot csv-state core. The rule stays stated in the prompt where the agent acts on it. Tests: 180/180 pass.
The min(16, cores-2) Workflow-runtime cap was repeated ~6x (blockquote, default bullet twice, opt-in bullet, config comment, rca-batch comment) and pinned the exact external formula everywhere — fragile if the tool's cap changes, and disproportionate since it only affects the opt-in path (the default ignores it). Keep the formula in one authoritative place (Step 5 opt-in bullet + the rca-batch.mjs code comment); reduce the blockquote/default-bullet/config comment to reference it without restating. Tests: 180/180 pass.
…rmula The config comment wrongly claimed concurrency is 'honored literally by the sequential harness (lib/loop.mjs)' — loop.mjs runs one test at a time and ignores it. Restate accurately: concurrency is an advisory fan-out width honored on the default direct-dispatch path (orchestrator batches that many coordinator subagents in one message; host may run slightly fewer — matches the observed ~10-15 parallel); sequential harness ignores it; opt-in Workflow path is runtime-capped. Also drop the exact min(16, cores-2) formula everywhere (it's the Workflow tool's own machine- dependent limit, not fed from config.concurrency, and drift-prone). Tests: 180/180.
…stering Clustering is now solely the server's failure themes (getBuildFailureThemes GET, which triggers via POST + polls if none exist yet, then clustersFromThemes). When the server returns no themes, pass empty buildThemes so every failed test becomes its own singleton — i.e. all tests are representatives. Remove the client-side text-signature path entirely: clusterRows/clusterAndPersist/computeSignature/ normalize deleted from signature.mjs (keep selectRepresentative + siblingPreSeed); rewrite SKILL Step 3 + clustering.md to the single server source + all-reps fallback; drop the removed tests. Tests: 172/172 pass.
…..CodeEvidence 'GitHub' is a product name; 'code' is the capability (the forge could be GitLab, Gitea, a local clone). Rename the two evidence-file functions + all callers (coordinator, workflow, SKILL.md, tests) for genericity. No behavior change.
renderGlimpse just tallied resolved/pending/failed from the CSV and formatted one line — nothing to compute that warrants a module. Step 6 now instructs the agent to count terminal states inline. Delete the module + its test; update API-ref and wiring allowlist.
… (-61)
Cut war-story anecdotes ('a real run…', measured X-of-Y stats), absence/migration
framing ('there are no kubectl literals here…', 'we removed…'), and multi-sentence
justifications wrapped around one-line rules — across github-evidence.md,
evidence-routing.md, clustering.md, and residual spots in SKILL.md. Every rule,
command, gate check, size-cap/routing/field-filter table, and section heading
preserved (verified). These docs load into agent context on demand, so the trim
is a real per-run token saving. Tests: 164/164.
SKILL.md loads on every run; the ~75-line lib/bin API reference is only consulted mid-execution (Step 2+), never at gate time. Move it into references/api.md (progressive disclosure) and leave a lazy pointer in SKILL.md. Net -73 lines of per-run context. The wiring drift guard now scans SKILL.md + references/api.md so an undocumented export still fails the suite.
The principle half restated default model behavior; trim to the skill-specific fan-out examples + the dependency exception.
The gate's connector discovery keyed disambiguation on a product-prefix naming convention (<product>-github, product families <product-a>-*), which breaks for any customer with a flat set of arbitrarily-named skills. The real contract is already the 'capability:' declaration, not the name. Rekey the prose off declared capability: group candidates by capability, frame 'exactly one skill per capability' as the normal case, and disambiguate only when >1 skill claims the same capability (by build/failure match, else one gate question). Shared product:/scope: is an optional coherence hint, never required. Nudge no longer prescribes a skill name.
… a guess
When >1 skill claims the same capability and signatures can't disambiguate,
headless mode picked the first alphabetically — a coin-flip that commits
coordinators to confidently-wrong repo/branch routing (worse than no product
connector). This also contradicted the gate's own headless rule ('skip asking,
record the gaps — never guess').
Degrade the capability to the raw/generic connector and record the gap instead,
matching the zero-skills nudge and the Part B headless policy.
Part A connector discovery: revert the net-new (a1f0e96) Step 0 apparatus — explicit ls sweep, capability-declaration reading, SUPERSEDES block, naming note, and the multi-skill/families disambiguation + headless fallback — back to the base's lean 'enumerate + validate + gap' form. Assume a single product (one connector skill per capability); keep only a one-clause note that a present connector skill supersedes the raw tool. Fix the two dangling 'Step 0' refs left behind (they meant Part A). Step 3 clustering: the poll cadence (GET/POST/3s/90s) and the preferred-path rationale already live in references/clustering.md — drop the duplication from SKILL.md, keeping the operational call sequence + load-bearing rules (rows = readRows, writeRows back, verify cluster_id). SKILL.md 821 -> 747 lines.
references/clustering.md is loaded on-demand at Step 3, so the operational call sequence (getBuildFailureThemes -> listTestsInFailureTheme -> clustersFromThemes) and the invariants (rows=readRows, writeRows+verify) belong there, not in the always-loaded SKILL.md. Add a 'Running it (Step 3)' section to the reference and cut SKILL.md Step 3 to an orientation line + pointer + the one hard verify gate. SKILL.md 747 -> 731 lines.
The concurrency rule, the rolling-queue rule, and the path preference were each stated 3-4 times across a preamble, a 'Path-specific behavior' list, a blockquote, and the path bullets. Collapse to one intro (concurrency read from config, never hardcode; rolling-queue in one sentence) + a single preference-ordered dispatch-path list. Drop the hardcoded '(default 20)' — the value lives in config/rca.config.json. Keep the load-bearing readTurn1 mapping. Also fix a stale pointer: coordinator prompts pointed at 'SKILL.md § API reference', which moved to references/api.md. SKILL.md 731 -> 704 lines.
Dave3130
changed the title
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
1 participant
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.
What
A cleanup pass over the
rca-buildplugin on top of PR #2's latency/token work, to make it customer-shippable: leaner per-run context, genericized (no BrowserStack-internal references), and correct. No behavior the plugin relies on is dropped — this removes dead code, duplication, narration, and speculative machinery.Net +727 / −2352 (−1625 lines);
SKILL.md1185 → 704 lines.Highlights
Genericize for release
Leaner per-run context (progressive disclosure)
lib//bin/API reference intoreferences/api.md(loaded on-demand at Step 2+, not every run).references/clustering.md; SKILL.md keeps an orientation line + pointer + the one hard verify gate.(default 20)— the value lives inconfig/rca.config.json.ai-tfa-coordinator.md, and the reference docs.Correctness / safety
kubectl/curlcaching.Removed dead code
lib/glimpse.mjs,lib/build-cleanup.mjsand their tests; stale wiring-allowlist entries.setGithubEvidence/contributeGithubEvidence→…CodeEvidence.Verification
node --test→ 164 passing.Step 0/§ API reference/<product>references remain.