fix(skills): inject Skill into --tools when explicit tools= is set (#977)

When users pass an explicit tools=[...] list alongside skills="all",
the CLI's --tools argument did not include Skill, so the Skill tool
was authorized but never loaded. Centralizes the injection inside
_apply_skills_defaults via an effective_tools return value used by
_build_command. Idempotent, no-op when skills is unset.

Adds 5 regression tests. mypy + ruff clean. 744 passed.

Alternative approach to #979.

Closes #977.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: Zeffut

src/claude_agent_sdk/_internal/transport/subprocess_cli.py

@@ -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)
@@ -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")
 
         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."""
@@ -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)])
 

tests/test_transport.py

@@ -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"
+        # 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"),
         [