Skip to content
Merged
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
5 changes: 5 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +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` 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: <path>")`, 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

Expand Down
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,27 @@ 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. 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
`runtime.mcp_servers` are translated to the SDK's own `stdio` / `http` /
`sse` config shapes and passed through `ClaudeAgentOptions`, so an agent can
Expand Down
4 changes: 2 additions & 2 deletions docs/providers/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. │
└─────────────────────────────────────────────────────────────────────┘
```
Expand Down Expand Up @@ -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) — 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). |

Expand Down
1 change: 1 addition & 0 deletions docs/workflow-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
84 changes: 76 additions & 8 deletions src/conductor/providers/claude_agent_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -414,9 +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,
# 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,
Expand Down Expand Up @@ -1063,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: <errno>" 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.

Expand All @@ -1080,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."
Expand Down Expand Up @@ -1134,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:
Expand Down
23 changes: 20 additions & 3 deletions tests/test_config/test_validator_capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -1502,16 +1502,18 @@ 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(
self,
*,
agents: list[AgentDef],
mcp_servers: dict[str, MCPServerDef] | None = None,
working_dir: str | None = None,
) -> WorkflowConfig:
from conductor.config.schema import ProviderSettings

Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions tests/test_providers/test_capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,14 @@ 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 (by
# per-server stamping or by subprocess inheritance) 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:
Expand Down
Loading
Loading