Skip to content

feat: add MCode as a built-in ACP agent - #503

Open
hetaoBackend wants to merge 3 commits into
openclaw:mainfrom
hetaoBackend:feat/mcode-agent
Open

feat: add MCode as a built-in ACP agent#503
hetaoBackend wants to merge 3 commits into
openclaw:mainfrom
hetaoBackend:feat/mcode-agent

Conversation

@hetaoBackend

@hetaoBackend hetaoBackend commented Aug 16, 2026

Copy link
Copy Markdown

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 mcode to 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 current session/load limitation without adding an adapter or sidecar.

User Impact

After installing and authenticating MiniMax Code, users can run:

acpx mcode exec 'summarize this repository'

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 passed
  • Focused registry test passed after first demonstrating the missing built-in as a failing test
  • Focused CLI integration test verifies mcode acp resolution on Unix and Windows .cmd launch paths
  • Live smoke with @minimax-ai/code 0.1.2 returned ACPX_MCODE_OK through acpx mcode exec; the redacted transcript is below
  • Live JSON smoke completed ACP initialize/session/prompt, advertised loadSession: false, streamed a terminal tool-call lifecycle, and returned the expected result; the redacted protocol excerpt is below
  • Project autoreview: clean, no accepted/actionable findings

Redacted real-agent transcript

This was rerun from the PR checkout against the installed, authenticated
@minimax-ai/code 0.1.2 binary. The working-directory path and per-session/message/tool
identifiers are omitted; no protocol result fields were changed.

$ mcode --version
0.1.2

$ ./node_modules/.bin/tsx src/cli.ts --format quiet --timeout 180 --deny-all --cwd <temporary-directory> mcode exec 'Reply with exactly ACPX_MCODE_OK and do not use tools.'
ACPX_MCODE_OK

The second run used --format json --json-strict --approve-all, asked MCode to execute
printf ACPX_TOOL_OK, and filtered the transcript to the initialize response, tool
lifecycle, 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

  • No MCode protocol changes are included.
  • MCode 0.1.2 does not advertise provider-session reload. The docs now show only one-shot exec usage 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.

@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@hetaoBackend
hetaoBackend marked this pull request as ready for review August 16, 2026 13:15
@hetaoBackend
hetaoBackend requested a review from a team as a code owner August 16, 2026 13:15
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 16, 2026
@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 28, 2026, 11:47 PM ET / August 29, 2026, 03:47 UTC.

ClawSweeper review

What this changes

This PR registers mcode -> mcode acp as a built-in ACP agent, with synchronized documentation and registry/integration tests.

Merge readiness

⚠️ Needs maintainer review before merge - 4 items remain

Keep open: the mapping and real-agent proof are credible, but an owner must explicitly approve reserving mcode as a permanent keyword when current configuration already supports custom ACP commands.

Priority: P3
Reviewed head: 5b40647f7cf48e7f747191afeacc9110ef5c3c36
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused implementation and real runtime evidence are credible, but the permanent registry contract remains unapproved.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body records a redacted run of the introduced registry mapping through an installed, authenticated native MCode ACP server, including a one-shot result and an ACP initialize/prompt/tool-call trace; it directly exercises the changed launch behavior.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body records a redacted run of the introduced registry mapping through an installed, authenticated native MCode ACP server, including a one-shot result and an ACP initialize/prompt/tool-call trace; it directly exercises the changed launch behavior.
Evidence reviewed 8 items Introduced permanent keyword: The pinned PR delta adds mcode to both command registries, changing an unreserved command into a built-in product keyword.
Existing configurable path: Current documentation says an ACP command need not be built in and supports named config-defined agents for repeated commands; invocation resolution gives configured agents precedence.
Product-surface policy: The vision identifies keywords as API surface and directs contributors to add fewer, intentional, stable conventions.
Findings 1 actionable finding [P1] Obtain approval before reserving mcode
Security None None.

How this fits together

ACPX resolves a friendly agent name into a command and launches that command as an ACP stdio server. The resulting protocol stream is handled by ACPX’s common client, permission policy, and session lifecycle.

flowchart LR
  User[Agent or automation] --> CLI[ACPX command line]
  CLI --> Registry[Built-in agent registry]
  Registry --> Launch[Launch mcode acp]
  Launch --> Server[MiniMax Code ACP server]
  Server --> Client[ACPX ACP client]
  Client --> Output[Messages and results]
Loading

Decision needed

Question Recommendation
Should ACPX reserve mcode as a permanent built-in agent keyword when configuration-defined agents already launch mcode acp? Approve the built-in: Accept mcode as a supported permanent keyword backed by its native ACP command.

Why: The implementation is narrow, but the repository treats keywords and naming conventions as long-term compatibility surface.

Before merge

  • Obtain approval before reserving `mcode` (P1) - The introduced registry entry creates a permanent keyword, while current docs support named structured-argv custom agents. The PR does not establish why that path is insufficient or include maintainer approval for the new product contract.
  • Resolve merge risk (P1) - Merging without an explicit registry decision creates a durable keyword and support contract despite an existing configurable launch path.
  • Complete next step (P2) - Human approval of the permanent keyword is the only remaining merge blocker; there is no narrow mechanical repair to dispatch.

Findings

  • [P1] Obtain approval before reserving mcodesrc/agent-registry.ts:54
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 7 files, +134/-0 The change is limited to one registry addition, matching documentation, and coverage.
Production versus tests production +2, tests +67 Runtime growth is limited to synchronized command and argv mappings; most code changes validate the behavior.

Merge-risk options

Maintainer options:

  1. Obtain explicit registry approval (recommended)
    Confirm that MCode should receive a permanent built-in keyword before merging the otherwise focused mapping.
  2. Keep configuration-first support
    Close the proposal without reserving the keyword, since named structured argv configuration supports the same launch command.

Technical review

Best possible solution:

Make an explicit registry-policy decision: merge the focused native mapping only if MCode merits permanent built-in support; otherwise retain configuration-defined agent support.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds a built-in shortcut rather than repairing a reported failure. The supplied terminal trace does exercise the native MCode command through ACPX.

Is this the best way to solve the issue?

Unclear pending product direction: the direct mapping is narrow if the keyword is accepted, but configuration-defined agents already cover the underlying launch capability.

Full review comments:

  • [P1] Obtain approval before reserving mcodesrc/agent-registry.ts:54
    The introduced registry entry creates a permanent keyword, while current docs support named structured-argv custom agents. The PR does not establish why that path is insufficient or include maintainer approval for the new product contract.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ef6b81e71756.

Labels

Label justifications:

  • P3: This is an optional discoverability and product-surface addition, not a broken existing workflow.
  • merge-risk: 🚨 compatibility: A built-in keyword becomes a durable command and support contract beyond the configurable path.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body records a redacted run of the introduced registry mapping through an installed, authenticated native MCode ACP server, including a one-shot result and an ACP initialize/prompt/tool-call trace; it directly exercises the changed launch behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body records a redacted run of the introduced registry mapping through an installed, authenticated native MCode ACP server, including a one-shot result and an ACP initialize/prompt/tool-call trace; it directly exercises the changed launch behavior.

Evidence

What I checked:

  • Introduced permanent keyword: The pinned PR delta adds mcode to both command registries, changing an unreserved command into a built-in product keyword. (src/agent-registry.ts:54, 5b40647f7cf4)
  • Existing configurable path: Current documentation says an ACP command need not be built in and supports named config-defined agents for repeated commands; invocation resolution gives configured agents precedence. (docs/custom-agents.md:6, 5b40647f7cf4)
  • Product-surface policy: VISION.md identifies keywords as API surface and directs contributors to add fewer, intentional, stable conventions. (VISION.md:79, 5b40647f7cf4)
  • Current main has not absorbed the change: The fetched current-main registry contains no mcode entry, so the PR remains necessary if the product decision is affirmative. (src/agent-registry.ts:39, ef6b81e71756)
  • Real behavior evidence: The supplied PR body includes a redacted authenticated MCode run through acpx mcode exec, plus an ACP initialize/prompt/tool-call trace that reports loadSession: false. (5b40647f7cf4)
  • Session documentation matches ACPX lifecycle: The added MCode guide warns against cross-invocation persistence, consistent with createSession closing its client after recording the session. (src/cli/session/session-management.ts:225, 5b40647f7cf4)

Likely related people:

  • Peter Steinberger: Commit f64e2ca established the structured launch boundary used by this proposal. (role: structured argv compatibility author; confidence: high; commits: f64e2ca4e4cc; files: src/agent-registry.ts)
  • JordanTheJet: Commit b34d7be added the recent ZeroClaw built-in in the same registry surface. (role: recent built-in registry contributor; confidence: high; commits: b34d7be64799; files: src/agent-registry.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Obtain an explicit owner decision on the permanent mcode registry contract.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (7 earlier review cycles)
  • reviewed 2026-08-16T13:21:18.339Z sha 79b5bf4 :: needs real behavior proof before merge. :: [P2] Remove the contributor-owned changelog entry
  • reviewed 2026-08-16T14:28:49.624Z sha ddafca4 :: needs changes before merge. :: [P2] Remove the persistent-session example
  • reviewed 2026-08-16T14:34:28.761Z sha 5b40647 :: found issues before merge. :: [P1] Avoid reserving mcode without an approved registry need
  • reviewed 2026-08-21T22:22:09.715Z sha 5b40647 :: found issues before merge. :: [P1] Avoid reserving mcode without an approved registry need
  • reviewed 2026-08-22T03:55:48.190Z sha 5b40647 :: found issues before merge. :: [P1] Avoid reserving mcode without an approved registry need
  • reviewed 2026-08-24T22:07:33.800Z sha 5b40647 :: found issues before merge. :: [P1] Obtain approval before reserving mcode
  • reviewed 2026-08-28T23:30:11.685Z sha 5b40647 :: found issues before merge. :: [P1] Obtain approval before reserving mcode

@hetaoBackend

Copy link
Copy Markdown
Author

Addressed the actionable review items in ddafca4:

  • Removed the contributor-owned CHANGELOG.md entry; release notes remain owned by the release process.
  • Reran the integration against the installed, authenticated @minimax-ai/code 0.1.2 binary.
  • Added a redacted, inspectable transcript to the PR body covering ACP v1 initialization, the advertised loadSession: false boundary, a completed terminal tool call, the final agent message, and end_turn.
  • Reran pnpm run check (918/918 tests, coverage gates passed) and pnpm run check:docs successfully on the updated checkout.

The remaining question—whether mcode should be accepted as a permanent built-in keyword—is intentionally left to maintainer product judgment.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 16, 2026
@hetaoBackend

Copy link
Copy Markdown
Author

Addressed the remaining author-side documentation finding in 5b40647:

  • Removed the sessions new plus later bare-prompt example from agents/MCode.md.
  • Documented that sessions new closes the ACP client after saving the local record and that, without provider-session reload, a later CLI invocation starts fresh MCode context.
  • Synchronized the same exec-first boundary in docs/agents.md.
  • Verified pnpm run check:docs and explicit Markdown lint for agents/README.md and agents/MCode.md.

The real-agent proof remains in the PR body. The only remaining question is maintainer approval of mcode as a permanent built-in keyword.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@hetaoBackend

Copy link
Copy Markdown
Author

@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 mcode acp can already be configured through the custom-agent path, a built-in entry gives users the same straightforward discovery and invocation experience as the other supported agents:

acpx mcode exec 'summarize this repository'

The implementation is intentionally narrow: it only registers the native mcode acp argv, without an adapter or sidecar. Cross-platform registry/CLI coverage, the full test suite, docs checks, and authenticated real-agent ACP smoke tests all pass. The current lack of provider-session reload is also documented explicitly.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: add MCode as a built-in ACP agent

1 participant