Skip to content
Open
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
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,8 @@ Conductor:
- 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 is the project key for the CLI's on-disk transcript directory, and it is where a project `.mcp.json` and `.claude/` tree would be looked for. The unconditional `strict_mcp_config=True` stops a `.mcp.json` there from injecting undeclared servers, and the unconditional `setting_sources=[]` (see **Skills** below) stops the CLI loading `CLAUDE.md`, project settings, and hooks from it — so cwd no longer drags ambient instructions in. `add_dirs` (the SDK's `--add-dir` passthrough) is a separate axis Conductor does not set.
- Knock-on effects: cwd is the project key for the CLI's on-disk transcript directory, and it is where a project `.mcp.json` and `.claude/` tree would be looked for. The unconditional `strict_mcp_config=True` stops a `.mcp.json` there from injecting undeclared servers, and the unconditional `setting_sources=[]` (see **Skills** below) stops the CLI loading `CLAUDE.md`, project settings, and hooks from it — so cwd no longer drags ambient instructions in. `add_dirs` (the SDK's `--add-dir` passthrough) is a separate axis, carrying the per-agent `settings_dir` and nothing else — see **Target-repository skills** below.
- **Target-repository skills** (`settings_dir`): the per-agent `AgentDef.settings_dir` is the **only** source of `ClaudeAgentOptions.add_dirs`, and it selects which directory's *project* settings tier contributes skills. It exists because cwd was doing two conflicting jobs on this provider. The CLI supports the MCP Roots protocol and advertises exactly one root — its cwd — so `@modelcontextprotocol/server-filesystem` **discards the directories in its own argv** and permits cwd alone; cwd is simultaneously what the `project` tier resolves against. Narrowing cwd onto a target repository to reach its skills therefore narrowed the agent's only MCP root below any sibling path the step still had to read. An earlier fork-local `_stdio_path_args` helper derived `add_dirs` from every stdio server's directory arguments, with a docstring asserting this "restores the declared scope"; it cannot — `--add-dir` takes no part in Roots negotiation, it widens the CLI's own file tools. That helper is **removed**; do not reintroduce deriving `add_dirs` from server arguments, which would also silently widen skill discovery to directories the author named as data. What `add_dirs` *does* do, measured: a named directory's `.claude/skills` become listed and invocable with cwd elsewhere entirely — and only those, not `CLAUDE.md`, `.claude/rules/*.md`, `.claude/settings.json` (so no `env`, no `hooks`) or `.claude/agents`, which all keep following cwd. That makes it the *skills third* of what a cwd-resolved `project` tier loads, not a replacement for it. It carries a second, **unconditional** effect the skills framing hides: `add_dirs` is "additional directories Claude can access" per the SDK's own contract, so a `settings_dir` widens the model's built-in `Read`/`Edit`/`Bash` to that tree with no settings tier enabled at all (measured: with `setting_sources` unset and `permission_mode: default`, a read outside cwd is refused without it and succeeds with it). It does not widen what an MCP server permits. `capabilities.py::settings_dir` gates the field and `config/validator.py` errors against a provider that cannot apply it, warning when it is set with no `setting_sources` (the skills half is then a no-op while the filesystem grant still applies). `WorkflowEngine._resolve_agent_directory` resolves both fields so they cannot drift, and `settings_dir` is per-agent only (no `runtime.` counterpart: the repository whose conventions apply is what varies between steps). `tests/test_integration/test_mcp_roots_negotiation.py` pins the negotiation rule itself against the real server with no LLM — two runs differing only in whether the client advertises `roots` — since every option here rests on it.
- **Skills** (issue #352): `supports_native_skills=True`. Skills are enabled through the SDK, not prompt injection, and three options move together in `execute`:
- `plugins=[{"type": "local", "path": <plugin root>}]` + `skills=["<plugin>:<skill>"]`. The SDK has no skill-*directory* surface, so `_resolve_skill_plugins` maps each directory back to the plugin that owns it via `skills/registry.py::resolve_skill_plugin`. That resolver is deliberately strict, because every one of these mistakes otherwise produces a name the CLI silently resolves to nothing: it bounds the upward walk (`_PLUGIN_SEARCH_DEPTH`), requires the skill to actually live under the candidate's `skills/` directory, requires `SKILL.md` to exist and its frontmatter `name` to equal the directory name (the CLI resolves by frontmatter name; Conductor sends the directory name), and rejects names outside `[A-Za-z0-9_.-]+` since they are joined into a comma-delimited `--allowedTools` value. A skill under no plugin root returns `None`; a plugin that is present but unusable raises `SkillPluginError`, which the provider re-raises as a `ProviderError` carrying the real reason rather than a blanket "not part of a plugin". Two plugins claiming one qualified name are refused too — deduping the clash away would drop a declared skill. All of it is `is_retryable=False`: these never become valid on a retry, and a checkout path containing "connection" would otherwise trip the retryability heuristic. Note providers are constructed lazily, so this surfaces when the first agent on this provider runs, **not** at `conductor validate`.
- `setting_sources=[]` **unconditionally**, for the same reason `strict_mcp_config=True` is unconditional a few lines away. Left unset, the CLI loads user, project, and local settings, which between them bring in ambient skills, `CLAUDE.md`, and hooks the workflow never declared, varying by machine and launch directory. Conductor surfaces instruction files through its own opt-in `--workspace-instructions`; settings and hooks have no equivalent. `skills=[]` and `skills=None` are **not** interchangeable upstream: `None` means "CLI defaults apply", and setting `skills` while leaving `setting_sources` at `None` makes the SDK re-default it to `["user", "project"]` — so the two options are coupled and dropping either re-opens the issue. The `[]` is also invisible in argv (it travels in the SDK's `initialize` control request), which is why the argv-based tests are paired with options-level assertions. The list is a context filter, not a sandbox: undeclared skills are hidden from the model's listing but their files stay readable.
Expand Down
2 changes: 1 addition & 1 deletion docs/providers/experimental.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,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 `reasoning_effort`, `prompt_injection` structured output, no `checkpoint_resume` (agents without a `session_key` carry no session state across a 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 `workflow_tools_passthrough`: a per-agent `tools:` allowlist is enforced by enumerating the declared stdio MCP servers and denying every tool not on the list (plus the built-in write/exec tools), since the CLI's `allowed_tools` only pre-approves and does not restrict. An allowlist alongside an http/sse server is refused — those cannot be enumerated, so the denial set would be unknown. `tools: []` alongside `mcp_servers:` is still refused (`mcp_servers_always_attached`): honoring an allowlist does not mean the provider can detach a declared server. Supports `working_dir` as of [#348](https://github.com/microsoft/conductor/issues/348); the CLI would load `CLAUDE.md` and `.claude/settings*.json` from that directory, but `setting_sources` is pinned empty as of [#352](https://github.com/microsoft/conductor/issues/352) so ambient instructions, settings, hooks, and skills are not inherited. Declares `session_continuity`: an agent with a `session_key` reuses one Claude session across executions, and the session map survives `conductor resume` — see [Session Continuity](../workflow-syntax.md#session-continuity-session_key). |
| `claude-agent-sdk` | `claude-agent-sdk>=0.2.82` | `@lesandiz (best-effort)` | no `reasoning_effort`, `prompt_injection` structured output, no `checkpoint_resume` (agents without a `session_key` carry no session state across a 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 `workflow_tools_passthrough`: a per-agent `tools:` allowlist is enforced by enumerating the declared stdio MCP servers and denying every tool not on the list (plus the built-in write/exec tools), since the CLI's `allowed_tools` only pre-approves and does not restrict. An allowlist alongside an http/sse server is refused — those cannot be enumerated, so the denial set would be unknown. `tools: []` alongside `mcp_servers:` is still refused (`mcp_servers_always_attached`): honoring an allowlist does not mean the provider can detach a declared server. Supports `working_dir` as of [#348](https://github.com/microsoft/conductor/issues/348); the CLI would load `CLAUDE.md` and `.claude/settings*.json` from that directory, but `setting_sources` defaults to empty as of [#352](https://github.com/microsoft/conductor/issues/352) so ambient instructions, settings, hooks, and skills are not inherited; a workflow opts back in per run with `runtime.provider.setting_sources`, and chooses per agent which directory the `project` tier reads skills from via `settings_dir` (cwd alone governs the CLI's sole MCP root, so the two are deliberately separate) — see [Target-Repository Skills](../workflow-syntax.md#target-repository-skills-settings_dir). Declares `session_continuity`: an agent with a `session_key` reuses one Claude session across executions, and the session map survives `conductor resume` — see [Session Continuity](../workflow-syntax.md#session-continuity-session_key). |
| `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
111 changes: 111 additions & 0 deletions docs/workflow-syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,117 @@ Because paths are normalized lexically instead of resolving to their real paths:
> 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.

### Target-Repository Skills (`settings_dir`)

`settings_dir` names a second directory whose Claude Code *project* settings
tier the agent reads skills from. It applies to `claude-agent-sdk` agents in a
workflow that sets `runtime.provider.setting_sources`, and is ignored by every
other provider.

```yaml
workflow:
runtime:
provider:
name: claude-agent-sdk
setting_sources: [project]

agents:
- name: judge
settings_dir: "{{ setup_worktree.output.worktree_path }}"
prompt: Review the change against this repository's conventions.
```

#### Why it is separate from `working_dir`

An agent's cwd does two unrelated jobs, and on this provider they conflict.
The `claude` CLI supports the MCP Roots protocol and advertises exactly one
root — its cwd. A filesystem MCP server therefore **discards the directories
in its own argv** and permits cwd alone; `--add-dir` takes no part in that
negotiation, so it cannot widen what a server allows. cwd is simultaneously
the directory the `project` settings tier resolves against.

So pointing `working_dir` at a target repository to pick up that repository's
skills also narrows the agent's only MCP root onto it, and any sibling path
the step still has to read — an artifacts directory, a second checkout — is
denied. Widening cwd back loses the repository's conventions.

`settings_dir` splits the two. Skills are discovered from cwd **and** from
`settings_dir`, so cwd can stay wide enough to contain everything the agent
must read:

```yaml
agents:
- name: judge
# No working_dir: cwd stays the launch directory, which contains both the
# worktree and the artifacts this judge reads through the filesystem MCP.
settings_dir: "{{ setup_worktree.output.worktree_path }}"
```

#### What it does and does not carry

Measured against the CLI:

| Named via `settings_dir` | Granted? |
|---|---|
| **Filesystem access for the model's built-in tools** (`Read`, `Edit`, `Bash`, …) | **yes — unconditionally**, see below |
| `.claude/skills` | **yes** — listed and invocable |
| `CLAUDE.md` | no |
| `.claude/rules/*.md` | no |
| `.claude/settings.json` `env` | no |
| `.claude/settings.json` `hooks` | **not established** — see below |
| `.claude/agents` | no |

> ⚠️ **The filesystem grant does not depend on `setting_sources`.** This field
> maps to the SDK's `add_dirs`, whose own contract is *"additional directories
> Claude can access beyond the current working directory"* — so naming a
> directory here widens the model's built-in file tools to that tree whether or
> not any settings tier is enabled. Measured with `setting_sources` unset and
> `permission_mode: default`: without `settings_dir` a read outside cwd is
> refused for permissions; with it, the same read succeeds.
>
> Skill discovery is the *reason* to set this field; the filesystem grant is
> its unavoidable companion. Point it at a directory the agent is entitled to
> read.

Note this grant is for the model's **built-in** tools only. It does not widen
what a filesystem MCP server permits — that stays cwd alone, which is the
whole reason this field exists.

**The `hooks` row is honestly unknown, not a measured negative.** The probe
behind it grepped the CLI's debug output for a hook's marker and found nothing
-- but it also found nothing in the control, where cwd *was* the directory and
the hook demonstrably should have run. A probe that returns nothing in its own
baseline cannot support a negative, so treat hooks as undetermined: do not
rely on them running, and do not rely on them being suppressed. Establishing
it needs a hook with an observable side effect, such as writing a file.

Setting aside the filesystem grant, this field is the *skills portion* of a
project tier, not a cwd-independent way to load one. It cuts favourably in one direction —
a target repository's skills arrive without its hooks also running — but it
does not compose with `working_dir` into "everything, anywhere":

> An agent that needs a target repository's **rules or instructions** as well
> as a cwd wide enough for its MCP servers cannot get both from these fields.
> One directory cannot be narrow and wide at once. `settings_dir` recovers the
> skills; anything else is a caller-side trade — keep `working_dir` on the
> repository and arrange for every path the agent reads to sit beneath it.

#### Resolution and restrictions

- Resolved exactly like `working_dir` — Jinja2-rendered, `~`-expanded,
relative paths resolved against the workflow file's directory, normalized
with `os.path.normpath`, and existence-checked before any provider call.
- Per-agent only. There is no `runtime.settings_dir`, because the repository
whose conventions apply is what varies between steps.
- Rejected on `wait`, `set`, `terminate`, `script`, `human_gate`, `questions`
and `workflow` step types — none has an LLM session to apply a settings tier
to, and accepting it silently would suggest conventions had been loaded when
none had.

> ⚠️ A settings tier brings everything that tier defines. Enable
> `setting_sources` and point `settings_dir` only at repositories trusted to
> the same degree as the workflow itself.

### Session Continuity (`session_key`)

By default each agent execution starts a fresh provider session, so an agent
Expand Down
Loading