feat(runtime): add Orka harness v2 ACP mode - #22
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
ACP update-delivery failures can leave framework state inconsistent with uncommitted protocol history.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
runtimes/common/agentkit_serve_common/acp.py — If emitting the update fails (for example, serve_acp_stdio.send rejects a response over 8 MiB),… |
What changed in this PR
Adds ACP stdio support for AgentKit runtimes behind Orka harness v2.
Changes:
- Implements ACP sessions, prompts, cancellation, MCP projection, and strict runtime binding.
- Adds adapter capabilities and MAF session rollback.
- Adds protocol tests and documentation.
Review finding: ACP output failures can leave MAF session state inconsistent with ACP history.
| File | Description |
|---|---|
README.md |
Documents ACP mode. |
docs/orka.md |
Describes harness v2 integration. |
docs/runtime-adapters.md |
Documents ACP architecture. |
runtimes/common/README.md |
Adds common ACP guidance. |
runtimes/common/agentkit_serve_common/acp.py |
Implements ACP stdio server. |
runtimes/common/agentkit_serve_common/cli.py |
Adds ACP CLI routing. |
runtimes/common/agentkit_serve_common/runtime.py |
Enables ACP offline echo. |
runtimes/common/tests/test_acp_protocol.py |
Covers ACP behavior and guardrails. |
runtimes/common/tests/test_cli_protocol.py |
Covers ACP CLI startup. |
runtimes/langgraph/agentkit_serve/agent_factory.py |
Advertises HTTP MCP support. |
runtimes/langgraph/tests/test_guardrails.py |
Tests ACP capability. |
runtimes/microsoft-agent-framework/agentkit_serve/agent_factory.py |
Adds session rollback and ACP capability. |
runtimes/microsoft-agent-framework/tests/test_guardrails.py |
Updates session recovery coverage. |
runtimes/pydantic-ai/agentkit_serve/agent_factory.py |
Detects HTTP MCP capability. |
runtimes/pydantic-ai/tests/test_unsupported_features.py |
Tests ACP capability. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Oversized input framing can execute a trailing fragment as another request, and valid null request IDs are rejected.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
runtimes/common/agentkit_serve_common/acp.py — readline(limit) returns a partial fragment when one input line exceeds the limit. After rejecting… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
runtimes/common/agentkit_serve_common/acp.py — If emitting the update fails (for example, serve_acp_stdio.send rejects a response over 8 MiB),… View resolved comment |
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
runtimes/common/agentkit_serve_common/acp.py:88
- ACP v1 permits a null JSON-RPC request ID (although it discourages clients from using one). This rejects such a valid request with
-32600instead of dispatching it, so allowNonewhile continuing to reject booleans and other types.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
ACP output frames can exceed Orka’s supervisor limits and terminate otherwise successful sessions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
|
runtimes/common/agentkit_serve_common/acp.py — The entire model response is emitted as one JSON-RPC line. Orka's supervisor configures its ACP… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
runtimes/common/agentkit_serve_common/acp.py — readline(limit) returns a partial fragment when one input line exceeds the limit. After rejecting… View resolved comment |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Invalid ACP request IDs can be echoed into malformed JSON-RPC error responses.
Review tier: Balanced
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
runtimes/common/agentkit_serve_common/acp.py — The entire model response is emitted as one JSON-RPC line. Orka's supervisor configures its ACP… View resolved comment |
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
runtimes/common/agentkit_serve_common/acp.py:268
- When the envelope is invalid, this can echo an object, array, boolean, or float from
idinto the error response. ACP request IDs are only strings or integers, and JSON-RPC requiresid: nullwhen a valid request ID cannot be established; normalize invalid values before sending this early error.
This issue also appears in the following locations of the same file:
- line 273
- line 282
runtimes/common/agentkit_serve_common/acp.py:273
- This early method-validation error also bypasses
_request_keyand echoes an invalid rawid, producing a response outside the ACP/JSON-RPC contract. Usenullunless the ID is a non-boolean string or integer.
await self._send_error(message.get("id"), _INVALID_REQUEST, "JSON-RPC method is required")
runtimes/common/agentkit_serve_common/acp.py:282
_request_keyrejects invalid ACP IDs, but the error response then echoes that same invalid value. This makes malformed input produce a malformed JSON-RPC response; useid: nullwhen ID validation fails.
await self._send_protocol_error(response_id, exc)
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
ACP initialization rejects unsupported versions instead of performing the protocol’s required version negotiation.
Review tier: Balanced
Findings: None
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
runtimes/common/agentkit_serve_common/acp.py:384
- ACP v1 requires an agent to return its latest supported protocol version when the requested version is unsupported. Rejecting every value other than
1with-32602prevents the required version negotiation for newer clients. Validate that the field is an integer, then let the response advertiseACP_PROTOCOL_VERSION.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Digest validation hashes a second read of a replaceable config, so the parsed runtime spec may differ from the approved content.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
|
runtimes/common/agentkit_serve_common/acp.py — The digest is computed from a second file read after cli._load_spec_or_exit() has already parsed… |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The critical MAF rollback path lacks direct adapter-level regression coverage.
Review tier: Balanced
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
runtimes/common/agentkit_serve_common/acp.py — The digest is computed from a second file read after cli._load_spec_or_exit() has already parsed… View resolved comment |
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
runtimes/microsoft-agent-framework/agentkit_serve/agent_factory.py:410
discard_sessionis the rollback path when cancellation or output delivery fails after MAF has already completed a turn, but the MAF guardrails only test resetting whenrun_agentitself raises. The common ACP tests use a recording fake, so a regression or no-op here would still pass. Add an adapter-level test that completes a run, callsdiscard_session, then retries the same session ID and verifies that a freshAgentSessionis used with explicit history enabled.
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Emit correlated, redacted tool lifecycle events across the LangGraph, Microsoft Agent Framework, and Pydantic AI adapters. Settle cancellation, normalize MAF provider messages, and propagate fatal MCP errors without replaying tool calls while preserving recoverable admitted errors. Validation: common and framework suites, Go tests, lint, amd64 image checks, and live Orka harness v2 conformance and lifecycle checks. Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
LangGraph can expose MCP diagnostics to the model, and ACP error serialization can strand oversized requests.
Review tier: Balanced
Findings: None
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
runtimes/common/agentkit_serve_common/acp.py:889
- Accepted requests can contain nearly 8 MiB of untrusted text, and several errors interpolate that text (for example the unknown method at line 501 and duplicate header name at line 685). JSON quoting can make the resulting error exceed
_MAX_MESSAGE_BYTES;send()then raises,_request_done()only retrieves that task exception, and the client waits without any response. Bound or sanitize error fields at this shared boundary (or terminate the server when a response cannot be written) so malformed-but-accepted frames cannot strand a request.
runtimes/langgraph/agentkit_serve/agent_factory.py:213 - This advertises ACP HTTP MCP support, but
_load_tools()still usesload_mcp_tools()with its defaulthandle_tool_errors=True. In the allowedlangchain-mcp-adapters0.3.2 release, an admittedCallToolResult(isError=True)becomes aToolMessagethat preserves the MCP error content verbatim, so broker diagnostics are sent into the model context; the new Pydantic AI and MAF paths replace those diagnostics with a generic recovery message. Please add the same redacting boundary here (while preserving model recovery), or do not advertise this capability until that boundary exists.
|
Both current-head Live Vekil/Copilot failures stop at authentication, before the AgentKit live agent is built or invoked. The pull-request job and push job each record Vekil exiting with At Live verification is blocked until the configured |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The mutable Vekil image can receive repository or cached credentials and must be pinned to an immutable digest.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
|
.github/workflows/ci.yml — Pin the credential-bearing Vekil image by digest |
|
scripts/live-copilot-agent-e2e.sh — Keep the local Vekil default immutable |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The large cross-runtime protocol, cancellation, and security change requires final human validation against the inaccessible companion Orka implementation.
Review tier: Balanced
Findings: None
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
|
scripts/live-copilot-agent-e2e.sh — Keep the local Vekil default immutable View resolved comment |
|
.github/workflows/ci.yml — Pin the credential-bearing Vekil image by digest View resolved comment |
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
ACP parsing and repeated-cancellation cleanup still contain correctness gaps.
Review tier: Balanced
Findings: None
Previously missed findings (2)
In code that hasn't changed since last review
runtimes/common/agentkit_serve_common/acp.py:401
json.loadscan raise a plainValueErrorfor an integer longer than Python's conversion limit andRecursionErrorfor deeply nested input. Both fit under the 8 MiB frame limit, so either currently escapes the protocol handler and terminates the ACP child instead of returning a parse error. Catch these decoder failures alongside malformed JSON.
runtimes/common/agentkit_serve_common/acp.py:455- EOF/close commonly follows a cancellation notification. If that prompt is still cleaning up, this unconditional second
Task.cancel()can interrupt the adapter's join/teardown path before child tool or provider tasks have stopped. Preserve the first cancellation's cleanup by only issuing cancellation for states not already marked cancelled.
This issue also appears on line 845 of the same file.
Summary
agentkit-serveso AgentKit runtimes can run behind Orka harness v2Companion Orka PR: orka-agents/orka#487
Validation
go test ./...make lintThe unfiltered common pytest collection still hits the existing Foundry dependency import error for
get_input_expanded; the focused common ACP and CLI suite passes.