feat: add ACP agent foundation (synced agents table, proxy, ws-tickets)#624
feat: add ACP agent foundation (synced agents table, proxy, ws-tickets)#624ital0 wants to merge 12 commits into
Conversation
- Add synced `agents` PowerSync table (schema, migration, sync rules) - Add agents, agent-proxy, and ws-ticket routes wired into the app - Add shared agent types and `agentId` on chat threads - Add ENABLED_AGENTS and ALLOW_CUSTOM_AGENTS settings with helper + tests
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Semgrep Security ScanNo security issues found. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
- Add _resetTicketsForTesting to clear in-memory ticket store - Reset between tests to prevent cross-test pollution
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
- tighten parseApiKey against non-object/null/non-string apiKey inputs - add parseClientMessage helper returning JSON-RPC parse error on bad input - surface upstream WS error detail and expose clearConnections for tests - rename _resetTicketsForTesting to clearTickets and cover expiry/capacity - return fulfilled provider agents when another provider rejects
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This comment has been minimized.
This comment has been minimized.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- reject binary/array/primitive WS frames in parseClientMessage - inject fetch into handleHttpMessage so upstream behavior is unit-testable - preserve JSON-RPC request id in non-JSON upstream error responses - join multi-line SSE `data:` lines with \n instead of concatenating - document rationale for unauthenticated /agents and co-located sync schema
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This comment has been minimized.
This comment has been minimized.
- Reject JSON primitives/arrays in parseClientMessage via isPlainObject guard - Drop unused :agentId path param from WS route (ticket carries identity) - Add agents table to encryptedColumnsMap (name, url, command, args, auth_method)
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
This comment has been minimized.
This comment has been minimized.
- extend ws:// guard to also reject http:// when an apiKey is set - http:// would leak Authorization header in cleartext - drop unused RemoteAgentDescriptor re-export from agents/types
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
…imeout - Queue client messages while upstream WS is CONNECTING, flush on open; cap at 64 messages / 256KB and close downstream with 4005 on overflow. - Clear the 30s initial-connect timeout once HTTP response headers arrive so long-running SSE streams are not aborted mid-stream. - Inject a WebSocket factory into openWsRelay for test isolation and export handleWsMessage / WsConnectionState for direct unit coverage. - Drop unused shared/agent-types.ts.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
- throw TicketQuotaError (HTTP 429 + Retry-After) after 20 active tickets per user - apply IP rate limit middleware to POST /ws-ticket - cover quota, independence per user, and expiry eviction in tests
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
- gate concurrent HTTP messages on a bootstrap promise so subsequent requests carry the Acp-Session-Id captured by the first response; reset the gate on failure so the next message can retry - close upstream WS before downstream on backlog overflow to avoid leaking an in-flight handshake until the peer's idle timeout - rename maxSseBufferBytes to maxSseBufferChars to match what is actually measured (string length, not bytes)
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
- add ENABLED_AGENTS and ALLOW_CUSTOM_AGENTS to backend .env.example - extend agents encrypted columns with install_path, package_name, description
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ce6819c. Configure here.
| releaseBootstrap() | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
Notification path discards session headers during bootstrap
Medium Severity
The notification/response code path can claim the bootstrap role (when it's the first message on the connection) but completely discards the fetch response, never capturing Acp-Session-Id or Acp-Connection-Id headers. It then calls releaseBootstrap() which, seeing sessionId is still null, resets bootstrapPromise to null and unblocks waiters. Any request that was awaiting the bootstrap gate proceeds without session headers, and the connection briefly enters a state where both bootstrapPromise and sessionId are null — allowing multiple concurrent messages to each claim a new bootstrap attempt, risking session thrashing.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ce6819c. Configure here.
2 participants
Summary
Backend foundation for multi-agent support via the Agent Client Protocol (ACP). This is PR1a of a 4-PR split of the original PR #531. No concrete agents yet — just the infrastructure everything else plugs into.
agentsPowerSync-synced table +agent_idcolumn onchat_threads(Drizzle migration0013)GET /agentsdiscovery endpoint with genericAgentProviderpattern (returns empty until Feature A/B register providers)POST /ws-ticket— short-lived auth token system (30s TTL, 10K max, hard cap enforced)WS /agent-proxy/ws/:agentId— generic WebSocket + HTTP/SSE relay with SSRF protection (validateSafeUrl+ DNS-pinnedsafeFetch)ENABLED_AGENTS(comma-separated allowlist) andALLOW_CUSTOM_AGENTS(enforced at ticket creation)agent-types.ts(RemoteAgentDescriptor) andpowersync-tables.tsupdatesrc/db/tables.ts,src/db/powersync/schema.ts) included to satisfy type-checksatisfies Record<PowerSyncTableName, …>Two-PR deploy requirement
Per CLAUDE.md PowerSync policy:
Deploying the frontend before sync rules are live would cause silent sync failure.
Known limitation
WebSocket upstream connections in `agent-proxy` lack DNS-pinning. `validateSafeUrl` provides synchronous hostname-only SSRF protection, but a DNS rebinding attack between validation and connection could bypass this. The HTTP path uses `safeFetch` with full DNS pinning. Documented inline; `createSafeWebSocket` is a follow-up.
Test plan
Review notes
The full 4-PR split plan is documented at `.ultraplan/ACP-Haystack-PR-Split-Plan.md`. This PR ships the foundation that PR1b, Feature A (Haystack), and Feature B (Custom Agents) all build on top of.
Reading order for reviewers:
Note
High Risk
Introduces new network-facing proxying and token issuance (
/agent-proxy,/ws-ticket) plus new PowerSync-synced data (agents,chat_threads.agent_id), which are security- and data-flow-sensitive areas where SSRF/credential-handling regressions could be impactful.Overview
Adds the foundation for ACP-backed multi-agent support by introducing a new PowerSync-synced
agentstable and associating chats to agents viachat_threads.agent_id(plus updated sync rules, shared table list, and frontend schema/encryption mappings).Exposes new backend endpoints: unauthenticated
GET /agentsfor agent discovery with env-based allowlisting (ENABLED_AGENTS), authenticatedPOST /ws-ticketfor short-lived one-time WS tickets (with per-user quota + 429Retry-After), andWS /agent-proxy/wsto relay JSON-RPC over either upstream WebSocket or HTTP/SSE with SSRF validation, bounded WS-connect backlogs, and safer handling of API keys (disallowing cleartextws:///http://when credentials are present).Reviewed by Cursor Bugbot for commit ce6819c. Bugbot is set up for automated code reviews on this repo. Configure here.