feat: add MCode as a built-in ACP agent - #503
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: found issues before merge. Reviewed August 21, 2026, 11:55 PM ET / August 22, 2026, 03:55 UTC. ClawSweeper reviewWhat this changesThe PR registers Merge readinessKeep open for an explicit product decision: the proof supports the narrow wiring, but adding a permanent built-in keyword duplicates an existing configured-agent path and creates a durable compatibility contract. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherACPX resolves a user-facing agent name into a launch command, then runs that command as an ACP server and manages its session and permissions. The built-in registry defines durable command keywords, while local configuration can add or override named agents. flowchart LR
A[User CLI command] --> B[Agent-name resolver]
C[Project or global configuration] --> B
D[Built-in agent registry] --> B
B --> E[ACP process launch]
E --> F[ACP client session]
F --> G[Agent response]
Decision needed
Why: The implementation is mechanically sound, but VISION and repository policy treat new keywords as long-term API surface; only maintainers can decide whether this convenience warrants an enduring support commitment. Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Either obtain explicit approval that Do we have a high-confidence way to reproduce the issue? Not applicable: this PR proposes a new built-in convention, not a report of broken behavior. Is this the best way to solve the issue? No: the current supported configuration path already provides the same named structured launch, so a permanent built-in should land only after explicit product approval. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2d735cf18220. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
|
Addressed the actionable review items in
The remaining question—whether @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
|
Addressed the remaining author-side documentation finding in
The real-agent proof remains in the PR body. The only remaining question is maintainer approval of @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@steipete Would you mind taking a look and making the product call on this one? I’d like to get MCode included as a built-in ACP agent. While acpx mcode exec 'summarize this repository'The implementation is intentionally narrow: it only registers the native ClawSweeper’s remaining concern is the long-term keyword/support contract rather than an implementation defect. I’m in favor of accepting that contract and merging the PR, but I’d appreciate your decision as the maintainer familiar with this registry boundary. If you’re open to the built-in, I’m happy to address any final changes you’d like. |
1 participant
Closes #502
What Problem This Solves
MiniMax Code already exposes a native ACP v1 server through
mcode acp, but acpx users currently have to configure that command manually. This makes MCode unavailable through the same built-in agent workflow as Codex, Claude, Gemini, and the other supported ACP agents.Why This Change Was Made
Add
mcodeto both built-in command registries as structured argv (["mcode", "acp"]), with registry and cross-platform CLI integration coverage. The agent guide documents installation, authentication, permission handling, and the currentsession/loadlimitation without adding an adapter or sidecar.User Impact
After installing and authenticating MiniMax Code, users can run:
MCode remains responsible for its own runtime, login state, model configuration, and tools; acpx only owns the ACP client lifecycle and permission policy.
Evidence
pnpm run check— 918 tests passed; coverage thresholds passed (94.73% lines, 87.94% branches)pnpm run check:docs— formatting, markdown lint, and docs site build passedmcode acpresolution on Unix and Windows.cmdlaunch paths@minimax-ai/code0.1.2 returnedACPX_MCODE_OKthroughacpx mcode exec; the redacted transcript is belowloadSession: false, streamed a terminal tool-call lifecycle, and returned the expected result; the redacted protocol excerpt is belowRedacted real-agent transcript
This was rerun from the PR checkout against the installed, authenticated
@minimax-ai/code0.1.2 binary. The working-directory path and per-session/message/toolidentifiers are omitted; no protocol result fields were changed.
The second run used
--format json --json-strict --approve-all, asked MCode to executeprintf ACPX_TOOL_OK, and filtered the transcript to the initialize response, toollifecycle, final agent message, and prompt result:
{"jsonrpc":"2.0","id":0,"result":{"protocolVersion":1,"agentCapabilities":{"loadSession":false,"mcpCapabilities":{"http":true,"sse":true},"promptCapabilities":{"image":false,"audio":false,"embeddedContext":false}},"agentInfo":{"name":"minimax-code","title":"MiniMax Code","version":"0.1.2"}}} {"jsonrpc":"2.0","method":"session/update","params":{"update":{"sessionUpdate":"tool_call","toolCallId":"<redacted>","title":"bash","name":"bash","kind":"execute","status":"in_progress","rawInput":{"command":"printf ACPX_TOOL_OK"}}}} {"jsonrpc":"2.0","method":"session/update","params":{"update":{"sessionUpdate":"tool_call_update","toolCallId":"<redacted>","status":"completed","rawInput":{"command":"printf ACPX_TOOL_OK"},"rawOutput":{"content":[{"type":"text","text":"ACPX_TOOL_OK"}]}}}} {"jsonrpc":"2.0","method":"session/update","params":{"update":{"sessionUpdate":"agent_message_chunk","messageId":"<redacted>","content":{"type":"text","text":"ACPX_TOOL_OK"}}}} {"jsonrpc":"2.0","id":2,"result":{"stopReason":"end_turn"}}Scope Notes
execusage and explicitly state that sequential CLI invocations start fresh MCode context.AI assistance was used to investigate the integration path, implement the change, run validation, and draft this PR. All generated changes and evidence were reviewed before submission.