feat(claude-agent-sdk): honor working_dir via ClaudeAgentOptions.cwd (#348) - #349
Merged
Merged
Conversation
…348) 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>
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>
jrob5756
force-pushed
the
feature/348-claude-agent-sdk-working-dir
branch
from
July 31, 2026 13:04
ffd038e to
296e506
Compare
jrob5756
marked this pull request as ready for review
July 31, 2026 13:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #348.
What
ClaudeAgentSdkProviderdeclaredworking_dir=False, soconductor validaterejected any workflow settingagent.working_dirorruntime.working_diragainst it. The declaration was accurate —execute()builtClaudeAgentOptions(...)without acwd, so theclaudeCLI and every stdio MCP server it spawned ran in whatever directory the Conductor process happened to be in.Until #335/#346 the stated reason was that MCP servers were rejected at the factory. That premise is gone, leaving a plain parity gap against
copilot(copilot.py:972) andclaude(claude.py:1147).This forwards the engine-resolved directory as
ClaudeAgentOptions.cwdand flips the capability.How
Verified against
claude-agent-sdk0.2.82:ClaudeAgentOptions.cwd(types.py:1880) reachesopen_process(cwd=...)andprocess_env["PWD"]in_internal/transport/subprocess_cli.py.claudesubprocess.McpStdioServerConfig(types.py:603) has onlytype/command/args/env— no cwd field — so unlikecopilot.py::_mcp_servers_for_cwdthere is no per-server stamping to add._translate_mcp_serversand_write_mcp_configare untouched.add_dirsis a separate axis (--add-dirflags that widen accessible directories, not a cwd). Conductor does not set it, so there is no interaction to handle.The path is passed verbatim.
WorkflowEngine._resolve_agent_working_dirhas already rendered, absolutized, normalised, and existence-checked it; re-resolving here would collapse the symlink aliases the engine preserves on purpose.The two open questions from the issue
Non-existent directory — no extra guard added. The engine raises
ExecutionErrorbefore any provider call, and a directory that disappears afterwards surfaces as the SDK'sCLIConnectionError("Working directory does not exist"), whichexecute()'s existingexcept Exceptionalready wraps inProviderError.cwdvsadd_dirs— orthogonal, as above.Worth knowing
cwd also determines which
CLAUDE.mdand local settings the CLI loads, and it is the SDK's session-store project key (_internal/session_resume.py). That is the point of the feature and matches Copilot. The security-relevant part is already covered:strict_mcp_config=Trueis unconditional, so a.mcp.jsonin the new directory cannot inject undeclared servers.Changes
providers/claude_agent_sdk.py— forwardcwd; flipCAPABILITIES.working_dirtoTruetests/test_providers/test_claude_agent_sdk.py— newTestWorkingDirectory(5 tests)tests/test_providers/test_capabilities.py— flip the matrix entrydocs/providers/experimental.md— carve-out row and sample bannerAGENTS.md— parity noteCHANGELOG.md— Unreleased entry (the[0.1.23]mention stays; it is historical)Out of scope: wiring
reasoning.effortto the SDK'seffortfield (a separate declared gap) and exposingadd_dirs.Testing
The new tests use the real
ClaudeAgentOptionsrather than aMock, so a renamed or removed SDK field fails here instead of passing silently. They cover the resolved directory reachingcwd, theos.getcwd()fallback, verbatim passthrough (symlink not collapsed), composition with MCP servers, and the descriptor matching the wiring.test_providers,test_config,test_engine(the provider skip is the unrelatedacaextra)make checkandmake validate-examplesboth cleanworking_diron this provider previously failed withConfigurationError: ... capabilities.working_dir=Falseand now validatesNote the SDK tests are
importorskip-gated; CI's test job installs the extra (ci.yml:109), the typecheck job does not.