Commit 838667eca1d2435db3ad096e64f096138a701569
1 parent
916c7609
fix: Validator's disallowedTools was silently disabling Bash entirely
Found running U-014's full real closed loop: the Validator gave an
INCONCLUSIVE verdict and explicitly said it had no Bash tool, despite its
frontmatter granting one. Isolated with a minimal test subagent: any
"Bash(...)" scoped pattern (e.g. "Bash(git commit:*)") in a subagent's
disallowedTools frontmatter field disables Bash *entirely*, not just that
subcommand -- confirmed by comparing a disallowedTools=[Edit,Write] agent
(Bash works) against one adding a single Bash(git commit:*) entry (Bash
breaks completely). The CLI's own --disallowedTools flag documents this
exact scoped syntax as supported; subagent frontmatter just doesn't honor
it the same way, and nothing in the docs says so.
Fixed agent_profile.py's validator FRONTMATTER to disallowedTools=
[Edit, Write] only -- dropping the four Bash(git *) entries that were
silently breaking test execution. The "don't commit/push/reset/clean"
constraint now relies on the prose instruction already in the migrated
AGENTS.md instead of a tool-level block that never actually worked; fixed
that file's own (now-false) claim that it was enforced at the tool level,
and documented the finding centrally in 05-实施约束与依赖.md §5.4.1.2 since
it'll matter for any future agent definition, not just this one.
Verified for real, redeployed: Bash now works for the actual Validator
identity, Edit/Write remain correctly blocked.
1 test fixed (was asserting the now-removed scoped pattern) + 1 new test
(disallowedTools frontmatter must never contain "Bash(" again). 174 total
across U-001-019, all passing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
4 changed files
with
38 additions
and
10 deletions
agents/validator/AGENTS.md
| @@ -41,7 +41,7 @@ | @@ -41,7 +41,7 @@ | ||
| 41 | 41 | ||
| 42 | ## 安全与停止条件 | 42 | ## 安全与停止条件 |
| 43 | 43 | ||
| 44 | -- 不提交、推送、reset、清理或覆盖用户工作树(本 Agent 的 `disallowedTools` 已禁用 `Bash(git commit:*)`/`Bash(git push:*)`/`Bash(git reset:*)`/`Bash(git clean:*)`)。 | 44 | +- 不提交、推送、reset、清理或覆盖用户工作树(**2026-07-31 订正**:本来想在 `disallowedTools` 里用 `Bash(git commit:*)` 这类按子命令限定的写法禁用,但实测发现 Claude Code 的 subagent frontmatter 里只要出现任意 `Bash(...)` 限定项,`Bash` 工具会被整体禁用,不是只挡住那个子命令——代价是 Validator 完全跑不了任何测试,比放行 `git commit` 的风险大得多。已经去掉这个失效的限定写法,现在这条约束只靠这句话本身,不是工具层强制的)。 |
| 45 | - 不做生产写入、真实支付/消息、破坏性迁移或权限提升。 | 45 | - 不做生产写入、真实支付/消息、破坏性迁移或权限提升。 |
| 46 | - 不把命令失败、0 tests、skip/xfail、flaky、超时或缺失依赖解释成通过。 | 46 | - 不把命令失败、0 tests、skip/xfail、flaky、超时或缺失依赖解释成通过。 |
| 47 | - 无法可靠测试时写入真实阻塞证据(对应 `inconclusive_escalate` 分支,见 04-P),不猜测、不伪造成功。 | 47 | - 无法可靠测试时写入真实阻塞证据(对应 `inconclusive_escalate` 分支,见 04-P),不猜测、不伪造成功。 |
docs/sdd/05-实施约束与依赖.md
| @@ -39,6 +39,13 @@ | @@ -39,6 +39,13 @@ | ||
| 39 | - **应对方式**:需要"某个 Org Agent 身份 + 结构化输出"同时成立时(如 `c_work_graph_plan`,见 03-C 3.4 节/U-011),不要依赖 `--json-schema`/`structured_output`,改为在 prompt 里明确要求模型把结果包在单个 ```` ```json ```` 围栏代码块里,再从 `result` 字段用正则提取解析,对"整段就是 JSON"和"围栏代码块"两种情况都做兜底解析(参考实现:`src/work_graph_plan.py` 的 `_extract_graph_manifest`) | 39 | - **应对方式**:需要"某个 Org Agent 身份 + 结构化输出"同时成立时(如 `c_work_graph_plan`,见 03-C 3.4 节/U-011),不要依赖 `--json-schema`/`structured_output`,改为在 prompt 里明确要求模型把结果包在单个 ```` ```json ```` 围栏代码块里,再从 `result` 字段用正则提取解析,对"整段就是 JSON"和"围栏代码块"两种情况都做兜底解析(参考实现:`src/work_graph_plan.py` 的 `_extract_graph_manifest`) |
| 40 | - 未在 Anthropic 官方文档里找到这条限制的说明,是靠实测发现的,不排除后续版本行为会变——如果这里升级过 Claude Code 版本,值得重新验证一次 | 40 | - 未在 Anthropic 官方文档里找到这条限制的说明,是靠实测发现的,不排除后续版本行为会变——如果这里升级过 Claude Code 版本,值得重新验证一次 |
| 41 | 41 | ||
| 42 | +## 5.4.1.2 subagent frontmatter 的 `disallowedTools` 不支持按子命令限定(发现,2026-07-31,U-014 真实闭环跑出来的) | ||
| 43 | + | ||
| 44 | +- 实测确认:`.claude/agents/<name>.md` frontmatter 里的 `disallowedTools`,只要出现任意 `Bash(某子命令:*)` 这种按子命令限定的写法(如 `Bash(git commit:*)`),`Bash` 工具会被**整体**禁用,不是只挡住那一条子命令——即使同一份 frontmatter 的 `tools:` 字段里明确写了 `Bash`。用一个只有 `tools:[Read,Bash]`、`disallowedTools:[Edit,Write]`(不含任何 `Bash(...)`)的最小测试 agent 验证过 Bash 能正常跑;一旦给 `disallowedTools` 加一条 `Bash(git commit:*)`,同一个 agent 立刻连 `echo` 都跑不了 | ||
| 45 | +- `claude --help` 里 `--disallowedTools`/`--allowedTools` **CLI 参数**本身明确支持这种按子命令限定的写法(帮助文本示例就是 `Bash(git *) Edit`),但 subagent **frontmatter** 里的同名字段不支持同一种语法——是两个不同的解析路径,行为不一致,官方文档没有说明这个差异 | ||
| 46 | +- **实际后果**:U-004 迁移 Validator 人设时,原本想用 `disallowedTools: [Edit, Write, Bash(git commit:*), Bash(git push:*), Bash(git reset:*), Bash(git clean:*)]` 把"不准改代码"和"不准动 git 历史"都在工具层锁死,结果是 Validator 真实调用时完全没有 Bash 权限,无法运行任何测试——这个问题一直存在到 U-019 之后才通过跑 U-014 完整真实闭环时暴露出来(Validator 给出 INCONCLUSIVE 判定时明确说"没有 Bash 工具")。已订正为 `disallowedTools: [Edit, Write]`(不再限定 Bash 子命令),"不准动 git 历史"这条约束现在只靠 AGENTS.md 里的文字指令,不是工具层强制——一个跑不了任何测试的 Validator 比一个理论上可能跑 `git commit` 的 Validator 风险大得多,两权相害取其轻 | ||
| 47 | +- 同 §5.4.1.1:这条限制没在官方文档里找到说明,是实测发现的,不排除后续版本行为会变——升级 Claude Code 版本后值得重新验证 | ||
| 48 | + | ||
| 42 | ## 5.4.2 Claude Code plan 额度等待重试参数(架构文档 7.5 节决策,2026-07-29) | 49 | ## 5.4.2 Claude Code plan 额度等待重试参数(架构文档 7.5 节决策,2026-07-29) |
| 43 | 50 | ||
| 44 | - 退避轮询间隔(5→10→20→30 分钟,上限 30 分钟)、超时兜底阈值(session 8 小时、weekly 9 天)是 MVP 阶段的硬编码默认值,不在管理后台开放用户配置,和 `log_retention_config` 的"保留天数可配、清理频率不可配"是同一个 MVP 简化原则 | 51 | - 退避轮询间隔(5→10→20→30 分钟,上限 30 分钟)、超时兜底阈值(session 8 小时、weekly 9 天)是 MVP 阶段的硬编码默认值,不在管理后台开放用户配置,和 `log_retention_config` 的"保留天数可配、清理频率不可配"是同一个 MVP 简化原则 |
src/agent_profile.py
| @@ -73,14 +73,24 @@ FRONTMATTER = { | @@ -73,14 +73,24 @@ FRONTMATTER = { | ||
| 73 | "model": "claude-sonnet-5", | 73 | "model": "claude-sonnet-5", |
| 74 | "skills": ["qa-task-validation", "unattended-agent-operations"], | 74 | "skills": ["qa-task-validation", "unattended-agent-operations"], |
| 75 | "tools": ["Read", "Bash", "Grep", "Glob"], | 75 | "tools": ["Read", "Bash", "Grep", "Glob"], |
| 76 | - "disallowedTools": [ | ||
| 77 | - "Edit", | ||
| 78 | - "Write", | ||
| 79 | - "Bash(git commit:*)", | ||
| 80 | - "Bash(git push:*)", | ||
| 81 | - "Bash(git reset:*)", | ||
| 82 | - "Bash(git clean:*)", | ||
| 83 | - ], | 76 | + # 2026-07-31 fix, found for real running U-014's full closed loop: |
| 77 | + # scoped Bash patterns here ("Bash(git commit:*)") don't just block | ||
| 78 | + # that one subcommand -- they silently disable Bash *entirely* for | ||
| 79 | + # the agent (confirmed by isolating it: a plain ["Edit","Write"] | ||
| 80 | + # disallowedTools list leaves Bash working fine; adding any | ||
| 81 | + # "Bash(...)" entry breaks it, even though the CLI's own | ||
| 82 | + # --disallowedTools flag documents this exact scoped syntax as | ||
| 83 | + # supported -- it just doesn't work the same way in subagent | ||
| 84 | + # frontmatter). A Validator that can't run tests at all is a much | ||
| 85 | + # worse outcome than the residual risk of it running `git commit`, | ||
| 86 | + # so the git-mutation restriction now relies on the prose instruction | ||
| 87 | + # already in the migrated AGENTS.md ("不提交、推送、reset、清理或覆盖用户 | ||
| 88 | + # 工作树") instead of a tool-level block that doesn't actually work. | ||
| 89 | + # ponytail: this is a real gap (no enforced ceiling on Bash git | ||
| 90 | + # mutations, just an asked-nicely one) -- upgrade path is whatever | ||
| 91 | + # Claude Code's correct scoped-disallowedTools frontmatter syntax | ||
| 92 | + # turns out to be, if this gets fixed/documented upstream. | ||
| 93 | + "disallowedTools": ["Edit", "Write"], | ||
| 84 | }, | 94 | }, |
| 85 | } | 95 | } |
| 86 | 96 |
src/test_agent_profile.py
| @@ -29,7 +29,18 @@ class RenderAgentDefinitionTest(unittest.TestCase): | @@ -29,7 +29,18 @@ class RenderAgentDefinitionTest(unittest.TestCase): | ||
| 29 | self.assertIn("disallowedTools:", content) | 29 | self.assertIn("disallowedTools:", content) |
| 30 | self.assertIn("- Edit", content) | 30 | self.assertIn("- Edit", content) |
| 31 | self.assertIn("- Write", content) | 31 | self.assertIn("- Write", content) |
| 32 | - self.assertIn("Bash(git commit:*)", content) | 32 | + |
| 33 | + def test_validator_disallowed_tools_has_no_scoped_bash_pattern(self): | ||
| 34 | + # 2026-07-31: confirmed for real that any "Bash(...)" entry in a | ||
| 35 | + # subagent's disallowedTools silently disables Bash *entirely*, not | ||
| 36 | + # just that pattern -- must never reappear in the actual frontmatter | ||
| 37 | + # (see agent_profile.py's FRONTMATTER["validator"] comment for the | ||
| 38 | + # full story; the migrated AGENTS.md body is allowed to *mention* | ||
| 39 | + # "Bash(...)" in prose explaining why it's no longer used there). | ||
| 40 | + content = agent_profile.render_agent_definition("validator") | ||
| 41 | + frontmatter = content.split("---", 2)[1] | ||
| 42 | + self.assertNotIn("Bash(", frontmatter) | ||
| 43 | + self.assertIn("- Bash", frontmatter) | ||
| 33 | 44 | ||
| 34 | def test_planner_and_coder_have_no_disallowed_tools(self): | 45 | def test_planner_and_coder_have_no_disallowed_tools(self): |
| 35 | for org_node in ("planner", "coder"): | 46 | for org_node in ("planner", "coder"): |