fix(acp): honor session/cancel by aborting the running turn#30145
Merged
Conversation
The ACP agent rejected every session/cancel with UnsupportedOperationError, so clients could not stop an in-flight turn — the prompt ran to completion regardless. This restores the pre-"next" behavior of aborting the backing session via sdk.session.abort, mirroring the closeSession abort path but leaving the ACP session in place so the client can keep prompting. Adds regression coverage: cancel aborts the backing session and keeps the ACP session usable, and a failed abort is swallowed best-effort.
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Contributor
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Exercising opencode with hydra-acp and could no longer cancel in ACP.
The ACP agent rejected every session/cancel with UnsupportedOperationError, so clients could not stop an in-flight turn — the prompt ran to completion regardless. This restores the pre-"next" behavior of aborting the backing session via sdk.session.abort, mirroring the closeSession abort path but leaving the ACP session in place so the client can keep prompting.
Adds regression coverage: cancel aborts the backing session and keeps the ACP session usable, and a failed abort is swallowed best-effort.
Issue for this PR
Closes #30146
Type of change
What does this PR do?
The ACP
cancelhandler insrc/acp/service.tsjust returnsUnsupportedOperationError, sosession/canceldoes nothing — the running turn keeps going. The old (pre-#29929) implementation actually aborted the session, and that got dropped when the "next" implementation was promoted.The fix puts the abort back.
cancelnow looks up the session and callssdk.session.abort({ directory, sessionID }), which is the same callcloseSessionalready uses to stop a turn — I just reuse the existingrequest("session")wrapper and the same best-effortEffect.catch/log.errorso a failed abort doesn't blow up the cancel. The one difference fromcloseSessionis thatcanceldoes not remove the ACP session, so the client can send another prompt afterwards instead of having to reopen the session.cancelis a notification (no reply), so there's nothing to return; logging on failure matches howcloseSessionhandles the same abort call.How did you verify your code works?
bun typecheckfrompackages/opencode— clean.bun test test/acp test/cli/acp --timeout 60000— 130 pass / 0 fail (includes the two new tests).session/cancel, which is how this regressed, so I added two intest/acp/service-session.test.ts: one assertscancelcallssession.abortand the session is still usable afterward (unlikecloseSession), the other asserts a rejected abort is swallowed.opencode acp, started a long-running turn from an ACP client, sentsession/cancel, confirmed the turn stops and a follow-up prompt still works.Screenshots / recordings
N/A — not a UI change.
Checklist