-
Notifications
You must be signed in to change notification settings - Fork 294
feat: implement Agent Client Protocol (ACP) for Copilot CLI #143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: implement Agent Client Protocol (ACP) for Copilot CLI #143
Conversation
- Implement CopilotAcpEngine using Agent Client Protocol - Add structured NDJSON communication with Copilot CLI - Support real streaming via agent_message_chunk events - Direct token count extraction from protocol responses - Better error handling with structured ACP responses - Add comprehensive unit tests for ACP engine - Integrate ACP engine as default for copilot - Keep legacy CopilotEngine for reference Benefits: - No more fragile text parsing - No more temporary file management - Proper streaming support - Protocol-based communication - Future-proof implementation
- Mark legacy CopilotEngine as deprecated - Add ACP documentation links to new implementation - Clarify that ACP is now the default for --copilot
- Use shell:true on Windows to handle .cmd wrappers - Fixes ENOENT error when spawning copilot on Windows - Tested and working successfully
- Copilot CLI's ACP implementation does not return usage/token data - promptResult only contains stopReason field - Removed unused token extraction code - Added documentation explaining this limitation - Token counts remain 0 (not available via ACP protocol)
|
@Shazwazza is attempting to deploy a commit to the Goshen Labs Team on Vercel. A member of the Team first needs to authorize it. |
|
Related Documentation 2 document(s) may need updating based on files changed in this PR: Goshen Labs's Space AI Engine IntegrationView Suggested Changes@@ -14,16 +14,16 @@
### Engine Overview
| Engine | CLI Command | Selection Flag | Permissions/Flags | Output Format | Token/Cost Reporting | Notable Behaviors & Limitations |
|---------------|--------------------|---------------------|------------------------------------|--------------------|----------------------|----------------------------------|
-| Claude Code | `claude` | `--claude` (default)| `--dangerously-skip-permissions` | `stream-json` | tokens + cost | Auth issues may be environment-specific. Windows: .cmd wrappers now supported. Failed commands return error messages with exit codes. |
-| OpenCode | `opencode` | `--opencode` | `run --format json` | JSON lines | tokens + cost | CLI must be in PATH. Windows: .cmd wrappers now supported. Failed commands return error messages with exit codes. |
-| Codex | `codex` | `--codex` | `exec --full-auto --json` | File + JSON | tokens (no cost) | Uses temp file for output; no token reporting. Failed commands return error messages with exit codes. |
-| Cursor | `agent` | `--cursor` | `--print --force --output-format stream-json` | stream-json | duration (ms) | No token reporting. Failed commands return error messages with exit codes. |
-| Qwen-Code | `qwen` | `--qwen` | `--output-format stream-json --approval-mode yolo` | stream-json | tokens | Model override via `--model`. Failed commands return error messages with exit codes. |
-| Factory Droid | `droid exec` | `--droid` | `--output-format stream-json --auto medium` | stream-json | duration (ms) | Model override via `--model`. Failed commands return error messages with exit codes. |
-| Trae Agent | `trae` | `--trae` | `--print --force --output-format stream-json` | stream-json | tokens, duration (ms) | Model override via `--model`. Trae CLI must be in PATH. |
-| GitHub Copilot| `copilot` | `--copilot` | `--yolo` | plain text | tokens (parsed from output) | Requires Copilot CLI. **Prompts are passed via temp files to preserve markdown. --yolo is always used for non-interactive mode. Only authentication errors with known output formats are detected and surfaced. Token usage is parsed and reported. Silent failures and infinite retry loops are fixed.** |
-| Gemini | `gemini` | `--gemini` | `--output-format stream-json --yolo` | stream-json | tokens + cost | Model override via `--model`. Failed commands return error messages with exit codes. |
-| Ollama | `claude` | `--ollama` | `--dangerously-skip-permissions` (Ollama env vars) | stream-json | tokens + cost | Runs local models via Claude Code CLI. Requires Ollama running locally and Claude Code CLI installed. Recommended models: `qwen3-coder`, `glm-4.7`, `gpt-oss:20b`, `gpt-oss:120b`. |
+| Claude Code | `claude` | `--claude` (default)| `--dangerously-skip-permissions` | `stream-json` | tokens + cost | Auth issues may be environment-specific. Windows: .cmd wrappers supported. |
+| OpenCode | `opencode` | `--opencode` | `run --format json` | JSON lines | tokens + cost | CLI must be in PATH. Windows: .cmd wrappers supported. |
+| Codex | `codex` | `--codex` | `exec --full-auto --json` | File + JSON | tokens (no cost) | Uses temp file for output; no token reporting. |
+| Cursor | `agent` | `--cursor` | `--print --force --output-format stream-json` | stream-json | duration (ms) | No token reporting. |
+| Qwen-Code | `qwen` | `--qwen` | `--output-format stream-json --approval-mode yolo` | stream-json | tokens | Model override via `--model`. |
+| Factory Droid | `droid exec` | `--droid` | `--output-format stream-json --auto medium` | stream-json | duration (ms) | Model override via `--model`. |
+| Trae Agent | `trae` | `--trae` | `--print --force --output-format stream-json` | stream-json | tokens, duration (ms) | Model override via `--model`. |
+| GitHub Copilot| `copilot` | `--copilot` | `--acp --stdio --yolo` | NDJSON (ACP) | none | Uses Agent Client Protocol (ACP) for structured communication and streaming. Token counts not available. Legacy engine deprecated. |
+| Gemini | `gemini` | `--gemini` | `--output-format stream-json --yolo` | stream-json | tokens + cost | Model override via `--model`. |
+| Ollama | `claude` | `--ollama` | `--dangerously-skip-permissions` (Ollama env vars) | stream-json | tokens + cost | Runs local models via Claude Code CLI. |
## Engine Integration Details
@@ -92,24 +92,32 @@
```
### GitHub Copilot
-Integrated as `CopilotEngine`. Uses the `copilot` CLI. **Prompts are always written to temporary markdown files (with unique UUID filenames) and passed via the `--yolo` flag for non-interactive mode, enabling autonomous operation and allowing all tools and paths without prompting.** The `--yolo` flag is always used for non-interactive mode. Streaming mode is not supported due to reliability issues on Windows; non-streaming execution is used for all platforms.
+Integrated as `CopilotAcpEngine`. Uses the `copilot` CLI with the Agent Client Protocol (ACP) for structured NDJSON communication and real streaming support. The ACP engine is now the default for the `--copilot` flag. Legacy CopilotEngine (text parsing) is deprecated and retained only for reference.
+
+**Key Features:**
+- Structured NDJSON protocol communication via ACP
+- Real streaming support with `executeStreaming()` and `agent_message_chunk` events
+- Protocol-based error handling (stop reasons)
+- Auto-approval for tool/permission requests in yolo mode
+- Cross-platform compatibility (Windows, macOS, Linux)
+- No fragile text parsing or temp file prompts
**Error Handling:**
-- Only authentication errors with known output formats are detected and surfaced. Specifically, if the Copilot CLI output starts with phrases like "no authentication", "not authenticated", "authentication required", or "please authenticate", a clear error message is shown with instructions to run `copilot /login` or set the `COPILOT_GITHUB_TOKEN` environment variable.
-- Other errors, such as rate limiting, network issues, or generic error messages, are not detected or surfaced as CLI errors unless they match the known authentication error patterns. This conservative approach avoids false positives, since Copilot CLI output may include phrases like "rate limit", "network error", or "error:" as part of valid responses (e.g., test results or code feedback).
-- Non-zero exit codes are not relied upon for error detection, as Copilot CLI may not use them consistently.
-- Fatal errors (such as authentication failure or missing CLI tools) abort all remaining tasks immediately and prevent infinite retry loops.
-
-**Token Usage Parsing:**
-- Input and output token counts are parsed from Copilot CLI output, supporting `k` and `m` suffixes (e.g., "17.5k in, 73 out").
-- Token usage is reported in the result when available.
-
-**Output Filtering:**
-- CLI artifacts, status messages, and stats sections are filtered out, returning only the meaningful response.
-- Markdown formatting is preserved in the prompt and response.
+- Errors are surfaced via ACP protocol stop reasons. Authentication errors are detected and reported with instructions to run `copilot /login` or set the `COPILOT_GITHUB_TOKEN` environment variable.
+- Other errors (rate limiting, network issues) are surfaced as protocol errors when available.
+- Fatal errors abort all remaining tasks and prevent infinite retry loops.
+
+**Token Usage Reporting:**
+- **Token counts are not available**: Copilot CLI's ACP implementation does not expose usage/token metadata. The result only contains `stopReason`. Telemetry loses token tracking for Copilot. This is a Copilot CLI limitation.
+
+**Streaming:**
+- Streaming is fully supported via ACP protocol. Progress updates are provided as response chunks arrive.
**Automatic Cleanup:**
-- Temporary prompt files are always cleaned up after execution, even on errors.
+- ACP connections and Copilot CLI processes are cleaned up after execution.
+
+**Legacy Engine:**
+- The old CopilotEngine (text parsing) is deprecated. Use CopilotAcpEngine for all new integrations.
Example:
```bash
@@ -150,8 +158,8 @@
## Adding a New Engine
To add a new AI engine:
1. Implement a new class extending `BaseAIEngine`, specifying the engine's name and CLI command.
-2. Parse output according to the engine's format (e.g., stream-json, plain text, file).
-3. Handle errors using the shared error detection utilities.
+2. Integrate using the engine's protocol or output format (e.g., ACP, stream-json, file, NDJSON).
+3. Handle errors using the shared error detection utilities and protocol stop reasons.
4. Support model overrides by appending the `--model <name>` flag if provided.
5. Register the engine in the CLI flag parser and engine registry.
6. Ensure CLI availability by checking the command exists in the system PATH.
@@ -165,7 +173,7 @@
- **OpenCode**: CLI must be in PATH. Windows is supported for npm global CLIs; .cmd wrappers are handled automatically.
- **Claude Code**: Authentication/connectivity issues may occur depending on environment; not considered a bug in Ralphy.
- **Codex**: No token reporting; uses temp files for output.
-- **GitHub Copilot**: Requires Copilot CLI installed and available in PATH. Prompts are passed via temp files. --yolo is always used for non-interactive mode. Only authentication errors with known output formats are detected and surfaced. Token usage is parsed and reported. Infinite retry loops on fatal errors are fixed.
+- **GitHub Copilot**: Requires Copilot CLI installed and available in PATH. Uses ACP protocol for structured communication and streaming. Token counts are not available. Legacy engine deprecated.
- **Ollama**: Requires [Ollama](https://ollama.com) running locally and Claude Code CLI installed and available in PATH. Only models with at least 64k context window are supported. If either dependency is missing, tasks will fail with a clear error message.
- **General**: Each engine requires its CLI tool installed and available in the system PATH.
@@ -178,7 +186,7 @@
ralphy --qwen "generate API" # Qwen-Code
ralphy --droid "create test suite" # Factory Droid
ralphy --trae "implement feature" # Trae Agent
-ralphy --copilot "add feature" # GitHub Copilot
+ralphy --copilot "add feature" # GitHub Copilot (ACP)
ralphy --gemini "summarize document" # Gemini CLI
ralphy --ollama "add feature" # Ollama (local models via Claude Code CLI)
@@ -194,10 +202,22 @@
## Project Standards
### v4.7.1
-- **Copilot engine improvements**: non-interactive mode (`--yolo`), conservative error detection (only authentication errors with known output formats), token usage parsing, temp file-based prompts for markdown preservation
+- **Copilot engine refactored to ACP protocol**: Structured NDJSON communication, streaming support, protocol-based error handling, auto-approval for yolo mode, Windows compatibility
+- **Token reporting limitation**: Copilot CLI ACP does not provide token counts; telemetry loses token tracking for Copilot
- **Fixed infinite retry loop**: tasks now properly abort on fatal configuration/authentication errors
- **Project standards**: added `.editorconfig` and `.gitattributes` for consistent coding styles
## Project Standards
-Ralphy now enforces consistent coding styles and line endings across all files using `.editorconfig` and `.gitattributes`. All text files use UTF-8 and LF line endings. Indentation is standardized (tabs for code, spaces for YAML/JSON). This ensures reliable cross-platform development and clean diffs.
+Ralphy enforces consistent coding styles and line endings across all files using `.editorconfig` and `.gitattributes`. All text files use UTF-8 and LF line endings. Indentation is standardized (tabs for code, spaces for YAML/JSON). This ensures reliable cross-platform development and clean diffs.
+
+---
+
+**References:**
+- [GitHub Copilot ACP Documentation](https://docs.github.com/en/copilot/reference/acp-server)
+- [Agent Client Protocol Spec](https://agentclientprotocol.com/protocol/overview)
+- [TypeScript SDK](https://agentclientprotocol.com/libraries/typescript)
+
+---
+
+**Note:** The Copilot engine now uses ACP protocol for all operations. The legacy engine is deprecated and retained only for reference.Copilot Engine IntegrationView Suggested Changes@@ -1,29 +1,51 @@
## GitHub Copilot AI Engine Integration in ralphy CLI
### Overview
-The ralphy CLI now supports GitHub Copilot as an AI engine for autonomous code generation and completion. Copilot is available alongside other engines such as Claude, OpenCode, Codex, Cursor, Qwen, and Droid. You can select Copilot via a dedicated CLI flag, use model overrides, and benefit from seamless integration with both the TypeScript CLI and the bash script interface.
+The ralphy CLI now integrates GitHub Copilot as an AI engine using the Agent Client Protocol (ACP) for structured, protocol-based communication. This replaces the previous approach of parsing Copilot CLI text output, resulting in improved robustness, real streaming support, and better error handling. Copilot is available alongside other engines such as Claude, OpenCode, Codex, Cursor, Qwen, and Droid. You can select Copilot via a dedicated CLI flag, use model overrides, and benefit from seamless integration with both the TypeScript CLI and the bash script interface.
----
+**Key benefits of ACP integration:**
+- Structured NDJSON protocol communication (no fragile text parsing)
+- Real streaming support with incremental output
+- Protocol-based error handling and stop reasons
+- Cross-platform compatibility (Windows, macOS, Linux)
+- Transparent migration for users (no CLI changes required)
+
+**Known limitation:**
+- Token usage counts are not available via ACP (Copilot CLI does not expose them in public preview). Telemetry for Copilot will show 0 tokens.
+
+The legacy CopilotEngine (text parsing) is still present for reference but is deprecated. All `--copilot` invocations now use the ACP-based engine by default.
### Implementation Details
#### TypeScript CLI
-The Copilot engine now provides more informative error messages when a command fails. In addition to checking for non-zero exit codes, the engine detects Copilot-specific error messages in the CLI output, since Copilot CLI may return exit code 0 even on errors (such as authentication or rate limiting issues). These errors are surfaced as clear messages, for example:
+The Copilot engine now uses the Agent Client Protocol (ACP) for all communication with the Copilot CLI. Instead of parsing text output, the engine communicates using structured NDJSON messages over stdio, as specified by the ACP protocol. This approach provides:
-- Authentication errors are detected only if the output starts with phrases like "not authenticated", "no authentication", "authentication required", or "please authenticate". These prompt you to log in or set the COPILOT_GITHUB_TOKEN environment variable.
-- Rate limiting errors are detected only if the output starts with phrases like "rate limit" or "too many requests" and the output is short (less than 200 characters), to avoid false positives from valid response content.
-- Network errors are no longer detected by searching for phrases like "network error" or "connection refused" in the output, since these may appear in valid responses (e.g., test results or code discussions). Instead, real network errors are surfaced via non-zero exit codes from the Copilot CLI.
-- Generic errors are only detected if the output starts with "Error:" (case-insensitive). Occurrences of "error:" in the middle of the output are not treated as CLI errors, to avoid misclassifying valid response content.
+- **Structured streaming:** Real streaming output is supported via `agent_message_chunk` events, allowing incremental progress updates.
+- **Protocol-based error handling:** Errors are surfaced via ACP stop reasons (e.g., `error`, `cancelled`), not by parsing CLI text or exit codes.
+- **Cross-platform support:** The engine works on Windows, macOS, and Linux, with special handling for Windows `.cmd` wrappers.
+- **Auto-approval:** Permission requests are auto-approved in yolo mode for seamless automation.
-The engine now always passes the prompt as a `-p <prompt>` argument, with special sanitization and escaping for Windows to handle problematic characters and newlines. The `--yolo` flag is now required for non-interactive mode to ensure tasks are executed without prompts; this replaces the previously required `--allow-all-tools` flag. If this flag is missing, the Copilot CLI may silently return without performing any work. This behavior is consistent with all supported engines in ralphy.
+**Token usage counts are not available:** The Copilot CLI's ACP implementation does not expose input/output token counts or usage metadata. As a result, all Copilot telemetry will report 0 tokens. This is a limitation of the Copilot CLI ACP server, not the ralphy implementation.
-Additionally, the engine now parses and reports input and output token usage from Copilot CLI output, supporting values with `k` and `m` suffixes (e.g., "17.5k"). This allows for better tracking of resource usage per task.
+**Legacy engine deprecated:** The previous CopilotEngine (which parsed CLI text output) is now deprecated and retained only for reference. All `--copilot` invocations use the ACP-based engine by default. No CLI flag or configuration changes are required for users.
-Error handling is further improved: fatal errors (such as authentication or configuration issues) will now abort all remaining tasks immediately, and failed tasks are always marked as complete to prevent infinite retry loops.
+**Error handling:**
+- Authentication errors are detected via ACP protocol responses and surfaced with clear messages (e.g., prompting to log in or set `COPILOT_GITHUB_TOKEN`).
+- Protocol-based errors (e.g., rate limits, cancellations) are handled via ACP stop reasons.
+- All errors are reported with structured messages; there is no reliance on parsing CLI output text.
+
+**Streaming:**
+- The `executeStreaming()` method streams output as it is received from Copilot, providing real-time feedback.
+
+**Testing:**
+- The ACP engine is covered by unit tests and has been verified end-to-end on all supported platforms.
#### Engine Integration
-Copilot is integrated into the engine factory in `cli/src/engines/index.ts`. The `createEngine` function instantiates `CopilotEngine` when the engine name is `copilot`, following the same interface as other engines. The engine name type (`AIEngineName`) is extended to include `copilot`, ensuring type safety and discoverability throughout the codebase.
-[Source](https://github.com/michaelshimeles/ralphy/blob/fc2df589969b5fe16d31eccb4e7ff91314e31776/cli/src/engines/index.ts#L9-L56)
+Copilot is integrated into the engine factory in `cli/src/engines/index.ts`. The `createEngine` function now instantiates `CopilotAcpEngine` when the engine name is `copilot`, making the ACP-based implementation the default for all Copilot invocations. The legacy `CopilotEngine` (text parsing) is still present in the codebase but is marked as deprecated and is no longer used by default.
+
+The engine name type (`AIEngineName`) continues to include `copilot`, ensuring type safety and discoverability throughout the codebase.
+
+[Source](https://github.com/michaelshimeles/ralphy/blob/main/cli/src/engines/index.ts)
#### CLI Flag and Argument Parsing
The CLI flag to enable Copilot is `--copilot`. This flag is parsed in `cli/src/cli/args.ts`, setting the `aiEngine` variable to `copilot`. Model override is supported via the `--model <name>` flag, which is passed to the engine and included in the Copilot CLI invocation if specified.
@@ -42,20 +64,22 @@
These standards help maintain code quality and prevent cross-platform issues.
### Internal Parsing Logic
-The Copilot engine output is parsed to remove CLI artifacts and extract the main response. The `parseOutput` method in `CopilotEngine` filters out lines that start with interactive prompts (`?`), command prompts (`›`), or contain status messages like "Thinking..." or "Working on it...". It also filters out stats sections, such as lines starting with "Total usage", "API time", "Breakdown by", and token count lines (e.g., `17.5k in, 73 out`). Only the meaningful response lines are returned. In addition, the engine parses and reports input and output token usage from the Copilot CLI output, supporting values with `k` and `m` suffixes (e.g., "17.5k").
+With the migration to the Agent Client Protocol (ACP), the Copilot engine no longer parses CLI text output to extract responses or token usage. All communication with Copilot CLI is handled via structured NDJSON messages as defined by the ACP protocol. The engine receives response chunks, stop reasons, and errors directly from the protocol, eliminating the need for output filtering or artifact removal.
-This logic is mirrored in the bash script implementation.
+**Token usage counts:**
+- The Copilot CLI ACP server does not provide input/output token counts or usage statistics. As a result, all Copilot telemetry will report 0 tokens.
+
+**Legacy parsing logic:**
+- The previous output parsing logic (removing CLI artifacts, extracting main responses, parsing token counts) is now only relevant for the deprecated CopilotEngine. It is not used in the ACP-based implementation.
### Model Override Support
-Model override is supported for Copilot and all other engines. Use the `--model <name>` flag to specify a custom model. This flag is parsed and passed to the Copilot CLI as `--model <name>`. For example, to use a specific Copilot model:
+Model override is supported for Copilot and all other engines. When using the ACP-based Copilot engine, the `--model <name>` flag is passed as an instruction in the ACP prompt content, ensuring the desired model is used. For example, to use a specific Copilot model:
```bash
ralphy --copilot --model my-copilot-model "Implement a REST API"
```
-[Source](https://github.com/michaelshimeles/ralphy/blob/fc2df589969b5fe16d31eccb4e7ff91314e31776/README.md#L81-L281)
-
----
+The model override is handled internally by the engine and does not require any changes to the CLI interface. This works seamlessly with the ACP protocol.
## Engine-Specific Arguments
You can pass additional arguments directly to the underlying engine CLI using the standard `--` separator. This allows you to leverage engine-specific features or flags that are not explicitly supported by ralphy.
@@ -130,8 +154,8 @@
### Testing Notes
- The Copilot CLI binary (`copilot`) must be installed and available in your `PATH`. The script checks for its presence and exits with an error if not found.
- Model override is supported and tested for all Copilot invocations.
-- The output parsing logic is tailored to the Copilot CLI's current output format; if the CLI changes, the parsing logic may need updates.
-- All engines, including Copilot, now return explicit error messages with exit codes or Copilot-specific error messages when commands fail (e.g., authentication errors, rate limits, or generic errors), instead of generic 'Unknown error'.
-- The `--yolo` flag is required for Copilot CLI non-interactive mode; without it, tasks may silently complete without execution. This replaces the previously required `-p` and `--allow-all-tools` flags. Copilot now outputs token usage statistics instead of duration.
-- Failed tasks, including those with non-retryable errors (such as authentication failures), are now always marked as complete after failure and will not be retried infinitely. Fatal errors (such as authentication or configuration issues) will now abort all remaining tasks immediately. This applies to both parallel and sequential execution modes, preventing infinite retry loops for failing tasks.
-- All engines are tested for correct CLI flag parsing, model override, output extraction, improved error reporting, and token usage reporting.
+- **Token usage counts are not available for Copilot ACP:** All Copilot telemetry will report 0 tokens due to ACP protocol limitations.
+- Output streaming and error handling are now protocol-based, not dependent on CLI text parsing.
+- All engines, including Copilot, are tested for correct CLI flag parsing, model override, output extraction, and improved error reporting.
+- The ACP engine is covered by unit tests and has been verified end-to-end on Windows, macOS, and Linux.
+- The legacy CopilotEngine is deprecated and not used by default.Note: You must be authenticated to accept/decline updates. |
Greptile OverviewGreptile SummaryRefactors Copilot CLI integration from fragile text parsing to protocol-based ACP (Agent Client Protocol), enabling structured NDJSON communication and real streaming support. The implementation addresses most issues from previous reviews including cross-platform process termination, stderr capture for diagnostics, and type-safe SDK usage via helper methods. Key improvements:
Notable limitation:
Remaining concerns:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as Ralphy CLI
participant Engine as CopilotAcpEngine
participant Process as Copilot Process
participant ACP as ACP SDK
participant Copilot as Copilot CLI
CLI->>Engine: execute(prompt, workDir, options)
Engine->>Process: spawn("copilot --acp --stdio")
Process->>Copilot: Start ACP server
Engine->>ACP: Create NDJSON streams (stdin/stdout)
Engine->>ACP: new ClientSideConnection(client, stream)
Engine->>ACP: initialize(protocolVersion, capabilities)
ACP->>Copilot: Initialize protocol
Copilot-->>ACP: Server capabilities
Engine->>ACP: newSession(cwd, mcpServers)
ACP->>Copilot: Create session
Copilot-->>ACP: sessionId
Engine->>Engine: buildPromptContent(prompt, options)
Note over Engine: Add model override as text if needed
Engine->>ACP: prompt(sessionId, promptContent)
ACP->>Copilot: Send prompt via NDJSON
loop Streaming response
Copilot-->>ACP: agent_message_chunk events
ACP-->>Engine: sessionUpdate callbacks
Engine->>Engine: Accumulate response chunks
opt Streaming mode
Engine->>CLI: onProgress(preview)
end
end
Copilot-->>ACP: promptResult (stopReason)
ACP-->>Engine: Return result
Engine->>Engine: Check stopReason (error/cancelled/end_turn)
Engine->>ACP: endSession(sessionId)
Engine->>Process: kill() / kill("SIGTERM")
alt Process exits in 2s
Process-->>Engine: exit event
else Timeout after 2s
Engine->>Process: kill("SIGKILL")
alt SIGKILL succeeds within 500ms
Process-->>Engine: exit event
else Still running
Engine->>Engine: Log warning
end
end
Engine-->>CLI: AIResult
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 files reviewed, 6 comments
- Fix cross-platform process termination (use kill() on Windows, SIGTERM on Unix) - Add SIGKILL after timeout to prevent leaked processes - Remove unsafe clientProvider override pattern - Create createChunkCapturingClient() helper to eliminate duplication - Improve code comments explaining model override limitation - Remove unused createAcpConnection() method - All tests passing (90/90)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 3 comments
- Fixed undefined variable references in executeStreaming error paths - All token counts explicitly set to 0 (not available via ACP) - Addresses remaining Greptile comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 files reviewed, 3 comments
…sleading comment, stderr capture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 4 comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 files reviewed, 1 comment
Summary
Refactors the Copilot engine to use GitHub Copilot CLI's Agent Client Protocol (ACP) instead of text output parsing. This modernizes the integration with a protocol-based approach that provides structured communication and real streaming support.
Changes
Core Implementation
CopilotAcpEngine: Implements ACP protocol using@agentclientprotocol/sdkexecuteStreaming()with properagent_message_chunkeventsrequestPermissionhandler for yolo modePlatform Support
shell: truefor .cmd wrapper supportTesting
Integration
--copilotflagCopilotEnginekept for reference (marked deprecated)Benefits
Known Limitations
promptResultonly containsstopReason. This is a Copilot CLI limitation, not an implementation issue.Testing
Migration
No action required - migration is automatic and transparent:
--copilotflag now uses ACP automaticallyTechnical Details
Dependencies Added:
@agentclientprotocol/sdk@0.13.1Files Changed:
cli/src/engines/copilot-acp.ts- New ACP implementation (425 lines)cli/src/engines/copilot-acp.test.ts- Unit tests (101 lines)cli/src/engines/index.ts- Use ACP as defaultcli/src/engines/copilot.ts- Marked deprecatedcli/package.json- Added SDK dependencyReferences
Checklist
Ready to merge ✅