Skip to content

feat(acp): add Command Code backend support - #1595

Closed
fshiori wants to merge 3 commits into
openabdev:mainfrom
fshiori:feat/commandcode-acp
Closed

fshiori wants to merge 3 commits into
openabdev:mainfrom
fshiori:feat/commandcode-acp

Conversation

@fshiori

@fshiori fshiori commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this solve?

Command Code is missing from OpenAB’s setup wizard, container variants, and backend documentation. This PR adds it as an opt-in backend using its native ACP server and OpenAB’s existing connection/session handling.

Related issue: Not linked.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365158868619404/1566712467349045288

Review Contract

Goal

Add Command Code support through setup, container images, build workflows, documentation, and integration tests.

Non-goals

Redesigning ACP transport, changing existing backend defaults, adding interactive question/plan approval UI, or fixing shared native Windows npm launcher handling.

Accepted Residual Risks

Proposed for maintainer approval:

  • Authenticated model turns remain opt-in; CI smoke tests cover initialization, working-directory validation, and authentication errors.
  • Native Windows runtime behavior was not validated and follows the existing shared launcher implementation.

Acceptance Criteria

  • Setup generates command-code acp, resolves local relative working directories, and preserves container paths.
  • Standalone, unified, and package images pin the CLI and expose the expected ACP/auth commands.
  • Existing build workflows include the new variant, and credential-free ACP smoke checks pass.
  • Live testing verifies streamed replies, tool execution, and session loading after an agent restart.
  • Existing backend defaults and child-process environment isolation remain unchanged.
  • Required upstream CI passes before merge.

Follow-ups

Shared native Windows npm launcher handling and dedicated chat UI for agent questions/plan approvals can be handled separately.

At a Glance

Discord / Slack / gateway adapters
|
v
OpenAB dispatcher + session pool
|
ACP over stdio
|
v
command-code acp

Prior Art & Industry Research

OpenClaw:

OpenClaw runs external coding harnesses through an ACP backend plugin, such as acpx. This provides a comparable approach to integrating an existing agent runtime through ACP. ACP agents documentation (https://docs.openclaw.ai/tools/acp-agents)

Hermes Agent:

Hermes exposes hermes acp as a stdio ACP server, including streamed messages, tool activity, and approval prompts. It reserves stdout for protocol traffic and sends logs to stderr. ACP Host Integration (https://hermes-agent.nousresearch.com/docs/user-guide/features/acp)

Other references:

Command Code’s native ACP documentation (https://commandcode.ai/docs/acp) describes the standard initialize → session/new → session/prompt flow used by this integration.

Proposed Solution

  • Add commandcode to the setup wizard and generate command-code acp.
  • Resolve local working directories to absolute paths, as required by Command Code.
  • Add Dockerfile.commandcode and matching targets in Dockerfile.unified and Dockerfile.package, pinning command-code to 1.88.2.
  • Include the variant in existing image build and smoke-test workflows.
  • Add credential-free CLI smoke checks and an opt-in authenticated Rust integration test.
  • Update backend documentation, configuration examples, and both READMEs.

Why this approach?

Command Code already implements ACP, and OpenAB already handles the required transport, permissions, streaming, and session lifecycle. Reusing that implementation keeps the integration consistent with other native ACP backends.

Alternatives Considered

  • A separate Command Code adapter: unnecessary because the CLI already exposes the required ACP interface.
  • Documentation-only configuration: would leave setup, container packaging, and automated compatibility checks unavailable.

Validation

  • cargo clippy --workspace -- -D warnings
  • Setup tests: cargo test -p openab-core --no-default-features setup:: — 8 passed.
  • Authenticated integration test: cargo test -p openab-core --no-default-features --test commandcode_acp -- --ignored --nocapture — passed using deepseek/deepseek-v4-flash; verified streaming, tool-written file contents, process restart, and restored conversation memory.
  • cargo build --no-default-features --features discord,slack
  • Docker runtime image built with a freshly compiled binary from this branch; verified OpenAB 0.10.0, binary hash, CLI version, non-root execution, and credential-free ACP checks.
  • Workflow YAML, shell syntax, Node base checks, and packaged CLI version consistency checks passed.
  • Manual Discord startup: mounted configuration, project directory, and existing credentials; confirmed authenticated Command Code, healthy container status, Discord connection, and slash-command registration.
  • Full cargo test --workspace and complete default/unified release builds remain for upstream CI.

Formatting note: with Rust 1.99.1, the baseline (9d0317b) and this branch report the same 37 unchanged files under cargo fmt --all -- --check. No formatting failures were introduced by this PR.

@fshiori
fshiori requested a review from thepagent as a code owner November 2, 2026 07:07
@openab-app openab-app Bot added closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. and removed closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. labels Nov 2, 2026
@chaodu-agent

Copy link
Copy Markdown
Collaborator

Important

CHANGES REQUESTED ⚠️ - Clean, well-tested backend addition that follows the existing variant pattern; four small items (local bake target, late working_dir failure, Helm path divergence, redistribution terms) should be settled before merge.

What This PR Does

Command Code ships a native ACP server (command-code acp), but OpenAB had no setup option, image, CI lane, or docs for it. This PR adds Command Code as an opt-in backend using the existing AcpConnection and session pool, with no changes to core runtime code.

How It Works

  • Setup wizard: commandcode maps to command-code acp. For local deployments, resolve_agent_working_dir turns a relative working_dir into an absolute path, because Command Code rejects relative ACP cwd values. Container paths are kept as-is.
  • Images: new Dockerfile.commandcode plus matching targets in Dockerfile.unified and Dockerfile.package, all pinning COMMAND_CODE_VERSION=1.88.2 on Node 22, running as the non-root node user, with OPENAB_AGENT_COMMAND / OPENAB_AGENT_AUTH_COMMAND set.
  • CI: the variant is added to the build-images, operator, pre-beta, snapshot, nightly, and pr-preview lists and to both smoke-test matrices. commandcode_acp_smoke.mjs runs in an isolated HOME with a stripped environment and checks initialize, relative-cwd rejection (-32602), and the unauthenticated session/new error (-32000) without credentials or model calls. Nightly auto-bump reads the npm latest version through the existing check_ver allowlist.
  • Tests and docs: an #[ignore] authenticated round-trip test (commandcode_acp.rs) covers streaming, tool execution, and session/load after a process restart. docs/commandcode.md covers API-key auth via inherit_env, persisted login, Helm, and troubleshooting.

Findings

# Severity Finding Location
1 🟡 docker-bake.hcl has no commandcode target, so docker buildx bake commandcode fails and the default bake group skips it docker-bake.hcl:16-21
2 🟡 Absolute working_dir is enforced only by the wizard; a hand-written relative path passes config load and fails later at session/new crates/openab-core/src/setup/wizard.rs:374, crates/openab-core/src/acp/connection.rs:608
3 🟡 Wizard Helm instructions diverge for this backend only (-f values.yaml instead of --set), which works around a chart gap shared by all API-key backends crates/openab-core/src/setup/wizard.rs:565
4 🟡 command-code is UNLICENSED, closed-source and obfuscated, and will be redistributed inside published OpenAB images; redistribution terms are not documented Dockerfile.commandcode:44
5 🟢 Follows the established per-backend pattern across standalone, unified, and package images and every build workflow Dockerfile.*, .github/workflows/
6 🟢 Credential-free ACP smoke gate with environment isolation that matches OpenAB's env_clear baseline; independently re-run against the real 1.88.2 CLI and all assertions passed .github/scripts/commandcode_acp_smoke.mjs
7 🟢 Meaningful opt-in live test (streaming, tool writes, restart plus session/load) crates/openab-core/tests/commandcode_acp.rs
8 🟢 Least-privilege auth guidance (inherit_env = ["COMMAND_CODE_API_KEY"], explicit warning against forwarding chat tokens); nightly bump stays injection-safe via check_ver docs/commandcode.md, .github/workflows/nightly-build.yml
Finding Details

🟡 F1: Missing docker-bake.hcl target

Every other variant (kiro, claude, codex, ... pi) has a bake target and is listed in group "default". CI uses plain docker build, so CI is unaffected, but local docker buildx bake commandcode fails and docker buildx bake silently omits the new image.

Requested change: add a target "commandcode" (dockerfile Dockerfile.commandcode, tag openab:commandcode, contexts = { builder = "target:builder" }) and add it to the default group.

🟡 F2: Relative working_dir fails late, outside the wizard

resolve_agent_working_dir only runs in run_setup. Config load accepts any string, and connection.rs passes working_dir straight through as the ACP cwd. A hand-edited working_dir = "." (or a config generated by other tooling) starts cleanly and then fails on the first message with a -32602 from the agent. docs/commandcode.md:28 does document the requirement, which helps, but this is a new per-backend invariant that other backends do not have.

Requested change (either is fine): normalize working_dir to an absolute path at config load (preferred; harmless for every backend and removes the agent == "commandcode" special case), or fail fast at startup with the same message as the docs when the command is command-code and the path is relative.

🟡 F3: Backend-specific Helm instructions

For every other agent the wizard prints a one-line helm install ... --set agents.<name>.discord.botToken=.... For commandcode it prints "Prepare values.yaml using docs/commandcode.md". If the reason is that secretEnv plus inherit_env cannot be expressed through --set, that limitation affects all API-key backends (Gemini, Codex, and others), not just this one.

Requested change: either provide an equivalent --set invocation, or add a one-line explanation in docs/commandcode.md of why values.yaml is required, so the divergence is intentional and documented rather than per-agent.

🟡 F4: Redistribution terms for a proprietary package

npm metadata for command-code@1.88.2: license: UNLICENSED, obfuscated build (publish:prod runs build:obfuscated), no linked source repository, published by the Command Code maintainers with npm registry signatures. It has no install-time lifecycle scripts, which is good. The risk here is redistribution, not supply-chain integrity: OpenAB would publish images that contain this package. There is precedent (other proprietary agent CLIs are already bundled), but the terms should be confirmed rather than assumed.

Requested change: link Command Code's terms that permit redistribution in third-party container images (in the PR description or docs/commandcode.md), or confirm with the vendor. If the terms do not allow it, ship the Dockerfile and docs but keep the variant out of the published image matrix.

🟢 F5-F8

  • Version pin asserted across all three Dockerfiles with assert_line, and the version plus OPENAB_AGENT_COMMAND / OPENAB_AGENT_AUTH_COMMAND are verified in the smoke job.
  • The smoke script was also run independently against the real CLI: agentInfo.name = "Command Code", protocolVersion: 1, loadSession: true, relative cwd gives -32602, unauthenticated session/new gives -32000.
  • docker-smoke-test-unified.yml now triggers on changes to itself, a small fix that is useful beyond this PR.
  • No secret logging, no permission fail-open, and Discord/Slack tokens are excluded from the forwarded environment.
Non-blocking follow-ups (not counted in the verdict)
  • commandcode_acp_smoke.mjs: env.PATH assumes process.env.PATH is defined; the /absolute|not authenticated/i checks pin upstream wording and may break on a nightly bump if Command Code rewords its errors; the responder resolves any message with a matching id, including agent-to-client requests (latent only, fails closed).
  • commandcode_acp.rs: exact-string asserts on agentInfo.name and stop_reason will break on an upstream rename (acceptable for an ignored test).
  • resolve_agent_working_dir uses to_string_lossy, which mangles non-UTF-8 paths; returning PathBuf would avoid this.
  • npm install ... --retry 3 is not an npm flag (--fetch-retries is). This follows the existing claude/kimi pattern, so a repo-wide cleanup is better than a one-off change.
  • /auth runs command-code login (browser OAuth with a localhost callback), which cannot complete inside a container. A docs line saying /auth is local-only would help.
  • operator/src/create.rs ECR map and chart NOTES.txt auth hints do not include commandcode; the same gap already exists for kimi/mimocode, so this belongs in a separate follow-up.
  • Considered and dismissed: extending validate_agent_command is not stale tech debt in the runtime path, because the function is #[cfg(test)] only.
Reviewer Verdicts
Reviewer Angle Verdict Key points
Reviewer A Architecture / general LGTM Consistent pattern, strong smoke and live coverage, least-privilege auth docs
Reviewer B Correctness Changes on F2, F3 Late working_dir failure, Helm divergence, smoke/test edge cases
Reviewer C Security / CI Approve after F1 Re-ran the smoke script against the real CLI, verified no install scripts and safe nightly bump, found the missing bake target
Coordinator Aggregation Changes requested Verified F1-F4 in source and npm metadata; dismissed the allowlist concern (#[cfg(test)])

No external reviewer comments were present at review time. The Review Contract check passed.

CI / Status

All completed checks pass at this SHA, including smoke-test (Dockerfile.commandcode, -commandcode, command-code, acp) and smoke-test-unified (commandcode, command-code). Full cargo test --workspace and full release builds run upstream, as the PR description states.

Baseline Check
  • PR opened: 2026-11-02
  • Head: 5716c9335c032c8682065754db5401df70551951
  • Base: main (not stacked), merge-base 9d0317bd0803947e75bc41edcb9adf9921f7ec20
  • Diff: 22 files, +625 / -23
  • Main already has: generic ACP transport, session pool, session/load, inherit_env, and the per-variant image/CI pattern. Running command-code acp was already possible with a hand-written config and a custom image.
  • Net-new value: first-class setup option, pinned images in three layouts, CI compatibility gate, nightly version tracking, and documentation.

5. Three Reasons We Might Not Need This PR

  1. Variant sprawl - each backend adds three Dockerfile targets, about seven workflow entries, nightly polling, and version-bump chores. A generic "bring your own ACP command" guide would deliver most of the value with no new matrix entries.
  2. Already possible today - since no core code changes, users can already run command-code acp with a custom image and config; the PR mainly adds packaging and convenience.
  3. Vendor-controlled contract - a closed-source, obfuscated CLI whose ACP behavior, error wording, and auth flow can change on any nightly bump, with OpenAB carrying the breakage in its published images.

None of these are implementation blockers. They are product trade-offs for maintainers to weigh against demand for Command Code.

@thepagent

Copy link
Copy Markdown
Collaborator

Setting this PR to draft. We won't proceed this PR until we have 10 👍
see https://discord.com/channels/1491295327620169908/1491365158868619404/1566784627740512397

@thepagent
thepagent marked this pull request as draft November 2, 2026 12:05
@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Caution

This PR has been waiting on the author for more than 2 days (labeled pending-contributor since 2026-11-02).
It will be automatically closed in 24 hours if there is no update.

@fshiori — You must add a new comment on this PR to remove the closing-soon label and keep it open. Pushing commits alone is not sufficient. Feel free to reopen a new PR later if it gets closed and you want to pick it back up.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🔒 Auto-closing: this PR has had the closing-soon label for more than 1 days without activity from the author.

If you'd like to continue working on this, please submit a new PR and link to this one if necessary.

@github-actions github-actions Bot closed this Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. pending-contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants