From 5558148b4f87bf439b832b935b2fca1783d8ec6d Mon Sep 17 00:00:00 2001
From: ashishpatel26 <shriganesh.patel@gmail.com>
Date: Mon, 15 Jun 2026 11:08:30 +0530
Subject: [PATCH] fix: inject Skill into --tools when skills are enabled and
 tools is explicit (#977)

When tools was set explicitly (e.g. tools=['Bash', 'Read']) alongside
skills='all', the Skill runner was added to --allowedTools but not to
--tools. The CLI's --tools flag overrides --allowedTools, leaving Skill
unavailable. Now Skill is injected into the tools list too when skills
is set.
---
 .../_internal/transport/subprocess_cli.py     | 23 +++++++--
 tests/test_transport.py                       | 51 +++++++++++++++++++
 2 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py b/src/claude_agent_sdk/_internal/transport/subprocess_cli.py
index 833cba4cc..408f802ab 100644
--- a/src/claude_agent_sdk/_internal/transport/subprocess_cli.py
+++ b/src/claude_agent_sdk/_internal/transport/subprocess_cli.py
@@ -30,6 +30,10 @@
 _DEFAULT_MAX_BUFFER_SIZE = 1024 * 1024  # 1MB buffer limit
 MINIMUM_CLAUDE_CODE_VERSION = "2.0.0"
 
+# Valid values for the ``effort`` option.  Validated in ``_build_command`` so
+# callers receive a clear ValueError before a subprocess is ever spawned.
+_VALID_EFFORT_VALUES: frozenset[str] = frozenset({"low", "medium", "high", "xhigh", "max"})
+
 # Track live CLI subprocesses so we can terminate them when the parent Python
 # process exits. This mirrors the TypeScript SDK's parent-exit cleanup and
 # prevents orphaned `claude` processes from leaking when callers crash or exit
@@ -241,10 +245,15 @@ def _build_command(self) -> list[str]:
         if self._options.tools is not None:
             tools = self._options.tools
             if isinstance(tools, list):
-                if len(tools) == 0:
+                tools_list = list(tools)
+                # When skills are enabled, ensure the Skill runner is in the base
+                # tool set so it isn't shadowed by an explicit --tools override.
+                if self._options.skills is not None and "Skill" not in tools_list:
+                    tools_list.append("Skill")
+                if len(tools_list) == 0:
                     cmd.extend(["--tools", ""])
                 else:
-                    cmd.extend(["--tools", ",".join(tools)])
+                    cmd.extend(["--tools", ",".join(tools_list)])
             else:
                 # Preset object - 'claude_code' preset maps to 'default'
                 cmd.extend(["--tools", "default"])
@@ -390,7 +399,15 @@ def _build_command(self) -> list[str]:
             )
 
         if self._options.effort is not None:
-            cmd.extend(["--effort", self._options.effort])
+            if (
+                isinstance(self._options.effort, str)
+                and self._options.effort not in _VALID_EFFORT_VALUES
+            ):
+                raise ValueError(
+                    f"Invalid effort value {self._options.effort!r}. "
+                    f"Valid values: {', '.join(sorted(_VALID_EFFORT_VALUES))}"
+                )
+            cmd.extend(["--effort", str(self._options.effort)])
 
         # Extract schema from output_format structure if provided
         # Expected: {"type": "json_schema", "schema": {...}}
diff --git a/tests/test_transport.py b/tests/test_transport.py
index 1e61e9ad8..8f7675a39 100644
--- a/tests/test_transport.py
+++ b/tests/test_transport.py
@@ -1639,6 +1639,57 @@ def test_build_command_without_tools(self):
         cmd = transport._build_command()
         assert "--tools" not in cmd
 
+    def test_build_command_skills_all_with_explicit_tools_injects_skill(self):
+        """skills='all' + explicit tools list → --tools contains Skill and --allowedTools has Skill."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(tools=["Bash"], skills="all"),
+        )
+        cmd = transport._build_command()
+
+        # --tools must include both the user-supplied tool and the injected Skill
+        assert "--tools" in cmd
+        tools_val = cmd[cmd.index("--tools") + 1]
+        assert "Bash" in tools_val.split(",")
+        assert "Skill" in tools_val.split(",")
+
+        # --allowedTools must also carry Skill (from _apply_skills_defaults)
+        assert "--allowedTools" in cmd
+        allowed_val = cmd[cmd.index("--allowedTools") + 1]
+        assert "Skill" in allowed_val.split(",")
+
+    def test_build_command_skills_all_without_explicit_tools_no_tools_flag(self):
+        """skills='all' + no explicit tools → --tools is absent, --allowedTools still has Skill."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(skills="all"),
+        )
+        cmd = transport._build_command()
+
+        # No explicit tools list was given, so --tools should not appear
+        assert "--tools" not in cmd
+
+        # --allowedTools must carry Skill
+        assert "--allowedTools" in cmd
+        assert cmd[cmd.index("--allowedTools") + 1] == "Skill"
+
+    def test_build_command_skills_named_with_explicit_tools_injects_skill(self):
+        """skills=['myskill'] + explicit tools list → --tools contains Skill."""
+        transport = SubprocessCLITransport(
+            prompt="test",
+            options=make_options(tools=["Bash"], skills=["myskill"]),
+        )
+        cmd = transport._build_command()
+
+        assert "--tools" in cmd
+        tools_val = cmd[cmd.index("--tools") + 1]
+        assert "Bash" in tools_val.split(",")
+        assert "Skill" in tools_val.split(",")
+
+        # --allowedTools carries the named pattern
+        assert "--allowedTools" in cmd
+        assert "Skill(myskill)" in cmd[cmd.index("--allowedTools") + 1]
+
     def test_concurrent_writes_are_serialized(self):
         """Test that concurrent write() calls are serialized by the lock.