PDA-115 Sync the fork with upstream after microsoft/conductor#510 and #514 - #9
Conversation
…y picker (microsoft#487) * fix(fleet): harden Windows sharing-violation retries and TUI directory picker Extract the Windows-only PermissionError retry loop out of _replace_with_retry into a shared _retry_on_windows_sharing_violation helper, and reuse it for record deletion (_safe_unlink) and quarantine rename (_delete_if_unchanged) so those paths also absorb a transient sharing violation from a concurrent reader instead of failing outright. FileNotFoundError is treated as "already gone" and never retried. Also fixes the Fleet TUI's directory picker so the tree's highlighted node only mirrors into the input while the tree has focus, and updates associated tests, docs, and changelog. * fix(fleet): apply PR microsoft#487 review findings Blocking fixes: - records.py: `_restore_if_absent` now routes `os.link` through the bounded Windows sharing-violation retry helper, closing the gap where a failed restore could orphan a live run's record as an unswept `.prune-*` file (AGENTS.md updated to match). - tui/actions.py, CHANGELOG.md, docs/fleet.md, test comment: corrected the false claim that the automatic root `NodeHighlighted` is caused by the background directory load -- it fires at mount, from `Tree`'s reactive initialisation, independent of any directory I/O. - tui/actions.py, docs/fleet.md: corrected the false "Enter in the input is the only accept path" claims -- a single click (or Enter) on a tree node also accepts via `NodeSelected` -> `DirectorySelected`; docs now describe both accept paths. Recommendations applied: - Guard `_retry_on_windows_sharing_violation` against a mistuned zero-valued retry constant silently reporting success without calling `op`; annotated the constant `Final[int]`. - Corrected the false "os.unlink is patchable, Path.unlink isn't" rationale in `_safe_unlink`'s docstring. - Fixed the CHANGELOG entry describing the self-cleanup retry (it understated the two-call rename+unlink sequence and was missing a relative pronoun). - Dropped `exc_info=True` (which produced an unattributed traceback on stderr/status output with no installed handlers) from the three new warning logs, interpolating the exception and stating the consequence instead. - Fixed the `Raises:` section on `_retry_on_windows_sharing_violation` to stop contradicting itself, and added an inline comment marking the deliberate `except PermissionError` (not `OSError`) choice. - Reworded the FILE_SHARE_DELETE rationale to lead with the observable source/destination contention asymmetry rather than presenting it as the sole cause. Recommendations skipped (see review response for reasoning): new `caplog`-based tests for the warning paths, gating the retry off in the bulk-prune scan, the KeyboardInterrupt handling change in cli/run.py, the `_replace_with_retry` inline-vs-wrapper judgement call, and the test hardening for global os.unlink/os.rename fakes -- each is a reasonable follow-up but changes behavior or adds test/production surface beyond what the blocking findings required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(fleet): wait for the picker modal's mount focus before driving it `test_tree_highlight_while_focused_mirrors_into_the_input` failed on Windows CI on both runs of this PR while passing on Linux, at `assert tree.has_focus` right after `tree.focus()`. The race is in the test, not the modal. `DirectoryPickerModal.on_mount` focuses `#dir-path`, and `Widget.focus()` defers the real work to `App.call_later`, so "the screen is composed" and "the modal's mount-time focus has landed" are two different moments. The test focused the tree in between: its own `call_later` ran first, the modal's arrived afterwards, and the input won -- leaving the tree unfocused. A single `pilot.pause()` closed that window on Linux but not on Windows. `Pilot.pause()` returns once `textual._wait.wait_for_idle` judges the process idle by comparing `time.process_time()` against wall clock, and Windows reports process time in ~15.6ms ticks, so the comparison reads no CPU used and returns after its first sleep however much work is queued. Reproduced locally by stubbing `wait_for_idle` to a no-op: the same test fails with the same assertion, and `screen.focused` is still `None` after the push. `_push` now takes the pilot and waits (via the existing `wait_for` condition helper) until the modal is mounted *and* its input actually holds focus, so every test in the class starts from a settled modal; the now-redundant `pilot.pause()` after each push is dropped. The focus assertion itself becomes a `wait_for` too, since `has_focus` is a reactive the tree only sets once it processes the `Focus` message. Test-only change. All 11 picker tests pass both normally and under the stubbed-idle emulation of Windows' weaker pause. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…crosoft#484) * fix(copilot): recover from a dead spawned Copilot runtime process Detect when the nested Copilot runtime subprocess has died (broken pipe/connection reset, or a check before sending an idle-recovery prompt) and transparently restart it on the next attempt instead of surfacing a confusing stuck-agent error. Externally-owned runtimes (runtime_url) are never restarted here -- that failure is reported as non-retryable so the owning orchestrator can act. A consecutive restart counter (reset on any successful SDK call) caps restart attempts so a runtime that keeps dying before ever succeeding fails fast rather than looping forever. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(copilot): address review findings on runtime restart state machine Blocking fixes (PR microsoft#484 review): - _restart_spawned_runtime no longer publishes the rebuilt client until it has actually started: _client/_started are invalidated first, so a failed start() (e.g. OOM at spawn) leaves the provider correctly believing no client is started, instead of silently disabling dead-runtime recovery for the rest of the process. - The consecutive-restart cap is now checked before incrementing the counter and is never left stale: the cap can no longer be tripped after zero actual restarts, the giving-up message reports the real restart count, and close() resets the counter so a cached provider isn't permanently wedged after a workflow crash-loops once. - Replaced the unfalsifiable cap-message assertion in test_copilot_runtime_recovery.py with one that pins the rendered clause and asserts the cap actually prevents the next rebuild. - Added tests/test_providers/conftest.py: an autouse fixture clearing COPILOT_PROVIDER_RUNTIME_URL/TOKEN so the runtime-recovery tests pass regardless of the developer's/CI runner's environment. - Added a regression test covering the corrupted-state bug: when the rebuilt client's start() raises, _started must end up False and a later _ensure_client_started() must re-attempt start(). Recommendations applied: - _runtime_unavailable_error now distinguishes a confirmed-dead process (poll() returned an exit code) from a broken connection to a still- alive process, instead of always claiming the process "died" and suggesting NODE_OPTIONS. - Client teardown during restart, and session.disconnect() in the per-agent finally block, now log a warning on failure instead of silently swallowing the exception (a leaked child / stranded session is diagnostically useful, especially given this PR's own OOM focus). - The session.error ProviderError path is now also routed through dead- runtime classification when retryable, instead of always surfacing a generic "Copilot SDK error" message that hides an exit-code 137 OOM kill. - Narrowed _spawned_runtime_process's return type from Any | None to subprocess.Popen[bytes] | None, matching the isinstance check the body already performs and the SDK's own annotation. - Added a one-time warning when a spawned, started client has no usable _cli_process handle, so a future SDK rename surfaces instead of silently degrading recovery to a no-op. - Fixed the inverted _FakeClient docstring/comments describing mock auto-vivification as looking "live" when it in fact reads as dead. - Scoped the restart-counter-reset comment to agent execution (several auxiliary paths increment without resetting). - Updated CHANGELOG.md, docs/configuration.md and AGENTS.md to name the restart cap (2, fixed, non-configurable), correct the "endlessly retrying" overstatement, and scope the SDK-boundary claim to agent-execution; documented the _cli_process vs _process split. Recommendations skipped (not applied): #5 (_interrupted_session reset + disclosure wording), #6 (max_session pre-flight), microsoft#12 (Liveness enum), microsoft#13 (_RestartBudget value type), microsoft#17 (per-generation client tracking for parallel groups), microsoft#18 (additional missing tests beyond the one added for finding #1), microsoft#19 (collapsing except clauses), microsoft#20 (extracting shared helpers) -- all correctness-neutral hardening/refactors judged to grow the diff beyond what this pass should touch; pyproject.toml dependency cap was also left alone as an unrelated, broader change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…icrosoft#489) * fix(fleet): replace bounded event-log reads with uncapped streaming The Runs/History/run-detail screens read a run's JSONL event log through three separate bounded windows (a 512 KiB tail, a 512 KiB head recovery read, and an 8 MiB full-log cap). A long-lived or resumed run outgrows these: a 9.72 MB / 20,361-line log lost its current step and token/cost totals, and a resumed run's second workflow_started event fell outside the head window, misreporting topology. summary.py::stream_event_log is a single streamed reader bounded only by the longest line, replacing all three windows. It supports a keep_types prefilter so the Runs screen's ~2s poll can skip uninteresting lines via regex before JSON-parsing. history.py now delegates to the same reader and takes the latest root workflow_started's timestamp for a resumed run's duration fallback. * fix(fleet): make run-detail generation-aware, harden step-detail reads Applies PR microsoft#489 review findings on top of the uncapped event-log streaming change: - `_scan_agent_details` (run-detail screen) now resets `open_steps`, `gated`, and `started_at_by_name` at every root `workflow_started`, matching `_scan_events`'s existing resume-boundary reset. Previously a generation killed mid-step (an open step or unresolved gate with no closing event) would survive across a resume forever, so the Runs screen correctly reported "running, nothing open" while run-detail simultaneously reported a dead step as still "at-gate" with a stale, ever-growing elapsed clock. - `derive_step_detail`'s consumption loop is extracted into `_scan_step_events`, returning its five locals in one tuple assigned atomically inside the caller's `try`/`except OSError`. Previously a mid-stream `OSError` left `status`/`output`/`activity` holding whatever a partial scan had accumulated, which was then returned as if it were authoritative (e.g. a completed step with real output rendering as "running, no output" forever). The same pass also resets a step whose `status` was still "running" across a resume boundary to "pending", and clears `prompt` alongside `output`/`activity` on a restart. Also applies several review recommendations: - `derive_run_detail` now prefilters its scan with `keep_types=_SUMMARY_EVENT_TYPES` (a verified superset of everything `_scan_agent_details` branches on), cutting its cost roughly 5x. - Promotes `history.py`'s `_finite_float` NaN/Infinity guard into `summary.py` and reuses it for token/cost accumulation in both `_scan_events` and `_scan_agent_details`, so a `NaN`/`Infinity` value from a corrupted log entry can no longer crash the Runs/run-detail poll loops. - Corrects several docstrings left describing the deleted bounded tail/head/full-log readers or overstating scan frequency/cost (`RunDetail`, `stream_event_log`'s `keep_types` Args/Raises, `derive_step_detail`'s Args/Returns, `step_detail.py`, `dag.py`, and history.py's resume-boundary paragraph). - Adds regression tests: run-detail generation reset, mid-stream `OSError` no longer surfacing a partial scan, a nested sub-workflow start not being mistaken for a resume boundary (both `summary.py` and `history.py`), a `_scan_agent_details` prefilter-equivalence test, a NaN/Infinity token/cost regression test, and a strengthened totals-accumulate-across-generations assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…calls (microsoft#490) * fix(providers): suppress idle watchdog during in-flight Copilot tool calls The Copilot SDK emits no events between tool.execution_start and tool.execution_complete, so a stale idle clock during a long-running tool call was indistinguishable from a genuinely stuck session, triggering a spurious "please continue" recovery prompt mid tool-call that could overwrite the agent's eventual structured output. In-flight tool calls (tracked by tool_call_id) now suppress idle recovery entirely while any remain outstanding; max_session_seconds / max_agent_iterations remain the backstop for a genuinely wedged tool. last_activity_ref's tool name is cleared (or rolled to another still-in-flight tool) on tool.execution_complete instead of only ever being set. Adds configurable runtime.idle_timeout_seconds and runtime.max_idle_recovery_attempts (Copilot-only) so workflows with legitimately long tool calls can tune the watchdog. Closes microsoft#488 * fix(providers): address PR microsoft#490 review findings on idle watchdog suppression Blocking fixes (microsoft#488): - Remove the "pop the oldest entry" fallback in the tool.execution_complete handler that could evict a different, still-running tool's active_tools entry on a duplicate/unmatched event, re-arming the watchdog mid-tool-call and reproducing microsoft#488 while appearing fixed. Replaced with a non-mutating debug log; max_session_seconds remains the backstop for a stale entry. - Strengthen TestOnEventActiveTools assertions so both tests actually pin the fix (verified to fail against the pre-fix provider, pass post-fix). - Add an end-to-end regression test driving a real tool.execution_start -> silence -> tool.execution_complete sequence through _send_and_wait, asserting the recovery prompt never clobbers response_content. - Add a warn-once latch (mirroring _context_window_anomaly_warned) so the first occurrence of extended idle-recovery suppression during a session is logged at warning level (console + logger), instead of silently degrading a previously console-visible 90s warning into up to 31.5 minutes of total silence. Recommendations applied: - Corrected the repeated false claim that the SDK "emits no events" during a tool call (it does not guarantee any, but tool.execution_progress / tool.execution_partial_result exist and are opt-in) across copilot.py, schema.py, docs/configuration.md, CHANGELOG.md, and the PR description; consolidated the rationale into one canonical docstring. - Corrected the inaccurate claim that max_agent_iterations backstops a wedged tool call (its counter only advances on tool.execution_start, so it's frozen for the whole wedge) — max_session_seconds is the sole backstop. - Added IdleRecoveryConfig.__post_init__ validation so directly-constructed configs (bypassing the Pydantic schema bounds) can't produce an unbounded busy-wait loop. - Simplified factory.py's IdleRecoveryConfig construction to a dict-filter + single constructor call instead of a three-way ternary per field. - Added ProviderCapabilities.idle_recovery (Copilot-only) with a workflow-level validator warning (not an error, since these are tuning knobs rather than safety bounds) when idle_timeout_seconds / max_idle_recovery_attempts are set against a provider that ignores them. - Bounded two previously-unbounded busy-wait test loops with asyncio.wait_for(..., timeout=5.0). - Added an overlapping-tool-calls end-to-end test keyed on tool_call_id (not tool_name), verified to reproduce the hang if the dict were mistakenly keyed by tool name instead. - Documented the max_session_seconds backstop in docs/configuration.md so a legitimately long tool call doesn't silently exceed it unexpectedly. Skipped: ACA forwarding of the two idle-recovery fields (larger, separate scope spanning factory/aca/aca_runner) and AGENTS.md documentation update (the two runtime knobs and active_tools mechanism are already documented in the config docs and code comments; deferring to keep this diff scoped to the review findings). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jason Robert <jasonrobert@microsoft.com>
microsoft#480) * refactor(engine): remove dead workflow lifecycle hooks (microsoft#476) The hooks: block (on_start/on_complete/on_error) was parsed and its templates rendered, but the result was always discarded, making the feature entirely unobservable. Remove HooksConfig, WorkflowDef.hooks, the engine's _execute_hook/LifecycleHookResult and all call sites, and the Hooks section of the workflow syntax docs. A workflow declaring hooks: now fails validation with a clear error. * refactor(engine): address review on hooks removal (microsoft#476) - Fix garbled except BaseException comment left by the removal, restoring the microsoft#116 rationale and documenting the is_base_exception flag. - Remove hooks: from the bundled conductor skill (SKILL.md, authoring.md, yaml-schema.md) so agents no longer generate workflows that hard-fail. - Scrub remaining stale 'on_error/on_complete hooks' prose in exceptions.py, AGENTS.md, authoring.md, and a test docstring. - Add a WorkflowDef model validator giving a targeted microsoft#476 error naming the removal and the workflow_completed/workflow_failed replacement, instead of the generic extra_forbidden message. - Rework/extend tests: schema, loader (YAML), and CLI (conductor validate) now assert the removal message; add invalid_hooks.yaml fixture. - Fix CHANGELOG spacing.
) (microsoft#469) * fix(cli): resolve doctor's glyph set per console encoding (microsoft#401) conductor doctor's table output hardcodes the U+2713/U+2717/U+25CB glyphs for its Installed/Credentials/Connection/Models columns. None of those are encodable in cp1252, so a run on a legacy Windows console raises UnicodeEncodeError mid-table, after the Environment section has already printed: the report is truncated and the process exits non-zero for a run that actually succeeded. The --json path already guards this with ensure_ascii=True (microsoft#381); the table path did not. Fix: resolve a Unicode-or-ASCII glyph set once per run_doctor() call, from the output console's actual stream encoding, and thread it through every cell helper (_tier_cell, _credentials_cell, _connection_cell, _models_cell, _format_tokens, _default_effort_cell, _rate_cell, _render_registries) instead of reaching for a module-level _CHECK/_CROSS/_DASH/_OPTIONAL_MARK constant. A console with no encoding attribute (e.g. an in-memory buffer) is treated as capable, matching the existing "nothing to protect against" default. Fixes microsoft#401 * fix(cli): address doctor review, add warn glyph and fix Models title dash jrob5756's review on microsoft#469 found the fix left one path open: the connection-note branch of _connection_cell still printed a bare warning glyph outside the resolved set, so conductor doctor --check still crashed on cp1252 for any provider whose probe came back inconclusive. Added a warn entry to _Glyphs (falls back to "!") and routed that branch through it. Also fixed the Models detail table title, which embedded a bare em dash and crashed on ascii/latin-1/cp437 independent of microsoft#401. Uses the already-resolved dash glyph's plain text instead. Corrected the _encodable and _resolve_glyphs docstrings per the review (StringIO.encoding is None, not absent; switched to MarkupFreeConsole.encoding instead of reaching through console.file). Reworded a docstring near _PRICING_SOURCE_CELLS that implied ASCII literals in general need no fallback rather than these four specifically. Test changes: added connection_note to the _prov helper and parametrized the cp1252 crash test over two diagnostic shapes crossed with no flags, --check and --models, since --check/--models are the only paths that render the Connection column at all. Fixed a misattributed comment and a tautological assertion in the same test. Filled in the Credentials and Notes cells in the missing-tier test so the dash assertion can only pass because of the branch under test.
…rectly (microsoft#498) * fix(plugins): resolve plugin flavor (Claude vs Copilot) manifests correctly Adds copilot_settings.py to parse Copilot-flavored plugin manifests and disambiguate resolution between Claude-plugin and Copilot-plugin conventions across manifest, marketplace, agents, and registry resolution. Updates validator, executor, and provider wiring (claude_agent_sdk, copilot) plus capability declarations to match. * fix(plugins): address code review findings for microsoft#497 flavor resolution Fixes seven blocking findings from PR microsoft#498's review: - agents.py: a Claude-flavor doc file with its own valid frontmatter (Docusaurus/Jekyll/Hugo/MkDocs) but no `name`/`description` no longer aborts the whole plugin as `PluginManifestError`; a new PluginNotAnAgentError treats it as documentation, matching the no-frontmatter case. - marketplace.py: a broken secondary catalog convention (e.g. a corrupt `.github/plugin/marketplace.json` alongside a healthy `.claude-plugin/marketplace.json`) no longer hard-fails the whole marketplace; only the primary convention is fatal. - marketplace.py: `plugin:` narrowing no longer collapses `flavored` to one key, which was silently suppressing the flavor-fallback warning; the key set is preserved and narrowing also finds a plugin published only in the secondary catalog. - registry.py: `_copilot_settings_marketplace_root` no longer discards the real cause of a settings-registered marketplace's failure behind a self-contradictory "neither declared nor installed" error. - registry.py: the installed-plugin flavor tie-break now determines a candidate's flavor from its manifest's location (not a full JSON parse), so a corrupt-but-flavor-matching build is still picked correctly instead of silently falling through to the wrong candidate; a genuine no-match now raises rather than silently picking one. - Added the missing regression tests at the executor/engine seam (claude-flavor plugin subagents actually reaching the provider, and two providers on a dual-catalog marketplace each getting their own build) and for the two provider `plugin_flavor` constants. - Added a CHANGELOG entry and a `conductor plugin list` test covering the flavor-aware cache key. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ent (microsoft#499) read_copilot_marketplaces() takes `home` as an explicit parameter, and its own docstring says why: "a parameter rather than a lookup so tests never read the developer's real ~". It then expanded a settings entry's leading `~` with os.path.expanduser(), which reads the ambient environment and so does exactly what the parameter exists to prevent. That contradiction was invisible on POSIX and broke Windows CI at d3c43a1. posixpath.expanduser reads $HOME; ntpath.expanduser prefers %USERPROFILE%. test_plugins' autouse isolation fixture sets both to an `isolated-home` directory, and the tilde test overrode only $HOME — so the expansion agreed with the test's `home` on Linux by luck and resolved to `isolated-home` on Windows. Expand a leading `~` against the `home` argument instead. The `~` in <home>/.copilot/settings.json names that same home, so this is the correct reading of the file, and it is behaviour-identical in production: every caller resolves `home` to Path.home(), which is itself expanduser('~'). `~otheruser` still defers to the ambient lookup, the only thing that can resolve another account's home. The test no longer overrides $HOME, so it now asserts the real contract — the ambient home stays pointed elsewhere, and the expansion must ignore it. Verified to fail on Linux against the old implementation, so the regression is caught on any platform rather than only on Windows. Adds a bare-`~` case for the new branch. Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rosoft#432) (microsoft#491) * docs(mcp): add solution design for `conductor mcp serve` (microsoft#432) Adds the engineering/architecture solution design for exposing a user's registry workflows as MCP tools, so any MCP host (Claude Code, VS Code, Cursor) can invoke a governed, routed, budget-capped, checkpointed workflow as a typed tool call instead of driving the CLI through a hand-written skill. Covers problem statement, goals/non-goals, requirements, the proposed design, alternatives considered, dependencies, impact, security, risks, and open questions. Absorbs and supersedes microsoft#135. Every claim about Conductor's own code is cited by file and symbol against `main` at 0554517; MCP spec and Python SDK claims are checked against primary sources, with several inherited from the source issue corrected inline. Two load-bearing SDK behaviours were verified empirically across both major versions (DD0). Filed under docs/projects/<project>/ to match the existing design docs (aca, fleet-manager, web-ui, agent-sdk). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(mcp): add implementation plan for `conductor mcp serve` (microsoft#432) Consumes the already-reviewed solution design and breaks it into 14 epics (E1-E14): bounding the `mcp` SDK dependency, the terminal run record and its retention, surfacing completed runs in `status` / `fleet list` / History, registry index fields and parse cache, the `mcp:` workflow block, the catalogue builder, the stdio server, detached invocation with a bounded wait, run lifecycle tools, `introspect` / `diagnose`, discovery above the tool cap, the `doctor` MCP section, and docs. Every path, symbol, and line reference is grounded against the tree at b6c5b11, and the pinned mcp 1.28.1 SDK surface was exercised in this repo's own venv rather than taken from the design's report. Four gaps the design left open were put to a stakeholder and are recorded as plan-level decisions R1-R4. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E1: Bound the mcp SDK dependency (DD0) Bound the mcp SDK dependency to >=1.28.1,<2 in pyproject.toml to prevent a lock refresh from silently pulling mcp 2.x, which renamed the camelCase Tool.inputSchema/CallToolResult.isError attributes read by the existing MCP client (mcp/manager.py:207) — a runtime AttributeError the module's except ImportError guard cannot catch. - Re-resolved uv.lock (mcp stays pinned at 1.28.1) - Added tests/test_mcp/test_sdk_bound.py regression suite asserting the SDK attribute surface and the declared specifier bound - Documented the fix in CHANGELOG.md under Unreleased/Fixed - Marked all five E1 tasks (E1-T1..T5) and acceptance criteria as DONE in docs/projects/mcp-server/conductor-mcp.plan.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E2: The terminal run record (DD1, P3, G5) Add TerminalRunRecord (frozen dataclass, fully absence-tolerant from_dict) and its read/write/remove API in fleet/records.py, stored under run_records_dir()/terminal/ — a subdirectory deliberately invisible to the three functions that non-recursively glob run_records_dir(). Wire the write into both run_workflow_async and resume_workflow_async in cli/run.py: each captures its terminal status/output/error on every exit path (clean success, explicit WorkflowTerminated, or an unexpected exception) and writes the tombstone in the existing finally block, immediately before the live record is removed, using a never-raising helper. Token/cost/unpriced- agent totals are read unconditionally from engine.get_execution_summary()['usage']. A resumed run replaces its predecessor's terminal record rather than duplicating it. A process that is kill -9'd (or otherwise dies before the finally runs) is documented and tested to leave no terminal record. Mark E2 DONE in the MCP server plan (epic status, all seven task rows, acceptance criteria). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E3: Terminal-record retention (DD13, FR12) Bound terminal run records by [fleet.retention].keep_last, pruned in the same sweep as the event log they point at, so a run_id resolves completely or not at all. - Terminal run records are treated as a fourth companion of their events log (alongside .bg.stderr.log/.bg.stdout.log), matched by the run_id embedded in the events log's filename - Added an orphan-only sweep for terminal records whose events log has already disappeared, sorted newest-first by ended_at, sharing keep_last and the keep_last < 1 guard - Liveness sourced from the same _live_event_log_paths() call already made by the main sweep (no second read_run_records() call) - Fixed run_id extraction regex to correctly handle the full alphanumeric/hyphen/underscore run_id contract instead of only hex characters Files: src/conductor/fleet/retention.py, tests/test_fleet/test_retention.py, docs/configuration.md, docs/fleet.md, docs/projects/mcp-server/conductor-mcp.plan.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E4: Surface completed runs in status, fleet list, and History - conductor status and conductor fleet list now surface recently-completed runs alongside live ones, bounded by [fleet.retention].keep_last, with a new --live flag on both to restore the pre-change 'live only' scope - HistoryEntry gained output/error_type/error_message fields, enriched by joining a matching TerminalRunRecord by run_id after the existing single-pass log scan - Fleet Manager TUI History screen surfaces failure reason / rendered output via row selection, extending the existing replay-command notification - Fixed: completed rows in conductor fleet list's table now show started_at (not ended_at) in the Started column Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E5: Registry index fields, offline ref pointer, and parse cache - T1: optional input/mcp fields on WorkflowInfo (config/schema.py, registry/index.py) - T2: SHA-keyed parse cache (_meta/<sha>/tools.json + tools.complete sentinel) via save_parsed_tools/load_parsed_tools (registry/cache.py) - T3: offline ref->SHA pointer (_meta/_refs/<slug>.json) modelled on plugins/fetch.py (registry/cache.py) - T4: allow_network seam on fetch_workflow/fetch_workflow_adhoc/resolve_and_fetch resolving from cache and raising typed RegistryError on a cold pointer - T5: comprehensive tests including the load-bearing "every function in registry/github.py patched to raise" test (tests/test_registry/test_cache.py) - Fix: corrected test_a_build_that_ignores_the_fields_still_loads to exercise a genuine legacy (pre-E5) model ignoring unknown fields, per review feedback Includes a minimal McpConfig prerequisite type in config/schema.py for E5-T1; E6 itself remains unimplemented. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E6: Implement the mcp: workflow block (DD4, FR11) - Wire WorkflowDef.mcp onto the existing McpConfig model in config/schema.py - Add validator cross-checks for _wait_seconds reserved-input collision and unslugifiable workflow names (_validate_mcp_exposure, slugify_workflow_name) - Add _report_mcp(...) CLI reporting modelled on _report_plugins - Document the mcp: block in docs/workflow-syntax.md - Add examples/mcp-serve.yaml - Add tests/test_config/test_mcp_block.py and extend tests/test_cli/test_validate.py - Mark E6 DONE in docs/projects/mcp-server/conductor-mcp.plan.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E7: Implement catalogue builder — exposure, schema ladder, naming, pinning Turns registry/CLI configuration into a frozen, immutable list of mcp.types.Tool objects at startup with zero process launching and zero network I/O on a warm cache. - options.py: frozen ServeOptions dataclass holding every startup argument (registries, workflow_dirs, allow, deny, toolsets, max_direct_tools, max_wait_seconds, tool_prefix, max_concurrent_runs, introspect_full). - naming.py: slugify() delegates to config.validator.slugify_workflow_name; build_tool_names() computes base slugs, qualifies all colliding identities with their registry (never only the loser, DD10), and applies --tool-prefix last. - sanitize.py: sanitize_description() strips control chars, invisible/ bidi-override Unicode, and instruction-marker shapes, and hard-caps length at 500 chars (NFR4). - toolgen.py: maps all 5 InputDef types to JSON Schema, injects the reserved _wait_seconds parameter, rejects workflows that declare _wait_seconds themselves, and publishes no outputSchema (DD5). - pinning.py: Pin dataclass (sha for GitHub registries, content hash for path registries and --workflow-dir); recheck helpers report drift without mutating catalogue state (DD6, DD3). - catalogue.py: build_catalogue() wires the four-rung exposure ladder and three-tier schema ladder together, degrading unparseable workflows to a permissive schema instead of dropping them (NFR2). Marks E7 and all E7-T1..T10 tasks/acceptance-criteria DONE in docs/projects/mcp-server/conductor-mcp.plan.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E8: Implement MCP stdio server (CLI, transport, tools/list) Adds `conductor mcp serve` Typer sub-app wired to the low-level mcp.server.lowlevel.Server, publishing a byte-identical tools/list response built from the frozen E7 catalogue over stdio (DD3). Keeps stdout protocol-pure by routing all server-side messages, including the FR10 startup summary (exposed tool/workflow counts, collisions, degraded schemas), through a dedicated stderr console (DD9). - New src/conductor/cli/mcp.py: `mcp serve` sub-app with --registry, --allow, --deny, --workflow-dir, --toolsets, --max-direct-tools, --max-wait-seconds, --tool-prefix, --max-concurrent-runs, and --introspect-full flags - New src/conductor/mcp/serve/server.py: catalogue -> lowlevel Server wiring, stdio transport, startup summary - Server reports Conductor's own package version in serverInfo.version - FR10 collision summary names every distinct registry/workflow pair, including same-registry collisions - Registered mcp sub-app on the root app (Environment panel) - Tests: in-memory stream pair drive of initialize/tools/list with cross-connection identity check, CLI flag/help/stdout-purity tests Marks epic E8 DONE in docs/projects/mcp-server/conductor-mcp.plan.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E9: Invocation: always detached, bounded wait, bounded fleet (FR4, FR5, G3, G4, DD2, R3) - Add build_typed_launch_inputs to fleet/launch.py for JSON-typed MCP inputs - Extend max_concurrent_runs docstring in mcp/serve/options.py - Add src/conductor/mcp/serve/invoke.py: tool dispatch, launch_background invocation (web_port=0, hardcoded skip_gates=False per DD11), FR5 wait resolution, bounded polling loop, result shaping (structuredContent + text, resource_link spill per NFR6), and LaunchTracker for R3 concurrency bounding - Add tests/test_mcp/test_serve_invoke.py - Mark E9 DONE in docs/projects/mcp-server/conductor-mcp.plan.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E10: Implement run lifecycle MCP tools (FR6, FR7, DD11) Add conductor_run_status, conductor_await_run, conductor_cancel_run, and conductor_list_runs plus the shared resolve_run/RunLookup three-source resolver in src/conductor/mcp/serve/runs.py. - resolve_run(run_id) tries a live run (read_run_record + derive_run_summary), then read_terminal_record, then find_event_log_for_run as a crash fallback, naming which source answered. - conductor_run_status shapes each source into a uniform status dict, including gate prompt/options/option_details and approval URL when at a gate. - conductor_await_run bounds polling at 2s cadence, returns early on terminal or at-gate status, and emits progress notifications when a token/sender are supplied. - conductor_cancel_run reuses cli/app.py::stop_records for the graceful stop ladder, reporting stopped/failed/already_terminal honestly. - conductor_list_runs unions live and terminal run records, dedupes by run_id (live wins), and filters by status/workflow. Updates docs/projects/mcp-server/conductor-mcp.plan.md: E10 and E10-T1..T6 marked DONE, acceptance criteria checked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E11: Implement introspect and diagnose MCP toolsets (FR8, DD12, NFR6, R4) Add two opt-in MCP toolsets, both off by default (DD3): - introspect (conductor_run_events, conductor_node_detail, conductor_plan_tree) in src/conductor/mcp/serve/introspect.py - diagnose (conductor_doctor, conductor_validate_workflow, conductor_run_logs) in src/conductor/mcp/serve/diagnose.py All six are thin adapters over existing Fleet Manager / diagnostics / validate machinery. R4's redaction posture is applied specifically where raw tool-call payloads live: conductor_run_events replaces agent_tool_start.arguments and agent_tool_complete.result with {name, status, byte_size} unless --introspect-full is set, while conductor_node_detail returns prompt/output in full and is proven (not assumed) to never carry a tool payload. conductor_run_logs follows DD12: ResourceLink content blocks plus bounded per-file metadata, never file bytes. - --toolsets validation added at ServeOptions construction, rejecting unknown toolset names at startup and surfacing the enabled set in the startup stderr summary (E11-T1). - Wired tools/list and tools/call for both toolsets in server.py, gated on options.toolsets. - Rejected tool-name collisions between generated workflow tools and the introspect/diagnose tool set at build_server time. - Fixed invoke.py's workflow-dir resolution to fail closed (UnknownToolError) rather than ambiguously rescanning when a recorded source path disappears. - Fixed conductor_run_events' event_types argument handling to distinguish an explicit empty filter from an absent one. Updates docs/projects/mcp-server/conductor-mcp.plan.md: E11 and E11-T1..T6 marked DONE, acceptance criteria checked. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E12: Implement discovery fallback above the tool cap (FR9, DD3, G7) - discovery.py: conductor_find_workflow(query) searches the frozen catalogue's own name/description/registry fields; conductor_run_workflow dispatches through invoke_workflow_tool, the same invocation layer a generated per-workflow tool uses, so a path-shaped/URL-shaped/registry- source-shaped `name` is refused the same way any unrecognized tool name is (NFR3) -- there is no separate shape check to bypass. - server.py: build_server now acts on catalogue.mode (decided once, at startup, by E7's build_catalogue): direct mode publishes the catalogue's per-workflow tools as before; discovery mode publishes the fixed conductor_find_workflow/conductor_run_workflow pair instead, never both. Both tools/list and tools/call are gated on the mode captured in build_server's closure, so it can never vary within or across a connection (DD3). One LaunchTracker is shared per server process (R3). - tests/test_mcp/test_serve_discovery.py: search/dispatch behavior, path- shaped-name refusal, above/below-cap tool-list and tool-call gating, mode stability across repeated calls/connections, and the startup log naming the exposed count and --max-direct-tools threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E12: Address review feedback on discovery fallback (FR9, DD3, G7) - conductor_find_workflow: bound result count (_MAX_RESULTS=25) and report total count/truncation - conductor_run_workflow: validate flattened inputs against the resolved catalogue entry's inputSchema before dispatch, raising LaunchError on mismatch (parity with direct-mode tools) - build_server: scope collision checks to tools actually published together, so discovery mode no longer checks against hidden catalogue names - build_server/_call_tool/_dispatch_discovery_tool: thread the request's progressToken and send_progress_notification into the discovery pair's dispatch for end-to-end progress reporting Resolves all four round-one review blockers. Focused discovery suite (31 tests) and full MCP suite (327 tests) pass; ruff and ty clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E13: Add conductor doctor MCP section (Impact Analysis → Operational) Adds a new `mcp` section to `conductor doctor` that shows what `conductor mcp serve` would expose as MCP tools, without starting a server or attaching a host. - Added `McpServeDiagnostic` + `gather_mcp_serve()` in providers/diagnostics.py, wrapping the existing E7 mcp.serve.catalogue.build_catalogue() pipeline offline (allow_network=False), wired into gather()/ALL_SECTIONS/DoctorReport. - Added McpServeToolInfo, McpServeCollision, McpServeRejectedWorkflow, and McpServeFailedRegistry dataclasses with to_dict() serialization. - Added a thin Rich-table renderer `_render_mcp_serve()` in cli/doctor.py following the existing `_render_registries` convention, covering tools, collisions, rejected workflows, and failed registries. - Updated `conductor doctor --help` text/examples in cli/app.py to mention the mcp section. - Added FailedRegistry dataclass and Catalogue.failed_registries field in mcp/serve/catalogue.py; threaded a failed_registries accumulator through build_catalogue/_collect_registry_candidates so a whole-registry resolution failure is captured structurally instead of only logged. - Marked E13 DONE in the MCP server plan document. Targeted suite: 382 tests passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * E14: Documentation and release for conductor mcp serve Documents the previously-undocumented conductor mcp serve feature (epics E1-E13): a new user-facing guide (docs/mcp-server.md) with host configuration snippets, the exposure ladder, toolsets, the mcp: workflow block, run lifecycle, and an explicit Limits section; a disambiguation/cross-link with the existing MCP client docs (docs/mcp-tools.md); a CLI reference entry for conductor mcp serve (docs/cli-reference.md); AGENTS.md architecture updates; and CHANGELOG entries. Also fixes two bugs surfaced during doc verification: - direct-mode generated workflow tools are now dispatchable through tools/call (previously listed but uncallable) - the default runs toolset (conductor_run_status/await_run/cancel_run/ list_runs) is now both listed in tools/list and dispatched in tools/call, matching documented default behavior make check and make validate-examples are green; make test is green except one pre-existing, environment-dependent failure unrelated to this epic's changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: restore web/ top-level bullet in AGENTS.md E14-T4 replaced the `- **web/**: Real-time web dashboard for workflow visualization` bullet with the new `- **mcp/**:` bullet instead of adding it, orphaning auth.py/server.py/frontend/static (all of which live in src/conductor/web/) as children of the mcp/ section and leaving web/ undocumented as a top-level package. Restore the web/ bullet and drop the stray doubled blank line left by the insertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(mcp): steer callers toward the non-blocking path (microsoft#432) An MCP-launched workflow was always detached, but the `_wait_seconds` description offered three equally-weighted options with no recommendation, so a calling model passed `_wait_seconds: 300` unprompted and held its turn open for a five-minute run -- which reads to a user as "the MCP server runs workflows in the foreground". Non-blocking is the intended path, so the tool schema now says so: - `_wait_seconds` leads with "leave this unset", states the run is detached either way, and says a value blocks only the caller's own call. `server.py::_DISCOVERY_TOOLS` hand-writes a second copy of that parameter, so `TestWaitSecondsSteersTowardBackground` asserts both copies steer identically. - The run handle carries `port`, an `observe` block of terminal commands (`conductor fleet` / `conductor status` / `conductor stop --port N`), and the capture-log paths -- so a caller has something to report instead of a reason to wait. - The immediate `next_action` no longer leads with `conductor_await_run`; it names `conductor fleet` first and conditions the blocking call on the user having actually asked to block. - Documented `--max-wait-seconds 0` as the operator-side hard guarantee: the ceiling applies to every blocking path including `mode: sync`. Every command in `observe` is resolved against the real Typer app by `test_every_observe_command_actually_exists_in_the_cli` -- a draft had fabricated `conductor fleet list --json`, which does not exist, and a plausible-looking invented command fails in front of the user at the exact moment they were told how to watch their run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(mcp): reconcile MCP serve with the fleet APIs main moved (microsoft#432) Two APIs this branch was written against changed on main while it was open, in ways a textual rebase cannot catch -- both are new files here, so they applied cleanly and then failed at import and call time. - `fleet/summary.py::read_event_log_full` was removed by microsoft#485, which replaced the bounded tail/head/full-log readers with the streamed, uncapped `stream_event_log`. That generator deliberately propagates `OSError` from its first `next()`, where the old reader returned `[]`. Every caller here is an MCP tool handler answering a question about some *other* process's log, where an unreadable or vanished file is an ordinary outcome rather than a server fault -- so `runs.py` grows one `read_event_log_events` adapter that materialises the stream and keeps the never-raise contract in a single place, rather than each call site growing its own `try`. - `fleet/records.py::_RUN_ID_PATTERN` was replaced by microsoft#435's shared `conductor.run_id::is_valid_run_id`, which the surrounding live-record functions already call. The terminal-record functions added here still referenced the removed module-level pattern, raising `NameError` on every `read_terminal_record` / `remove_terminal_record` call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(mcp): strip ANSI from the serve --help flag assertions (microsoft#432) `typer.rich_utils` evaluates `FORCE_TERMINAL` at import time and sets it whenever `GITHUB_ACTIONS` is present, so on CI the help panel is styled even though nothing is a TTY. Its option highlighter emits the leading dash as its own span (`\x1b[1;36m-\x1b[0m\x1b[1;36m-registry\x1b[0m`), so the literal `--registry` never appears in the raw output and every flag assertion in `test_serve_help_renders` failed on all three CI platforms. The flag is set before any test can patch the environment and pinning `COLUMNS` does not disable colour, so strip the SGR escapes before asserting -- the same `_ANSI_RE` approach `test_replay_command.py` already uses. Width is pinned wide as well, mirroring `test_help_panels.py`, so a long flag such as `--max-concurrent-runs` cannot be wrapped mid-token in a narrow option column. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(doctor): write registry paths as TOML literal strings (microsoft#432) `TestGatherMcpServe`'s two end-to-end fixtures interpolated a real `tmp_path` into a TOML *basic* string, which is parsed for escape sequences. On Windows that path opens an invalid `\U` unicode escape (`C:\Users\...`), so `registries.toml` failed to parse and both tests saw a `McpServeDiagnostic.error` instead of the catalogue they asserted on -- a Windows-only failure the branch's first CI run surfaced. TOML literal strings perform no escape processing, which is TOML's own answer for paths, so render the source through a small `_toml_str` helper. The hardcoded `owner/repo` source a few lines below has no backslashes and is deliberately left alone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Finalizes the 0.1.36 release section, bumps the project version, and re-locks uv.lock. Also repairs a changelog defect introduced when microsoft#491 was rebased: the `conductor mcp serve` Added/Changed entries had landed inside the already-published [0.1.34] section, and the `mcp>=1.28.1,<2` Fixed entry inside [0.1.31]. Both are relocated verbatim into [0.1.36], where the work actually ships. Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rosoft#494) Bumps the github-actions group with 1 update: [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv). Updates `astral-sh/setup-uv` from 10.0.0 to 10.0.1 - [Release notes](https://github.com/astral-sh/setup-uv/releases) - [Commits](astral-sh/setup-uv@ae62891...20cfd1b) --- updated-dependencies: - dependency-name: astral-sh/setup-uv dependency-version: 10.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…microsoft#502) * feat(claude-agent-sdk): opt-in setting_sources for target-repo skills The provider passed setting_sources=[] unconditionally, so no workflow could load the settings tier of the repository it operates on. An agent working a target repo could not use that repo's own .claude/skills/, CLAUDE.md, or .claude/rules/*.md; those conventions had to be duplicated into the workflow's prompts. Adds runtime.provider.setting_sources (user/project/local), defaulting to [] so behaviour is unchanged unless a workflow asks. The empty default is load-bearing rather than cosmetic: the SDK re-defaults an unset setting_sources to ["user", "project"] whenever skills is set, so [] has to be sent explicitly to keep a run hermetic. Two things this needed beyond the plumbing, both found by running it: - Grant the Skill tool when setting_sources is non-empty. CLI-discovered skills never pass through skill_names, so gating on that alone listed a repo's skills to the model with no tool to invoke them. - Resolve ClaudeAgentOptions.skills to "all" when tiers are enabled and the workflow named no skills. Otherwise every call failed with "not in this session's skills allowlist" — observed with 28 discovered skills, all rejected. A declared skills:/plugins: list still wins; discovery does not widen what the author asked for. Note that `project` loads the whole tier, hooks included: pointing it at untrusted code runs that code's hooks. Documented at the schema field. Verified against claude-agent-sdk 0.2.87 with a fixture repo whose .claude/rules is a symlink to a tool-agnostic .agents/rules and a CLAUDE.md that never references it. With [] the agent reports no rule; with ["project"] it returns the rule's contents. Skills, CLAUDE.md and rules all arrive through the one tier, symlinks included. Refs microsoft#501 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(claude-agent-sdk): enforce, serialize and scope setting_sources Addresses review on microsoft#502. The field was added to the schema and the provider without the enforcement, serialization and per-agent scoping every other structured provider field already has. - Reject `setting_sources` unless `name: claude-agent-sdk`. Only that factory branch reads it, so on any other provider it was accepted and silently dropped -- worst on `aca`, where the runner's four-key `inner_provider_settings` allowlist never forwards it to the sandbox. - Count it in `has_structured_config()`, so the model serializer no longer collapses the object to a bare string (the opt-in survives a `model_dump` round-trip), `--provider` overrides warn before discarding it, and `_describe_provider` shows it under `-v`. - A per-agent `skills: []` now opts that agent out of the tiers entirely, hooks included, keeping it the one opt-out. The tier is workflow-global while `working_dir` is per agent, so without this an agent that asked for no skills got every skill in the target repo. - Warn once per provider when tiers are enabled, naming hooks and the actual scope. Nothing in the run output distinguished a run that loaded the target repo's hooks from one that did not. - Carry the `Literal["user", "project", "local"]` narrowing through the constructor and `_resolve_skill_filter`, and guard the factory read by provider name instead of `getattr`. Against the real SDK symbols `ty` now reports no `invalid-argument-type` at the options call site. - Correct the comments describing the `"all"` branch: `skills=[]` alongside a tier empties the session allowlist and *hides* the discovered skills from the model's listing rather than causing a rejection loop, so the failure being avoided is "load the repo's skills, then hide every one". `"all"` is kept deliberately. - Docs: AGENTS.md, comparison.md, experimental.md and skills/discovery.py stated the empty `setting_sources` as an unconditional guarantee; they now say "empty by default, opt-in per workflow" and carry the trust caveat. Adds the CHANGELOG entry, a compatibility-table row, a workflow-syntax section, and `examples/claude-agent-sdk-setting-sources.yaml`. - Tests: `TestSettingSourcesWiring` no longer swallows six `TestMcpOptionsWiring` tests (it opened without closing the previous class); it now sits after `TestSkillsWiring` and reuses its `_capture_options`/`_argv` helpers. Adds argv assertions for the bare `Skill` grant on the `"all"` path, the `skills: []` opt-out, declared skills not widened by a tier, the omitted-`tools:` preset path, tier parametrization over user/project/local/all-three, schema rejection per provider, and a serialization round-trip. Drops three weaker duplicates and renames `test_setting_sources_isolated_unconditionally` to `..._by_default`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(claude-agent-sdk): cover the tier warning, --provider discard and -v output Follows up the review fixes: the hook warning, the `--provider` override discard path and `_describe_provider` had no test behind them, and the `skills: []` opt-out asserted only the absence of `--allowedTools` with no positive argv anchor beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…c-ai providers (claude, openai) (microsoft#503) * build(deps): add pydantic-ai-harness and bump pydantic-ai-slim floor * feat(providers): add context-window and output-limit resolution cascades for compaction * feat(providers): assemble tiered compaction capability for pydantic-ai agents * feat(providers): enable always-on tiered compaction for claude and openai providers * feat(providers): emit compaction lifecycle events for pydantic-ai agents * feat(cli): render compaction lifecycle events in console output - Add ConsoleEventSubscriber branches for agent_compaction_config, start, and complete. - Use styled()/join() for markup-safe interpolation per AGENTS.md rules. - Extend test_logging.py with config/start/success/failure render tests. - Add JSONL verbatim round-trip test and replay skip-list guard. * feat(dashboard): show compaction lifecycle events in the activity stream - Add agent_compaction_config/start/complete to the EventType union and payload interfaces. - Append activity-log entries for compaction lifecycle events on the active node. - Add Vitest coverage for config/start/complete/success/error and replay-state neutrality. - Regenerate static/ assets via make build-frontend. * docs: document automatic context compaction for pydantic-ai providers * feat(providers): unify default max_tokens at 16384 for claude and openai * feat(providers): resolve effective anthropic max_tokens and read model output cap for compaction * fix(providers): additive compaction reserve from effective max_tokens and tool-output config * docs: align compaction reserve docs with effective-max-tokens formula * feat(providers): add vendor model-listing token-limit parser for compaction * refactor(providers): drop the global compaction context-window env override * fix(providers): paginate the full Anthropic model listing for token-limit metadata * feat(providers): read vendor-advertised model token limits in the OpenAI provider * fix(providers): handle OpenAI SDK AsyncPaginator in model metadata fetch * test(providers): pin compaction cascade behavior with provider-advertised limits * docs: document provider-advertised token limits and drop the window env override * fix(providers): harden compaction metadata parsing and close lifecycle Defensive model-listing parser for hostile Mapping/property access. Broad exception handling in Claude metadata cache population. Reset Claude unavailable-listing latches in close(). Restore OpenAI get_model_capabilities token fields to None and plain in membership. Add regression tests for parser hostility, proxy-prefix negative pin, and close reset. * fix(providers): emit compaction start event before the inner strategy runs The fail-open wrapper emitted agent_compaction_start only after the inner tiered strategy (including the summarizer's network call) had already returned, and hardcoded elapsed to 0.0, so console/JSONL/dashboard showed an instant start/complete pair instead of a live lifecycle. Emit start immediately before delegating when the estimate crosses the trigger, and measure the real elapsed time. * fix(providers): warn only for explicit models and single-flight model listing The available-models warning fired for the hardcoded provider default (e.g. gpt-5-mini) even when every agent overrides it with its own model:, producing a misleading warning on proxies that don't list the default. Warn only when the model was explicitly requested via the constructor. Also move the models.list() fetch under the cache lock with a double-checked pattern so concurrent first-callers issue exactly one round-trip instead of a stampede, and honor a per-agent max_tokens attribute (issue microsoft#471 groundwork) when resolving the compaction output limit in both providers. * test(providers): pin explicit-model warning, listing single-flight, per-agent compaction limit Cover the companion provider fixes: no warning for the hardcoded default model when agents override it (warning retained for an explicitly requested model), exactly one models.list() call under concurrent first-callers, and the compaction output limit honoring a per-agent max_tokens attribute with source "settings". * fix(providers): address PR microsoft#503 review findings on context compaction Blocking: - measure post-compaction tokens via heuristic reclaim (mirroring TieredCompaction._escalate) so tokens_saved is non-zero on real compactions instead of always reporting before == after - replace the degenerate trigger=1 floor with resolve_compaction_plan: the tool buffer is clamped to 25% of the window, the target keeps a window-scaled 5% hysteresis margin below the trigger, and a plan with no viable headroom is disabled (reported on agent_compaction_config via enabled/disabled_reason) instead of compacting on every request - keep max_tokens off the OpenAI wire when unset; the compaction reserve still falls back to the 16384 default internally, so reasoning models keep the server's full output allowance Recommended: - fold ANTHROPIC_BASE_URL into ClaudeProvider._base_url so has_custom_base_url gates registry lookups for env-configured proxies - bound Anthropic/OpenAI model-listing drains at 2000 entries, handle partial listings explicitly, never cache an empty listing, and narrow the catch to transport errors so parser bugs surface - pin pydantic-ai-harness (<0.25) and pydantic-ai-slim (<3), and declare genai-prices as a direct dependency - delete the dead _ThresholdGatedCompaction gate (its branches were identical and duplicated the wrapper's own estimate) - split the wrapper's failure handling into three zones so a telemetry failure no longer latches compaction off or reports false failure, and name degraded tiers / still-over-trigger on agent_compaction_complete - implement AgentProvider.get_max_output_tokens on the Copilot provider - style the four compaction activity types in the dashboard, render tokens_saved, and surface disabled/degraded states - rebuild examples/compaction.yaml around a multi-turn MCP tool loop (loop-back iterations never accumulated provider history) and fix the inverted trigger-direction comment - correct docs claiming ModelInfo.max_tokens caps the wire value, the dashboard-bar refresh timing, and split the CHANGELOG entry into Added/Changed (dropping the nonexistent 64k-fallback removal note)
* fix(providers): retry transient OpenAI stream errors
* fix(providers): match stream-error retry classification to actual OpenAI payloads
The retryable set for a bare openai.APIError used Anthropic's
"rate_limit_error" string, which a real OpenAI 429 never sends — its
mid-stream rate limits arrive as type "requests"/"tokens" with code
"rate_limit_exceeded", so they still failed on the first attempt.
Widen the marker set to the vocabulary OpenAI and Anthropic-shaped
gateways actually emit, with sources cited, and treat a payload with no
parseable type (a non-object error value from an Ollama/vLLM gateway,
or an Azure-style {"code": ...} shape) like a broken stream: by the
time a stream has started, auth and request validation have passed.
The module docstring also claimed only the translated ModelHTTPError/
ModelAPIError types are relied on at runtime — now false, since the
bare-APIError branch depends on pydantic-ai continuing not to translate
it. Rewrite it to say so, drop the stale 1.44.0 floor note, and pin the
upstream assumptions with canary tests: pydantic-ai's _map_api_errors
leaves a bare APIError untranslated, and a real openai.Stream puts the
SSE error object itself into APIError.body. Every marker in the set is
now pinned in both payload positions, and the exact-type guard has a
test that fails under an isinstance mutation.
* fix(providers): surface declined and retry_on-filtered retries honestly
Three gaps in how execute_with_retry reports a failure the retry policy
did not take:
- A retryable error declined by a narrowed retry_on filter escaped
through a bare raise, so a raw SDK exception (e.g. a mid-stream
openai.APIError) flew past callers that catch ProviderError. The gate
now wraps it in a non-retryable ProviderError naming the declined
category, chained to the original.
- A retry taken logs a warning and emits agent_retry; a retry declined
logged only at debug, which Conductor's default no-handler logging
never emits. The decline is now logged at warning level too.
- A bare openai.APIError's str() carries neither the payload type/code
nor a non-object body, and the SDK substitutes a generic "An error
occurred during streaming" message, so a fatal stream error told the
user to check their API key with no evidence attached. The
ProviderError message now includes the payload details via
_describe_stream_error.
…oft#507) * fix(providers): guard compaction against token estimate drift * fix(providers): rework the compaction window guard after review The byte-bound guard added to catch token estimate drift had four defects surfaced in review: - It delegated to TieredCompaction.before_model_request, which re-gates on the same ~4-chars-per-token heuristic that under-counted the content, so the guard no-opped exactly where it was needed while still emitting a success-shaped agent_compaction_complete. - A raw UTF-8 byte count runs ~4x a real token count for English prose, so the guard tripped at roughly a quarter of the window on ordinary text, producing a per-request no-op loop and a spurious user-facing warning. - before_estimate was promoted to max(primary, bytes), corrupting every downstream telemetry field (tokens_before, tokens_saved, and still_over_trigger, which read True for essentially every guard-triggered compaction). - The regression test hard-coded a target_tokens the production resolver never produces; under the real formula the motivating scenario did not compact at all. Rework the guard around a density-calibrated estimate instead of a byte count. _density_text_token_bound matches the primary heuristic on ordinary prose, counts text with a substantial non-ASCII share at ~1 token per character, and whitespace-poor ASCII blobs at ~2 characters per token, so the guard fires only on genuinely token-dense content. When it fires, the tier chain is driven directly against that measurement (with the request's model context, mirroring the harness's context_for_request) until the estimate fits the target — the inner strategy's heuristic gate can no longer veto the safety compaction. Telemetry stays on the token scale: agent_compaction_start gains trigger_reason ("trigger" / "window_guard") and a separate density_tokens field, and agent_compaction_complete gains degraded_estimators and still_over_window so a guard compaction that cannot get back under the window reads as degraded rather than as false success, and cli/run.py renders both. The primary-failure fallback is now genuinely independent (it walks the message list directly with a per-part guard instead of sharing the harness text-collection path that took the old fallback down with the primary), and a double failure emits a dedicated agent_compaction_skipped event with reason="estimate_unavailable" instead of vanishing into stderr. Tests derive their configuration from resolve_compaction_plan so they cannot pass against an unreachable parameterisation, use genuinely token-dense fixtures (CJK) instead of repeated ASCII punctuation, assert the safety post-condition (density estimate back under the window) rather than a harness keep_messages constant, and cover the previously untested error branches: double estimator failure, density-only failure, shared harness failure, the inclusive window boundary, and a negative test that ordinary prose below the trigger neither compacts nor emits events. Docstrings that described the removed single-measurement contract are rewritten, and the change is documented in the changelog, the workflow syntax guide, and AGENTS.md.
Bumps [httpcore2](https://github.com/pydantic/httpx2) from 2.3.0 to 2.10.0. - [Release notes](https://github.com/pydantic/httpx2/releases) - [Commits](pydantic/httpx2@v2.3.0...v2.10.0) --- updated-dependencies: - dependency-name: httpcore2 dependency-version: 2.10.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@vitest/mocker](https://github.com/vitest-dev/vitest/tree/HEAD/packages/mocker) to 5.0.0 and updates ancestor dependency [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest). These dependencies need to be updated together. Updates `@vitest/mocker` from 3.2.7 to 5.0.0 - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v5.0.0/packages/mocker) Updates `vitest` from 3.2.7 to 5.0.0 - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v5.0.0/packages/vitest) --- updated-dependencies: - dependency-name: "@vitest/mocker" dependency-version: 5.0.0 dependency-type: indirect - dependency-name: vitest dependency-version: 5.0.0 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [httpx2](https://github.com/pydantic/httpx2) from 2.10.0 to 2.12.0. - [Release notes](https://github.com/pydantic/httpx2/releases) - [Changelog](https://github.com/pydantic/httpx2/blob/main/src/httpx2/CHANGELOG.md) - [Commits](pydantic/httpx2@v2.10.0...v2.12.0) --- updated-dependencies: - dependency-name: httpx2 dependency-version: 2.12.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…s_dir (microsoft#514) * feat(claude-agent-sdk): select the project settings tier with settings_dir `working_dir` was doing two unrelated jobs on this provider. The Claude CLI supports MCP Roots and advertises exactly one root -- its cwd -- so a filesystem MCP server discards the directories in its own argv and permits cwd alone. That makes cwd the only handle on what an agent can read, while it is simultaneously the directory the `project` settings tier resolves against. Narrowing cwd onto a target repository to pick up that repository's skills therefore narrowed the agent's MCP root below any sibling path the step still had to read; widening it back lost the repository's conventions. Add a per-agent `settings_dir`, forwarded to `ClaudeAgentOptions.add_dirs`, so the two are separable: the skills of the directory named there are discovered and invocable regardless of cwd, leaving cwd free to stay wide enough for the agent's MCP servers. The split is deliberately partial. A directory named here contributes its `.claude/skills` and nothing else -- not `CLAUDE.md`, `.claude/rules/*.md`, `.claude/settings.json` (so no `env`, no `hooks`) or `.claude/agents`, all of which keep following cwd. It is the skills portion of a project tier, not a cwd-independent way to load one. `add_dirs` also carries an unconditional effect the skills framing hides: per the SDK's own contract it is "additional directories Claude can access", so a `settings_dir` widens the model's built-in Read/Edit/Bash to that tree with no settings tier enabled at all. It does not widen what an MCP server permits. Both effects are documented on the schema field, and `conductor validate` warns when the field is set with no `setting_sources`, since the skills half is then a no-op while the filesystem grant applies. `WorkflowEngine._resolve_agent_directory` is extracted from `_resolve_agent_working_dir` so `working_dir` and `settings_dir` resolve identically and cannot drift. `capabilities.py::settings_dir` gates the field; `config/validator.py` errors against a provider with nowhere to put it rather than dropping it silently. The Roots behaviour every option here rests on is pinned by tests/test_integration/test_mcp_roots_negotiation.py, which drives the real filesystem server with no LLM -- two runs differing only in whether the client advertises `roots`. * fix(claude-agent-sdk): address review on settings_dir Blocking: - docs/providers/experimental.md: the capability row had been rewritten with text describing per-agent `tools:` allowlist enforcement -- enumerating stdio MCP servers, refusing an allowlist alongside http/sse, an `mcp_servers_always_attached` capability. None of that exists here: `workflow_tools_passthrough` is False, `_resolve_tool_config` refuses any non-empty allowlist, and the identifier appears nowhere in `src/`. The row also lost the accurate `no workflow_tools_passthrough` carve-out. Restored to upstream's text plus one additive sentence for `settings_dir`, so the table no longer promises a security property the provider does not have. - Added `AgentExecutor._reject_unsupported_settings_dir`, mirroring the `capabilities.settings_dir` check at run time. `conductor run` never calls the static validator, and the engine resolves the directory for every provider, so `provider: copilot` with a `settings_dir` previously ran against whatever conventions its cwd supplied and exited 0. Follows the four existing `_reject_*` helpers and `session_key`'s precedent; the two docstrings that promised validate-time-only rejection now describe both. - An empty `settings_dir` reached `add_dirs` as a real filesystem grant. `Path("")` is `Path(".")`, which is not absolute, so it resolved to the workflow file's own directory and passed the `is_dir()` check. Four layers disagreed on what "set" meant; they now agree: the field takes `StringConstraints(strip_whitespace=True, min_length=1)` (matching `session_key`), the seven step-type guards use `is not None` so a blank cannot bypass them, and the engine refuses a *template* that renders empty. Recommended: - `_project_tier_enabled` replaces `_setting_sources_enabled`: `settings_dir` feeds only the `project` tier, so `['user']`/`['local']` no longer validate silently, and a per-agent `skills: []` -- which zeroes the tier in `execute` -- is now detected. - The no-skills warning branches on its cause. Under a per-agent provider override it no longer advises adding `setting_sources`, which the schema rejects unless `runtime.provider` is claude-agent-sdk. - docs/workflow-syntax.md said the field was "ignored by every other provider"; it is refused. The provider comment led with the skills effect and omitted the unconditional filesystem grant -- the grant now comes first. - The schema docstring example used a mapping under `agents:`, which the schema rejects. - `_resolve_agent_directory` takes `field` keyword-only as a `Literal["working_dir", "settings_dir"]`, so the two adjacent `str` parameters can no longer be transposed (`ty` now rejects a bad name). - The validator sub-agent sets `settings_dir=None` explicitly: it runs with `tools=[]`, so neither half would apply, and inheriting it would grant a tree it cannot use. Not switched to `model_copy`, which would also carry `validator` (recursive validation), `session_key` and `routes`. - The roots-negotiation test is marked `real_api` (it fetches from npm and pins upstream behaviour, not ours), passes the resolved `npx` path so it can launch `npx.cmd` on Windows, and writes stderr to a plain file rather than reopening a `NamedTemporaryFile` by name. - The bundled example declares its three inputs, which turns on the unknown-input check for the `settings_dir` template this PR added -- a typo there was previously undetectable by `make validate-examples`. - Tests: the missing `questions` step-type case, an argv assertion that `--add-dir` still reaches the CLI (with a negative control), the blank and empty-render refusals, all three warning causes, and the run-time capability refusal. Every new branch was mutation-tested. * feat(claude-agent-sdk): report settings_dir in the agent lifecycle events `settings_dir` is a trust decision -- it loads another repository's conventions and, unconditionally, widens the model's built-in Read/Edit/Bash to that tree -- and nothing in the run output mentioned it. Neither the dashboard nor the JSONL event log recorded which directory an agent had been granted, so the grant could not be audited after the fact. Emitted alongside `working_dir` at all three sites that already carry it: `agent_started`, `parallel_agent_started` and `for_each_agent_started`. Emitting on one path only would have left the other two silent about the same grant. Always present (null when unset) so a consumer can distinguish "no grant" from "this Conductor did not report one". * fix(claude-agent-sdk): restore the empty-render guard and close review gaps The empty-render guard added in 0b1598a had been narrowed to `field == "settings_dir"` in 9b15ba7 -- a mutation-test edit that leaked into that commit rather than being reverted. Restored to unconditional, and the `working_dir` half is now pinned by its own test, which is what would have caught the leak. Also from a further review pass: - The `working_dir` behaviour change is now declared rather than silent. An empty-rendering `working_dir` previously resolved to the workflow file's own directory and ran there; it is now an error. Recorded in CHANGELOG.md under Changed, in the shared helper's docstring, and tested in its own right so a later edit narrowing the guard back to `settings_dir` cannot silently restore the old behaviour. - CHANGELOG.md had no entry for this feature at all. Added under Unreleased/Added, alongside the Changed note above. - The validator sub-agent's comment justified `settings_dir=None` with "no skill can be invoked without the Skill tool", which is false: `_resolve_tool_config` grants `Skill` back for `tools: []`, so with a settings tier enabled the grader does hold it (measured: `(['Skill'], None)`). The conclusion stands on the other half -- no file tool for the grant to widen -- so the reason is corrected rather than the code. Pinned by a test, since the consequence is latent today and becomes load-bearing the next time a tool is granted back. - AGENTS.md still described `settings_dir` as enforced only in config/validator.py. It is enforced twice; the file now says so, in the same words the skills and plugins bullets use. - The schema docstring stated the filesystem grant as a live exposure. Conductor reaches only `bypassPermissions` (full preset, where reads already succeed everywhere) or `tools: []` (at most the `Skill` loader, no file tool), so the grant is a property of the SDK contract to design against rather than something reachable from a workflow today. The docs page already carried that hedge; the docstring, which is what surfaces on hover, did not. - `settings_dir` had no coverage inside parallel groups or for-each loops. Both are now tested, the for-each one asserting per-iteration resolution -- that site resolves after loop-variable injection, so a refactor could otherwise load a different iteration's skills unnoticed. * test(claude-agent-sdk): pin settings_dir on the two group event paths The event emission added for `settings_dir` covered three paths but only `agent_started` was tested: both group payload lines could be deleted with the suite fully green (8626 passing, and 264 passing across every file that mentions those event names). `TestSettingsDirInGroups` does not close the gap -- it asserts the value reaching `provider.execute`, which routes through `_resolve_agent_working_dir`, a different path from the payload. That is the same shape as the mutation leak the previous commit fixed: an unpinned line in a shared payload where the loss is silent. The tree already had the precedent -- `TestWorkingDirEvents` exists to pin these same two additive events for `working_dir`, with an `is None` control for each -- and the emission commit did not follow it. Adds `parallel_agent_started` and `for_each_agent_started` assertions plus a negative control, so the class now covers what its docstring claims and what `AGENTS.md` asserts. The for-each case checks per-iteration values, since that is where a templated `settings_dir` varies and most needs auditing. Verified by deleting both payload lines: all three new tests fail. Also records why the guard comment covers both fields (the `settings_dir`-only rationale sitting on that line is the reasoning that produced the earlier narrowing), and why the argv helper is local rather than reusing another test class's private one. * fix(claude-agent-sdk): warn at run time when settings_dir discovers no skills A `settings_dir` whose `project` tier is not enabled discovers nothing -- and the filesystem grant applies regardless, so the one effect the author did not ask for was the only one they got, with no diagnostic. Traced through the real provider: `add_dirs=['/target']` with `setting_sources=[]` and `skills=[]`, and no log output. `conductor validate` warned about this; `conductor run` never calls the static validator. That is the same gap this branch already closed twice for `capabilities.settings_dir`, and forgetting `setting_sources` is the likelier mistake of the two -- it lives at workflow scope while `settings_dir` is per agent, and the bare-string `provider: claude-agent-sdk` shorthand cannot carry it at all. Warned rather than raised, matching validate's own choice: the workflow is not wrong, just ineffective. Placed where `effective_sources` is already computed, so it also covers the two cases a workflow-level check would miss -- a non-`project` tier, and a per-agent `skills: []` that zeroes the tier for that agent. Parametrised over all five, and mutation-verified. Also: the schema docstring and the docs section opened with "Directory whose Claude Code *project* settings tier this agent loads", which over-promises (a reader expects CLAUDE.md, rules, env, hooks) and does not mention the grant at all. Both now lead with what it carries and what it grants; the prose already had the detail, but the first line is what a tooltip shows and the field name is what appears in every workflow file. * fix(claude-agent-sdk): latch the tier warning, and correct two stale descriptions The previous commit added a run-time mirror of the "settings_dir discovers no skills" warning and then updated only three of the five places that describe it: - AGENTS.md carried the new parenthetical AND the sentence it was meant to replace, stating the same thing twice. That file is loaded as agent instructions, so a garbled invariant there is asserted as fact by whatever reads it next. - The schema docstring still said `conductor validate` warns -- true before this branch, false after it, and an understatement of the guarantee the previous commit exists to provide. The same docstring already applies the twice-enforced idiom to the capability refusal six lines earlier. Both now match the code. The other three sites (provider message, docs section, CHANGELOG) were already correct. Also latches the warning per agent. It was firing once per execution, so a 50-item for_each emitted 50 identical lines naming the same agent and the same directory (measured: 5 warnings for 5 executions). Keyed by agent name rather than a bare flag, because a global latch would silence a *second* affected agent -- and naming the directory is the point of the warning. Follows the `_warned` latch convention in claude.py and engine/workflow.py. Two mutations confirm the test pins both properties: removing the latch, and making it global, each fail it. * fix(claude-agent-sdk): key the tier-warning latch so a for_each dedupes The latch added in e17c0cb did not fix the case its own message cited. The engine renames a for_each member per item (`<agent>[<key>]`, engine/workflow.py), so keying on `agent.name` gave every iteration a distinct key: an 8-item loop still emitted 8 warnings naming the same directory. Measured before and after. Keyed on the resolved directory instead, which dedupes both a for_each fan-out and repeated loop-backs. The trade is recorded at the declaration: two differently-named agents naming the SAME directory now warn once, naming only the first. Accepted because the actionable content is the directory and the remedy is workflow-global, so the second line would add nothing -- and the alternatives are worse. A bare flag would hide a second directory entirely (`settings_dir` is rendered per execution, so one agent can name several), and any name-bearing key reinstates the for_each noise. The test now drives the engine's own naming -- eight `fan[N]` iterations over one directory, plus one agent naming another -- rather than a name repeated verbatim, which no group path produces. Three mutations confirm it: no latch, a global flag, and the name key each fail it. Also branches the run-time remedy on its cause, as config/validator.py already does. With `setting_sources: [project]` and an agent's own `skills: []`, the message told the author to add 'project' -- already present, so the advice was a no-op and the warning kept firing. That is the same unactionable-advice defect raised against the validate-time message earlier in this branch; the run-time mirror had inherited only one of its three branches. * fix(claude-agent-sdk): key the tier warning by cause too, and cover the third cause Two defects the previous commit introduced while fixing its own. The directory-only latch was justified with "the remedy is workflow-global, so the second line would add nothing" -- true of the unbranched message, and falsified by the remedy branch added in the same commit. Measured: no tier enabled, one agent with `skills: []` and one without, both naming the same directory -> a single line, naming only the first, prescribing a fix that is wrong for the agent it does not name. Now keyed on `(directory, cause)`, which keeps the for_each collapse (all members share a cause) and bounds the output at two lines per directory. The residual trade -- same directory AND same cause warns once -- is recorded, and holds, because the remedy is then identical for both. `config/validator.py` distinguishes three causes; the run-time mirror had ported two. The missing one is a per-agent `provider: claude-agent-sdk` override under a different `runtime.provider`: `providers/registry.py` forwards structured settings only to the matching provider, so the agent reaches the provider with no `setting_sources` and the warning fired telling the author to add it -- which the schema rejects unless `runtime.provider` is itself claude-agent-sdk, exactly the unactionable advice this branch fixed at validate time earlier. The provider cannot see the workflow-level provider name, so rather than plumb it through, the non-opt-out arm now names the requirement instead of prescribing the edit, which is true on both paths. AGENTS.md records the key and its accepted cost. Three mutations confirm the tests pin it: dropping the cause from the key, reverting to the agent name, and collapsing the remedy branch each fail. * docs(claude-agent-sdk): record the tier warning's limits and pin its measurement No behaviour change; three gaps a review pass found in the prose around the settings_dir tier warning. The opt-out remedy assumes the `project` tier is otherwise present. An agent with `skills: []` under a `user`/`local`-only tier is told to remove the opt-out, which is necessary but not sufficient, so that author reaches a working config in two warnings rather than one. Both layers say the same thing, so there is no divergence -- but the limit was undocumented. Distinguishing it would need a third cause value and a wider latch key, to serve a combination requiring two unusual settings at once, so AGENTS.md records the limit instead. Two test docstrings described the code as it was rather than as it is: the latch test named only the directory half of a key that is now `(directory, cause)`, and the no-tier-remedy test claimed to exercise the per-agent provider override, which it cannot -- the provider never receives the workflow-level provider name, which is exactly why one shared remedy arm is right. That docstring now says what the test can and cannot pin, and points at the validate-time test that does cover the override path. The filesystem-grant measurement is now pinned to `claude` CLI 2.1.263. Later builds dropped `default` from `--permission-mode`'s accepted names, so the stated reproduction needs the version named. Conductor never passes that mode explicitly, so nothing in the code is affected.
* refactor(providers): add opaque execution continuation state
* feat(providers): preserve pydantic ai message history
* refactor(providers): align continuation parameter signatures
* fix(engine): continue agents after validator feedback
* docs(validation): explain pydantic ai continuation
* docs(validation): note pydantic ai continuation in AGENTS.md
* test(engine): cover validator continuation fail-open path
* refactor(providers): remove unrelated unused-argument del from claude execute
* fix(executor): refuse continuation state a provider cannot resume
The continuation path discards the rendered prompt, the workspace-
instructions preamble, and the eager skill injection, keeping only the
follow-up turn. That is only safe when the provider actually holds the
completed conversation. Add AgentProvider.supports_continuation (True on
the Pydantic AI providers, claude and openai) and make AgentExecutor
raise ExecutionError when handed state from a provider that does not
declare it, instead of silently sending the model a bare feedback turn.
Tests: the guard and the follow-up-only prompt are covered at executor
level, provider declarations are pinned for all six providers, and the
stateless validator retry now asserts the re-run prompt is the original
prompt plus feedback rather than a feedback-only stub.
* test(providers): pin continuation state forwarding into the pydantic run
Nothing asserted that a continuation_state handed to provider.execute()
actually reaches agent.iter(): deleting the message_history forwardings
in claude.py, openai.py, and runner.py left the suite green, so every
validator retry would have silently degraded to a feedback-only turn.
Spy on run_with_interrupt from both Pydantic AI providers and assert the
history object arrives as message_history, the follow-up text arrives as
user_prompt, and the continued run's messages extend the first run's.
* refactor(providers): keyword-only execute kwargs with continuation_state last
Inserting continuation_state between custom_agents and extra_mcp_servers
silently changed the meaning of positional argument nine for every caller
and subclass, and the duplicated provider.execute call it came with could
not guard anything: the kwarg is only ever forwarded when a provider
returned it, and that provider already accepts it. Make every argument
after rendered_prompt keyword-only on AgentProvider and all six
providers, move continuation_state to the end of the signature, collapse
the executor's two call arms into one unconditional call, and update the
test doubles and the ACA runner's one positional call accordingly.
* refactor(providers): type continuation_state as object at the boundary
Any opted the field out of type checking at exactly the two lines where
an unvalidated value crosses into run_agent_pipeline's correctly typed
Sequence[ModelMessage] | None parameter. Spell opacity as object
instead: the value is unchecked by construction, and the two Pydantic AI
providers now re-narrow it with a documented cast at the one place that
knows the value can only be this provider's own all_messages() list.
* fix(dashboard): keep the original prompt on validator continuation retries
On the continuation path rendered_prompt is only the follow-up turn, and
agent_prompt_rendered carried it verbatim, so the node's Prompt panel and
the JSONL log lost the agent's actual task prompt at exactly the moment a
user opens it. Tag the event with continuation: true and have the store
append the turn to the existing prompt (top-level and for-each item)
instead of replacing it.
* refactor(engine): explain the validator re-run guidance selection
The continuation branch packed three non-obvious decisions into an inline
conditional nested two calls deep: guidance_section is deliberately
dropped (it already lives in the provider-held conversation), the
feedback is lstripped (the builder prefixes newlines for the append
case), and the concatenated guidance was computed unconditionally while
being dead on the continuation path. Hoist the selection to a named
local with the reasoning attached.
* fix(engine): surface the cause of a failed validator re-run
When the feedback re-run itself fails, the fail-open path kept the
original output but reported through channels that never said why: the
agent_validation_failed event carried only {issues, will_retry,
rerun_errored}, so even in verbose mode the user learned *that* the
retry failed but never *why*. Continuation retries make this worse
because the re-run now replays the whole first conversation and can
overflow the context window where a rebuilt prompt used to fit. Put the
error on the event payload (with a continued flag), render the cause in
the console and the dashboard activity entry, and note the context-
growth trade-off in the validator docs.
* docs: align continuation wording across docstrings, schema, and AGENTS.md
The prose in the feature docs was updated carefully but the docstrings
on the implementing functions were not: AgentExecutor.execute still
described an unconditional prompt render, guidance_section was said to
be appended (on the continuation path it becomes the entire prompt), and
the four providers that ignore the kwarg had no Args entry for it,
making 'deliberately ignored' indistinguishable from 'not wired up yet'.
Update each site, document the ignore with a del, and spell out the
skipped-prefix consequences. ValidatorConfig's user-facing docstring
contradicted itself (feedback 'appended to its prompt' vs. no prompt
re-sent) and leaked the internal 'Pydantic AI providers' label; rewrite
it provider-neutrally. Finally, add the missing Provider Parity bullet:
the PR changed the AgentProvider.execute contract and AgentOutput, so
the parity checklist now states the continuation rule alongside the
other per-provider invariants.
* feat(providers): continue hermes conversations across validator retries
Hermes has the same in-memory continuation surface as the Pydantic AI
providers: run_conversation accepts a conversation_history list, and the
provider already persists result["messages"] for checkpoint resume with
exactly the system_message + history call shape a continuation needs.
Declare supports_continuation, forward a handed-back state as
conversation_history (winning over the checkpoint-resume file, mirroring
claude_agent_sdk's precedence), and populate the field from the
completed run's own messages. The state is withheld on a partial run and
when a parse-recovery call produced the output, since that output lives
in the recovery conversation the outer result never carries — continuing
from the original run's list would show the model an answer the
validator never graded. On the retry turn the schema instruction is
appended as usual, so prompt-injection structured output keeps working.
* test: cover the remaining continuation branches
Continuation was tested only against a plain-text agent, but validator
retries always run against schema-output agents whose history ends in
the output tool's ToolCall/ToolReturn pair — pin that case. Pin the
pre-run interrupt on a continued run as a decision: the partial request
is based on the prior conversation and the follow-up prompt never
reaches the model (the engine discards partial re-runs, so it fails
safe). And pin the never-serialized invariant end to end: a provider
returning an arbitrary object as continuation state leaves neither the
event payloads nor a saved checkpoint's JSON.
…osoft#510) * fix(gates): read a terminal dialog reply as one multi-line turn The dialog gate read each reply with single-line `Prompt.ask`, so pasting a block of text into an interactive terminal dispatched every line as its own turn. A three-line paste became three separate questions to the model, each answered against a fragment of the intended message, and the paste's trailing newline added a fourth turn with empty content. The human gate already had a multi-line reader for exactly this shape, so the loop is extracted to `read_multiline_lines(console, sentinel)` in human.py and the dialog's main turn reads through it. `HumanGateHandler._read_multiline` delegates with its historical `.` sentinel, so that gate's behaviour is unchanged. The reader returns `(text, hit_eof)` rather than a bare string because the two EOF cases are different answers: an EOF that terminates a paste should submit the accumulated content, while an EOF with nothing accumulated is a deliberate Ctrl-D and dismisses. Collapsing them would make the dismissal branch unreachable, since the reader converts EOF into a returned string. The dialog's sentinel is `/send` rather than the human gate's `.` because a lone `.` is a plausible line of prose in a free-form reply. Because that makes `/send` load-bearing for submitting a turn, `_display_dialog_start` advertises it — gated on the same `sys.stdin.isatty()` condition as the reader, since off a tty the turn falls back to `Prompt.ask` and the sentinel does nothing. Off a tty (a pipe, CI, or the web dashboard, which returns via `_web_handle_dialog` and never renders this banner) the single-line path is untouched. Verified: tests/test_gates 69 passed; full suite 8308 passed with three failures that reproduce identically on an unpatched checkout (chmod 0o000 and case-sensitivity tests that do not hold on this filesystem). Each new test fails against the unpatched gate. ruff check, ruff format --check and ty are clean. * fix(gates): compare stripped text in the dialog submission guards The multi-line reader drops trailing newlines but deliberately keeps a whitespace-only line, since a real strip would eat the closing indentation of a pasted code block. Both new guards compared exactly against "", so a buffer of spaces or tabs survived as a truthy string and slipped past both. Two consequences, the second worse than the first. A whitespace-only submission was dispatched to the model as a turn and spliced into the agent's re-execution guidance. And an EOF with only whitespace typed did not reach the "user is leaving, not pasting" branch, so Ctrl-D submitted the whitespace as a turn and *then* dismissed -- the user asked to leave and sent a message instead. Nothing was logged on either path. Both guards now test stripped text, matching _is_dismiss, which already normalises this way and was the outlier's neighbour in the same file. Addresses a blocking review finding. * test(gates): cover the reader gate, and tighten the reader's contract The condition choosing between the multi-line and single-line readers had no test in either direction: deleting the isatty() half, or the `prompt_text is None` half, left the whole suite green. The first would activate the multi-line reader under a pipe or in CI, waiting for a sentinel nobody can type; the second would make the yes/no confirmation demand /send after "yes" on every interactive run. Both halves now have a test, and the Prompt.ask fallback -- previously never executed by any test -- is exercised. The rule itself moves into a `_reads_multiline_turn()` predicate so the reader and the banner's sentinel hint consult one source, rather than two isatty() calls kept in agreement by a comment. The banner's hint is pre-rendered as a Text and spliced into a fixed-arity template: styled() raises IndexError on a mismatch and silently drops an extra argument, and the arity was previously maintained by hand across a `+`-concatenated fragment and its args tuple, on a branch no test reached. On a tty the rendered output is byte-identical to before, ANSI spans included. Off a tty the sentence regains the plural it has upstream ("Type your responses below."), which the previous revision had silently made singular on the one path this series leaves alone; that banner is now byte-identical to upstream, and a test pins the wording, since a byte comparison was otherwise the only thing that would catch it drifting again. Requiring the sentinel to submit also made the terminal UI's own exit instruction inert: the banner said "Say done or /done when finished", but on a tty a dismiss keyword is only seen once the turn is submitted, so `done` alone left the user at a prompt that never responded -- driving the real handler with input() returning "done" every time, the dialog never exited in 30 reads. The failure-recovery notice repeated the same premise, on the one screen where the user most needs a reliable way out. `_dismiss_instruction()` now states that rule once beside `_reads_multiline_turn()`, both sites render it, and both directions are pinned. Off a tty the sentence is unchanged from upstream, since every line is already a turn there. Also tightens the extracted reader: - `sentinel` is keyword-only and required. It is public, the two gates use different sentinels, and `read_multiline_lines(console)` was silently valid -- it would truncate a user's prose at any lone "." with no signal. - Only `except EOFError` remains. The extraction had also caught StopIteration, which a real stdin never produces here -- an exhausted or non-tty stream raises EOFError and a closed one ValueError -- so the catch was wider than any reachable input, and it let a test double read past what it supplied and still pass as a clean submission. The reason it is not caught is recorded at the clause, and a test pins it, so it is not reinstated as an oversight. - The sentinel's whitespace tolerance and the reader's trailing-line handling are pinned; both survived mutation before. So is the keyword-only signature itself, since restoring one default silently re-opens the hazard and nothing else would fail. `test_ctrl_d_at_empty_prompt_dismisses` now uses a bounded EOF source. A bare `side_effect=EOFError()` re-raises forever, so deleting the dismissal branch hung the suite instead of failing it, and there is no pytest-timeout configured -- a hung CI job rather than a readable failure. That test also pins `read_on_daemon_thread` as the dispatch: a cancelled asyncio.to_thread leaves its worker blocked in input() holding a slot in the shared default executor, which that function's own docstring explains at length. A pasted block's indentation is pinned end to end, leading edge included: the guards strip only to decide whether anything was submitted, while the text itself must reach the provider verbatim, and adding a strip to the returned turn text previously passed the whole suite while silently reindenting a pasted code block. Every guard in this series is mutation-tested: reverting any of them fails at least one test. Addresses a blocking review finding plus seven recommended ones. * docs(gates): correct three false claims about the dialog reader All three were verified wrong by execution, having been asserted as verified. "Ctrl-C dismisses rather than propagating" is removed. CPython runs signal handlers on the main thread only and the read happens on a daemon thread, so a real SIGINT never reaches that except clause: sending one to a child blocked in the reader on a pty raises CancelledError at the await and KeyboardInterrupt escapes asyncio.run. Ctrl-C tears the run down as it does everywhere else, and a reader who trusted this line would lose their session expecting to keep it. The except clause is correct for exceptions the reader itself raises and stays; its test is renamed and its docstring now says explicitly that it does not cover a real SIGINT. "Off a tty the single-line path is unchanged" is corrected. Prompt.ask returns "" for a blank line and the empty-submission guard sits above the tty branch, so a blank piped line is now skipped where it was previously dispatched as a turn with empty content. The new behaviour is right, but that path is the contract for anyone driving a dialog from CI, and it said nothing had changed. Grouping the web dashboard with "a pipe, CI" is corrected. It returns before either reader and has always received whole messages, so the old wording implied its chat box could not take a multi-line reply. Two things the prose did not say at all are now stated. A terminal accepts an EOF keystroke only at the start of a line, and its EOF does not persist -- the read returns and the terminal is readable again -- so Ctrl-D after entering a line now submits it and a second Ctrl-D is needed to leave, where one used to exit. On an empty prompt it still exits in one keystroke, so the habitual exit only changes once something has been entered, and that text is now sent rather than discarded. And the "trailing blank lines are stripped" overstatement is dropped: a whitespace-only trailing line is kept verbatim, which is deliberate -- stripping it would eat a pasted code block's closing indentation -- and is now pinned by a test. Addresses three recommended review findings. --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com>
Audits all 12 commits since v0.1.36 and reconciles CHANGELOG.md. Also corrects a changelog defect: PR microsoft#503 appended its Added/Changed entries to the already-published 0.1.36 section (merged 2026-09-04, two days after v0.1.36 shipped), so the released notes claimed changes that were not in that release. Those entries move to 0.1.37, restoring the 0.1.36 section to byte-identical with the v0.1.36 tag. Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat: add deterministic type: mcp workflow step (microsoft#392) Add a new workflow step type that calls a single tool on a configured MCP server directly, with no LLM in the loop: - Engine-owned lazy stdio connection pool keyed by (server, runtime working_dir), with per-server asyncio locks, an idle-manager bound, and unconditional cleanup in finally run()/resume() - Structured envelope result {content, structured, is_error} in the workflow context; is_error is routable data, not a step failure - Redacted event contract: mcp_started/mcp_completed/mcp_failed carry only metadata (server, tool, argument_keys, elapsed, is_error, result_bytes, truncated, spill_path) — never argument or result values — surfaced in console, JSONL, web replay, and fleet summary - Dispatch in all three engine positions: main loop, parallel groups, and for_each; fail-fast groups cancel and drain siblings before pool cleanup - Static off-network validation: server declared, tool allowlisted, stdio transport only, argument templates checked - Dashboard: mcpNode graph node + McpDetail panel (metadata only) - Docs, examples/mcp-step.yaml, bundled skill references, CHANGELOG * fix(engine): harden mcp step failure redaction, pooling, and interrupts Review-driven fixes for the type: mcp step (PR microsoft#518): - Pool admission is now atomic under the pool guard: an in-flight connect reserves its slot (_mcp_step_pending), so two concurrent first-time connects can no longer both pass a size-only capacity check and overshoot _MCP_STEP_POOL_MAX with nothing evicted. The cap is documented as a soft threshold (busy entries are never evicted mid-call). - Eviction cleanup no longer swallows task cancellation: the evicted manager's close() runs as its own shielded task (close() deliberately absorbs CancelledError while draining), tolerates repeated cancellation, and re-raises CancelledError before any new connect or invoke happens. - The "see debug logs" failure message pointed nowhere: --log-file wires a Rich console, not a Python logging handler. Redacted mcp failures now write the full exception (including cause chains) to a private per-run *.mcp-diagnostics.log next to the run's event log, and the redacted message points at that file. Safe categories keep their authored messages verbatim (timeouts keep their duration; the name-only runtime checks propagate as before). - Structured result keys "outputs"/"errors" are now reserved from the top-level merge: WorkflowContext duck-types group outputs by exactly those keys, so a flattened pair would misclassify a step's output as a group output in every context mode. They stay under output.structured. - call_tool_structured strips server-supplied "truncated"/"spill_path" content-block fields at ingestion; only Conductor's own truncation pass may set them, and a shared typed reader (mcp_truncation_metadata) is the single read-side for live events and synthetic replay alike. - Synthetic replay of parallel/for-each groups no longer publishes saved MCP envelopes in the aggregate outputs: mcp members/items are replayed as the metadata-only event sequence live execution emits. - MCPManager.connect_server gains redact_errors=True for the deterministic step path: connection failures log safe metadata only (no server stderr or traceback at ERROR); provider-facing behavior is unchanged. - Main-loop mcp steps watch interrupt_event across the slot/connect/call waits: a dashboard Stop cancels the in-flight call (never auto-replayed — side effects are unknown) and enters the existing pause flow (agent_paused -> Resume/Kill; CLI menu without a dashboard). Group members mirror LLM members and are reached via group cancellation. - The runtime tool-allowlist check now uses wildcard membership ("*" in tools), exactly matching conductor validate; and validate now parses every nested argument template for Jinja2 syntax errors (reference analysis deliberately swallows TemplateSyntaxError). - Docs: argument coercion describes the actual whole-render YAML parsing (embedded templates can coerce to non-strings); the validation section enumerates which checks runtime repeats vs validate-only ones; the secrets section scopes the no-values guarantee (item_key identifiers and authored downstream outputs stay visible) and documents the diagnostic file; restrictions lists gain settings_dir (rebase follow-up). * fix(web): render mcp members in groups and stop double-counting them Review-driven dashboard fixes for the type: mcp step (PR microsoft#518): - A parallel group's MCP member no longer increments workflow completion on its mcp_completed (the group increments it on parallel_completed) — a one-member MCP group reported 2/1 completed before. - Parallel group members keep their declared step type: workflow_started seeds their nodes from the agents' declared types (both root and sub-workflow initialization), and graph layout picks the renderer from the declared type via a shared flowNodeTypeFor mapping, so a parallel MCP member renders as McpNode and opens McpDetail. - For-each item rows factor MCP metadata into expandability and render it (server, tool, result size with the truncation marker, spill path); items whose tool reported is_error: true stay "completed" but show a visible warning. No raw result body is exposed — none exists on items by design. Static dashboard assets rebuilt (make build-frontend). * fix(web): stop republishing stored mcp truncation markers on resume replay The synthetic replay path built mcp_completed events from checkpoint- restored envelopes and forwarded their truncated/spill_path content-block fields as trusted Conductor metadata. Those markers are only trustworthy on the live path, where call_tool_structured strips server-supplied fields of those names at ingestion: a checkpoint written before the stripping existed can carry a server-supplied spill_path, and replaying it presents server-controlled data as Conductor-generated metadata. Synthetic events now report no truncation (the stored envelope itself stays intact in the workflow context for routing and templates), and mcp_truncation_metadata is documented as the live-path-only reader. * fix(engine): park interrupted mcp calls when no one can resume them A Stop cancelling an in-flight mcp tool call used to be followed by an automatic re-execution whenever no human resolved the pause: every browser client disconnecting mid-pause returned the same pause outcome as an explicit Resume click, and a dashboard with zero connected clients auto-resumed. For a side-effecting tool call that silently repeats work whose external outcome is unknown. WebPauseOutcome now carries an explicit reason (resume/guidance/disconnect/unavailable), and the disconnect arm no longer emits agent_resumed — the LLM caller emits it there when it auto-resumes, preserving its event stream. An explicit Resume completed in the same wait batch wins over a simultaneous disconnect. The mcp dispatch re-enters the step only on resume/guidance (or the CLI interrupt menu, which is an explicit decision by definition); on disconnect or a clientless dashboard it raises _McpStepOutcomeUncertain, an InterruptError subclass, so the run stops flagged stopped_by_user with a failure checkpoint and conductor resume becomes the explicit at-least-once re-execution boundary. No mcp_failed is emitted: the tool reported no failure, the call's outcome is simply unknown. * fix(web): preserve declared node types for nested static mcp members Members of parallel groups inside statically previewed sub-workflows were seeded as generic 'agent' nodes, and ensureNode never updates an existing node's type, so the runtime workflow_started landing on the reused placeholder never corrected them either — nested mcp members ended up routed to AgentDetail instead of McpDetail. buildStaticChildContext now seeds parallel members with their declared types (the same map the root and child workflow_started handlers already use), and the child workflow_started re-syncs every declared agent's node type from the runtime topology onto reused placeholder nodes, leaving group nodes untouched. --------- Co-authored-by: Jason Robert <jasonrobert@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…crosoft#482) * build(deps): add opentelemetry-api core dep and telemetry extra * feat(telemetry): env-driven OpenTelemetry tracing with unified provider spans Activated via OTEL_EXPORTER_OTLP_ENDPOINT. A TelemetrySubscriber maps workflow events to spans with a detached-span lifecycle; a delegating global tracer provider nests native provider spans under Conductor's orchestration spans for one unified trace tree per run. The OTLP protocol is latched per run and shared between the engine and the Copilot client; native Copilot CLI spans are captured over OTLP HTTP, while standard gRPC disables them with a per-run warning. Per-execution native span activity is signalled on agent start events, and engine agent lifecycle events expose provider name and token breakdown. Includes the telemetry docs, example workflow, and regression suite. * fix(telemetry): route HTTP OTLP exports through the traces path and make init transactional The HTTP span exporter uses an explicit endpoint= verbatim, so the documented base URL (http://localhost:4318) posted spans to / instead of /v1/traces and any standard collector rejected them. Derive the per-signal URL with the same path-appending rule the SDK applies to OTEL_EXPORTER_OTLP_ENDPOINT, honoring an explicit OTEL_EXPORTER_OTLP_TRACES_ENDPOINT override. The base URL latched for the Copilot SDK is unchanged, and the example recipe now uses port 4318 with http/protobuf so the advertised native Copilot spans actually export. Initialization is also transactional now: a failure after the TracerProvider is constructed (exporter build, global-provider discovery) shuts the partial provider down instead of leaking its batch worker, and the provider is built with shutdown_on_exit=False so the SDK's atexit shutdown can no longer stall interpreter exit behind a hung collector — TelemetrySubscriber.close() owns the lifecycle. * fix(telemetry): bound exporter drain and report interrupted runs at close BatchSpanProcessor.force_flush ignores its timeout and drains synchronously on the calling thread (one exporter timeout per queued batch), so a slow or unreachable collector stalled workflow teardown and cancellation on the asyncio thread. The flush/shutdown pair now runs on a daemon thread bounded by a single overall deadline; an incomplete export is logged with the run id instead of being waited on, shutdown is attempted independently of the flush outcome, and the telemetry guards reset in an outer finally so cleanup failures cannot mask an in-flight workflow or dashboard exception. close() also takes the terminal outcome: spans still open when the engine never emitted a terminal event (KeyboardInterrupt, external cancellation, an escaped exception) are ended as failed with the real error type instead of looking like clean completions. Both CLI helpers wrap dashboard.stop() in try/finally so telemetry cleanup always runs. * fix(telemetry): repair nested-workflow and for-each span identity Three span-hierarchy fixes in the subscriber: - parent_for_path now prefers an open workflow span at the exact path over the remembered outer invocation, so a child workflow's agents and groups attach to the child workflow span instead of flattening the exported tree into delegate -> inner-agent. - For-each validator spans are keyed by the full execution identity (workflow path, group, item index) instead of the group's shared display name, so two overlapping item validators can no longer close each other's spans and inherit each other's token/error metadata. - A remembered subworkflow parent records whether it is a sequential agent or a for-each item span; subworkflow_completed/failed closes only sequential parents, leaving item termination to for_each_item_completed/failed so the item's terminal metadata (aggregated cost, tokens) still lands on an open span. Covered by new subscriber tests for overlapping validators and early item-span closure. * fix(telemetry): gate native-span reporting on the active run and the executing provider native_otel_spans_active answered from static capability alone, so a run with no endpoint configured, tracing disabled, or a failed init still reported native_otel_spans_active=true on agent start events for Claude and OpenAI. It now requires an active-run telemetry check first; the static capability answer stays in has_native_otel_spans. The engine call sites also read the child workflow's own runtime settings, which breaks down when a child engine inherits the parent's provider registry: a parent configured with an external Copilot runtime_url plus a child on default settings reported native spans as active under HTTP even though Conductor never configured that runtime's telemetry, and the subscriber then suppressed its fallback tool spans against an uninstrumented runtime. Provider settings now resolve through the registry that actually constructs the providers (new ProviderRegistry.provider_settings_for, mirroring the name-matching rule used at construction), applied identically at the sequential, parallel, and for-each start events. * docs(telemetry): qualify the privacy guarantee and Copilot correlation claims Prompt and response capture is disabled by default, but exception messages can still contain input or response values — a failed output validation embeds the rejected value in the error message recorded on the span — so the docs no longer promise that disabling content capture keeps response content out of traces, and advise treating traces as potentially sensitive regardless. The correlation claim is narrowed to reality: Conductor's own spans and native Pydantic AI spans carry the workflow run id as gen_ai.conversation.id, while native Copilot CLI spans never receive that attribute and are correlated by shared trace id and parent relationship instead. * test(telemetry): guard optional-SDK coverage and strengthen span tests The five instrumentation cases that call init_tracer_provider assumed a real SDK with no importorskip guard, so they failed outright on a base install while the rest of the suite skipped. They are guarded now, and a new SDK-absent case pins the other half of the contract: with an OTLP endpoint configured but no telemetry extra installed, initialization declines and execution continues uninstrumented. CI and the release workflow install the telemetry extra for the test and typecheck jobs so this suite actually runs there (the typecheck job also needs it to resolve the telemetry modules' TYPE_CHECKING imports). Two assertions that could not catch the failures they claimed to cover are rebuilt: - The tool-span dedup test drove its own hand-built Pydantic agent with instrumentation explicitly disabled and synthesized the tool callbacks afterwards. It now runs the real OpenAI builder and runner through the engine with only the model swapped for TestModel, asserting the native invocation, chat, and exactly one tool span reach the exporter with the run id as conversation identity and correct ancestry. - The cross-task detach test defined but never installed its spy, and keyed on a nonexistent loop attribute. It now records attach/detach ownership per token (contextvars.Token is unhashable, so keyed by id) around the fail-fast execution and asserts worker tokens are observed, never detached by another task, and the caller's context is restored. Covers span-hierarchy regression tests: a real nested engine execution asserts the child workflow's agents parent under the child workflow span rather than the delegate invocation span. * fix(telemetry): close pre-push review gaps Honor trace-specific OTLP endpoint and protocol precedence from one immutable environment snapshot, roll back exporter and processor allocation failures, and avoid advertising Copilot native spans without a general endpoint. Resolve provider identity through the inherited registry used for execution, make run and resume teardown preserve primary workflow failures while closing every resource, contain drain-thread startup failures, and qualify external-runtime correlation guarantees. Add regressions for every path. * fix(telemetry): snapshot OTLP environment atomically Resolve general and trace-specific endpoints and protocols from one copied environment mapping, keep validate-time SDK diagnostics aligned with signal-specific activation, and cover both drain-thread lifecycle failures. * test(telemetry): prove atomic OTLP snapshot semantics Use a non-dict live mapping whose direct reads mutate backing state, so the regression fails without the production environment copy. Cover both general and trace-specific endpoint diagnostics when the optional SDK is absent. * fix(mcp): preserve item identity in lifecycle events * feat(telemetry): trace direct MCP workflow steps * docs(telemetry): document direct MCP step spans
* fix(providers): clarify structured output completion * docs: note structured output completion guidance
Takes upstream's now-merged versions of the PDA-35 dialog fix (microsoft#510) and the PDA-95 settings_dir field (microsoft#514), dropping our superseded copies. Conflict resolutions: - pyproject.toml, tests/test_gates/test_dialog.py: both-sides-true additions; kept both. - providers/factory.py: took upstream's stricter provider-name guard over our getattr. - config/schema.py, docs/providers/experimental.md: upstream's settings_dir prose is a superset. experimental.md keeps our workflow_tools_passthrough capability claim, which upstream cannot supply. - providers/claude_agent_sdk.py (11 hunks): kept our _server_tool_filters, _server_filter_denials and the 4-tuple _resolve_tool_config allowlist enforcement; took upstream's SettingSource retype, effective_sources threading, hooks-trust warning and settings_dir tier warning. add_dirs is upstream's settings_dir expression, which left the _stdio_path_args local unread -- the dead assignment is removed and the helper left defined, since its removal is PDA-95's remaining acceptance criterion and is measured separately. - tests/test_providers/test_claude_agent_sdk.py: both sides' test classes are disjoint and both kept, except TestSettingSourcesWiring, which both sides defined -- upstream's is a behavioural superset, so ours is dropped (Python would otherwise have silently discarded the first definition). - uv.lock, web/static/, tsconfig.tsbuildinfo: regenerated with uv lock and make build-frontend rather than resolved by hand. Fork-local changes verified present after the merge: the per-agent and per-server tool allowlists (workflow_tools_passthrough=True), the OutputPane XSS fix (FN-5100), and the anyio>=4.14.2 floor (FN-5092, which moved the lock from 4.12.1 to 4.15.1). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Per-file conflict resolutions (11), and the AC2 test comparisonThe eleven conflicts
The ticket predicted nine conflicts at
|
| hunk | resolution |
|---|---|
| 1 | kept our _server_tool_filters + _stdio_path_args; took upstream's retyped _resolve_skill_filter signature |
| 2, 7, 9, 10, 11 | took upstream — prose accuracy plus the effective_sources threading |
| 3 | kept our _server_filter_denials (upstream side empty) |
| 4, 5 | took upstream — Sequence[SettingSource] retype and the new hooks-trust warning |
| 6 | both — our enumeration/denial block and 4-tuple call site, plus upstream's effective_sources and settings_dir tier warning |
| 8 | took upstream's add_dirs line; removed the now-dead local; left _stdio_path_args defined |
AC3 — ty diagnostics, both parents measured
| diagnostic | our main |
upstream/main |
this branch |
|---|---|---|---|
call-non-callable (execute_dialog_turn, fork-local) |
2 | 0 | 2 |
unused-ignore-comment (claude_agent_sdk.py) |
4 | 4 | 4 |
unused-ignore-comment (aca.py) |
0 | 2 | 2 |
| total | 6 | 6 | 8 |
make typecheck runs ty check src (not src tests), which is the figure above.
One measurement trap worth recording: an initial run reported 17 diagnostics, 11 of them unresolved opentelemetry imports. That was a stale venv — uv sync had run before uv lock pulled in upstream's new opentelemetry-api dependency — not a merge defect. Re-syncing cleared it.
AC2 — pytest, both trees, compared in both directions
| this branch | clean upstream/main |
|
|---|---|---|
| passed | 9095 | 9064 |
| failed | 4 | 4 |
| skipped | 57 | 57 |
The failure sets are byte-identical (comm -23 and comm -13 on the sorted test ids are both empty):
tests/test_plugins/test_registry.py::TestUnreadableTrees::test_unreadable_skill_subdirectory_is_reported
tests/test_providers/test_claude_performance.py::TestClaudeProviderPerformance::test_concurrent_request_handling
tests/test_skills/test_path_entries.py::TestUnreadableParent::test_unreadable_parent_is_reported_not_raised_raw
tests/test_skills/test_path_entries.py::TestSkillsRootDiagnostics::test_mis_cased_skill_md_is_reported
Three of the four chmod(0o000) a directory and expect a "could not be read" error; the running
user's privileges bypass the permission, so the path reads as absent instead — the condition those
tests' own skip reason names ("chmod(0o000) blocks neither Windows owners nor root"). Both runs used
-p no:randomly for comparability.
The 31-test difference is upstream's new tests plus ours that survived the merge.
🤖 Generated with Claude Code
The helper is uncalled after the merge took settings_dir as the only source of add_dirs, but its docstring still asserted that forwarding a stdio server's directory args "restores the declared scope" -- the claim upstream measured and disproved. AGENTS.md and tests/test_integration/test_mcp_roots_negotiation.py both now say the opposite: --add-dir takes no part in Roots negotiation, and a client's sole advertised root replaces every argv root. Docstring only; the helper stays defined and uncalled, since removing it is PDA-95's remaining acceptance criterion and is measured separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t path The merge dropped the fork's TestSettingSourcesWiring on the grounds that upstream's same-named class was a behavioural superset. It is not: the two same-named test_declared_sources_grant_the_skill_tool methods drive different arms of _resolve_tool_config. Upstream's uses tools=[] and asserts options.tools == ["Skill"]; the fork's used a non-empty allowlist and asserted "Skill" in allowed_tools. Measured, not reasoned: replacing `if skills_enabled:` with `if False:` at the non-empty-allowlist arm leaves the full suite green at 9095 passed -- identical to the unmutated run. Nothing guarded it. The restored test fails under that mutation and passes without it. Unguarded failure mode: an agent with setting_sources (or resolved skills) and a non-empty tools: allowlist would have the CLI list the tier's discovered skills to the model while refusing every invocation as outside the session allowlist -- discovery without execution, silently. Re-added into the fork-local TestMcpAllowlistEnforcement under a distinct name rather than reinstating a colliding class. Also corrects _resolve_tool_config's docstring, which took upstream's Args: block wholesale: it claimed the tools: [] carve-out was the Skill grant's "one carve-out" (false once the fork added the second), and dropped the entry for the fork-local enumerated_mcp_tools parameter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review outcomeTwo reviewers at Head is now Fixed1. A dropped test that was the sole guard on a fork-local branch ( 2. A docstring asserting a premise upstream measured and disproved (
Confirmed by both reviewers
Corrections to this PR's earlier claims
Not a defect in this PR, but worth raisingCI has never successfully run on this fork. The one run on 🤖 Generated with Claude Code |
How to size this PR, and what the sync did to the fork deltaThe headline The diff to actually reviewThat is the entire fork delta once synced — everything this fork carries on top of
Did the sync lose anything? The fork delta, before and after
Every one of those 280 lines accounted forTwo files left the delta entirely, which is the sync's whole purpose — upstream absorbed our
Four files shrank:
Every other file in the delta has an identical line count before and after — 22 of them, including all four must-survive changes' files ( No file entered the delta that was not in it before. The one apparent addition ( Reading the file counts
🤖 Generated with Claude Code |
Pandora-as-engine: two A/B Deliver runs, no regression foundTL;DR — this branch works as Pandora's Conductor engine. Two full Deliver runs of the same ticket, one per engine, both Scope: whether Pandora's Deliver flow runs on this branch. Not a review of the 252-file upstream sync's correctness — the body is right that reviewing that figure is the wrong move. Nothing here is a readiness judgement; that belongs to this project. The two runsPandora drives and is also the target repo. Same ticket (PDA-143), same gate answers, same model, engine the only deliberate variable.
Engine identity was verified rather than assumed from the env override: the runs self-report Step-by-step equivalence — what "identical for 49 events" coversEvery route and every deterministic gate exit code matched:
Both human gates presented the same four options ( Raw-mode outputs parsed through Pandora's deterministic extractors on both engines ( The commit path was genuinely exercised: Run 2's full sequence is run 1's sequence plus one review-fix cycle inserted at
|
| Filter | Result |
|---|---|
src/ raw |
14 files, +522 −99 |
src/ excluding generated assets + tsbuildinfo |
12 files, +519 −96 |
src/**/*.py only |
9 files, +482 −79 |
Nothing lands on 11 / +518 / −95. The headline figure is right and the conclusion is unaffected; the sub-figure looks measured at an earlier commit.
Three comparisons that could not be made — recorded so they are not mistaken for passes
- Multi-line revise at the human gate. Every option reports
multiline: falseon both engines, so a multi-line answer is not accepted by this gate on either. Not a branch defect — an incorrect expectation in my own verification plan. - A hooks-trust warning expected only on the branch. Never fired on either engine. The
setting_sources: [project]string appears in both logs as workflow YAML comment text, not as a warning. - Traceback counts are not comparable between these two runs. Both logs contain "Traceback" in agent-authored text — the ticket under test is about a
TypeError, and this branch's planner chose a test-first strategy that reproduces the defect on purpose (RC 1,TypeError, by design, before the fix). Counted by a naive grep, that reads as a branch regression that does not exist. Counted on engine-written surfaces only (stderr,error,*_failedevents), both runs are 0.
What this does and does not license
The runs establish that Pandora's Deliver flow works end to end on 0.1.37, through planning, a typed human-gate revise, worktree setup, implementation, commit-through-hooks, a two-judge panel, a review-fix cycle, and PR creation.
They cannot attribute any behaviour to an individual change inside a 252-file sync, and the two engines differ by a whole version — the entire upstream delta, not only the fork-local allowlist work. "No regression" here means no regression from that delta as a whole.
🤖 Generated with Claude Code
|
Thanks — the A/B runs cover the exact gap AC4 could not: The Engine identity verified from the runs' self-report rather than the env override is what makes the comparison worth anything, and it is worth saying that explicitly for anyone repeating this. The
|
Fresh review at current head — clean, plus a correction to AC3The two reviews above ran at The delta changes no executable line. Proven mechanically rather than by reading the diff: parsing The restored guard is real, and uniquely load-bearing. Independently mutation-tested: Every docstring claim in the two commits was checked against the code: Correction: the
|
| Tree | Exit | Diagnostics |
|---|---|---|
upstream/main |
0 | 4 warnings |
our main (pre-merge) |
1 | 4 warnings + 2 errors |
| this branch | 1 | 4 warnings + 2 errors (same sites) |
The 4 warnings are upstream's own, byte-for-byte. The 2 call-non-callable errors are in execute_dialog_turn — fork-local code upstream does not have — and already fail on our main. So the merge inherits the failure rather than introducing it, and AC3 holds in the weaker sense of "no new diagnostics" rather than "the gate passes". The description now says so, as attention item 4.
My original baseline run used --all-extras and reported 6 diagnostics for upstream; with CI's extras it is 4 and exits 0. Same tree, different environment — the figure I compared against was measured in a configuration CI never uses.
Gate outcomes at 92107b5
uv run ruff check src tests All checks passed! exit 0
uv run ruff format --check 492 files already formatted exit 0
uv run ty check src Found 8 diagnostics exit 1 (inherited)
pytest tests/test_providers/test_claude_agent_sdk.py 207 passed exit 0
One caveat the reviewer raised about its own work, worth preserving: it could not complete a whole-suite run under the mutation, so its "only guard on that arm" finding is proven per-file (1 failed / 206 passed) rather than suite-wide. My own earlier full-suite mutation run — 9095 passed with the grant deleted, identical to unmutated — is the suite-wide half, and the two agree.
🤖 Generated with Claude Code
|
Reviewed at
if query is None or ClaudeAgentOptions is None:
raise ProviderError("Claude Agent SDK not available")Fork-local code, so no upstream conflict risk. Turns AC3 from "no new diagnostics" into "the gate passes". |
`ty check src` exited 1 with two `call-non-callable` errors here: the import fallback types `query` and `ClaudeAgentOptions` as `... | None`, and a `CLAUDE_AGENT_SDK_AVAILABLE` check does not narrow them. `execute` already guards on the symbols themselves; this matches it. Runtime behaviour is unchanged -- one import statement binds both symbols or neither, so the flag and the None checks are equivalent. Verified the guard still raises ProviderError when `query` is None, and the flag is still read at its two other call sites. Turns AC3 from "no new diagnostics" into "the gate passes": `ty check src` now exits 0 with 4 diagnostics, matching upstream/main exactly. Review feedback from @joaomena. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Fixed in
Two things I checked before taking it, since the change swaps a runtime guard for a type-narrowing one:
Tests: 4 dialog-turn, 66 dialog-related, 207 in the provider file, all pass. Full suite unchanged at 4 failed / 9096 passed / 57 skipped. One measurement artefact worth recording, since it briefly looked like a regressionMy first full-suite run after the fix reported 9095 passed / 58 skipped — one test having moved from passed to skipped. That was not the fix. To reproduce your Worth noting for anyone reproducing these figures: the suite totals depend on which extras are installed, and 🤖 Generated with Claude Code |
TL;DR
Syncs the fork with
microsoft/conductorafter upstream merged our PDA-35 and PDA-95 work. Ignore the 252-file diff — that is three weeks of upstream arriving. The fork's own code, once synced, is 11 files / +518 −95, and two-thirds of that is one file.The sync converged the fork: its delta went 29 files/+1628 → 27 files/+1348, because upstream absorbed two files of our work outright. All four must-survive fork changes verified present by content.
Verified: ruff clean ·
tyexit 0, matching upstream exactly · 4 failed / 9096 passed, failure set byte-identical to a cleanupstream/maincheckout · 5/5 Pandora workflows validate · Pandora ran two full end-to-end Deliver runs on this engine, bothdelivered, no regression ·mainuntouched.1.
add_dirsis a real behavioural deletion, and it reaches Pandora. Both sides wroteClaudeAgentOptions.add_dirs; ours derived it from stdio MCP servers' directory args, upstream's from the new per-agentsettings_dir. Not a union — upstream's line was taken, because upstream measured our premise and disproved it (--add-dirtakes no part in MCP Roots negotiation). No Pandora workflow setssettings_dir, so Pandora'sadd_dirsis now empty wheremainpassed two directories. Pandora's runs show this is currently invisible to it — all its tools are<server>__<tool>, and zero built-in tool calls occurred — but a future step declaring a built-in tool would be the first to notice.2.
_stdio_path_argsis left deliberately dead._stdio_path_argsis defined with zero callers. Removing it is PDA-95's remaining acceptance criterion, kept measurable on purpose — please don't "tidy" it, and don't wire it back in (its docstring previously argued for exactly that;2b4a7d3corrects it).3. CI has never run on this fork. The one run here is
startup_failureat 0s with zero jobs, not caused by this merge (trigger block byte-identical tomain; an earlier merge tomainmatching the same trigger produced no run either). The local runs above are the only verification behind this PR. Reported to the project owner separately.Where the detail lives
What the merge is, and how the conflicts were resolved
Merges
upstream/main(29 commits,64b7167) into the fork, taking upstream's now-merged versions of the PDA-35 dialog fix (#510) and the PDA-95settings_dirfield (#514) and dropping our superseded copies. Ticket PDA-115.11 conflicts, not the 9 the ticket predicted against an older upstream head —
pyproject.tomlandtsconfig.tsbuildinfoare new at64b7167. Full per-file and per-hunk resolutions are in the conflict-resolutions comment.The substantive file is
src/conductor/providers/claude_agent_sdk.py(11 hunks, +267 −61 of the 518): our_server_tool_filters,_server_filter_denialsand the 4-tuple_resolve_tool_configallowlist enforcement kept; upstream'sSettingSourceretype,effective_sourcesthreading, hooks-trust warning andsettings_dirtier warning taken. The other 10 source files are +251 −34 between them.uv.lock,src/conductor/web/static/andtsconfig.tsbuildinfowere regenerated (uv lock,make build-frontend), not hand-resolved.Fork-local changes verified present after the merge
Verified by content, not ancestry —
git merge-base --is-ancestorreports all our commits as fork-local even where upstream took them as rewritten commits, so it cannot answer this question.4a81064,104de88)workflow_tools_passthrough=True, both filter helpers defined, 4-tuple_resolve_tool_configintact, 22 enforcement tests passcfe1418, FN-5100)dangerouslySetInnerHTMLabsent from both sinks the fork fixed (OutputPane.tsx,OutputViewer.tsx); upstream still has both619560c, FN-5092)anyio>=4.14.2inpyproject.toml;uv lockmoved 4.12.1 → 4.15.17e83791,cc0d58e)Acceptance criteria — all five met, with method
git grep).uv run pytest— 4 failed, 9096 passed, reproduced twice. Failure set byte-identical to a cleanupstream/maincheckout (4 failed / 9064 passed there), compared test-id by test-id in both directions; bothcommdirections empty. Three distinct environmental causes: twochmod(0o000)tests this machine's privileges bypass, one mis-casedSKILL.mdtest a case-insensitive filesystem cannot fail, oneelapsed < 1.0timing assertion. None of the four files is touched by this merge.uv run ty check src(whatmake typecheckruns) — exit 0, 4 diagnostics, byte-identical toupstream/main's. Two fork-localcall-non-callableerrors inexecute_dialog_turnfailed this gate on ourmaintoo;7023bfbfixes them by guarding on the SDK symbols themselves rather than the availability flag, asexecutealready did.uv run ruff check src testsandruff format --checkboth clean, exit 0.conductor validate5 passed, 0 failed. Caveat:conductor validatedoes not parse!filebodies, so this asserts nothing about the shell inside steps. Pandora's A/B runs cover that gap by executing every rendered script for real, and independently re-derived this AC.mainunchanged — still5b06b92, identical toorigin/main.One measurement note, since it produced a false alarm worth not repeating: an early
tyrun reported 17 diagnostics, 11 of them unresolvedopentelemetryimports. That was a stale venv —uv synchad run beforeuv lockpulled in upstream's newopentelemetry-apidependency — not a merge defect. Upstream's ownci.ymladds--extra telemetryfor the same reason.How this was reviewed
Two reviewers at the merge commit, each in its own worktree — one with full context (PR, ticket, conventions), one blind: given the diff and the two parent SHAs only, barred from
gh, the tracker, commit messages, and my measurements, and asked to state what it thought the change did before judging it.Both found the reconciliation correct. The full-context reviewer reproduced the mechanical merge with
git merge-treeand confirmed all 198 hand-edited lines are deletions of one side of a conflict hunk, ruling out silent rewrites during resolution.Two findings, both fixed and pushed:
92107b5— my first resolution dropped the fork'sTestSettingSourcesWiringas a subset of upstream's same-named class. Wrong. The two same-named methods drive different arms of_resolve_tool_config. Proved by mutation and reproduced: deleting the Skill grant on the non-empty-allowlist arm left the full suite green at 9095 passed. The restored guard kills that mutant.2b4a7d3—_stdio_path_args' docstring still asserted the premise upstream disproved, contradictingAGENTS.mdand upstream's owntest_mcp_roots_negotiation.py. Also fixes_resolve_tool_config'sArgs:block, which took upstream's wholesale and so mis-described the Skill grant and dropped the fork-localenumerated_mcp_toolsentry.Details and the corrections to this PR's earlier claims are in the review-outcome comment.
🤖 Generated with Claude Code