Request cancellation for in-flight requests (ADR 0008)

[drshade]

Implements ADR 0008: in-flight requests can now actually be interrupted, per the spec's "SHOULD stop work as soon as practical, MUST NOT send any further messages for that request".

stdio

• Every request (except the transport-level subscriptions/listen) runs in its own async task. Tasks are registered in an in-flight table by request id before the handler body starts (a gate MVar closes the register/cancel race) and deregister themselves on any exit, including cancellation.
• notifications/cancelled now falls through from subscription teardown to cancelling the referenced in-flight task. cancel waits for the task to die, so once it returns nothing further is written for that id. Unknown or already-completed ids are ignored, as the spec requires.
• Writes to stdout are uninterruptibleMask_-protected inside the existing write lock, so a cancellation arriving mid-write can never leave a half-written line on the shared channel.
• EOF cancels all in-flight tasks before the graceful subscription closures.

HTTP

• For SSE responses (handleStreamingRequest), the handler runs raced against a keep-alive writer that doubles as the disconnect detector: when the client closes the stream, the next keep-alive write throws and the handler task is cancelled. The interval is 5s, which bounds cancellation latency for abandoned handlers.
• Stream writes are serialized behind a per-stream lock and made uninterruptible, so an SSE event can never be split by a concurrent writer or a cancellation.
• Single-JSON requests need no new machinery — Warp already tears the request thread down on disconnect (noted in the haddock).

Contract

Cancellation reaches handler code as an ordinary GHC asynchronous exception. Documented in the README (new Cancellation section) and on both transport entry points: handlers are interruptible wherever they block in IO, and handlers that acquire resources must release them via bracket/finally.

Testing

• New Spec.Cancellation verifies the contract the transports are built on: a cancelled handler task emits nothing further, never yields a response, and bracket cleanup runs (186 examples total, all green).
• Live smoke tests on both transports:
  • stdio: cancelled a 3s tool call mid-flight — no response ever arrives for that id, the tool's completion marker file is never written, no post-cancel progress leaks, the server keeps answering, and unknown-/completed-id cancellations are ignored.
  • HTTP: curl --max-time 2 aborting a 12s streamed tools/call — the handler is cancelled (marker never written), and the server keeps serving; an unaborted control call streams progress and completes normally.

New dependency: async >=2.2 && <2.3. CHANGELOG updated under 0.2.0.1; ADR 0008 marked Landed (0.2.0.1).

[drshade]

Review verdict: two documentation fixes requested — the code is right, but the contract it creates isn't fully told. The concurrency engineering itself held up under scrutiny:

• The register-before-start gate genuinely closes the register/cancel race, and self-deregistration via finally covers every exit path including cancellation.
• cancel-waits semantics make "nothing further for that id" true once teardown returns; the inherent race where a cancellation lands during an uninterruptible response write is exactly the case the spec's cancellation rules permit ("may arrive after processing has completed").
• Interruptible lock-take + uninterruptible write is the correct masking shape — no half-written frames, no deadlock on cancellation while waiting for the lock.
• The keep-alive-as-disconnect-detector race on HTTP is a neat two-birds mechanism, and writing the final response after the race (keep-alive already cancelled, lock held) is ordering-safe.
• Cancel-then-check subscription-then-inflight for notifications/cancelled, ignoring unknown ids silently, matches the cancellation spec.

One engineering note to record rather than fix: uninterruptibleMask_ around writes means a stalled-but-open client (reads nothing, connection alive) can block a writer beyond cancellation until TCP gives up. That's the standard framing-vs-liveness tradeoff and I agree with the choice — but it's worth a sentence in the haddock, and a System.Timeout.timeout guard around the masked write is a reasonable future hardening if it ever bites.

186/186 locally; merge on the doc fixes + green CI.

[drshade]

Requested fix 1: document that stdio handlers now run concurrently. This branch changes a real contract: before this PR the stdio transport processed requests strictly sequentially, so handlers with shared mutable state (an IORef cache, a file the tools append to) were race-free by construction. Now two in-flight requests run their handlers in parallel. That's the right design — HTTP always ran handlers concurrently under Warp, so portable handlers needed thread safety anyway — but stdio-only users could have relied on the old serialization, and nothing in the README's new Cancellation section or this module's haddock says it changed. One or two sentences in both places ("requests are served concurrently on both transports; handlers touching shared state must synchronize") plus a CHANGELOG line under 0.2.0.1, since this is behavioral, not just additive.

[drshade]

Done in 6cc8355 — README (new paragraph in the Cancellation section), the stdio transport haddock, and the CHANGELOG (flagged as a behavioral break under 0.2.0.1) now all state that requests are served concurrently on both transports and that handlers touching shared mutable state must synchronize.

[drshade]

Requested fix 2: the single-JSON disconnect claim is stronger than Warp's actual behavior. A WAI application doesn't learn the client is gone until it writes — Warp doesn't poll the socket while the handler runs. For streaming responses your keep-alive writer is precisely what surfaces the disconnect; for single-JSON responses there is no mid-handler write, so an abandoned request runs the handler to completion and only then fails the response write. The thread does get torn down — but after the work is done, which is not "cancellation" in the spec's SHOULD-stop-work sense. Suggest rewording here, in the README, and in the CHANGELOG fragment to something like: "for single-JSON responses a disconnect is only detected at the final write, so mid-handler cancellation applies to streaming (SSE) requests — clients wanting cancellable calls should opt into streaming via a progressToken." That last clause is genuinely useful guidance for client authors.

[drshade]

Done in 6cc8355 — reworded in the HTTP haddock, README and CHANGELOG: single-JSON responses only detect a disconnect at the final write (handler runs to completion first), so mid-handler cancellation applies to streaming requests, with the suggested guidance that clients wanting cancellable calls opt into streaming via a progressToken.

[drshade]

Approved. Both documentation fixes verified on 6cc8355: the stdio concurrency change is stated in the README, transport haddock and CHANGELOG (with the honest BREAKING-behavioral label), and the single-JSON disconnect semantics are corrected everywhere with the progressToken guidance for client authors. The ADR-index status sync in 811cf6b is a welcome bonus. 186/186 tests, CI green. Merging — roadmap Batch 2 (ADR 0007 + 0008) is complete: the library now delivers the spec's full long-running-tool story (progress, client logs, per-request SSE, and real cancellation on both transports).