Repository navigation
Conversation
_partition_tools treated every NativeTool in tool_handlers as offered, so an empty or missing AI Config tool list still turned on Bash, Read, and the rest. User-defined tools were already gated on the variation's keys. A key the variation does not declare is not offered. Co-authored-by: alexis <alexis@launchdarkly.com>
Co-authored-by: alexis <alexis@launchdarkly.com>
jeffdupont
left a comment
There was a problem hiding this comment.
The fix is right. A built-in now reaches tools only when the variation attaches its key, for create_claude_agents_handler() and for every node to_claude_agents() runs. This matches partitionTools in launchdarkly/js-ai-sdk's claude-agents handler. I checked each variation shape: no tools, an empty tool map, only user-defined tools, some or all of the registered built-ins, and a key with no handler. All of them come out right, and Skill is still added when the variation has skills. _native_tool_aliases is still unfiltered, but ToolCatalog only walks config.tools, so a built-in the variation doesn't attach never reaches the span catalog. The claude-agents tests pass locally on 12b148c (304 passed) and ruff is clean.
1. Blocking: the README still describes the old behaviour (packages/claude-agents/README.md:189 and the example under it). It says to "place them as values in tool_handlers to enable the corresponding native Claude tool", and the example registers {"web-search": ClaudeWebSearch} without saying the variation must attach web-search. Anyone who follows it after this change gets no WebSearch. Fix: say that the built-in is enabled only when the variation's tools include that key, and note it next to the example. I couldn't leave this inline because the README isn't in the diff.
2. Non-blocking: call out the behaviour change in the release notes. Today, registering a NativeTool enables that built-in on every variation. After this change, it's enabled only on variations that attach it. Anyone who relied on the old behaviour loses the tool with no warning, and a plain fix: commit puts that in the changelog as a quiet patch. Fix: add a sentence to the squash commit body, e.g. "Registering a NativeTool no longer enables it by itself; the variation must attach its key." Use fix! if you want it to count as breaking.
One inline comment on a missing test case.
| config, | ||
| handlers, | ||
| ) | ||
| assert omitted.tools == [] |
There was a problem hiding this comment.
Non-blocking: these cases cover all-attached and none-attached, but not the one this fix is for: some registered built-ins attached and others not. Please add a variation that attaches only read, keep both read and bash in handlers, and assert opts.tools == ["Read"]. That's the test that catches a regression back to enabling everything registered.
_partition_toolsenabled everyNativeToolintool_handlers, so a variation with a missing or empty tool list still got Bash, Read, and the rest of the registered built-ins. User-defined tools were already limited to keys on the variation.A key the variation does not declare is no longer offered.
tools=[]is still sent when nothing is attached, which is what turns the Claude Agent SDK's default built-in set off. The skills handler still addsSkillwhen the variation names a skill.OpenCode and the JS claude-agents handler already did this. Native graph goes through the same partition, so it follows the same rule.