fix!: drop the coordinator's tools allowlist — it reduced it to Bash + Read - #4
Merged
Merged
Conversation
…+ Read
A live run's coordinators could not work: dispatched as
`tfa-rca:ai-tfa-coordinator`, they reported having only `Bash` and `Read` — no
`ToolSearch`, no `tfaRcaTurn`, no `getTfaTurnResult` — and terminated honestly rather
than fabricating a result. Two retries did the same.
I caused it. The frontmatter carried
tools: [Bash, Read, Grep, Glob, Task, mcp__*__tfaRcaTurn, mcp__*__getTfaTurnResult, mcp__github__*]
and that line had been INERT for months because the YAML block failed to parse. Fixing
the parse so `claude plugin validate` would pass activated an allowlist that had
therefore never once been exercised — and it is wrong three ways:
1. `mcp__*__tfaRcaTurn` does not match the real `mcp__plugin_tfa-rca_bstack__tfaRcaTurn`;
2. `Task`, `Grep` and `Glob` were not granted either;
3. `ToolSearch` is absent — and MCP tools are DEFERRED, so without it no schema can be
loaded and `tfaRcaTurn` is unreachable even when its name is permitted.
That is the lesson I should have drawn at the time: I noted in the validate fix that
`tools:` and `model:` "were being discarded at runtime" and treated restoring them as
pure upside, without asking whether a list nothing had ever enforced was correct.
Restoring an unexercised constraint is a behaviour change, not a repair.
**Removed rather than corrected**, because an allowlist cannot express what this agent
needs. The coordinator routes evidence to whatever the CUSTOMER has — a log store, a
cluster, a metrics surface, a forge — and those are unknowable when this file is
written. Inheriting the session's tools is not laxness here; it is the only thing
consistent with "generic over product and infra", and it is what worked for months.
`model: sonnet` is kept, but flagging it: it was dropped by the same parse failure, so
it is newly in force. Coordinators now run on Sonnet where they previously inherited the
session model. That is what the file asks for and it is a deliberate cost choice, but it
shipped unexamined alongside the tools change and is worth a conscious decision.
Guard added, mutation-proven against the exact old line, against a plausible "better"
allowlist, and against the frontmatter being un-indented back into unparseability — the
failure mode that hid all of this. The guard states why a list cannot work, so the next
person adding one has to argue with the reason rather than rediscover it.
340 tests, up from 339.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
(cherry picked from commit a3df342a4a8c42bfa4f9980ff02b3f56689751f8)
gaurav-singh-9227
approved these changes
Aug 31, 2026
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
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.
Production fix. Coordinators cannot run. A live RCA dispatched three of them and
each reported having only
BashandRead— noToolSearch, notfaRcaTurn, nogetTfaTurnResult— then terminated honestly rather than fabricating a result:Cause
agents/ai-tfa-coordinator.mdcarried:That line had been inert for months, because the YAML frontmatter did not parse — the
runtime dropped every field and derived the agent's name from the filename. An earlier PR
fixed the parse so
claude plugin validatewould pass, and in doing so activated anallowlist that had never once been exercised. It is wrong three ways:
mcp__*__tfaRcaTurndoes not match the realmcp__plugin_tfa-rca_bstack__tfaRcaTurn.Task,GrepandGlobwere not granted either.ToolSearchis missing — fatal on its own, because MCP tools are deferred andtheir schemas cannot be loaded without it. Even a correctly-named
tfaRcaTurnwouldbe unreachable.
Removed, not corrected
An allowlist cannot express what this agent needs. The coordinator routes evidence to
whatever the customer has — a log store, a cluster, a metrics surface, a forge —
and none of that is knowable when this file is written. Inheriting the session's tools
is not laxness here; it is the only thing consistent with "generic over product and
infra", and it is the behaviour that worked before the allowlist came into force.
Guard
tests/wiring.test.mjsnow fails if anytools:key returns to the frontmatter, andseparately if a continuation line drifts back to column 0 — the parse failure that hid
all of this. Mutation-proven against the exact old line, against a plausible "better"
list (
[Bash, Read, ToolSearch, Agent, mcp__*]), and against re-breaking the indent.The guard carries the reasoning, so the next person adding a list argues with the reason
rather than rediscovering this.
Still newly in force — worth a decision
model: sonnetwas dropped by the same parse failure, so it is now actually applying:coordinators run on Sonnet where they previously inherited the session model. That is
what the file asks for and it is a deliberate cost choice, but it shipped unexamined
alongside the tools change. Not touched here; flagging it.
Verification
claude plugin validate .passes plain and--strict; frontmatter parses, sodescriptionandmodelload rather than being silently dropped.--plugin-dir.🤖 Generated with Claude Code