From 8404cc9796ed4bcc430d7af71ad80ec0935826c8 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Thu, 30 Jul 2026 14:13:14 -0400 Subject: [PATCH 1/2] feat(claude-agent-sdk): honor working_dir via ClaudeAgentOptions.cwd (#348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider declared `working_dir=False`, so `conductor validate` rejected any workflow setting `agent.working_dir` or `runtime.working_dir` against it. The declaration was accurate but left the provider out of step with `copilot` and `claude`, both of which honor the engine-resolved directory. `execute()` now forwards the resolved directory as `ClaudeAgentOptions.cwd`, which the SDK applies as the `claude` subprocess's cwd. Stdio MCP servers pick it up by inheriting from that subprocess, so there is no per-server stamping as in `copilot.py::_mcp_servers_for_cwd` — the SDK's `McpStdioServerConfig` has no cwd field, leaving `_translate_mcp_servers` untouched. The path is passed verbatim: `WorkflowEngine._resolve_agent_working_dir` has already rendered, absolutized, normalised, and existence-checked it, and re-resolving would collapse symlink aliases the engine preserves on purpose. No provider-side `is_dir()` guard either — a directory that disappears after that check surfaces as the SDK's own `CLIConnectionError`, which the existing `except Exception` already wraps in `ProviderError`. Note that cwd also selects which `CLAUDE.md` and local settings the CLI loads, and is the SDK's session-store project key. The unconditional `strict_mcp_config=True` still prevents a `.mcp.json` in that directory from injecting undeclared servers. Tests use the real `ClaudeAgentOptions` rather than a Mock so a renamed or removed SDK field fails loudly instead of passing silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 1 + CHANGELOG.md | 15 +++ docs/providers/experimental.md | 4 +- src/conductor/providers/claude_agent_sdk.py | 18 ++- tests/test_providers/test_capabilities.py | 8 +- tests/test_providers/test_claude_agent_sdk.py | 108 ++++++++++++++++++ 6 files changed, 142 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7f61dfc2..d0bdc7b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -270,6 +270,7 @@ Conductor: - A narrowing per-server `tools:` filter (anything other than the default `["*"]`) is **refused**, not ignored: forwarding the server unfiltered would grant more tools than declared, the same security regression that justifies refusing the per-agent allowlist. A dropped `timeout` only warns, since losing it cannot widen tool access. - **Tool execution**: Per-agent `tools:` allowlists remain unsupported (`workflow_tools_passthrough=False`). The provider refuses any non-empty per-agent list because workflow tool names do not translate to CLI tool IDs. Note the SDK's `tools` option governs **built-in** tools only, and `allowed_tools` is a permission auto-approve list rather than an availability filter — so honoring an allowlist would require a permission-mode redesign, not just a name mapping. An agent with `tools: []` runs with no built-in tools (MCP servers still attach); omitting `tools:` grants the full `claude_code` preset. - **Runtime config**: `temperature` and `max_tokens` are rejected at the factory — the CLI controls sampling behavior. +- **Working directory** (issue #348): the engine-resolved `agent.working_dir` / `runtime.working_dir` **is** forwarded, as `ClaudeAgentOptions.cwd`. The SDK applies it as the `claude` subprocess's cwd (`_internal/transport/subprocess_cli.py` passes it to `open_process` and sets `PWD`), so stdio MCP servers pick it up by **inheriting** it from that subprocess. There is deliberately no per-server stamping as in `copilot.py::_mcp_servers_for_cwd`: the SDK's `McpStdioServerConfig` has no cwd field, so `_translate_mcp_servers` is left alone. The provider passes the path verbatim — `WorkflowEngine._resolve_agent_working_dir` has already rendered, absolutized, normalised, and existence-checked it, and re-resolving here would collapse the symlink aliases the engine preserves on purpose. No provider-side `is_dir()` guard either: a directory that disappears after that check surfaces as the SDK's own `CLIConnectionError("Working directory does not exist")`, which `execute`'s existing `except Exception` already wraps in `ProviderError`. Two knock-on effects worth knowing: cwd also selects which `CLAUDE.md` and local settings the CLI loads, and it is the SDK's session-store project key (`_internal/session_resume.py`). The unconditional `strict_mcp_config=True` still stops a `.mcp.json` in that directory from injecting undeclared servers. `add_dirs` (the SDK's `--add-dir` passthrough) is a separate axis Conductor does not set. #### `aca.py` parity notes diff --git a/CHANGELOG.md b/CHANGELOG.md index e7c91a19..771fd272 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`claude-agent-sdk` provider now honors `working_dir`** — the directory + resolved from `agent.working_dir` / `runtime.working_dir` is forwarded to + `ClaudeAgentOptions.cwd`, so the `claude` CLI runs there and every stdio MCP + server it spawns inherits the same directory. Previously the provider + declared `working_dir=False` and `conductor validate` rejected any workflow + that set it, which was accurate but left the provider out of step with + `copilot` and `claude`. There is no per-server stamping as there is for + Copilot, because the SDK's stdio server config has no working-directory + field — inheritance from the CLI subprocess covers it. A missing directory + still fails before the provider is reached, and `strict_mcp_config` remains + enabled so a `.mcp.json` sitting in the new directory cannot inject + undeclared servers. See + [`docs/mcp-tools.md`](docs/mcp-tools.md). + ([#348](https://github.com/microsoft/conductor/issues/348)) + - **`claude-agent-sdk` provider now supports MCP servers** — workflow-level `runtime.mcp_servers` are translated to the SDK's own `stdio` / `http` / `sse` config shapes and passed through `ClaudeAgentOptions`, so an agent can diff --git a/docs/providers/experimental.md b/docs/providers/experimental.md index c2171e8a..55f124bb 100644 --- a/docs/providers/experimental.md +++ b/docs/providers/experimental.md @@ -25,7 +25,7 @@ prints a one-time banner per provider: │ (claude-agent-sdk>=0.2.82) maintained by @lesandiz (best-effort) │ │ Limitations: no per-agent tools allowlist, reasoning_effort │ │ ignored, structured output via prompt injection, no checkpoint │ -│ resume, working_dir ignored. │ +│ resume. │ │ See docs/providers/experimental.md for stability policy. │ └─────────────────────────────────────────────────────────────────────┘ ``` @@ -99,7 +99,7 @@ adopting one does not inflate the install surface for others. | Provider | Upstream pin | Maintainer | Capability carve-outs | |---|---|---|---| -| `claude-agent-sdk` | `claude-agent-sdk>=0.2.82` | `@lesandiz (best-effort)` | no `workflow_tools_passthrough`, no `reasoning_effort`, `prompt_injection` structured output, no `checkpoint_resume`, no `working_dir`. Supports `mcp_tools` as of [#335](https://github.com/microsoft/conductor/issues/335), except that a narrowing per-server `tools:` filter is refused (no SDK equivalent). | +| `claude-agent-sdk` | `claude-agent-sdk>=0.2.82` | `@lesandiz (best-effort)` | no `workflow_tools_passthrough`, no `reasoning_effort`, `prompt_injection` structured output, no `checkpoint_resume`. Supports `mcp_tools` as of [#335](https://github.com/microsoft/conductor/issues/335), except that a narrowing per-server `tools:` filter is refused (no SDK equivalent). Supports `working_dir` as of [#348](https://github.com/microsoft/conductor/issues/348). | | `hermes` | `hermes-agent` | `(community contribution)` | no `mcp_tools`, `prompt_injection` structured output, no `working_dir` | | `aca` | `azure-identity>=1.19.0` | `(unassigned)` | no `workflow_tools_passthrough` (the wrapped in-container `CopilotProvider` never applies the `tools:` allowlist to the SDK session), no `working_dir` (only the separate, container-relative `sandbox.working_dir` is honored — not the generic host-resolved field), `prompt_injection` structured output (inherits the inner Copilot provider), no `checkpoint_resume` (ephemeral sandbox sessions, no volume mount). Declares `interrupt`/`max_session_seconds` as `True`, but the shipped runner MVP doesn't fully back either yet — see [Known Gaps](./aca.md#known-gaps-runner-mvp). | diff --git a/src/conductor/providers/claude_agent_sdk.py b/src/conductor/providers/claude_agent_sdk.py index c73bd221..f0857a19 100644 --- a/src/conductor/providers/claude_agent_sdk.py +++ b/src/conductor/providers/claude_agent_sdk.py @@ -329,12 +329,12 @@ class ClaudeAgentSdkProvider(AgentProvider): # No global mutable state shared across calls — the SDK spawns # an independent subprocess per query() invocation. concurrent_safe=True, - # The provider never forwards the engine-resolved working directory - # to ``ClaudeAgentOptions.cwd``, so an agent's ``working_dir`` would - # be silently ignored (both for the CLI itself and for any stdio MCP - # servers it spawns). Declared False so ``conductor validate`` errors - # instead of lying about where the agent runs. - working_dir=False, + # The engine-resolved working directory is forwarded to + # ``ClaudeAgentOptions.cwd``, which the SDK applies as the ``claude`` + # subprocess's cwd. Stdio MCP servers inherit it from that subprocess + # rather than being stamped individually as they are for Copilot: + # the SDK's ``McpStdioServerConfig`` has no cwd field. + working_dir=True, # Skill content is eagerly injected into the rendered prompt by # AgentExecutor (the claude-agent-sdk surfaces no # ``skill_directories`` kwarg today; once it does we can flip @@ -417,6 +417,12 @@ async def execute( options = ClaudeAgentOptions( model=model, system_prompt=agent.system_prompt, + # The engine resolves ``agent.working_dir`` (Jinja render, + # absolutize, is_dir check) before dispatching here, so pass it + # through verbatim; ``None`` keeps the process-cwd behavior. The + # CLI subprocess starts in this directory and every stdio MCP + # server it spawns inherits it. + cwd=agent.working_dir or os.getcwd(), output_format=_build_output_format(agent.output) if agent.output else None, max_turns=max_turns, permission_mode=permission_mode, diff --git a/tests/test_providers/test_capabilities.py b/tests/test_providers/test_capabilities.py index af613139..34a5ad31 100644 --- a/tests/test_providers/test_capabilities.py +++ b/tests/test_providers/test_capabilities.py @@ -206,13 +206,13 @@ def test_every_production_provider_has_capabilities(self, provider_name: str) -> @pytest.mark.parametrize( ("provider_name", "expected"), [ - # Requirement: copilot and claude honor agent/runtime ``working_dir`` - # for the SDK session and its MCP servers; hermes and - # claude-agent-sdk do not (declared False so validate errors out). + # Requirement: copilot, claude, and claude-agent-sdk honor + # agent/runtime ``working_dir`` for the SDK session and its MCP + # servers; hermes does not (declared False so validate errors out). ("copilot", True), ("claude", True), ("hermes", False), - ("claude-agent-sdk", False), + ("claude-agent-sdk", True), ], ) def test_working_dir_capability_matrix(self, provider_name: str, expected: bool) -> None: diff --git a/tests/test_providers/test_claude_agent_sdk.py b/tests/test_providers/test_claude_agent_sdk.py index 1ac71183..c7fe449d 100644 --- a/tests/test_providers/test_claude_agent_sdk.py +++ b/tests/test_providers/test_claude_agent_sdk.py @@ -2087,3 +2087,111 @@ def test_config_errors_are_not_retryable(self) -> None: {"timeout-probe": {"type": "stdio", "command": "d", "tools": ["connection"]}} ) assert exc.value.is_retryable is False + + +class TestWorkingDirectory: + """The engine-resolved ``working_dir`` must reach ``ClaudeAgentOptions.cwd``. + + The SDK applies ``cwd`` to the ``claude`` subprocess it spawns + (``_internal/transport/subprocess_cli.py``), so it governs both where the + CLI runs and where the stdio MCP servers it spawns inherit their cwd from. + These tests use the real ``ClaudeAgentOptions`` rather than a Mock so a + renamed or removed SDK field fails here instead of silently passing. + """ + + @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) + async def test_resolved_working_dir_becomes_cwd(self, tmp_path: Path) -> None: + captured: dict = {} + + async def fake_query(**kwargs): + captured["cwd"] = kwargs["options"].cwd + yield _result(result="ok") + + with patch("conductor.providers.claude_agent_sdk.query", fake_query): + provider = ClaudeAgentSdkProvider() + await provider.execute( + agent=AgentDef(name="t", prompt="hi", working_dir=str(tmp_path)), + context={}, + rendered_prompt="hi", + ) + + assert captured["cwd"] == str(tmp_path) + + @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) + async def test_unset_working_dir_falls_back_to_process_cwd(self) -> None: + """No ``working_dir`` keeps the legacy process-cwd behavior.""" + captured: dict = {} + + async def fake_query(**kwargs): + captured["cwd"] = kwargs["options"].cwd + yield _result(result="ok") + + with patch("conductor.providers.claude_agent_sdk.query", fake_query): + provider = ClaudeAgentSdkProvider() + await provider.execute( + agent=AgentDef(name="t", prompt="hi"), context={}, rendered_prompt="hi" + ) + + assert captured["cwd"] == os.getcwd() + + @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) + async def test_working_dir_passed_verbatim(self, tmp_path: Path) -> None: + """The engine already renders, absolutizes, and existence-checks the + path (``WorkflowEngine._resolve_agent_working_dir``). The provider must + not re-resolve it -- ``resolve()`` here would collapse symlink aliases + the engine deliberately preserves.""" + real = tmp_path / "real" + real.mkdir() + link = tmp_path / "link" + link.symlink_to(real, target_is_directory=True) + captured: dict = {} + + async def fake_query(**kwargs): + captured["cwd"] = kwargs["options"].cwd + yield _result(result="ok") + + with patch("conductor.providers.claude_agent_sdk.query", fake_query): + provider = ClaudeAgentSdkProvider() + await provider.execute( + agent=AgentDef(name="t", prompt="hi", working_dir=str(link)), + context={}, + rendered_prompt="hi", + ) + + assert captured["cwd"] == str(link) + + @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) + async def test_working_dir_composes_with_mcp_servers(self, tmp_path: Path) -> None: + """``cwd`` and the MCP config path are independent: stdio servers pick + the directory up by inheriting it from the CLI subprocess, so nothing + is stamped onto the per-server config (``McpStdioServerConfig`` has no + cwd field).""" + captured: dict = {} + + async def fake_query(**kwargs): + options = kwargs["options"] + captured["cwd"] = options.cwd + captured["payload"] = json.loads(Path(options.mcp_servers).read_text()) + yield _result(result="ok") + + with patch("conductor.providers.claude_agent_sdk.query", fake_query): + provider = ClaudeAgentSdkProvider( + mcp_servers={"docs": {"type": "stdio", "command": "docs-server"}} + ) + await provider.execute( + agent=AgentDef(name="t", prompt="hi", working_dir=str(tmp_path)), + context={}, + rendered_prompt="hi", + ) + + assert captured["cwd"] == str(tmp_path) + assert captured["payload"] == { + "mcpServers": {"docs": {"type": "stdio", "command": "docs-server"}} + } + + def test_capability_declares_working_dir_support(self) -> None: + """The descriptor is a contract -- it must match the wiring above.""" + assert ClaudeAgentSdkProvider.CAPABILITIES.working_dir is True + assert ( + "working_dir ignored" not in ClaudeAgentSdkProvider.CAPABILITIES.declared_limitations() + ) From 296e506a0c9bd642ffb2bad5e2f619d599f36358 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Thu, 30 Jul 2026 16:19:53 -0400 Subject: [PATCH 2/2] fix(claude-agent-sdk): make working_dir failures actionable (#348) Review follow-ups on the working_dir wiring. `os.getcwd()` was evaluated outside `execute()`'s try, so a deleted process cwd escaped as a bare `FileNotFoundError` with no path and no suggestion, breaking the method's `ProviderError` contract. It now resolves through a dedicated handler that names the agent, the subsystem, and two remedies, rather than falling through to the generic "check the CLI is installed" arm. The SDK reuses `CLIConnectionError` for failures to *spawn* the CLI, so a missing working directory, a path that is a file (ENOTDIR), and an unreadable one (EACCES) were all reported as connection problems -- "check the binary is executable and that no firewall is blocking" -- and all marked retryable even though none can succeed on a second attempt. `_classify_startup_failure` now distinguishes them, shared by both classifiers so detection lives in one place. Genuine connection drops keep the old advice and stay retryable. The ENOTDIR/EACCES hint names both possible causes because the errno text does not say whether the offending path is the working directory or the binary. Skipping the provider-side `is_dir()` guard is only defensible if the wrapped error is actionable, so this is a prerequisite of that decision rather than a separate improvement. Also pins the capability flip where it is user-visible: nothing asserted that `conductor validate` now accepts these workflows. The real-descriptor cross-check covers both the per-agent and workflow-level validator branches, which are separate code paths. Documents that the CLI loads `CLAUDE.md` and `.claude/settings*.json` from its working directory, so pointing an agent at an untrusted checkout runs that checkout's instructions -- `strict_mcp_config` covers MCP servers but not hooks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 6 +- CHANGELOG.md | 10 +- docs/providers/experimental.md | 2 +- docs/workflow-syntax.md | 1 + src/conductor/providers/claude_agent_sdk.py | 78 ++++++++++-- .../test_validator_capabilities.py | 23 +++- tests/test_providers/test_capabilities.py | 3 +- tests/test_providers/test_claude_agent_sdk.py | 120 +++++++++++++++++- 8 files changed, 220 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d0bdc7b3..2506cd46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -270,7 +270,11 @@ Conductor: - A narrowing per-server `tools:` filter (anything other than the default `["*"]`) is **refused**, not ignored: forwarding the server unfiltered would grant more tools than declared, the same security regression that justifies refusing the per-agent allowlist. A dropped `timeout` only warns, since losing it cannot widen tool access. - **Tool execution**: Per-agent `tools:` allowlists remain unsupported (`workflow_tools_passthrough=False`). The provider refuses any non-empty per-agent list because workflow tool names do not translate to CLI tool IDs. Note the SDK's `tools` option governs **built-in** tools only, and `allowed_tools` is a permission auto-approve list rather than an availability filter — so honoring an allowlist would require a permission-mode redesign, not just a name mapping. An agent with `tools: []` runs with no built-in tools (MCP servers still attach); omitting `tools:` grants the full `claude_code` preset. - **Runtime config**: `temperature` and `max_tokens` are rejected at the factory — the CLI controls sampling behavior. -- **Working directory** (issue #348): the engine-resolved `agent.working_dir` / `runtime.working_dir` **is** forwarded, as `ClaudeAgentOptions.cwd`. The SDK applies it as the `claude` subprocess's cwd (`_internal/transport/subprocess_cli.py` passes it to `open_process` and sets `PWD`), so stdio MCP servers pick it up by **inheriting** it from that subprocess. There is deliberately no per-server stamping as in `copilot.py::_mcp_servers_for_cwd`: the SDK's `McpStdioServerConfig` has no cwd field, so `_translate_mcp_servers` is left alone. The provider passes the path verbatim — `WorkflowEngine._resolve_agent_working_dir` has already rendered, absolutized, normalised, and existence-checked it, and re-resolving here would collapse the symlink aliases the engine preserves on purpose. No provider-side `is_dir()` guard either: a directory that disappears after that check surfaces as the SDK's own `CLIConnectionError("Working directory does not exist")`, which `execute`'s existing `except Exception` already wraps in `ProviderError`. Two knock-on effects worth knowing: cwd also selects which `CLAUDE.md` and local settings the CLI loads, and it is the SDK's session-store project key (`_internal/session_resume.py`). The unconditional `strict_mcp_config=True` still stops a `.mcp.json` in that directory from injecting undeclared servers. `add_dirs` (the SDK's `--add-dir` passthrough) is a separate axis Conductor does not set. +- **Working directory** (issue #348): the engine-resolved `agent.working_dir` / `runtime.working_dir` **is** forwarded, as `ClaudeAgentOptions.cwd`. + - The SDK applies it as the `claude` subprocess's cwd (`_internal/transport/subprocess_cli.py` as of 0.2.87 passes it to `open_process` and sets `PWD`), so stdio MCP servers pick it up by **inheriting** it from that subprocess. There is deliberately no per-server stamping as in `copilot.py::_mcp_servers_for_cwd`: the SDK's `McpStdioServerConfig` has no cwd field, so `_translate_mcp_servers` is left alone. Inheritance is a property of the CLI binary, not of the SDK, so it is documented rather than asserted by a test. + - The path is passed **verbatim** — `WorkflowEngine._resolve_agent_working_dir` has already rendered, absolutized, normalised, and existence-checked it, and re-resolving here would collapse the symlink aliases the engine preserves on purpose. The `ClaudeAgentOptions(...)` construction lives **inside** `execute`'s `try` so the `os.getcwd()` fallback can't escape as a bare `OSError` when the process cwd has been deleted (`copilot.py` resolves its cwd inside its try for the same reason). + - There is no provider-side `is_dir()` guard: a directory that vanishes after the engine's check surfaces as the SDK's `CLIConnectionError("Working directory does not exist: ")`, wrapped in `ProviderError`. That is only defensible because `_classify_startup_failure` special-cases it — `CLIConnectionError` otherwise yields firewall/binary advice and `is_retryable=True`, which is wrong for all three launch failures (missing dir; path is a file → `ENOTDIR`; unreadable dir → `EACCES`; the latter two reach the SDK's generic "Failed to start Claude Code" arm, not its dedicated one). + - Knock-on effects: cwd selects which `CLAUDE.md` and local settings the CLI loads (Conductor never sets `setting_sources`, so the CLI's load-everything default applies) and is the project key for the CLI's on-disk transcript directory. The unconditional `strict_mcp_config=True` still stops a `.mcp.json` in that directory from injecting undeclared servers, but it does **not** cover hooks or instructions — point `working_dir` only at trees you trust. `add_dirs` (the SDK's `--add-dir` passthrough) is a separate axis Conductor does not set. #### `aca.py` parity notes diff --git a/CHANGELOG.md b/CHANGELOG.md index 771fd272..685be147 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,8 +20,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 field — inheritance from the CLI subprocess covers it. A missing directory still fails before the provider is reached, and `strict_mcp_config` remains enabled so a `.mcp.json` sitting in the new directory cannot inject - undeclared servers. See - [`docs/mcp-tools.md`](docs/mcp-tools.md). + undeclared servers. Note that the `claude` CLI also reads `CLAUDE.md` and + `.claude/settings*.json` from its working directory, so pointing an agent at + an untrusted checkout means running that checkout's instructions and hooks. + Launch failures caused by a bad working directory are now reported as such + rather than as connection problems, and are no longer treated as retryable. + See + [`docs/workflow-syntax.md`](docs/workflow-syntax.md#working-directory) and + [`docs/providers/experimental.md`](docs/providers/experimental.md). ([#348](https://github.com/microsoft/conductor/issues/348)) - **`claude-agent-sdk` provider now supports MCP servers** — workflow-level diff --git a/docs/providers/experimental.md b/docs/providers/experimental.md index 55f124bb..ad900f24 100644 --- a/docs/providers/experimental.md +++ b/docs/providers/experimental.md @@ -99,7 +99,7 @@ adopting one does not inflate the install surface for others. | Provider | Upstream pin | Maintainer | Capability carve-outs | |---|---|---|---| -| `claude-agent-sdk` | `claude-agent-sdk>=0.2.82` | `@lesandiz (best-effort)` | no `workflow_tools_passthrough`, no `reasoning_effort`, `prompt_injection` structured output, no `checkpoint_resume`. Supports `mcp_tools` as of [#335](https://github.com/microsoft/conductor/issues/335), except that a narrowing per-server `tools:` filter is refused (no SDK equivalent). Supports `working_dir` as of [#348](https://github.com/microsoft/conductor/issues/348). | +| `claude-agent-sdk` | `claude-agent-sdk>=0.2.82` | `@lesandiz (best-effort)` | no `workflow_tools_passthrough`, no `reasoning_effort`, `prompt_injection` structured output, no `checkpoint_resume`. Supports `mcp_tools` as of [#335](https://github.com/microsoft/conductor/issues/335), except that a narrowing per-server `tools:` filter is refused (no SDK equivalent). Supports `working_dir` as of [#348](https://github.com/microsoft/conductor/issues/348) — note the `claude` CLI also loads `CLAUDE.md` and `.claude/settings*.json` from that directory, so point it only at trees you trust. | | `hermes` | `hermes-agent` | `(community contribution)` | no `mcp_tools`, `prompt_injection` structured output, no `working_dir` | | `aca` | `azure-identity>=1.19.0` | `(unassigned)` | no `workflow_tools_passthrough` (the wrapped in-container `CopilotProvider` never applies the `tools:` allowlist to the SDK session), no `working_dir` (only the separate, container-relative `sandbox.working_dir` is honored — not the generic host-resolved field), `prompt_injection` structured output (inherits the inner Copilot provider), no `checkpoint_resume` (ephemeral sandbox sessions, no volume mount). Declares `interrupt`/`max_session_seconds` as `True`, but the shipped runner MVP doesn't fully back either yet — see [Known Gaps](./aca.md#known-gaps-runner-mvp). | diff --git a/docs/workflow-syntax.md b/docs/workflow-syntax.md index 3b6af170..0b256c34 100644 --- a/docs/workflow-syntax.md +++ b/docs/workflow-syntax.md @@ -326,6 +326,7 @@ Because paths are normalized lexically instead of resolving to their real paths: > ⚠️ **Warning: Working directory is NOT a sandbox** > Setting `working_dir` doesn't restrict the model's filesystem access. The model can still read and write files outside this directory if it uses absolute paths or parent directory traversals (e.g., `../`). Avoid relying on this configuration to sandbox untrusted model execution. +> On the `claude-agent-sdk` provider the directory is also a trust boundary in the other direction: the `claude` CLI loads `CLAUDE.md` and `.claude/settings*.json` (including hooks) from wherever it runs, so pointing `working_dir` at an untrusted checkout means running that checkout's instructions. ### Sandbox Configuration (ACA) diff --git a/src/conductor/providers/claude_agent_sdk.py b/src/conductor/providers/claude_agent_sdk.py index f0857a19..6b116a18 100644 --- a/src/conductor/providers/claude_agent_sdk.py +++ b/src/conductor/providers/claude_agent_sdk.py @@ -414,15 +414,33 @@ async def execute( sdk_tools, permission_mode = self._resolve_tool_config(tools, agent) + # ``os.getcwd()`` raises ``OSError`` when the process cwd has been + # deleted or an ancestor lost traversal permission. Resolve it here + # with a dedicated handler rather than leaning on the generic arm + # below, which would report a vanished cwd as a CLI installation + # problem and hand back a bare pathless errno. + try: + resolved_cwd = agent.working_dir or os.getcwd() + except OSError as exc: + raise ProviderError( + f"Agent '{agent.name}' declares no working_dir and the process working " + f"directory could not be resolved: {exc}", + suggestion=( + "The directory conductor was launched from has been deleted or is " + "no longer readable. Re-run from an existing directory, or set an " + "explicit working_dir on the agent or runtime." + ), + is_retryable=False, + ) from exc + options = ClaudeAgentOptions( model=model, system_prompt=agent.system_prompt, - # The engine resolves ``agent.working_dir`` (Jinja render, - # absolutize, is_dir check) before dispatching here, so pass it - # through verbatim; ``None`` keeps the process-cwd behavior. The - # CLI subprocess starts in this directory and every stdio MCP - # server it spawns inherits it. - cwd=agent.working_dir or os.getcwd(), + # Already resolved by ``WorkflowEngine._resolve_agent_working_dir`` + # (agent over runtime, rendered, absolutized, existence-checked), + # so pass it through verbatim rather than re-resolving — that would + # collapse the symlink aliases the engine preserves. + cwd=resolved_cwd, output_format=_build_output_format(agent.output) if agent.output else None, max_turns=max_turns, permission_mode=permission_mode, @@ -1069,6 +1087,45 @@ def _safe_callback(callback: EventCallback, event_type: str, data: dict[str, Any logger.debug("Error in event_callback for %s", event_type, exc_info=True) +def _classify_startup_failure(msg: str) -> str | None: + """Return a launch-failure hint for a ``CLIConnectionError`` message. + + The SDK reuses ``CLIConnectionError`` for failures to *spawn* the CLI, not + just to talk to a running one. A missing working directory gets a dedicated + message; ``ENOTDIR`` (the path is a file) and ``EACCES`` arrive through the + generic "Failed to start Claude Code: " arm instead. The generic + connection advice sends users to check firewalls for what is a bad path. + + Matching on upstream free text was audited against ``claude-agent-sdk`` + 0.2.87: CLI stderr never reaches a ``CLIConnectionError`` message (a + non-zero exit becomes ``ProcessError``, which this function never sees), so + a tool emitting "permission denied" cannot be misfiled as a launch failure. + + Args: + msg: Lower-cased exception message. + + Returns: + A tailored hint, or ``None`` when the message is not a launch failure + and the generic connection advice applies. + """ + if "working directory does not exist" in msg: + return ( + "The working directory disappeared between the engine's existence " + "check and the CLI launch — the agent's working_dir, or the process " + "cwd when none is set. Check whether an earlier step (e.g. a script " + "agent) deletes or moves it mid-run." + ) + if "not a directory" in msg or "permission denied" in msg: + # The offending path may be the working directory or the CLI binary -- + # the errno text does not say which -- so name both. + return ( + "The `claude` CLI could not be started. Check that the agent's " + "working_dir points at an existing, readable directory and that " + "the `claude` binary is executable." + ) + return None + + def _classify_error_suggestion(exc: BaseException) -> str: """Build a remediation hint tailored to the kind of failure observed. @@ -1086,6 +1143,9 @@ def _classify_error_suggestion(exc: BaseException) -> str: "https://docs.anthropic.com/claude/docs/claude-code and verify with `claude --version`." ) if cls == "CLIConnectionError": + startup_hint = _classify_startup_failure(msg) + if startup_hint is not None: + return startup_hint return ( "Could not connect to the `claude` CLI. Check that the binary is " "executable and that no firewall is blocking its spawned subprocess." @@ -1140,8 +1200,10 @@ def _is_retryable_exception(exc: BaseException) -> bool: return False if cls == "CLIConnectionError": - # Connection drops to a local subprocess — often transient. - return True + # A failure to *launch* the CLI (bad working_dir, non-executable + # binary) is deterministic — a retry lands on the same path. Only + # genuine connection drops to a running subprocess are transient. + return _classify_startup_failure(msg) is None if cls == "ProcessError": if "auth" in msg or "401" in msg or "403" in msg or "unauthorized" in msg: diff --git a/tests/test_config/test_validator_capabilities.py b/tests/test_config/test_validator_capabilities.py index 73d6cc64..8bc5e798 100644 --- a/tests/test_config/test_validator_capabilities.py +++ b/tests/test_config/test_validator_capabilities.py @@ -1502,9 +1502,10 @@ def test_sandbox_working_dir_against_real_aca_capabilities_passes( class TestClaudeAgentSdkRealCapabilitiesCrossCheck: - """#335 flipped ``mcp_tools`` to True for ``claude-agent-sdk``. Pin what - that means for the validator against the REAL descriptor, not a synthetic - one — mirrors ``TestAcaRealCapabilitiesCrossCheck``. + """#335 flipped ``mcp_tools`` and #348 flipped ``working_dir`` to True for + ``claude-agent-sdk``. Pin what those mean for the validator against the + REAL descriptor, not a synthetic one — mirrors + ``TestAcaRealCapabilitiesCrossCheck``. """ def _sdk_workflow( @@ -1512,6 +1513,7 @@ def _sdk_workflow( *, agents: list[AgentDef], mcp_servers: dict[str, MCPServerDef] | None = None, + working_dir: str | None = None, ) -> WorkflowConfig: from conductor.config.schema import ProviderSettings @@ -1522,6 +1524,7 @@ def _sdk_workflow( runtime=RuntimeConfig( provider=ProviderSettings(name="claude-agent-sdk"), mcp_servers=mcp_servers or {}, + working_dir=working_dir, ), ), agents=agents, @@ -1566,6 +1569,20 @@ def test_non_empty_tools_is_still_rejected(self, patch_caps: Any) -> None: with pytest.raises(ConfigurationError, match="does not honor per-agent tool allowlists"): validate_workflow_config(config) + def test_agent_working_dir_is_accepted(self, patch_caps: Any) -> None: + """The whole point of #348: an agent-level working_dir no longer fails + validate (it raised ConfigurationError before the capability flip).""" + self._patch(patch_caps) + config = self._sdk_workflow(agents=[AgentDef(name="a", prompt="hi", working_dir="/repo")]) + validate_workflow_config(config) # no raise + + def test_runtime_working_dir_is_accepted(self, patch_caps: Any) -> None: + """The workflow-level branch is separate code from the per-agent one, + so the flip has to be pinned on both paths.""" + self._patch(patch_caps) + config = self._sdk_workflow(agents=[AgentDef(name="a", prompt="hi")], working_dir="/repo") + validate_workflow_config(config) # no raise + class TestSkillsCrossCheck: """Requirement: ``skills`` (per-agent or runtime-wide) against a provider diff --git a/tests/test_providers/test_capabilities.py b/tests/test_providers/test_capabilities.py index 34a5ad31..2a584517 100644 --- a/tests/test_providers/test_capabilities.py +++ b/tests/test_providers/test_capabilities.py @@ -207,7 +207,8 @@ def test_every_production_provider_has_capabilities(self, provider_name: str) -> ("provider_name", "expected"), [ # Requirement: copilot, claude, and claude-agent-sdk honor - # agent/runtime ``working_dir`` for the SDK session and its MCP + # agent/runtime ``working_dir`` for the SDK session and (by + # per-server stamping or by subprocess inheritance) its MCP # servers; hermes does not (declared False so validate errors out). ("copilot", True), ("claude", True), diff --git a/tests/test_providers/test_claude_agent_sdk.py b/tests/test_providers/test_claude_agent_sdk.py index c7fe449d..93507374 100644 --- a/tests/test_providers/test_claude_agent_sdk.py +++ b/tests/test_providers/test_claude_agent_sdk.py @@ -2093,14 +2093,18 @@ class TestWorkingDirectory: """The engine-resolved ``working_dir`` must reach ``ClaudeAgentOptions.cwd``. The SDK applies ``cwd`` to the ``claude`` subprocess it spawns - (``_internal/transport/subprocess_cli.py``), so it governs both where the - CLI runs and where the stdio MCP servers it spawns inherit their cwd from. - These tests use the real ``ClaudeAgentOptions`` rather than a Mock so a - renamed or removed SDK field fails here instead of silently passing. + (``_internal/transport/subprocess_cli.py``, as of 0.2.87), so it governs + where the CLI runs. Stdio MCP servers are expected to inherit it as + children of that subprocess — not asserted here, since the CLI binary owns + server spawning. These tests use the real ``ClaudeAgentOptions`` rather + than a Mock so a renamed or removed SDK field fails here instead of + silently passing. """ @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) async def test_resolved_working_dir_becomes_cwd(self, tmp_path: Path) -> None: + """Requirement: the directory the engine resolved is the one the CLI + subprocess starts in.""" captured: dict = {} async def fake_query(**kwargs): @@ -2119,7 +2123,12 @@ async def fake_query(**kwargs): @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) async def test_unset_working_dir_falls_back_to_process_cwd(self) -> None: - """No ``working_dir`` keeps the legacy process-cwd behavior.""" + """No ``working_dir`` keeps the process-cwd fallback. + + Passing the explicit string rather than ``None`` is what makes the SDK + stamp ``PWD`` on the child and disambiguate a missing directory from a + generic launch failure, so this is not merely cosmetic. + """ captured: dict = {} async def fake_query(**kwargs): @@ -2192,6 +2201,103 @@ async def fake_query(**kwargs): def test_capability_declares_working_dir_support(self) -> None: """The descriptor is a contract -- it must match the wiring above.""" assert ClaudeAgentSdkProvider.CAPABILITIES.working_dir is True - assert ( - "working_dir ignored" not in ClaudeAgentSdkProvider.CAPABILITIES.declared_limitations() + + @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) + async def test_concurrent_agents_get_their_own_cwd(self, tmp_path: Path) -> None: + """``concurrent_safe=True`` has to hold for cwd too: no instance-level + caching may let one agent's directory leak into an overlapping + sibling's options.""" + dirs = [] + for i in range(4): + d = tmp_path / f"d{i}" + d.mkdir() + dirs.append(str(d)) + seen: dict[str, str] = {} + + async def fake_query(**kwargs): + await asyncio.sleep(0.05) # force the executions to overlap + seen[kwargs["prompt"]] = kwargs["options"].cwd + yield _result(result="ok") + + with patch("conductor.providers.claude_agent_sdk.query", fake_query): + provider = ClaudeAgentSdkProvider() + await asyncio.gather( + *( + provider.execute( + agent=AgentDef(name=f"a{i}", prompt="hi", working_dir=d), + context={}, + rendered_prompt=f"p{i}", + ) + for i, d in enumerate(dirs) + ) + ) + + assert seen == {f"p{i}": d for i, d in enumerate(dirs)} + + @patch("conductor.providers.claude_agent_sdk.CLAUDE_AGENT_SDK_AVAILABLE", True) + async def test_unresolvable_process_cwd_raises_provider_error(self) -> None: + """``os.getcwd()`` raises when the process cwd has been deleted. It must + surface as ``ProviderError`` naming the real subsystem -- the generic + handler would call it a CLI installation problem and hand back a bare + pathless errno, which is close to useless.""" + + async def fake_query(**kwargs): + raise AssertionError("query should never be reached") + yield # pragma: no cover - keeps this an async generator + + with ( + patch("conductor.providers.claude_agent_sdk.query", fake_query), + patch( + "conductor.providers.claude_agent_sdk.os.getcwd", + side_effect=FileNotFoundError(2, "No such file or directory"), + ), + pytest.raises( + ProviderError, match="working directory could not be resolved" + ) as exc_info, + ): + await ClaudeAgentSdkProvider().execute( + agent=AgentDef(name="t", prompt="hi"), context={}, rendered_prompt="hi" + ) + + assert "working_dir" in (exc_info.value.suggestion or "") + assert "installed" not in (exc_info.value.suggestion or "") + assert exc_info.value.is_retryable is False + + @pytest.mark.parametrize( + "message", + [ + "Working directory does not exist: /gone", + "Failed to start Claude Code: [Errno 20] Not a directory: '/x/afile'", + "Failed to start Claude Code: [Errno 13] Permission denied: '/x/noexec'", + ], + ) + def test_launch_failures_are_not_misdiagnosed(self, message: str) -> None: + """Skipping the provider-side ``is_dir()`` guard is only defensible if + the wrapped SDK error is actionable. These three all arrive as + ``CLIConnectionError``, whose generic advice is about firewalls, and + none of them can succeed on a retry.""" + from claude_agent_sdk import CLIConnectionError + + from conductor.providers.claude_agent_sdk import ( + _classify_error_suggestion, + _is_retryable_exception, ) + + exc = CLIConnectionError(message) + suggestion = _classify_error_suggestion(exc) + assert "firewall" not in suggestion + assert "working_dir" in suggestion + assert _is_retryable_exception(exc) is False + + def test_genuine_connection_drop_stays_retryable(self) -> None: + """The launch-failure carve-out must not swallow the transient case.""" + from claude_agent_sdk import CLIConnectionError + + from conductor.providers.claude_agent_sdk import ( + _classify_error_suggestion, + _is_retryable_exception, + ) + + exc = CLIConnectionError("subprocess died unexpectedly") + assert "firewall" in _classify_error_suggestion(exc) + assert _is_retryable_exception(exc) is True