Skip to content

fix: answer pending permission requests when the client cancels or closes a session - #156

Open
EugeneTheDev wants to merge 1 commit into
masterfrom
eugenethedev/permission-cancellation-fix
Open

EugeneTheDev wants to merge 1 commit into
masterfrom
eugenethedev/permission-cancellation-fix

Conversation

@EugeneTheDev

@EugeneTheDev EugeneTheDev commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

When a v1 client sends session/cancel, it MUST answer every pending session/request_permission of that session with the cancelled outcome (docs/protocol/v1/prompt-turn.mdx). The v1 client runtime didn't: a pending request was released only if the agent sent the optional $/cancel_request, so the app had to track and answer its own requests next to every cancel(). close() had the same gap in both runtimes. The v2 runtime already handled cancel; this brings v1 in line and covers close in both.

Fixes

  • New internal PendingPermissionCancellation holds one cancel signal per session and races each permission handler against it. The race is the only answering path, so each request gets exactly one response. The app's handler is cancelled, and the cancelled reply goes out once it has finished.
  • cancel() (v1) completes the signal in a finally, so a failed session/cancel send still releases pending requests. Requests arriving after it and before the next turn are answered cancelled without reaching the app. v1 prompt() resets the signal only after its active-prompt check passes, so a rejected concurrent prompt can't undo a cancel.
  • close() (v1 and v2) completes the signal before sending session/close. Otherwise an agent whose close waits for its turn would deadlock with a client waiting for the close response. A closed session stays cancelled through a CLOSED sentinel that no later turn replaces. A failed close still propagates its error; the holder (v1) and onClosed() (v2) still wait for a successful response.
  • v2 now uses the same helper; its cancel() behaviour is unchanged.
  • KDoc for cancel(), close() and both permission handler methods describes the new behaviour, including that handlers must respond to cancellation promptly. AGENTS.md gains a lifecycle rule.
  • No public API change; acp/api/acp.api is unchanged.

Relation to #155

Independent and can merge in either order. With this change, an agent whose cooperative AgentSession.cancel() waits for its turn receives the client's cancelled answer instead of waiting for the user. Expect a trivial AGENTS.md conflict in the lifecycle list; keep both bullets.

Verification

  • New v1 client tests (ClientPermissionCancellationTest), v2 close tests in client/v2/ClientSessionTest, and a cross-transport test in SimpleAgentTest (STDIO and WebSocket). Against the pre-fix runtime, the v1 tests (apart from the control) and the integration test fail.
  • ./gradlew :acp:apiCheck and ./gradlew check pass.

@EugeneTheDev
EugeneTheDev force-pushed the eugenethedev/permission-cancellation-fix branch from 3fa9ce3 to 8bbf921 Compare November 3, 2026 17:51
…oses a session

The v1 client runtime now answers every pending `session/request_permission`
of a session with the `cancelled` outcome when the app calls
`ClientSession.cancel()` or `close()`, as `prompt-turn.mdx` requires for
`session/cancel`. It no longer waits for the agent's optional
`$/cancel_request`. The v2 runtime already did this on cancel; it now does
it on `close()` too.

- `PendingPermissionCancellation` holds one cancel signal per session and
  races each permission handler against it. The race is the only answering
  path, so each request gets exactly one response. The app's handler is
  cancelled and the reply goes out once it has finished.
- `cancel()` completes the signal in a `finally`, so a failed
  `session/cancel` send still releases pending requests. Requests that
  arrive after it and before the next turn are answered without reaching
  the app. v1 `prompt()` resets the signal only after its active-prompt
  check passes.
- `close()` completes the signal before sending `session/close`, so an agent
  whose close waits for its turn cannot deadlock. A closed session stays
  cancelled through a `CLOSED` sentinel that no later turn replaces. A failed
  close still propagates its error.
- KDoc for `cancel()`, `close()` and both permission handler methods
  describes the behaviour. `AGENTS.md` gains a lifecycle rule for it.

Tests cover cancel, several pending requests, a failed send, requests after
cancel, a rejected concurrent prompt, independent sessions, an app answer
before cancel, close before its response, close failure, and a prompt after
close, for v1. For v2 they cover close with non-compliant and compliant
agents in both orders, close failure, and a prompt after close. A
cross-transport test checks that an SDK agent gets the `cancelled` outcome
over STDIO and WebSocket.

Spec checked: `agent-client-protocol` at `dc011f0`.
Design: docs/superpowers/specs/2026-11-03-permission-cancellation-design.md
@EugeneTheDev
EugeneTheDev force-pushed the eugenethedev/permission-cancellation-fix branch from 8bbf921 to 9941000 Compare November 4, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant