-
Notifications
You must be signed in to change notification settings - Fork 93
Per-request cancellation mechanism #183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| --- | ||
| title: "Cancellation" | ||
| description: "Mechanisms for request cancellation" | ||
| --- | ||
|
|
||
| ACP uses JSON-RPC 2.0 for making requests and getting responses. | ||
|
|
||
| The JSON-RPC specification doesn't define any standard mechanism for request cancellation and keeps it up to the implementation. | ||
|
|
||
| - The agent and the client **MUST** implement [$/cancelRequest](./schema#%24%2Fcancelrequest) notification method to support per-request cancellation. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a common pattern to do a $ for a wildcard?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I decided to reuse the same name as in LSP. In my inderstanding the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All good. Borrowing from LSP whenever possible makes sense to me! |
||
|
|
||
| - When a cancellation request is received from the remote side, the corresponding request activity and all nested activities (including outgoing requests) **MUST** be cancelled, then one of the responses **MUST** be sent back: | ||
| - an error response with the Cancelled [error code `-32800`](./schema#param-code) | ||
| - a valid response with the appropriate data (e.g., a partial result or a valid result with the Cancelled marker) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this also allow for the opposite side to also do some sort of graceful cancellation? Like we do for prompts where the agent can still send a session/update for currently in-progress items before responding with the cancelled stop reason? Obviously the client would need to handle this as you pointed out in the line below, I'm just curious what the boundaries of both sides would be.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, the 2nd item is exactly to cover this. So if the handler handles a cancellation manually it may send a non-error response and the calling side will receive it. The 1st item covers the default case, when a handler execution is cancelled and not processed, in this case the default JsonRpcErrorCode.Cancelled should be sent |
||
|
|
||
| - The calling side **MAY** implement graceful cancellation processing by waiting for the cancelled response (error code `-32800`) from the remote side. | ||
|
|
||
| - The request **MAY** be also cancelled from inside the request handler activity (e.g. when a heavy action is being performed in IDE by a request and the user cancels it explicitly). In this case the response with the [error code `-32800`](./schema#param-code) and the appropriate message **MUST** be sent back and the cancellation **SHOULD** be propagated on the calling side. | ||
|
|
||
| - Cancellation **MAY** also be done explicitly on a per-feature basis to cover the bigger scope of things to cancel (e.g., cancellation of a [prompt turn](./prompt-turn#cancellation)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In your mind, do you think we should move to a world where prompt cancellation would also fall under this cancellation paradigm? I think it might fit, I'm just curious in your mind where you see the boundary of specialized cancellation vs generic
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
TBH when I explored the In this case you can bind all the updates to a particular prompt, because now it's not handy to maintaint something like |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,135 @@ title: "Schema" | |
| description: "Schema definitions for the Agent Client Protocol" | ||
| --- | ||
|
|
||
| ## JSON RPC | ||
|
|
||
| The Agent Client Protocol uses JSON RPC 2.0 for communication. | ||
|
|
||
| ### <span class="font-mono">JsonRpcRequest</span> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, we should expose this and make sure it is added to the schema |
||
|
|
||
| A JSON-RPC request is represented by sending a request object to the remote side. | ||
|
|
||
| **Type:** Object | ||
|
|
||
| **Properties:** | ||
|
|
||
| <ResponseField name="id" type={"integer | string"} required> | ||
| An identifier established by the calling side. The id must be a integer or | ||
| string. | ||
| </ResponseField> | ||
| <ResponseField name="method" type={"string"} required> | ||
| The name of the method to be invoked (e.g., "initialize", "session/new", | ||
| "session/prompt"). | ||
| </ResponseField> | ||
| <ResponseField name="params" type={"object"}> | ||
| A structured value that holds the parameter values to be used during the | ||
| invocation of the method. | ||
| </ResponseField> | ||
| <ResponseField name="jsonrpc" type={"string"} required> | ||
| The version of the JSON-RPC protocol. Must be exactly "2.0". | ||
| </ResponseField> | ||
|
|
||
| ### <span class="font-mono">JsonRpcResponse</span> | ||
|
|
||
| When a JSON-RPC call is made, the server replies with a response. | ||
|
|
||
| **Type:** Object | ||
|
|
||
| **Properties:** | ||
|
|
||
| <ResponseField name="id" type={"integer | string"} required> | ||
| This member is required. It must be the same as the value of the id member in | ||
| the request object. | ||
| </ResponseField> | ||
| <ResponseField name="result" type={"any"}> | ||
| This member is required on success. This member must not exist if there was an | ||
| error invoking the method. The value is determined by the method invoked on | ||
| the server. | ||
| </ResponseField> | ||
| <ResponseField name="error" type={<a href="#jsonrpcerror">Error</a>}> | ||
| This member is required on error. This member must not exist if there was no | ||
| error triggered during invocation. | ||
| </ResponseField> | ||
| <ResponseField name="jsonrpc" type={"string"} required> | ||
| The version of the JSON-RPC protocol. Must be exactly "2.0". | ||
| </ResponseField> | ||
|
|
||
| ### <span class="font-mono">JsonRpcError</span> | ||
|
|
||
| When a JSON-RPC call encounters an error, the response contains the error member with a value that is an object. | ||
|
|
||
| **Type:** Object | ||
|
|
||
| **Properties:** | ||
|
|
||
| <ResponseField name="code" type={"integer"} required> | ||
| A number that indicates the error type that occurred. | ||
|
|
||
| Standard error codes: | ||
|
|
||
| - `-32700`: Parse error - Invalid JSON was received | ||
| - `-32600`: Invalid request - The JSON sent is not a valid request object | ||
| - `-32601`: Method not found - The method does not exist or is not available | ||
| - `-32602`: Invalid params - Invalid method parameter(s) | ||
| - `-32603`: Internal error - Internal JSON-RPC error | ||
| - `-32800`: Cancelled - The request was cancelled | ||
|
|
||
| </ResponseField> | ||
| <ResponseField name="message" type={"string"} required> | ||
| A short description of the error. | ||
| </ResponseField> | ||
| <ResponseField name="data" type={"any"}> | ||
| A primitive or structured value that contains additional information about the error. This may be omitted. | ||
| </ResponseField> | ||
|
|
||
| ### <span class="font-mono">Notification</span> | ||
|
|
||
| A notification is a request object without an "id" member. Notifications express a lack of interest in the corresponding response and as such no response needs to be returned. | ||
|
|
||
| **Type:** Object | ||
|
|
||
| **Properties:** | ||
|
|
||
| <ResponseField name="method" type={"string"} required> | ||
| The name of the method to be invoked (e.g., "session/cancel", | ||
| "session/update"). | ||
| </ResponseField> | ||
| <ResponseField name="params" type={"object"}> | ||
| A structured value that holds the parameter values to be used during the | ||
| invocation of the method. | ||
| </ResponseField> | ||
| <ResponseField name="jsonrpc" type={"string"} required> | ||
| The version of the JSON-RPC protocol. MUST be exactly "2.0". | ||
| </ResponseField> | ||
|
|
||
| ### <span class="font-mono">$/cancelRequest</span> | ||
|
|
||
| Cancels a previously sent request by its ID. | ||
|
|
||
| This is a standard JSON-RPC notification that can be sent by either party to signal | ||
| that they are no longer interested in the response to a previously sent request. | ||
|
|
||
| #### <span class="font-mono">CancelNotification</span> | ||
|
|
||
| Notification parameters for cancelling a request. | ||
|
|
||
| **Type:** Object | ||
|
|
||
| **Properties:** | ||
|
|
||
| <ResponseField name="id" type={"integer | string"} required> | ||
| The ID of the request to cancel. Must match the id from a previously sent | ||
| request. | ||
| </ResponseField> | ||
| <ResponseField name="method" type={"string"} required> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a payload here? Or just a request id?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe to add a message? I can add arbitrary payload as well, but how to interpret it when received? |
||
| The name of the method to be invoked (e.g., "session/cancel", | ||
| "session/update"). | ||
| </ResponseField> | ||
| <ResponseField name="params" type={"object"}> | ||
| A structured value that holds the parameter values to be used during the | ||
| invocation of the method. | ||
| </ResponseField> | ||
|
|
||
| ## Agent | ||
|
|
||
| Defines the interface that all ACP-compliant agents must implement. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,7 @@ | ||
| { | ||
| "jsonRpcMethods": { | ||
| "cancel_request": "$/cancelRequest" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting question: I wonder if we need to have agent/client methods to handle this. Which means the agent/client will need to keep track of which request body was attached to each request id. If we require the cancellation to pass more params, we are close to just defining individual cancellation requests... 🤔 interesting question. It might be fine to require on the SDK level that this is handled nicely for the implementer. But I think all the more reason to put this behind a capability for now, so we can also potentially roll this out initially as "unstable" and work out some of the wrinkles
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm not sure about implementations in Rust or TS, but in Kotlin I maintatin a map in incoming requests, the map is [requestId->requestJob]. So while a request is being executed and cancel notification is received we know what exactly to cancel. BUT, it works exactly on JsonRpc level and doesn't touch semantic entities of ACP protocol, it's agnostic to what exactly should be cancelled
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think we can definitely do that, but for the graceful cancellation I wonder if there is a way to route it... It would be possible though |
||
| }, | ||
| "agentMethods": { | ||
| "authenticate": "authenticate", | ||
| "initialize": "initialize", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately I think we may need to make this a capability given the number of in-progress agents/clients.
However, I think we could make this a "MUST" for a v2 of the protocol, which I think would make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, no problem at all, so I'll describe it as a capability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For v1 at least 😄 I'll separately start a document we can collaborate on for v2 breaking change candidates