feat(acp): support ACP service for OpenCodeReview

[chaojixinren]

ACP Module Architecture

The module keeps the ACP protocol boundary independent from OCR's core review engine. The adapter owns client-facing sessions and orchestration, while the existing ocr binary remains responsible for review execution and its review LLM.

flowchart LR
    C["ACP clients<br/>Paseo / Zed / JetBrains"]
    T["JSON-RPC 2.0 transport<br/>stdio"]
    S["ocr-acp<br/>ACP Server"]
    SE["Session and command layer<br/>available_commands_update<br/>session/prompt / session/cancel"]
    IP["Intent parser<br/>slash commands / natural language<br/>clarification state"]
    O["OCR process orchestrator<br/>timeouts / cancellation / cleanup<br/>stdout-stderr isolation"]
    CLI["Existing ocr CLI<br/>review / scan"]
    R["Existing CLI output<br/>stderr logs / stdout JSON"]

    C <--> T
    T <--> S
    S --> SE
    SE --> IP
    SE --> O
    O --> CLI
    CLI --> R
    R --> O
    O --> SE
    SE --> S
    S --> T

Loading

The client sends prompts and commands through the transport. ocr-acp parses them into a review or scan intent, asks for clarification when required, launches the existing OCR CLI as a child process, and streams progress and structured results back through the same ACP session. Cancellation and timeout signals flow into the process orchestrator, which is responsible for terminating and cleaning up the managed process tree before delivering the final outcome.

The implemented transport is stdio and the exposed OCR operations are review and scan. Streamable HTTP and OCR session-command integration are outside this PR's scope.

Why ACP Support

OpenCodeReview (OCR) is Alibaba's open-source AI code review CLI, implemented in Go under the Apache-2.0 license. It has been used by tens of thousands of developers inside Alibaba and has found millions of code defects. OCR uses a hybrid architecture of deterministic engineering and agents: file selection, file-level context isolation, rule matching, comment locations, and reflection are protected by deterministic engineering logic, while agents handle dynamic decisions and context retrieval. This improves accuracy and reduces token consumption under the same model.

Today, OCR maintains a separate plugin or skill for each host, such as Claude Code, Codex, Cursor, and OpenCode. This makes integration work grow with every new host and makes behavior and documentation harder to keep consistent. OCR also cannot currently be discovered or consumed by ACP clients such as Paseo, Zed, and JetBrains ACP Agent Registry, so it has no standard entry point for multi-agent workflows where a general coding agent writes code and OCR acts as a dedicated review agent.

Agent Client Protocol (ACP) is an open protocol for connecting editors/clients with AI agents, with a role similar to LSP. A client implements ACP once and can then use any compatible agent. The client communicates with the agent through a JSON-RPC 2.0 session, sends prompts and commands, receives streaming progress and tool-call updates, gets structured results with file/line locations, and can cancel an in-flight session. The agent only needs to provide an ACP server. This lets OCR replace “one host, one integration” with “one server, every compatible client” and enables standard multi-agent collaboration.

Existing Work and Motivation

OCR already provides:

• CLI commands: ocr review, ocr scan, ocr session, and ocr delegate;
• a VS Code extension;
• CI/CD integrations such as GitHub Actions and GitLab CI;
• integrations with multiple LLM providers; and
• host-specific plugins or skills for Claude Code, Codex, Cursor, and OpenCode.

Issue #674 requested registering OCR as an independent provider through ACP for tools such as Paseo. PR #679 added an “ACP server adapter” to the H2 2026 roadmap, with the explicit direction to expose OCR as a dedicated review agent while keeping it decoupled from the core review engine. That roadmap PR is a planning placeholder and contains no implementation.

Design and Scope of This PR

This work follows the roadmap boundary: an independent ACP adapter process wraps the existing ocr binary instead of embedding protocol code into OCR's internal review engine.

• The adapter now provides a standalone ACP Server executable and communicates with clients using JSON-RPC 2.0 over stdio. Streamable HTTP remains future work.
• The adapter invokes ocr review and ocr scan as child processes and consumes their structured JSON output; OCR session-command integration remains outside the implemented scope.
• The adapter owns ACP sessions, prompt parsing, clarification, progress/display updates, cancellation, timeout handling, and protocol-channel hygiene.
• OCR continues to own review execution and the review LLM; the adapter does not import OCR internal packages, duplicate OCR's provider system, or change its source or prompts.
• Progress uses one collapsible execution entry with the latest CLI activity, total elapsed time, command and bounded logs. No custom CLI progress protocol or inferred group-stage timers are required.
• Windows uses Job Objects and cancellable stdio. Graceful CTRL_BREAK delivery requires a shared console; without one, cancellation relies on Job Object termination and may not preserve a partial JSON result.
• Release builds cover Linux, macOS and Windows on amd64/arm64. Documentation is synchronized across English, Chinese, Japanese, Korean and Russian.
• available_commands_update now exposes /review, /scan, and parameter hints so clients can discover the supported entry points.

Final Project Goal

Deliver a usable, tested, and documented standalone ACP Server for OCR. A user should be able to configure one command in Paseo or another compatible ACP client, use natural language or slash commands to start a review or scan, see streaming progress and structured line-level findings in the client UI, and cancel the operation at any time. The completed work will land the ACP roadmap item in the upstream alibaba/open-code-review repository and close issue #674.

Type of Change

• New feature (non-breaking change that adds functionality)

How Has This Been Tested?

• Local root-module make check, make test and make build passed after restoring CLI source to the PR baseline.
• Local ACP make check, make test and make build passed, including the two stderr regression fixes.
• ACP race-enabled coverage passed at 91.1% after removing the custom progress protocol; the subsequent stderr fixes passed the full race-enabled test suite.
• Regression tests reproduce and fix false truncation warnings for lines larger than the 32 KiB read buffer but below the 64 KiB line limit, and preserve the pending log event before a non-EOF read error. Over-limit truncation remains tested.
• Official Python ACP SDK smoke passed using ordinary CLI logs and final JSON: handshake, command discovery, review/scan, findings and locations, no-findings completion, partial coverage/budget failure, ResourceLink, cancellation, repeated cancellation and EOF cleanup.
• Opt-in real CLI binary contract tests passed against the rebuilt CLI without the removed source changes.
• Local Windows amd64/arm64 adapter builds passed; current-head ACP CI cross-compilation also passed for Windows amd64/arm64, Linux arm64 and macOS amd64/arm64.
• Documentation site production build passed with two asset/entrypoint size warnings; git diff --check and LF checks passed.

Checklist

• My code follows the project's coding style
• I have performed a self-review of my code
• I have added tests that prove my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly
• I have signed the CLA

Related Issues

• Related to 希望支持ACP #674 (ACP provider support request)
• Related roadmap: [WIP] feat: add ACP support #679

AI / LLM Disclosure

This PR was developed with assistance from OpenAI Codex gpt-6-astra. The implementation and tests were reviewed locally and with the repository's OpenCodeReview automation. No external LLM API or API key is required for the test suite; parser LLM integration is covered by deterministic fakes, with real-endpoint coverage remaining opt-in.

[github-actions]

🔍 OpenCodeReview found 10 issue(s) in this PR.

• ✅ Successfully posted inline: 10 comment(s)

[Qiyuanqiii]

The overall Phase 4 direction looks solid, and the amount of validation and test coverage here is good. CI, ACP CI, and CodeQL are green.

I’m requesting changes for two clarification-state issues before merge:

1. I confirmed the existing finding in finalizeReview: a commit review with a missing commit is stored back as reviewRange, and reviewClarifyQuestion(["commit"]) consequently asks for --to instead of --commit. This should preserve reviewCommit, use a commit-specific question, and have regression coverage for both the pending state and the question text.

2. In the natural-language path, the pending slots are included in the next model prompt but are not deterministically merged with the next tool-call result. A valid follow-up response containing only the newly supplied field can therefore discard fields already known from the previous turn. I left an inline comment on the fresh pendingSlots construction with a concrete example.

I also agree that protecting structural ExtraBody fields and the managed anthropic-version header would be worthwhile hardening, but I would not make the lower-severity URL/error-snippet/Makefile findings blockers by themselves.

Once the two state-machine issues above are addressed and covered by multi-turn tests, I think this will be much closer to approval.

[Qiyuanqiii]

Please explicitly complete the AI/LLM disclosure required by the current PR template: either state that no AI/LLM was used, or, if it was used, disclose the tool(s) and specific model(s).

[chaojixinren]

@Qiyuanqiii Thanks for the detailed review. I addressed the two clarification-state issues you identified.

1. Commit reviews now preserve reviewCommit when --commit is missing. The parser emits a commit-specific clarification question and keeps the pending state typed as a commit review. Regression coverage now verifies both the pending state and the question text.

2. Natural-language follow-ups now merge the new tool-call fields with the adapter-owned compatible pending slots. A follow-up response may provide only the newly supplied field without discarding values collected in the previous turn. A two-turn regression test covers the from=main followed by to=feature case.

I also added coverage for clearing clarification state when the model returns no partial intent, and for preventing stale slash-command selectors from leaking across review types.

The fixes are included in commits 39cd254 and c094012. Verification passes with:

• make -C acp test
• make -C acp check
• make -C acp coverage (approximately 90.1% total coverage)

I agree that protecting structural ExtraBody fields and the managed Anthropic headers is worthwhile hardening. I kept those items separate from the clarification-state fix so the current change remains focused. The lower-severity URL, error-snippet, and Makefile suggestions are also reasonable follow-up improvements.

I have also updated the AI/LLM disclosure to explicitly identify the tools and models used during development.

[chaojixinren]

@PrayWithYou Thanks for the follow-up review. Commit 42d84af addresses the remaining clarification and cancellation issues.

• Clarification state: actionClarify now merges compatible pending state instead of replacing it. Omitted fields retain previously collected values, explicit lists replace previous lists, and switching intent types does not carry incompatible fields forward. Regression tests cover consecutive clarification turns followed by a completed request.
• Cancellation and deadlines: The runner now checks termination before setup and immediately before cmd.Start(). It also rechecks observable cancellation/deadline state when selecting the final outcome, including when process completion is already available. This avoids relying on watcher scheduling or select ordering. Tests cover requests cancelled or expired before startup, the completion/cancellation race, and preserving an already-delivered outcome.
  The startup check and OS process creation are not atomic; cancellation occurring between them is handled by the running-process cleanup path.
  The PR disclosure now explicitly names OpenAI Codex with gpt-6-astra. ACP CI is green on the latest commit. The non-blocking hardening items remain separate from this fix.

[chaojixinren]

[chaojixinren]

Follow-up: the tool-call details added by c9279e7 cannot be seen in the VS Code / Cursor ACP client

While re-verifying the structured progress work on Windows, I found that the client used for the VS Code side of the manual acceptance does not render tool-call content at all. This is a client-side limitation, not an adapter or OCR defect, but it makes a few statements in this PR and its docs over-claimed, so I am recording it here.

Client behaviour

formulahendry.acp-client@0.2.0 (the "ACP Client" extension; latest release) renders a tool call from toolCallId, title and status only. In src/ui/ChatWebviewProvider.ts:

• case 'tool_call' calls addToolCall(tc.toolCallId, tc.title, tc.status); addToolCallInline() builds only <span class="tc-icon"> and <span class="tc-title">.
• case 'tool_call_update' calls updateToolCall(update.toolCallId, update.status, update.title); it rewrites the icon and the title, nothing else.

The content array does reach the webview — the extension host forwards the whole notification in handleSessionUpdate — but it is never read in the render path, and the only <details> element in the webview is the "Thinking..." block. So in this client there is no expandable body: Command:, the working directory, the bounded [ocr] log tail and the new **Review stages** markdown are all dropped.

I filed the gap upstream with the code references, a generic payload example and a minimal repro:
formulahendry/vscode-acp#69

What this means for the adapter

Nothing needs to change here. With an ocr built from this branch, the CLI emits the structured [ocr-progress] {...} stderr snapshots (same one-file review: 0 lines with the v1.12.4 release binary, 17 with the branch build), and the adapter forwards them as tool_call_update.content together with the command, working directory and diagnostics. Clients that render tool-call content — Zed — keep working, and this is the protocol-correct representation regardless of any single client.

[chaojixinren]

One thing I want to clarify before treating the current ACP interface as stable is that both the command coverage and the natural-language intent contract are still expected to evolve.

The current ACP adapter starts with /review and /scan, but the overall command coverage is not finalized yet. OCR already exposes a broader set of CLI commands, modes, and flags, and it is still unclear which of those should also be exposed through ACP, which should remain CLI-only, and whether some capabilities should be represented differently at the ACP layer instead of mirroring the CLI one-to-one.

This also applies to future OCR features. For example, #1510 proposes a new ocr review --staged mode for reviewing a frozen Git index snapshot. If that lands, ACP will need to decide whether and how staged review should be exposed, rather than automatically mapping every new OCR CLI flag into ACP. That decision would affect the slash-command surface, structured intent schema, validation rules, clarification behavior, command discovery, documentation, and tests.

The natural-language parsing layer is also still an initial version. It currently maps free-form requests into the structured review/scan intent schema, but the conventions around intent selection, clarification, rejection, argument inheritance, unsupported capabilities, and how newly added OCR features should be described to the parser LLM still need further refinement and testing. The current prompt and tool schema should therefore not be treated as a finalized long-term contract yet.

My intention for this PR is primarily to establish the ACP foundation: the protocol boundary, session lifecycle, OCR process orchestration, cancellation and timeout handling, progress/result mapping, and an initial working interface for review and scan.

Once the ACP integration is in use and the expected command coverage becomes clearer, I plan to follow up on the higher-level command surface and natural-language parser contract separately, including how new OCR capabilities such as #1510 should be represented.

[chaojixinren]

The CI failures were caused by the Go toolchain vulnerability scan, not by the ACP implementation.

The ACP workflow was using Go 1.26.6, which contains standard-library vulnerabilities fixed in Go 1.26.9. I have updated the ACP CI workflow separately to use Go 1.26.9.

The same issue also affects the root project CI, together with vulnerabilities in the root module's golang.org/x/net dependency. That fix is being handled separately in PR #1062.

The ACP code and tests were not the cause of the CI failures.

[lizhengfeng101]

LGTM