Skip to content

refactor(acp): migrate server to coder/acp-go-sdk#368

Merged
pocky merged 1 commit into
mainfrom
feature/F105-acp-server-migration-to-coderacp-go-sdk
Jun 6, 2026
Merged

refactor(acp): migrate server to coder/acp-go-sdk#368
pocky merged 1 commit into
mainfrom
feature/F105-acp-server-migration-to-coderacp-go-sdk

Conversation

@pocky

@pocky pocky commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Replace the hand-rolled pkg/acpserver package (~590-line custom JSON-RPC/SSE server) with the upstream github.com/coder/acp-go-sdk, eliminating ~2,000 lines of bespoke protocol code that duplicated what the SDK provides
  • Decompose the monolithic ACP infrastructure layer into focused adapters: Agent (session lifecycle), Emitter (SDK event dispatch), PermissionClient (tool-approval requests), and a simplified Renderer that writes directly to SDK streams
  • Fix session lifecycle bugs discovered during migration: Shutdown now permanently tears down session context rather than only interrupting the current run, and NewSession correctly validates the type assertion on the session ID field
  • Add a full functional test suite (acp_serve_functional_test.go) and architecture enforcement tests that were previously absent

Changes

Removed — custom ACP server package

  • pkg/acpserver/server.go: Deleted (~590 lines of custom JSON-RPC/SSE server)
  • pkg/acpserver/protocol.go: Deleted (custom framing protocol)
  • pkg/acpserver/types.go: Deleted (custom request/response types)
  • pkg/acpserver/doc.go: Deleted
  • pkg/acpserver/architecture_test.go: Deleted
  • pkg/acpserver/goroutine_leak_test.go: Deleted
  • pkg/acpserver/protocol_test.go: Deleted
  • pkg/acpserver/server_test.go: Deleted
  • pkg/acpserver/types_test.go: Deleted
  • pkg/acpserver/writeframe_internal_test.go: Deleted
  • internal/infrastructure/acp/message.go: Deleted (custom message DTOs replaced by SDK types)
  • internal/infrastructure/acp/message_test.go: Deleted

Infrastructure — new SDK adapters

  • internal/infrastructure/acp/agent.go: New Agent handler implementing session create/resume/run against the SDK
  • internal/infrastructure/acp/agent_test.go: Unit tests for Agent covering success, validation errors, and type assertion edge cases
  • internal/infrastructure/acp/emitter.go: New Emitter wrapping SDK connection to dispatch SessionUpdate events; extracted sessionUpdater interface for testability
  • internal/infrastructure/acp/emitter_internal_test.go: Internal tests for Emitter internals
  • internal/infrastructure/acp/emitter_test.go: Behavioural tests for Emitter with fake connection
  • internal/infrastructure/acp/errors.go: Centralized ACPHandlerError transport-neutral error type
  • internal/infrastructure/acp/errors_test.go: Tests for error construction and unwrapping
  • internal/infrastructure/acp/permission.go: New PermissionClient handling tool-approval round-trips via the SDK
  • internal/infrastructure/acp/permission_test.go: Tests for PermissionClient including error propagation
  • internal/infrastructure/acp/architecture_test.go: New architecture enforcement tests (SDK import confinement, documentation drift detection)
  • internal/infrastructure/acp/doc_test.go: Public surface and documentation coverage tests
  • internal/infrastructure/acp/event_projector.go: Updated to consume SDK event types instead of custom message structs
  • internal/infrastructure/acp/event_projector_test.go: Revised to match new SDK-based projector
  • internal/infrastructure/acp/renderer.go: Simplified to emit directly to SDK streams; added concurrency invariant documentation
  • internal/infrastructure/acp/renderer_test.go: Substantially refactored to match simplified renderer surface
  • internal/infrastructure/acp/doc.go: Rewritten to document new component decomposition and SDK integration contract

Application — session service fixes

  • internal/application/acp_session.go: Updated session lifecycle; Shutdown permanently tears down context
  • internal/application/acp_session_service.go: Wired new SDK-backed adapters; improved NewSession result validation
  • internal/application/acp_session_service_parking_test.go: New parking/prompt-routing test coverage
  • internal/application/acp_session_service_test.go: Expanded to cover NewSession error paths and session ID validation
  • internal/application/acp_session_service_concurrency_test.go: Minor updates for renamed types
  • internal/application/acp_audit_fixes_test.go: Minor updates

CLI wiring

  • internal/interfaces/cli/acp_serve.go: Updated serve command to wire SDK server instead of custom server; fixed signal-watch goroutine lifetime
  • internal/interfaces/cli/acp_serve_lifecycle_test.go: New lifecycle tests for server startup/shutdown sequence
  • internal/interfaces/cli/acp_serve_test.go: Revised to match new wiring surface
  • internal/interfaces/cli/acp_wiring.go: Simplified; removed pkg-acpserver dependency, wires SDK components directly
  • internal/interfaces/cli/acp_wiring_test.go: Updated with sync/atomic import for streamFlaggingEmitter tests

Architecture & configuration

  • .go-arch-lint.yml: Removed pkg-acpserver component; added go-sdk-acp vendor; updated infra-acp dependency rules
  • go.mod / go.sum: Added github.com/coder/acp-go-sdk dependency

Tests — integration

  • tests/integration/acp/acp_serve_functional_test.go: New end-to-end functional suite covering session create, run, streaming, and shutdown flows
  • tests/integration/acp/acp_jsonrpc_e2e_test.go: Updated to use SDK-based server
  • tests/integration/acp/acp_goroutine_leak_test.go: Updated for SDK lifecycle
  • tests/integration/acp/testhelpers_test.go: Refactored test helpers for SDK server setup

Documentation

  • docs/ADR/020-acp-server-migration-to-coder-sdk.md: New ADR documenting the migration rationale, tradeoffs, and SDK adoption decision
  • docs/ADR/018-acp-transparent-agent-server-protocol.md: Minor update to reference ADR-020
  • docs/ADR/README.md: Added entry for ADR-020
  • CHANGELOG.md: Release entry for F105
  • README.md: Updated feature status

ZPM knowledge base

  • .zpm/kb/pr_feature_f105_acp_server_migration_to_coderacp_go_s/journal.wal: PR tracking journal
  • .zpm/kb/pr_feature_f105_acp_server_migration_to_coderacp_go_s/knowledge.pl: Prolog knowledge facts for this PR

Test plan

  • make build produces a clean binary with no compilation errors
  • make test passes including the new acp_serve_functional_test.go functional suite and all unit tests for Agent, Emitter, PermissionClient, and Renderer
  • make lint reports zero violations; architecture lint confirms infra-acp no longer imports pkg-acpserver and only the SDK vendor is used
  • awf acp-serve starts an ACP server, accepts a session, processes a prompt, and streams events correctly end-to-end

Closes #367


Generated with awf commit workflow

@pocky pocky force-pushed the feature/F105-acp-server-migration-to-coderacp-go-sdk branch from f251001 to e3ce139 Compare June 6, 2026 21:51
- `.go-arch-lint.yml`: Remove pkg-acpserver component, add go-sdk-acp vendor dependency
- `.zpm/kb/pr_feature_f105_acp_server_migration_to_coderacp_go_s/journal.wal`: Add PR knowledge journal
- `.zpm/kb/pr_feature_f105_acp_server_migration_to_coderacp_go_s/knowledge.pl`: Add PR Prolog knowledge base
- `CHANGELOG.md`: Document F105 ACP server SDK migration
- `README.md`: Update ACP server documentation reference
- `docs/ADR/018-acp-transparent-agent-server-protocol.md`: Update ADR cross-reference
- `docs/ADR/020-acp-server-migration-to-coder-sdk.md`: Add ADR documenting SDK migration decision
- `docs/ADR/README.md`: Register new ADR 020
- `go.mod`: Replace custom acpserver with github.com/coder/acp-go-sdk
- `go.sum`: Update checksums for new dependency
- `internal/application/acp_session.go`: Adapt session types to SDK interfaces
- `internal/application/acp_session_service.go`: Wire SDK-based agent handler
- `internal/application/acp_audit_fixes_test.go`: Update tests for SDK session contract
- `internal/application/acp_session_service_concurrency_test.go`: Update concurrency tests
- `internal/application/acp_session_service_parking_test.go`: Add parking flow tests
- `internal/application/acp_session_service_test.go`: Expand session service test coverage
- `internal/infrastructure/acp/agent.go`: Add SDK-backed Agent handler implementation
- `internal/infrastructure/acp/agent_test.go`: Add full unit tests for Agent handler
- `internal/infrastructure/acp/architecture_test.go`: Add SDK import confinement test
- `internal/infrastructure/acp/doc.go`: Update package documentation for SDK migration
- `internal/infrastructure/acp/doc_test.go`: Add documentation drift detection tests
- `internal/infrastructure/acp/emitter.go`: Add Emitter wrapping SDK session updates
- `internal/infrastructure/acp/emitter_internal_test.go`: Add internal Emitter unit tests
- `internal/infrastructure/acp/emitter_test.go`: Add public Emitter integration tests
- `internal/infrastructure/acp/errors.go`: Add typed ACP error hierarchy
- `internal/infrastructure/acp/errors_test.go`: Add error type unit tests
- `internal/infrastructure/acp/event_projector.go`: Adapt projector to SDK event types
- `internal/infrastructure/acp/event_projector_test.go`: Update projector tests for SDK types
- `internal/infrastructure/acp/message.go`: Remove custom message DTO (replaced by SDK)
- `internal/infrastructure/acp/message_test.go`: Remove message DTO tests
- `internal/infrastructure/acp/permission.go`: Add SDK-backed PermissionClient adapter
- `internal/infrastructure/acp/permission_test.go`: Add permission client unit tests
- `internal/infrastructure/acp/renderer.go`: Rewrite renderer to emit directly via SDK
- `internal/infrastructure/acp/renderer_test.go`: Update renderer tests for direct SDK emission
- `internal/interfaces/cli/acp_serve.go`: Replace custom server with SDK server lifecycle
- `internal/interfaces/cli/acp_serve_lifecycle_test.go`: Add server lifecycle tests
- `internal/interfaces/cli/acp_serve_test.go`: Update serve command tests for SDK wiring
- `internal/interfaces/cli/acp_wiring.go`: Rewire ACP components against SDK interfaces
- `internal/interfaces/cli/acp_wiring_test.go`: Update wiring tests with SDK fakes
- `pkg/acpserver/` (10 files): Delete custom ACP server package superseded by SDK
- `tests/integration/acp/acp_goroutine_leak_test.go`: Adapt goroutine leak test to SDK server
- `tests/integration/acp/acp_jsonrpc_e2e_test.go`: Update e2e test for SDK transport
- `tests/integration/acp/acp_serve_functional_test.go`: Add functional integration test suite
- `tests/integration/acp/testhelpers_test.go`: Update test helpers for SDK server setup

Closes #367
@pocky pocky force-pushed the feature/F105-acp-server-migration-to-coderacp-go-sdk branch from e3ce139 to 8afce0b Compare June 6, 2026 22:43
@pocky pocky marked this pull request as ready for review June 6, 2026 22:49
@pocky pocky merged commit b20c85f into main Jun 6, 2026
5 checks passed
@pocky pocky deleted the feature/F105-acp-server-migration-to-coderacp-go-sdk branch June 6, 2026 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

F105: ACP server migration to coder/acp-go-sdk

1 participant