feat(cli): arctl init --mcp adds MCP_SERVERS_CONFIG for remote servers#501
Open
xytian315 wants to merge 15 commits into
Open
feat(cli): arctl init --mcp adds MCP_SERVERS_CONFIG for remote servers#501xytian315 wants to merge 15 commits into
xytian315 wants to merge 15 commits into
Conversation
xytian315
force-pushed
the
xytian315/run-remote-mcp
branch
2 times, most recently
from
1fee963 to
3195dea
Compare
# Conflicts: # internal/cli/declarative/run.go
- writeMCPServersConfig now strips any existing MCP_SERVERS_CONFIG= line (and its marker comment) before writing, so a re-run replaces rather than appends. dotenv's last-wins parsing would otherwise silently mask the older line. - arctl init now warns on stderr when remote MCP headers are written to .env in plaintext, calling out the shared-catalog token-leak risk. - Reject empty --mcp "" upfront instead of producing a confusing "not found: @latest" registry error. - Regression test pins the dedup behavior.
- ResolvedMCP.Tag was populated but never consumed by any caller. Drop it (and the test assertion that was the only thing keeping it alive). - arctl run's remote-only mcp.yaml error mentioned the URL twice; one mention plus the inspector hint is enough.
CI's lint-go and verify jobs caught: - declarative.go: third-party cobra must precede the localmodule block. - init_internal_test.go / mcpresolve_test.go: stretchr/testify must precede the local v1alpha1 import, blank-line separated. - init.go: replace `for i := 0; i < len(lines); i++` with range loop (modernize linter).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends arctl init agent --mcp <ref> to resolve catalog MCPServer refs against the registry up-front and, when the ref is a remote MCP (spec.remote set), append an MCP_SERVERS_CONFIG entry to the agent's .env so arctl run reaches the remote without manual editing. It also makes arctl run fail fast (with an MCP Inspector hint) on remote-only mcp.yaml projects.
Changes:
- New
internal/cli/declarative/mcpresolvepackage with aFetcherinterface andResolvethat returns URL/headers for remote MCPs, with unit tests and a fake-fetcher injection point ininit.go. arctl init agentnow resolves--mcprefs before any file writes, merges remote entries with--local-mcpentries into a single rewrittenMCP_SERVERS_CONFIG=line, warns when catalog headers (potential tokens) are written verbatim, and adds anapiClientMCPFetcheradapter pluslookupPersistentFlag/lookupRegistryURL/lookupRegistryTokenhelpers because the init subtree skips the rootPersistentPreRunE.arctl runshort-circuits with a clear error pointing atnpx -y @modelcontextprotocol/inspector --server-url <url>whenmcp.yamldeclares onlyspec.remote; new unit + e2e tests cover the happy path, source-mode no-write, registry-failure no-partial-writes, and the run-fast-fail.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/cli/declarative/mcpresolve/mcpresolve.go | New package resolving an MCPServer ref via an injectable Fetcher and flattening remote headers. |
| internal/cli/declarative/mcpresolve/mcpresolve_test.go | Unit tests for remote, source-mode, and fetcher-error paths. |
| internal/cli/declarative/init.go | Pre-resolve --mcp refs, unify MCP_SERVERS_CONFIG writer (with strip-and-replace), warn on header leakage, refactor readMCPYAML/readMCPName. |
| internal/cli/declarative/init_internal_test.go | Tests for env-merge, replace-not-duplicate behavior, remote/source init flows, and registry-failure cleanup. |
| internal/cli/declarative/declarative.go | Adds apiClientMCPFetcher, lookupPersistentFlag, and registry URL/token lookups for the offline init path. |
| internal/cli/declarative/run.go | Fail-fast for remote-only mcp.yaml with MCP Inspector guidance. |
| internal/cli/declarative/run_internal_test.go | New test asserting the remote-only run error message. |
| e2e/init_build_run_test.go | E2E coverage for init-wire-env happy path and run remote-only error. |
| docs/declarative-cli.md | Documents --mcp/--local-mcp wiring behavior and the run-on-remote-MCP error guidance. |
Drop redundant rationale and re-statements; same info in ~35% fewer bytes. No behavioral changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What changed:
arctl init agent --mcp <ref>now auto-appends the remote URL to.envwhen the catalog ref is a remote MCP.
arctl runon a remote-only MCP folder fails fast with a clear messagepointing at the MCP Inspector (
npx -y @modelcontextprotocol/inspector ...).Change Type
/kind feature
Changelog
Additional Notes