feat(acp): integrate ACP harness for agent communication - #4072
Open
floreks wants to merge 50 commits into
Open
Conversation
- Added new `acp` package for handling ACP session lifecycle. - Implemented provider-neutral Agent Client Protocol (ACP) harness. - Created `acp.go`, `acp_session.go`, `acp_turn.go`, and `acp_client.go` for structuring ACP tool logic and session management. - Extracted opencode session export functionality into `ExportSession` method for reusability. - Updated imports to add support for ACP logic and made necessary code refactoring for cleaner management. - Removed redundant session export logic from `artifacts.go`. - Updated `go.sum` with new dependencies for ACP SDK integration.
Soffi AI SummaryThis PR migrates the AI agent harness in the deployment operator from bespoke, per-agent communication implementations to a unified Agent Client Protocol (ACP) layer. Previously, each AI coding agent (Claude, Opencode, Codex, Gemini) embedded its own session management, tool-call handling, and artifact streaming logic. The new Updated: 2026-09-11 14:18 UTC |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Member
Author
|
plural deploy to dev |
Contributor
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Add context cancellation checks in `ReadTextFile` for robustness. - Introduce a limit on text file read size with `maxTextFileBytes`. - Implement `contextReader
Member
Author
- Bump OPENCODE_VERSION from 1.17.3 to 1.18.23 for deployment compatibility.
- Add `TestNewOpenCodeSelectsConfiguredAgentForRunMode` to verify agent selection based on run mode. - Update `opencode_args_test.go` to use `Default*Agent` constants. - Change `opencode_types.go` to export `Default*Agent` constants. - Modify `opencode.go` files to utilize exported `Default*Agent` constants.
Member
Author
|
plural deploy to dev |
- Introduce `sessionAttempt` and `sessionDetails` structs for clearer separation of attempts and session configurations - Implement session attempt lifecycle methods: `startAttempt`, `run`, `stop`, `initialize`, `openSession`, `createSession`, `resumeSession` - Add error handling improvements: `fail`, `promptFailure` - Consolidate process rejection logic into `rejectProcess` - Refine session configuration with `setModelConfig` and `setModeConfig` functions for cleaner configuration flow
- Bump `opencode` version from 1.17.3 to 1.18.23 in deployment workflow file for compatibility.
- Add test `TestToolOutputPrefersContentOverRawOutput` to prioritize content over raw output in tool messages. - Add test `TestToolOutputFallsBackToRawOutputWithoutContent` to ensure raw output is used when content is unavailable. - Simplify `normalizeUsage` function logic to use `max` for total token calculation. - Refactor `acp_turn.go` to improve tool output handling by checking and formatting content and raw output effectively.
…-consider-moving-agent-harness-to-use-agent-client-protocol
- Update `ConfigureSystemPrompt` and `systemPromptInput` methods to use pointer receivers in `tool/v1/tool.go` - Modify `ConfigureSkills` and related methods to use pointer receivers in `v1/skills.go` - Change test functions in `acp_test.go` to test new tool behaviors - Simplify tool management logic by removing redundant flusher handlers in `acp.go` and `acp_turn.go` - Enhance cumulative cost tracking by introducing a new `RecordCumulativeCost` method
- Introduced `newSessionUpdates` to handle pre-response session updates - Enhanced `NewSession` to process updates using `SessionUpdate` - Added test `TestRunPromptHandlesUpdatesSentBeforeNewSessionResponse` to verify update handling order - Added test `TestRunPromptRejectsMismatchedInitialSessionUpdate` to ensure proper session update matching - Added test `TestSessionUpdateRejectsEmptySessionIDBeforeBinding` to enforce session ID validation - Refactored session config functions into methods: `setSessionConfig`, `setModelConfig`, `setModeConfig` - Introduced `startAttempt` for clearer session initiation logic - Created `validate` method for comprehensive tool configuration checks - Updated error messages for consistency with lowecase conventions - Reorganized `toolCall` and `toolUpdateEvents` to streamline tool communication logic
Member
Author
…-consider-moving-agent-harness-to-use-agent-client-protocol
- Adjust `opencode.go` to use `DefaultReviewAgent` consistently across logic. - Resolve potential errors due to incorrect agent name casing.
- Change object creation in `templates_test.go` to use pointer receiver syntax - Correct casing of `DefaultReviewAgent` in `opencode_args_test.go`
…-consider-moving-agent-harness-to-use-agent-client-protocol
…-consider-moving-agent-harness-to-use-agent-client-protocol # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
…verage - Introduced ACP transport for Opencode with provider-specific session handling in `transport.go`. - Refactored tool runtime API to abstract provider-neutral operations in `runtime_types.go`. - Added comprehensive tests for ACP transport in `transport_test.go` and Agent lifecycle in `runtime_test.go`. - Enhanced tool configuration logic, consolidating system prompt methods with `configureSystemPrompt`. - Improved model selection and OpenCode provider prefix handling in `opencode_templates_test.go` and `settings_test.go`. - Simplified file-system handling and lifecycle hooks for agents.
- Deleted `codex` package and its implementation files (`artifacts.go`, `codex.go`, `codex_stream.go`) to streamline codebase. - Removed Codex-specific tests from `codex_stream_test.go`.
…ion and tests - Updated Codex configuration to use `Settings` instead of `Profile` for better semantic alignment. - Replaced `Profile`-based logic with `Settings` in tests and runtime handling (`agent_config_test.go`, `templates_test.go`, and others). - Removed outdated profiles: `analysisProfile`, `autonomousProfile`, and `reviewProfile` in favor of streamlined ACP settings. - Enhanced diagnostic handling with detailed process STDERR tailing in `exec_stdio` package. - Deprecated `chat` wire API, enforced `responses` wire API throughout Codex logic (`agent_config.go`, `runtime_config.go`, etc.). - Improved runtime failure handling by introducing structured error propagation in `session.go`. - Adjusted template rendering to align with the updated Codex `settings` structure (`config.toml.gotmpl`).
…escapes - Refactored `openTextFile` to enforce root-relative path validation. - Updated file opening logic to use `root.OpenFile` for secure operations. - Added tests to validate rejection of file reads outside root. - Introduced symlink escape prevention with comprehensive test cases.
Member
Author
|
plural review this |
- Introduced `authenticationMethod` field in `Engine` for configurable ACP authentication. - Added `WithAuthenticationMethod` option to set authentication method on the engine. - Updated session logic to authenticate using the specified method before session operations. - Enhanced `NewEngine` to support authentication method configuration. - Modified `Transport` to pass `gemini-api-key` authentication method for Gemini agent. - Added tests to validate authentication behavior, including method configuration, failure cases, and call order.
… transport - Renamed `geminiPromptUsage` to `toUsage` and converted it to a method of `Transport`. - Refactored `geminiTokenCount` to `toTokenCount`, making it a method of `Transport`. - Updated `engine` initialization to use the new `toUsage` method for usage resolution in Gemini transport.
- Introduced mutex for tool call state (`toolMu`) to ensure thread-safe access during updates. - Refactored tool call logic with `startToolLocked` and `upsertPermissionTool` methods for enhanced modularity and error handling. - Added `permissionToolCallStart` and `permissionToolCallUpdate` utility methods for streamlined tool call creation and updates. - Improved session handling with stricter error checks and conditionally locking mechanisms. - Enhanced tool call metadata management with `setName` and `displayName` adjustments, providing better clarity for `title` and `kind` fallback behavior. - Updated file client logic to improve read/write behavior and added error coverage for edge cases. - Added new tests to validate tool call fallbacks, session-based permission errors, and redundant calls prevention.
- Adjusted file reading logic to correctly terminate loop on EOF without returning an error. - Improved error handling and messaging for read failures to enhance debugging clarity.
- Bumped Gemini version to `0.59.0` across deployment configurations and Dockerfiles. - Disabled `SessionResume` in Gemini transport due to session corruption issues in v0.59.0 (`No previous sessions found`). - Added comments referencing upstream issue for future revision. - Refactored session request handling by introducing `acpRequest` to streamline session creation logic. - Removed unused session loading and history patching logic from Dockerfile. - Added new tests to validate behavior of fresh session initiation in transport logic.
- Added support for recovering `ToolCallUpdate` events arriving before `ToolCall` starts via the `WithToolCallUpdateRecovery` option. - Introduced `recovered` state handling for tool calls, ensuring smooth reconciliation of past terminal states. - Enhanced `ToolCall` metadata and status validation with stricter checks to prevent redundant or invalid updates. - Refactored `turnState` logic to conditionally emit start, output, and terminal tool messages based on recovery scenarios. - Updated engine and transport to propagate tool call recovery logic for Gemini agent. - Added comprehensive tests for tool call recovery, late updates, and reconciliation correctness.
…ibility - Introduced `WithToolCallStartContentAsInputWithoutRawInput` option for treating start content as tool input when `RawInput` is unavailable. - Updated `toolCall` logic to support conditional input mapping from content. - Enhanced `turnState` to utilize `startContentIsInputWithoutRawInput` for flexible tool updates. - Added Gemini compatibility instructions to system prompts, detailing command substitution restrictions. - Updated tests to validate tool call input-output behaviors with Gemini-specific configurations.
- Removed recovery logic for tool calls and redundant content input mappings. - Streamlined `applyToolUpdate` by dropping unused flags and dependencies. - Updated Gemini runtime to utilize streamlined transport with auto-resume support. - Simplified tests by eliminating unnecessary tool call recovery scenarios.
- Added `UpdateTopicTool` to `excludeTools` in settings template. - Introduced test to validate exclusion of `UpdateTopicTool` for progress-only mode.
- Removed legacy `Pi` implementation, including runtime, tool, and test definitions. - Replaced with a streamlined, modular `Agent` structure for Pi runtime. - Introduced `runtime_config.go`: - Centralized runtime configuration handling and improved provider resolution logic. - Simplified agent initialization using consistent model and settings resolution. - Added granular methods for managing Pi's session, prompt, and skill paths: - Consolidated export logic for session directories in `artifacts`. - Improved type safety and eliminated redundant dependencies.
Member
Author
|
plural review this |
…ation - Replaced implicit empty string initialization with explicit `var` declaration for `prefix`. - Added a newline for readability before sink message construction.
…modernize transport logic - Replaced `coreTools` with `tools.core` and adjusted related test logic for consistency. - Renamed and updated various tools in `settings.json.gotmpl` template for better representation and conformance. - Deprecated `excludeTools` in favor of the nested `tools.exclude` structure. - Removed `RepositoryDir` field from `ConfigTemplateInput` and transport logic. - Updated transport to use `repositoryDir` instead of `workDir` for execution context. - Enhanced templates and tests for quoting model names and excluding auxiliary directories from settings. - Refined Gemini compatibility instructions to explicitly note restrictions on `.git` internals. - Improved test coverage for tool settings and transport execution, including stricter validation for deprecated fields.
- Enhanced `readTextFile` with cancellation handling and introduced `cancelableTextFile` for better resource cleanup. - Refactored core file processing into modular methods to streamline operations and improve readability. - Updated `settings_test.go` to use `httpUrl` instead of `url` for MCP server configuration, adjusting error messages accordingly. - Added new test to validate cancellation handling during file reads. - Updated Gemini settings template to rename `url` to `httpUrl`, ensuring consistency with test logic.
Member
Author
|
plural review this |
![plural-copilot[bot]](https://avatars.githubusercontent.com/in/916369?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Medium-depth review completed for 59a76fe.
The ACP migration is structurally sound: the shared engine has focused lifecycle/configuration coverage; ACP filesystem access is rooted, symlink-contained, size-bounded, and write-gated by run mode; prior FIFO/read-cancellation findings are addressed with nonblocking opens and async close-on-cancel. I also reviewed the Claude/Codex/OpenCode/Pi migration paths, Gemini’s retained raw transport, artifact staging, and the updated pinned runtime images.
No additional actionable correctness or security findings.
Note: CI is still running, and the existing Test Build Docker image failure should be resolved/understood before merge.
- Updated tool result handling to replace empty-output values with default success message. - Adjusted test logic for `GeminiInactivityTimeout` to prioritize explicit environment values. - Introduced private default path and policy file creation for Gemini runtime setup. - Enhanced ACP client for text file reads with resource management and context cancellation. - Modified terminal-related methods to return unavailable state in unattended runs. - Refined settings and templates with improved variable redaction and modernized permissions handling. - Added stricter validation and error handling in ACP transport and file management.
… tests - Renamed `workDir` to `repositoryDir` across Gemini transport and configuration for clearer execution context. - Refined `defaultGeminiInactivityTimeout` constant handling for consistency and clarity. - Added new transport tests utilizing `repositoryDir` to validate execution behavior. - Introduced `Gemini` tool with enhanced runtime, including babysitting and session management. - Implemented model validation and argument parsing for Gemini execution. - Replaced `tools.core` with `coreTools` in settings and aligned tests accordingly. - Consolidated tests to modernize and ensure consistency with refactored transport and tool settings.
Member
Author
|
plural review this |
…anges - Replaced `settings.local.json.gotmpl` with `settings.json.gotmpl` in templates. - Updated tests to reflect the renamed settings file and adjusted expected model references. - Changed default model in tests from `claude-sonnet-4-6` to `claude-sonnet-5`.
- Updated `anthropic.model` and `anthropic.toolModel` in settings.yaml - Changed `config.claude.model` in claude.yaml runtime config - Modified `defaultModel` constant in runtime_config.go
- Adjusted expected model value in `agent_test.go` from `claude-sonnet-5` to `claude-sonnet-4-6` to match runtime changes.
- Adjusted `transport_test.go` to reflect model change from `claude-sonnet-5` to `claude-sonnet-4-6`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the deployment operator's agent harness to use newer versions of multiple AI agents and introduces support for ACP (Agent Client Protocol) adapters, especially for Claude, Codex, and Pi agents. It also adds a new ACP client implementation, improves Dockerfile practices, and includes minor bug fixes and test improvements.
Agent Version Upgrades and ACP Adapter Integration
2.1.236), Codex (0.153.4), and Opencode (1.18.23) across workflow files, Go controller defaults, and Dockerfiles, ensuring all components use the latest agent releases.@agentclientprotocol/claude-agent-acp), Codex (@agentclientprotocol/codex-acp), and Pi (pi-acp) in their respective Dockerfiles, allowing the agent harness to communicate with these agents through the ACP protocol.Dockerfile and Build Improvements
libatomic1where needed).ACP Client Implementation
tool/acp/client.goto support file operations, permission requests, and session handling, enabling unattended runs to interact with the filesystem and handle ACP protocol requests.github.com/coder/acp-go-sdk, required by the new ACP client.Bug Fixes and Tests
AgentRun.IsProxyEnabled()to handle nil pointers safely.TestHandleAgentMessageCorrelatesRecoveredTerminalToolCall) to verify correct correlation and cleanup of recovered terminal tool calls.Test Plan
Test environment: https://console.plrl-dev-aws.onplural.sh/
Checklist
Plural Flow: console