feat: add ACP transport for cross-tool consultations#6
Merged
Conversation
Add ACP (Agent Client Protocol) as an additional transport alongside existing CLI subprocess invocation. ACP provides standardized JSON-RPC 2.0 communication supported by all major AI coding tools. New files: - acp/client.js: AcpClient class handling spawn, initialize, session, prompt, streaming chunk collection, and graceful shutdown - acp/providers.js: Provider registry mapping tool names to ACP adapter configs (claude, gemini, codex, copilot, kiro, opencode) - acp/run.js: CLI entry point that encapsulates full ACP lifecycle into a single Bash-invocable command with --detect mode Updated: - SKILL.md: ACP Transport section with provider table, command templates, detection, and transport field in session schema - consult.md: ACP detection in Phase 2b, kiro in tool allow-list - consult-agent.md: node/npx/kiro-cli tool permissions
- test-command-templates.js: 17 new assertions for ACP provider table, command templates, transport selection docs, kiro in tool lists - test-acp-client.js: 94 tests covering AcpClient constructor, provider registry, run.js argument validation, output sanitization patterns, module exports, and file existence - package.json: wire test:acp and test:templates scripts
- Remove excessive inline comments from client.js and run.js - Fix consult-agent.md: add Kiro to non-continuable list - Fix test-acp-client.js: replace setTimeout race with proper async/await, move IIFE tests into sequential async runner - Add mock ACP subprocess test (connect, initialize, session, prompt, chunk events, close) - Add missing edge case tests (timeout=0, NaN timeout, leading-dash session-id, detectAllAcpSupport, isCommandAvailable positive path) - Test count: 94 -> 121
Security: - Sanitize error messages in writeError() (run.js) - Add question-file path containment check (reject paths outside cwd) Performance: - Replace O(N^2) string concat with array+join for chunk accumulation - Close readline interface on unexpected subprocess exit (resource leak) Correctness: - Accept --effort arg in run.js (was hardcoded to 'medium') - Fix model fallback (was using agentInfo.name as model ID) - Add .catch handler to test runner (prevent silent crash) - Add path-containment test case Test count: 121 -> 122
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.
Summary
Changes
New files (acp/)
acp/client.js- AcpClient JSON-RPC 2.0 class: spawn, initialize, session, prompt, streaming chunks, timeout, shutdownacp/providers.js- 6-provider registry with command detectionacp/run.js- CLI entry point with--detectmode, output sanitization, path containmentUpdated
skills/consult/SKILL.md- ACP Transport section, Kiro provider, transport field in session schemacommands/consult.md- ACP detection in Phase 2b, kiro in tool allow-list, node/npx/kiro-cli toolsagents/consult-agent.md- Extended tool permissions, Kiro in non-continuable listTests
scripts/test-acp-client.js- 122 tests (mock ACP protocol, provider registry, arg validation, sanitization)scripts/test-command-templates.js- 17 new ACP regression assertionsSecurity
Test plan
npm testpasses (122 ACP + template tests)Closes agent-sh/agentsys#281