fix(mcp): launch bundled cao-mcp-server without a per-launch network fetch#403
fix(mcp): launch bundled cao-mcp-server without a per-launch network fetch#403tedswinyar wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #403 +/- ##
=======================================
Coverage ? 87.97%
=======================================
Files ? 124
Lines ? 15690
Branches ? 0
=======================================
Hits ? 13803
Misses ? 1887
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR removes the per-launch uvx --from git+https://...@main fetch for the bundled orchestration MCP server and instead launches the already-installed cao-mcp-server, with shared runtime/install-time resolution to make that invocation PATH-independent and version-aligned with the installed CAO.
Changes:
- Update bundled + example agent profiles and docs to declare
command: cao-mcp-server(no network fetch). - Add a shared resolver (
utils/mcp_resolution.py) and integrate it across providers, install-time config materialization, and OpenCode translation. - Remove the E2E uvx cache warmup and add regression/unit tests to prevent reintroducing the fetch form.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils/test_opencode_config.py | Adjusts OpenCode translation tests to assert passthrough for custom uvx commands and resolution for bundled cao-mcp-server. |
| test/utils/test_mcp_resolution.py | Adds unit tests for the new MCP command resolver (runtime vs persisted behavior and fallback modes). |
| test/utils/test_bundled_profiles.py | Adds a regression guard ensuring all bundled wheel profiles use bare cao-mcp-server and never reference the git+https uvx fetch. |
| test/services/test_install_service.py | Extends Kiro timeout injection tests to detect new command forms (bare, resolved path, module entrypoint). |
| test/providers/test_codex_provider_unit.py | Ensures Codex provider runs bundled cao-mcp-server entries through the shared resolver. |
| test/providers/test_antigravity_cli_unit.py | Ensures Antigravity provider persists a resolved, PATH-stable cao-mcp-server command into its config. |
| test/e2e/conftest.py | Removes the session-scoped uvx cache warmup fixture (no longer needed). |
| src/cli_agent_orchestrator/utils/opencode_config.py | Resolves bundled cao-mcp-server before flattening into OpenCode’s command-list schema. |
| src/cli_agent_orchestrator/utils/mcp_resolution.py | Introduces shared resolution for launching bundled cao-mcp-server without PATH dependency or network fetch. |
| src/cli_agent_orchestrator/services/install_service.py | Resolves MCP server commands at install time for persisted provider configs; expands Kiro timeout detection. |
| src/cli_agent_orchestrator/providers/kimi_cli.py | Applies shared MCP command resolution before emitting Kimi MCP config. |
| src/cli_agent_orchestrator/providers/cursor_cli.py | Applies shared MCP command resolution when materializing Cursor plugin MCP manifests. |
| src/cli_agent_orchestrator/providers/copilot_cli.py | Replaces inline resolution logic with the shared resolver (intended no behavior change). |
| src/cli_agent_orchestrator/providers/codex.py | Applies shared MCP command resolution before emitting -c mcp_servers.* overrides. |
| src/cli_agent_orchestrator/providers/claude_code.py | Applies shared MCP command resolution before writing the per-session MCP config file. |
| src/cli_agent_orchestrator/providers/antigravity_cli.py | Resolves bundled cao-mcp-server for persisted config output (prefers stable PATH launcher). |
| src/cli_agent_orchestrator/agent_store/workflow_scout.md | Switches bundled profile MCP server from uvx git fetch to bare cao-mcp-server. |
| src/cli_agent_orchestrator/agent_store/reviewer.md | Switches bundled profile MCP server from uvx git fetch to bare cao-mcp-server. |
| src/cli_agent_orchestrator/agent_store/memory_manager.md | Switches bundled profile MCP server from uvx git fetch to bare cao-mcp-server. |
| src/cli_agent_orchestrator/agent_store/developer.md | Switches bundled profile MCP server from uvx git fetch to bare cao-mcp-server. |
| src/cli_agent_orchestrator/agent_store/code_supervisor.md | Switches bundled profile MCP server from uvx git fetch to bare cao-mcp-server. |
| examples/orchestration/reviewer-opus.md | Updates example profile to use bare cao-mcp-server. |
| examples/orchestration/dev-sonnet.md | Updates example profile to use bare cao-mcp-server. |
| examples/orchestration/dev-opus.md | Updates example profile to use bare cao-mcp-server. |
| examples/orchestration/dev-kimi.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/report_generator_codex.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/README.md | Updates documentation snippet to use bare cao-mcp-server. |
| examples/cross-provider/data_analyst_kiro_cli.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/data_analyst_kimi_cli.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/data_analyst_copilot_cli.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/data_analyst_codex.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/data_analyst_claude_code.md | Updates example profile to use bare cao-mcp-server. |
| examples/cross-provider/cross_provider_supervisor.md | Updates example profile to use bare cao-mcp-server. |
| examples/codex-basic/codex_reviewer.md | Updates example profile to use bare cao-mcp-server. |
| examples/codex-basic/codex_documenter.md | Updates example profile to use bare cao-mcp-server. |
| examples/codex-basic/codex_developer.md | Updates example profile to use bare cao-mcp-server. |
| examples/aws/stepfunction/stepfunction-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/aws/sqs-send/sqs-send-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/aws/sqs-monitor/sqs-monitor-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/aws/sqs-dlq-check/sqs-dlq-check-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/aws/dynamodb-query/dynamodb-query-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/aws/dynamodb-delete/dynamodb-delete-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/aws/cloudwatch-logs/cloudwatch-logs-agent.md | Updates AWS example profile to use bare cao-mcp-server. |
| examples/assign/report_generator.md | Updates assign example profile to use bare cao-mcp-server. |
| examples/assign/README.md | Updates documentation snippet to use bare cao-mcp-server. |
| examples/assign/data_analyst.md | Updates assign example profile to use bare cao-mcp-server. |
| examples/assign/analysis_supervisor.md | Updates assign example profile to use bare cao-mcp-server. |
| docs/agent-profile.md | Updates agent-profile documentation snippet to use bare cao-mcp-server. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for label, candidate in order: | ||
| if candidate: | ||
| logger.debug("Resolved %s via %s: %s", command, label, candidate) | ||
| return candidate, [] |
There was a problem hiding this comment.
Good catch. Fixed in 04b5486: resolution now preserves caller-supplied args in every tier (script resolution returns them unchanged; the module-entrypoint fallback appends them after -m <module>). Added a TestArgsPreservation suite covering all three tiers plus a no-aliasing check.
| # Module entrypoint via the current interpreter — runnable without any | ||
| # console script on PATH. Falls back to a bare ``python3`` only if | ||
| # sys.executable is unavailable (best effort in degenerate environments). | ||
| interpreter = sys.executable or "python3" | ||
| logger.debug("Resolved %s to module entrypoint via %s", command, interpreter) | ||
| return interpreter, ["-m", CAO_MCP_SERVER_MODULE] |
There was a problem hiding this comment.
Done in 04b5486 — the fallback now returns [-m, <module>, *args] so flags reach the server in that tier too. Covered by test_module_fallback_appends_args_after_module.
tedswinyar
force-pushed
the
fix/cao-7le-mcp-server-command
branch
from
0d71958 to
04b5486
Compare
gutosantos82
left a comment
gutosantos82
left a comment
There was a problem hiding this comment.
PR Review: #403 — Point bundled agent profiles at the installed cao-mcp-server console script (drop the per-launch uvx git fetch)
Summary
Replaces the uvx --from git+https://github.com/awslabs/cli-agent-orchestrator.git@main cao-mcp-server cold-fetch in bundled/example agent profiles with the bare cao-mcp-server console script, and adds utils/mcp_resolution.py — a shared 3-tier resolver (interpreter-sibling → PATH → python -m …server) with a persisted flag — wired into 8 sites (6 providers + install_service + opencode_config). This fixes agents coming up without orchestration tools when a cold uvx clone exceeds the provider MCP-startup timeout, and removes an unpinned @main network dependency. The change is well-constructed, thoroughly documented, and verified end-to-end (265/0 tests pass, resolver claims confirmed, the target console script launches). Recommend merge after minor cleanups; no blocking or correctness-critical issues.
Important (should fix)
- [correctness] src/cli_agent_orchestrator/utils/mcp_resolution.py:124-129 (surfaces in
providers/codex.py:287-291,claude_code.py:257,cursor_cli.py:510) —resolve_mcp_server_configunconditionally setscommand/argseven when the input entry has nocommandkey.profile.mcpServersis typedDict[str, Any]and isn't validated against the command-requiredMcpServermodel, so a URL/transport server like{"type":"http","url":"..."}would gaincommand=""/args=[]after resolution. Incodex.pytheif "command" in cfg:guard then becomes always-true and emitsmcp_servers.<name>.command="", clobbering a command-less custom server that previously passed through untouched. Introduced. Likelihood is low (all bundled profiles + theMcpServermodel are stdio/command-based), but it's a real behavior change for command-less custom entries and is unguarded by tests. Fix: early-return an untouched copy when"command"is absent. (Path-weighted: lives inproviders/+utils/; kept at Important rather than Blocking only because it can't affect the PR's actual targets.) - [tests] provider wiring — claude_code / cursor_cli / kimi_cli / copilot + install_agent block — 4 of 8 wired sites have no test that would fail if the
resolve_*call were removed. Notablytest_kimi_cli_unit.pyfixtures usecommand: "uv", args: ["run","cao-mcp-server"](not the bare command), so the resolver just passes through andkimi_cli.py:342is never meaningfully exercised.install_agent's persisted-resolution block (install_service.py:290-296) only runs against non-caocommand: service-mcpfixtures (no-op). Recommend at minimum a kimi test with a barecao-mcp-serverand aninstall_agenttest asserting the written Kiro/Q JSON command is resolved (persisted=True). (antigravity/codex/opencode already have dedicated assertions.) - [conventions] src/cli_agent_orchestrator/providers/copilot_cli.py:11,13 —
import shutilandimport sysare now dead after the inline 3-tier block was replaced byresolve_cao_mcp_command()(Path/subprocessstill used). Flagged by 3 reviewers. Won't fail CI (repo runs black/isort/mypy, not ruff/flake8), so it's cleanup, not a gate — drop both imports. Introduced. - [conventions] CHANGELOG.md — no entry for a user-facing behavior fix (bundled profiles now launch cao-mcp-server via the installed console script instead of a per-launch
uvx git+…fetch). Other PRs add entries referencing their number; add one under## [Unreleased]→### Fixedreferencing #403. Introduced.
Nits (optional)
- [security] src/cli_agent_orchestrator/providers/codex.py:288,290 — the resolved command/args are interpolated into TOML
-coverrides via unescaped f-strings (f'{prefix}.command="{cfg["command"]}"'). No shell injection (whole list isshlex.join'd at :325), but a resolved absolute install path containing"or\(legal on Linux) would emit malformed TOML. The adjacentdeveloper_instructionshandling at :271 already escapes\/"/\n, so this is an inconsistency. Value is install-path-controlled, not remote/agent-controlled → low risk; harden for defense-in-depth. Introduced (the resolver widens what flows here from a fixed literal to a resolved path). - [tests] utils/mcp_resolution.py:36-38 — the
sys.platform=='win32'.exefilename branch and_sibling_script().exists()are never exercised (all tests mock_sibling_scriptwholesale). The one resolver branch with zero coverage; low risk (mechanical). - [correctness] utils/mcp_resolution.py:126-129 — for non-cao passthrough servers,
argsis force-set to[]when the original omitted it. Harmless for stdio servers; folds into the command-less fix above.
Prior feedback
None — the only existing PR comments are empty-body entries from the author (@tedswinyar); no maintainer feedback to dedupe against. All findings above are net-new.
Tests
Strong resolver unit coverage: new test/utils/test_mcp_resolution.py (194 lines) covers both persisted modes, all 3 tiers, args preservation + copy semantics, idempotency, non-cao passthrough, and empty sys.executable; test/utils/test_bundled_profiles.py dynamically enumerates bundled profiles and guards against reintroducing the uvx git+https form (good regression guard). Markers are correct (plain unit tests, no live CLI/tmux). Deleting the e2e uvx cache-warmup fixture is safe — it was best-effort and only warmed a cache the profiles no longer use. Gap is provider-integration coverage (see Important); the codex mock is signature-faithful.
Verification
Run in a clean Docker env (ghcr.io/astral-sh/uv:python3.12-bookworm-slim), uv sync succeeded (numpy built in-container — no local-host blocker):
- Baseline:
uv run pytestover the 6 new/touched files → 265 passed, 0 failed (mcp_resolution 97% cov, opencode_config 98%, install_service 85%, codex 94%). - ✓ VERIFIED — passthrough leaves non-cao commands (and abs paths) unchanged.
- ✓ VERIFIED — persisted=False prefers sibling; persisted=True prefers PATH; reverse fallbacks confirmed.
- ✓ VERIFIED — module-entrypoint fallback when neither sibling nor PATH exists; caller args appended in tier 3.
- ✓ VERIFIED — idempotency:
resolve_mcp_server_config(resolve_mcp_server_config(cfg)) == resolve_mcp_server_config(cfg), other keys preserved. - ✓ VERIFIED — the central claim:
cao-mcp-serveris a real, shipped console script (pyproject [project.scripts]→server:main), resolvable (which→ venv path) and launchable (live FastMCP stdio launch confirmed; tier-3python -m …serveralso launches). - ⁇ NOT VERIFIED — the full end-to-end "agent comes up with orchestration tools inside Codex's 10s timeout" outcome (needs a live provider CLI session); the mechanism it depends on (fast local script vs. slow uvx clone) is confirmed.
Verdict
Approve with nits. A well-scoped, well-documented, end-to-end-verified fix with no blocking or correctness-critical issues. Recommended before merge: guard the command-less-entry edge case, add the copilot dead-import + CHANGELOG cleanups, and close the kimi/install_agent test gaps.
Code Review: resolver introduces a live TOML-escaping gap on the Codex pathVerdict: Request changes (or merge together with a companion escaping fix — see below) This PR adds a shared Finding: this PR turns a previously-inert Codex TOML gap into a live oneFile: if "command" in cfg:
command_parts.extend(["-c", f'{prefix}.command="{cfg["command"]}"'])Codex's This PR changes that.
So this PR is what makes Test coverage gap: the new test Suggested fix: either land this together with (or immediately followed by) a fix that escapes What's solid (no changes needed)
|
…fetch The bundled agent profiles configured the orchestration MCP server as `uvx --from git+https://github.com/awslabs/cli-agent-orchestrator.git@main cao-mcp-server`, which cold-clones and builds the whole package from GitHub on every agent launch. The project's own e2e cache-warmup fixture documents this as ~20s on a cold cache, which exceeds Codex's default 10s MCP startup timeout, so the agent starts WITHOUT its orchestration tools (handoff / assign / send_message) and silently no-ops -- the supervisor looks healthy but cannot delegate. It also pinned the MCP server to @main regardless of the installed CAO version, so the orchestration client could drift out of sync with the API server it talks to. cao-mcp-server is a console script installed alongside cao / cao-server, so the running CAO install already has a matching one. Point the bundled profiles (and examples/docs) at it, and add utils/mcp_resolution.py to turn the bare command into a PATH-independent invocation, since the agent subprocess's PATH may not contain the script dir (unactivated venv, devcontainer, pip --prefix): 1. the cao-mcp-server next to the running interpreter (exact env match), else 2. cao-mcp-server resolved on PATH, else 3. <python> -m cli_agent_orchestrator.mcp_server.server (no script needed). Runtime providers (Claude Code, Codex, Kimi, Copilot) rebuild the launch config each time and prefer the interpreter sibling. Install-time providers (Kiro, OpenCode) and the Antigravity/Cursor providers write the command to a config file they read at a later launch, so they resolve with persisted=True and prefer the stable PATH launcher (e.g. ~/.local/bin/...) -- which survives `uv tool upgrade`, whereas the versioned venv path would not. Copilot's previously-inline resolution now uses the shared helper. Also: guard the resolver against an empty sys.executable; delete the e2e uvx cache-warmup fixture (no network fetch to warm now); update the example profiles; and add resolver and bundled-profile tests.
tedswinyar
force-pushed
the
fix/cao-7le-mcp-server-command
branch
from
04b5486 to
89b23ee
Compare
|
Thanks for the careful review — you're right that this PR is what turns the Codex The companion escaping fix is #404 (linked from the description of that PR back here). It routes Two options from my side, happy with either:
One practical note on the Windows scenario specifically: CAO's tmux/libtmux dependency means the orchestrator doesn't currently run natively on Windows (WSL paths are POSIX-shaped), so the backslash path is defensive rather than reachable today — but agreed it should be correct regardless, and env values are caller-supplied either way. |
|
Thanks for the thorough review (and for running it end-to-end in a clean container — the differential verification is much appreciated). All four Important items are addressed in the updated head (
Also took the nits: The one line codecov flags as uncovered is the |
What happens
The bundled agent profiles start the orchestration MCP server with:
On a cold uvx cache this clones and builds the package from GitHub before the server starts. If that runs longer than the provider's MCP startup timeout, the agent comes up without its orchestration tools (
handoff,assign,send_message) and can't delegate, even thoughcao-serveris running fine.The repo's own test suite already documents this.
test/e2e/conftest.pypre-warms the cache with this note:The tests work around it by warming the cache first; a real cold start hits the failure.
It also pins the MCP server to
@mainregardless of the installed CAO version, so the orchestration client can drift out of sync with the API server it talks to.The fix
cao-mcp-serverships in the same package ascao-server(declared in[project.scripts]since the initial launch), so the running install already has a matching copy. This change:agent_store/),docs/agent-profile.md, and 26 example profiles at the barecao-mcp-serverconsole script instead of the uvx network fetch.utils/mcp_resolution.pyto turn the bare command into a PATH-independent invocation, since the agent subprocess's PATH may not contain the script dir (unactivated venv, devcontainer,pip install --prefix). It keeps the Copilot provider's 3-tier fallback, now shared by all providers:cao-mcp-servernext to the running interpreter (exact env match), elsecao-mcp-serverresolved on PATH, else<python> -m cli_agent_orchestrator.mcp_server.server(no console script needed).persisted=True, preferring the stable PATH launcher (e.g.~/.local/bin/cao-mcp-server) that survivesuv tool upgrade, whereas the versioned venv path would not._inject_kiro_mcp_timeoutdetection to match the new command forms (bare script, resolved absolute path, module entrypoint) in addition to the legacy uvx args form, so custom-named cao-mcp-server entries keep the raised kiro tool-call timeout.Notes
cao-ops-mcp-serversnippets in the README are intentionally untouched: they are configured by an external agent where CAO isn't installed, so the uvx form is correct there. Same forexamples/fleet/deploy/bootstrap.sh, which installs CAO itself.Testing
test/utils/test_mcp_resolution.py: resolution order for runtime vs persisted, passthrough for non-CAO commands, module-entrypoint fallback, idempotency.test/utils/test_bundled_profiles.py: every bundled profile declares the bare console script; regression guard against the uvx form._inject_kiro_mcp_timeouttests for all command forms.Implements the fix discussed with the maintainers in the internal Slack channel (approach and the two refinements confirmed there: shared 3-tier fallback, runtime injection over static paths in profiles).