Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions src/claude_agent_sdk/_internal/transport/subprocess_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,24 @@ def _build_settings_value(self) -> str | None:

def _apply_skills_defaults(
self,
) -> tuple[list[str], list[str] | None]:
"""Compute effective allowed_tools and setting_sources for skills.
) -> tuple[list[str], list[str] | None, list[str] | None]:
"""Compute effective allowed_tools, setting_sources, and tools for skills.

When ``options.skills`` is ``"all"``, injects the bare ``Skill`` tool;
when it is a list, injects ``Skill(name)`` for each entry. In either
case ``setting_sources`` defaults to ``["user", "project"]`` when
unset so the CLI discovers installed skills without the caller having
to wire up both options manually. ``None`` is a no-op.

When the caller provides an explicit ``tools`` list (the base set of
tools the CLI is allowed to load), the bare ``Skill`` tool is also
appended to that list when missing. Without this, ``skills="all"``
would add ``Skill`` to ``allowed_tools`` but ``Skill`` would still be
absent from the base tool set, so the model could not invoke it
(issue #977). When ``tools`` is ``None`` or a preset object, no
injection into ``tools`` is needed because the default tool set
already includes ``Skill``.

Does not mutate the original options object.
"""
allowed_tools: list[str] = list(self._options.allowed_tools)
Expand All @@ -199,24 +208,38 @@ def _apply_skills_defaults(
if self._options.setting_sources is not None
else None
)
# Only mirror an explicit list-form ``tools`` option here. Preset
# objects and ``None`` are returned as ``None`` so ``_build_command``
# uses its existing path for them.
tools: list[str] | None = (
list(self._options.tools)
if isinstance(self._options.tools, list)
else None
)

skills = self._options.skills
if skills is None:
return allowed_tools, setting_sources
return allowed_tools, setting_sources, tools

if skills == "all":
if "Skill" not in allowed_tools:
allowed_tools.append("Skill")
if tools is not None and "Skill" not in tools:
tools.append("Skill")
else:
for name in skills:
pattern = f"Skill({name})"
if pattern not in allowed_tools:
allowed_tools.append(pattern)
# The CLI's --tools flag matches the bare tool name; allowed_tools
# supplies the per-skill filtering. Ensure Skill is loadable.
if tools is not None and "Skill" not in tools:
tools.append("Skill")

Comment on lines +227 to 238
if setting_sources is None:
setting_sources = ["user", "project"]

return allowed_tools, setting_sources
return allowed_tools, setting_sources, tools

def _build_command(self) -> list[str]:
"""Build CLI command with arguments."""
Expand All @@ -237,22 +260,22 @@ def _build_command(self) -> list[str]:
["--append-system-prompt", cast(SystemPromptPreset, sp)["append"]]
)

effective_allowed_tools, effective_setting_sources, effective_tools = (
self._apply_skills_defaults()
)

# Handle tools option (base set of tools)
if self._options.tools is not None:
tools = self._options.tools
if isinstance(tools, list):
if len(tools) == 0:
if effective_tools is not None:
# List-form tools, possibly with Skill injected by skills option.
if len(effective_tools) == 0:
cmd.extend(["--tools", ""])
else:
cmd.extend(["--tools", ",".join(tools)])
cmd.extend(["--tools", ",".join(effective_tools)])
else:
# Preset object - 'claude_code' preset maps to 'default'
cmd.extend(["--tools", "default"])

effective_allowed_tools, effective_setting_sources = (
self._apply_skills_defaults()
)

if effective_allowed_tools:
cmd.extend(["--allowedTools", ",".join(effective_allowed_tools)])

Expand Down
61 changes: 61 additions & 0 deletions tests/test_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,67 @@ def test_build_command_skills_does_not_duplicate_entries(self):
cmd = transport._build_command()
assert cmd[cmd.index("--allowedTools") + 1] == "Skill(pdf)"

def test_build_command_skills_all_injects_skill_into_explicit_tools(self):
"""Regression for #977: skills='all' alongside an explicit tools list must
add Skill to --tools so the model can actually invoke it."""
transport = SubprocessCLITransport(
prompt="test",
options=make_options(
tools=["WebSearch", "WebFetch", "Read"],
skills="all",
),
)
cmd = transport._build_command()
assert "--tools" in cmd
assert cmd[cmd.index("--tools") + 1] == "WebSearch,WebFetch,Read,Skill"
Comment on lines +641 to +642
# And Skill is in allowedTools too.
assert cmd[cmd.index("--allowedTools") + 1] == "Skill"

def test_build_command_skills_list_injects_skill_into_explicit_tools(self):
"""Named skills list must also add the bare Skill tool to --tools so the
per-name patterns in allowedTools can take effect."""
transport = SubprocessCLITransport(
prompt="test",
options=make_options(
tools=["Read"],
skills=["pdf"],
),
)
cmd = transport._build_command()
assert cmd[cmd.index("--tools") + 1] == "Read,Skill"
assert cmd[cmd.index("--allowedTools") + 1] == "Skill(pdf)"

def test_build_command_skills_tools_injection_is_idempotent(self):
"""If the caller already put Skill in tools, do not duplicate it."""
transport = SubprocessCLITransport(
prompt="test",
options=make_options(
tools=["Read", "Skill"],
skills="all",
),
)
cmd = transport._build_command()
assert cmd[cmd.index("--tools") + 1] == "Read,Skill"

def test_build_command_skills_none_does_not_inject_into_tools(self):
"""Without skills set, an explicit tools list must be passed through unchanged."""
transport = SubprocessCLITransport(
prompt="test",
options=make_options(tools=["Read"]),
)
cmd = transport._build_command()
assert cmd[cmd.index("--tools") + 1] == "Read"

def test_build_command_skills_tools_injection_does_not_mutate_options(self):
"""Tools injection must not leak back into the caller's options object."""
options = make_options(
tools=["Read"],
skills="all",
)
transport = SubprocessCLITransport(prompt="test", options=options)
transport._build_command()
assert options.tools == ["Read"]

@pytest.mark.parametrize(
("skills", "extra", "want_tools", "want_sources", "want_init_skills"),
[
Expand Down