feat(acp): support serverless remote sessions - #1589
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Direction looks right: sessionId = Conversation id, durable mailbox/worker path, and StateAdapter-backed transport/cancellation instead of inventing a side ACP runtime. The multi-instance recovery coverage is also the right shape.
One real contract gap before I'd merge this:
streamPrompt returns the prompt end_turn as soon as it sees turn_completed, but API Turn control is only cleared in finishCancellationAfterTerminal after mailbox acknowledge. A client that immediately sends the next session/prompt can still observe the previous control as active. The recovery path in acceptPrompt refuses to clear that control while the message remains pending, so the follow-up fails with "already has an active prompt" even though the previous Turn already completed. The old in-process path finished cancellation before returning the prompt result, so this is a back-to-back prompt regression.
Please release durable control at/after the terminal Turn event (or otherwise make admit treat terminal+pending-ack as finished), and add an integration case that sends the next prompt as soon as end_turn arrives without waiting for worker cleanup.
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, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e5c65a6. Configure here.
There was a problem hiding this comment.
Looks good to me now. The terminal handoff waits until the mailbox input is acknowledged before emitting the prompt result, so a follow-up cannot overlap retryable work; once acknowledgement is visible, acceptPrompt can safely clear any lagging control and admit the next Turn. The cross-instance integration coverage exercises both sides of that boundary.
| if (requestId === undefined || requestId === null) { | ||
| return textResponse("Initialize request must include an ID", 400); |
There was a problem hiding this comment.
Initialize handler rejects valid null JSON-RPC request IDs
handleInitialize rejects valid id: null JSON-RPC initialize requests with HTTP 400, but the schema and every downstream path accept null as a valid request identifier.
Evidence
jsonRpcIdSchemainroute.tsisz.union([z.string(), z.number().finite(), z.null()]), soacpCallSchemaparsesnullas a valididhandleConnectedPostonly guards withrequestId === undefined, allowingnullto proceed as a request ID throughacceptAcpRequestand stream output pathsrequestKeyinroute.tsexplicitly branches forcall.id === null, showing the durable transport layer is designed to handlenullIDsresultMessageanderrorMessageboth acceptacp.JsonRpcId, which includesnull, confirming the rest of the pipeline expects it
Also found at 1 additional location
packages/junior/src/api/acp/route.ts:896-896
Identified by Warden · code-review · EWA-5F8

Make remote ACP sessions use Junior's durable Conversation and
StateAdapterpaths so requests can move across serverless instances without affinity. ACP
session IDs are Conversation IDs. Prompts use the existing mailbox, queue,
lease, checkpoint, and event paths. Redis stores connection ownership, retry
receipts, stream cursors, stream leases, and durable cancellation.
This replaces process-local ACP transport state and keeps accepted work running
after an SSE disconnect. A client can reconnect and call
session/loadtoreplay Conversation history. Admission is fenced at the mailbox mutation
boundary, overlapping prompts remain exclusive across instances, and bounded
stream admission returns
503instead of dropping undelivered output.The route remains experimental and opt-in. ACP v1 still requires clients to
reconnect and load after a live request reaches the hosting platform's request
duration limit.