Feature/add support for acp agents

[pikann]

Summary

Adds ACP (Agent Client Protocol) agents — a second agent "shape" alongside the existing LLM agents. An ACP agent delegates work to a coding CLI (Claude Code, Codex, Gemini CLI, or a custom ACP server) that the user runs locally, in their own already-checked-out source folder, connected to Paca through a small local bridge daemon. No source code is cloned into Paca's infrastructure for this agent type — the bridge only relays task requests and streams conversation events back over an authenticated WebSocket.

What's new

• apps/acp-bridge — new local daemon (PyPI package paca-acp-bridge, run via uvx), wrapping the OpenHands SDK's ACPAgent. Spawns the chosen ACP CLI as a subprocess in the current directory, streams events back to Paca, and reconnects automatically on a dropped connection.
• services/api — agent_type (llm/acp), acp_provider, acp_command, and acp_bridge_token_hash columns on agents (migration 000022); ACP-specific creation/update validation; new endpoints POST /{agentId}/acp-bridge-token (generate a bridge token) and GET /{agentId}/acp-bridge-status (connection status). Bridge tokens are stored only as a SHA-256 hash — the plaintext is shown once at generation time.
• services/ai-agent — bridge connection management (acp_bridge.py), dispatch of /paca-skill-triggered messages to connected ACP agents (acp_dispatch.py), and handling of ACPToolCallEvent plus synthetic finish actions in the executor/stream pipeline. A new internal API key (AI_AGENT_INTERNAL_KEY / INTERNAL_API_KEY) authenticates the API service calling back into ai-agent's internal-only routes.
• services/realtime — plumbing to relay ACP bridge connection status.
• apps/web — AcpBridgeSetup component for generating/copying the local bridge run command, agent-creation flow updates to request an ACP bridge token, and translations for the new agent type/setup copy across all 9 supported locales.
• CI/CD — new acp-bridge-pr-ci workflow (ruff lint/format, pytest, package build) scoped to apps/acp-bridge/**, and a publish-acp-bridge job in cd.yml that publishes the package to PyPI via trusted publishing (OIDC) on release.
• Deploy — docker-compose.* and Caddyfile updates to wire the new internal key and any related routing.

Security notes

• Bridge tokens are never persisted in plaintext (hash-only storage), shown once, and scoped per agent.
• The ACP CLI runs with the user's own local credentials and git/gh access — Paca never sees, stores, or forwards that auth, and doesn't manage or inject MCP servers/skills for ACP agents (the local ACP client's own configuration is used as-is).

Testing

• Go: new unit tests in agent_service_test.go / agent_handler_test.go for ACP validation and the new endpoints.
• Python (ai-agent): new tests for bridge connection handling, dispatch, prompt building, and worker changes (test_acp_bridge.py, test_acp_dispatch.py, test_prompt.py, test_worker.py).
• Python (acp-bridge): unit tests for the bridge client and runner (test_bridge_client.py, test_runner.py).
• Web: tests for the ACP-aware conversation-to-thread-messages mapping.

[pullfrog]

Run failed. View the logs →

  ^{｜ Rerun failed job ➔ ｜ View workflow run ｜ via Pullfrog ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Code review — ACP agents support

Thanks for this substantial feature. The local-bridge model is a good fit for letting users run agents in their own environment, but the distributed dispatch path has a few reliability and security gaps that I'd like to see tightened before merge.

Critical (blocking)

1. Fire-and-forget dispatch can leave conversations stuck in RUNNING

• services/ai-agent/src/agent/acp_bridge.py:158-168 — dispatch() checks is_online() and then publishes via Valkey Pub/Sub. Pub/Sub drops messages immediately if the subscriber is reconnecting or if the daemon disconnected after the is_online() check.
• services/ai-agent/src/agent/acp_dispatch.py:52-56 — The conversation is set to RUNNING before dispatch() returns. If the latter returns False, we do call _fail_offline, but if it returns True and the message is then lost in transit, the conversation never recovers.
• Recommended fix: use a per-agent Redis Stream (or a request/response pattern over the WebSocket with ACK + timeout). At minimum, add a per-conversation watchdog timer that fails the conversation if no terminal turn_status arrives within agent_config.timeout_minutes.

2. No timeout for ACP turns

• services/ai-agent/src/agent/acp_dispatch.py:41-76 — After dispatching, neither dispatch_acp_trigger nor the worker waits for completion. timeout_minutes exists on the agent but is unused for ACP.
• Recommended fix: start a watchdog after a successful dispatch; fail the conversation on timeout and emit agent.conversation.failed.

3. Daemon silently drops outbound status/event messages while disconnected

• apps/acp-bridge/src/paca_acp_bridge/bridge_client.py:42-45 — _send() returns without raising when self._ws is None. A final turn_status: finished/failed can be dropped during a reconnect, so the server never learns the turn ended.
• The test_send_drops_message_when_not_connected test actually codifies this behavior.
• Recommended fix: raise on critical messages or queue status/final-event messages until reconnect; callers should be able to detect loss.

High

4. Same bridge token can be used concurrently from multiple sessions

• services/ai-agent/src/agent/acp_bridge.py:103-114 — register() overwrites _connections[agent_id] and creates a new forwarder, but the old WebSocket remains open and the Pub/Sub subscription still exists. Two daemons presenting the same token both receive start_turn dispatches and can both emit events/status for the same agent/conversation.
• Recommended fix: enforce one active bridge session per agent. Close the previous WebSocket when a new successful hello arrives, or reject the new connection.

Medium

5. WebSocket is accepted before credentials are verified

• services/ai-agent/src/routes/bridge.py:37-54 — websocket.accept() runs before the hello message is read and the token verified. An attacker can open many WebSockets and never send a valid hello, consuming server resources.
• Recommended fix: read the first message, validate the token, and only then call accept(). Add a short timeout for the hello message.

6. Existing bridge sessions survive token regeneration

• services/api/internal/service/agent/agent_service.go:284-303 — Regenerating the token writes a new hash to the DB, but services/ai-agent only checks the token at handshake time. A daemon connected with the old token stays connected indefinitely.
• Recommended fix: on token regeneration, delete the agent's presence key and/or broadcast a close to active bridge sessions.

7. Missing DB index on bridge-token hash lookup

• services/api/migrations/000022_add_acp_agents.sql:37 — acp_bridge_token_hash TEXT is queried on every WebSocket handshake but has no index.
• Recommended fix: add CREATE UNIQUE INDEX ... ON agents (acp_bridge_token_hash) WHERE agent_type = 'acp' AND deleted_at IS NULL;

8. API status proxy uses an http.Client with no timeout

• services/api/internal/transport/http/handler/agent_handler.go:49 — h.httpClient = &http.Client{} has no timeout, so GetACPBridgeStatus can hang indefinitely.
• Recommended fix: configure a 10s (or similar) timeout on the client.

9. CORS wildcard on privileged bridge endpoints

• services/api/internal/transport/http/router/router.go — Access-Control-Allow-Origin: * is applied globally, including to POST /projects/:projectId/agents/:agentId/acp-bridge-token.
• Recommended fix: switch to an allow-list of configured frontend origins or set credentials policy explicitly.

Low

10. llm_base_url now required by handler despite DB default

• services/api/internal/transport/http/handler/agent_handler.go:168-170 enforces non-empty llm_base_url even though the DB column has DEFAULT ''. This changes the create-agent contract for existing LLM callers.

11. UpdateAgent applies LLM/ACP fields independently of agent_type

• services/api/internal/service/agent/agent_service.go:196-220 — LLM fields can be set on ACP agents and vice versa. Since agent_type is immutable through the API this is mostly cosmetic, but it's still confusing and can leave an ACP agent carrying an encrypted LLM API key.

12. Malformed acp_command JSONB is silently swallowed

• services/api/internal/repository/postgres/agent_repository.go:833-837 — if err := json.Unmarshal(...); err == nil silently ignores parse errors.
• Recommended fix: return the unmarshal error.

Tests

• apps/acp-bridge/tests/test_bridge_client.py tests URL formatting and message routing but the only connectivity test asserts the silent-drop behavior.
• The Go handler/service tests for ACP paths are stubs and don't exercise ACP validation.

Bottom line

The schema, service wiring, and UI look reasonable, but the distributed dispatch and message-durability issues are real blockers for a usable ACP experience. I'd recommend addressing at least the ACK/watchdog/timeout story and the single-active-session enforcement before merging.

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The latest commit resolves the prior review's reliability/auth concerns, but a race in token-regeneration eviction can still close a legitimate newer ACP bridge session.

Reviewed changes — the incremental diff from d055843 to cc0d86d tightened ACP bridge reliability, session binding, and API hardening.

• Replaced silent message drops with a queued outbox in BridgeClient, so turn_status events survive reconnects.
• Added a per-turn watchdog in acp_dispatch.py to fail ACP conversations that never receive a terminal status.
• Enforced one active bridge session per agent via control-channel eviction and session-aware unregister().
• Moved WebSocket credential verification before accept() by sending the agent id and token as handshake headers.
• Tightened CORS from a blanket wildcard to a configurable CORS_ORIGINS allow-list.
• Added a unique partial index on agents.acp_bridge_token_hash so handshakes do not scan the whole table.
• Fixed handler/service edge cases: llm_base_url is no longer required at creation, LLM/ACP fields are isolated by agent_type on update, malformed acp_command JSONB surfaces as an error, and the ai-agent HTTP proxy has a 10s timeout.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

The latest incremental diff hardens the ACP bridge outbox, turn concurrency, event ownership, Caddy scope, and the custom-provider DB check. However, the token-regeneration eviction race flagged in the previous review is still present: the new token is persisted and returned before the force-disconnect runs, so a legitimate newer bridge session can be evicted if it reconnects slightly ahead of the disconnect call.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Clarification on review #4722784492: the suggestion block in that review was only meant to anchor the comment to the current code, not a proposed change. I’m requesting the race be fixed before merge.

Issue: GenerateACPBridgeToken persists the new token, returns it to the client, and then runs disconnectACPBridge in the background. In that window a daemon can reconnect with the new token; the late /agent-bridge/disconnect/{agentId} call then force-evicts by agent ID and closes the legitimate new session.

Fix options (either works):

1. Synchronously force-disconnect the old session (with a short timeout) before persisting/returning the new token.
2. Keep the async disconnect, but scope the eviction to the replaced token hash/session so a connection authenticated with the new token is not affected.

Right now this is the only remaining item blocking approval of the latest incremental diff.

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}