subagent: say why we refuse an empty --tools, not what one would do
The comments and docs around the empty-`--tools` assert stated a mechanism: that an empty value parses as *unset* and therefore grants MORE built-ins than omitting the flag. That claim came from a measurement, and the installed `claude --help` (2.1.268) says the opposite in as many words — `Use "" to disable all tools`. One of the two is wrong and this repo is not the place that settles it. The rule the code enforces is right under both readings, which is precisely why it should not be argued from either: we never emit an empty `--tools`, because what an empty one means is release-dependent and nothing here wants a subagent with no built-in tools regardless. So the prose now states the rule and the ambiguity, and asserts nothing about `""` in either direction — replacing the claim with its opposite would be the same mistake sourced differently. No behaviour change: the assert, the resolution and every test are untouched. Two test comments also gained the limit they were quietly missing. `no_spawned_tool_escapes_the_session` hardcodes ten names, so it is a canary for tools someone thought to list, not a guarantee — the guarantee is the subset property, which covers tools nobody has invented yet. And `a_subagent_gets_no_builtin_its_parent_lacks` resolves both sides in one process off one env var, so it catches a code divergence but not the two real systemd units disagreeing about `HIVE_TOOL_GROUPS` — which is what they did until the previous commit, and is not a thing a unit test can reach. Refs #4416
This commit is contained in:
parent
4121ccf1f3
commit
9cd30a58ba
3 changed files with 50 additions and 28 deletions
|
|
@ -69,11 +69,15 @@ pub fn builtin_tools_for(groups: &[ToolGroup]) -> Vec<&'static str> {
|
|||
/// The value for claude's `--tools` flag: which built-in tools exist in this
|
||||
/// session at all.
|
||||
///
|
||||
/// ⚠️ **Never emit this as an empty string.** An empty `--tools` value parses
|
||||
/// as *unset* and yields **more** tools than omitting the flag, so there is
|
||||
/// no way to spell "no built-in tools" — an empty list is a fail-loudly bug,
|
||||
/// not a lockdown. [`ALLOWED_BUILTIN_TOOLS`] is non-empty, which is what
|
||||
/// keeps that from arising.
|
||||
/// ⚠️ **Never emit this as an empty string** — not because of what an empty
|
||||
/// value does, but because we don't know. Our own measurement and the
|
||||
/// installed `claude --help` disagree about whether `--tools ""` means "no
|
||||
/// tools" or parses as the flag being unset, and the answer is a property of
|
||||
/// whichever claude release is installed, not of this code. So an empty
|
||||
/// value is refused rather than relied on: the rule holds under either
|
||||
/// reading, and no caller has to know which one is true today.
|
||||
/// [`ALLOWED_BUILTIN_TOOLS`] is non-empty, which keeps the case from
|
||||
/// arising in the first place.
|
||||
///
|
||||
/// ⚠️ This governs built-ins only. `mcp__*` tools are not filtered by
|
||||
/// `--tools` at all; they are governed by `--strict-mcp-config` plus
|
||||
|
|
@ -439,8 +443,10 @@ mod tests {
|
|||
assert!(!without.contains(&"WebFetch") && !without.contains(&"WebSearch"));
|
||||
}
|
||||
|
||||
/// An empty `--tools` value parses as *unset* and grants more than
|
||||
/// omitting the flag, so no resolution may ever produce one.
|
||||
/// No resolution may produce an empty `--tools` value. What an empty one
|
||||
/// means is disputed and release-dependent (see [`builtin_tools_arg`]),
|
||||
/// so this pins the rule that makes the question moot rather than any
|
||||
/// answer to it.
|
||||
#[test]
|
||||
fn no_group_combination_resolves_to_an_empty_tools_arg() {
|
||||
assert!(!ALLOWED_BUILTIN_TOOLS.is_empty());
|
||||
|
|
|
|||
Loading…
Reference in a new issue