Make a cancelled turn actually unwind in the Go and .NET servers - #514
Merged
Conversation
Cancelling a turn was a mute button, not a stop button. The runner walks away at its first cancellation check and the sink is gagged, but the engine's agent loop keeps going: it folds every tool failure back to the model as a tool result and iterates, and after a cancel that failure is the context error a tool returns or the denial the write-confirmation gate returns once the park is unwound. So the loop made another model call and acted on the answer, invisibly. Neither engine's loop checks cancellation, and cancellation here is cooperative rather than the preemptive future-drop Rust gets for free. Stop the loop where it re-enters shared state — the model call: wrap the turn's chat client so a cancelled context fails the call instead of issuing it, which unwinds RunStream / RunStreamingAsync. This also clears the DATA RACE the Go race detector reports on the shared corpus's cancel-mid-turn scenario, where the cancelled turn's goroutine and the next turn's drove the engine mock concurrently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 3d350e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…osts The live gateway client fails that call on its own cancelled context, so the defect is that the servers relied on the transport to stop a cancelled turn — not a standing spend leak. Say that, rather than let the comment claim a bill that is not there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
brentrager
added a commit
that referenced
this pull request
Aug 19, 2026
#514 made Go and .NET actually unwind a cancelled turn, and the corpus proved it twice, unaided: * Go was UNMARKED and went green on its own, scenario untouched, race detector included. The corpus said Go was broken, the port was fixed, and the corpus now agrees without anyone editing it. * .NET was marked, so the xpass guard FIRED on its first real opportunity: 'remove dotnet from knownDivergences in cancel-mid-turn — it now passes'. Marker removed; .NET is back to 19/19. That is the whole point of an expiring marker over a skip: it told us the moment it became a lie, instead of rotting into a green check that proved nothing. The five interaction-ordering markers (th-ef78d0) are untouched — those still genuinely fail and the ruling stands: Rust's order is correct. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
brentrager
added a commit
that referenced
this pull request
Aug 19, 2026
* th-eae69d: add cancel + Rich Interaction conformance scenarios
The shared conformance corpus every server replays had no cancel scenario
and no interaction scenario. Cancellation and Rich Interactions — the two
newest features — were cross-checked only by fixture SHAPE, never by
cross-language BEHAVIOR, so a port could be fully green on parity while
implementing neither correctly. That is why six reviewers independently
found the same divergences in four languages: nothing in CI was positioned
to catch them.
Eight scenarios, test-only, no runner or server changes:
cancel-mid-turn terminal cancelled/499 echoing the
TURN's requestId, then the slot is free
cancel-no-active-turn-noop a stray cancel emits nothing
interaction-park-resume identity_intake parks + resumes
interaction-invalid-retryable invalid values stay PARKED, twice
interaction-stale-id-rejected INTERACTION_MISMATCH, stays parked
interaction-declined declined:true resolves without values
interaction-conversational-fallback no capability -> no park
interaction-choices-park-resume the choices kind, same envelope
A mock turn finishes faster than a cancel can race it and the format has no
slow-tool directive, so cancel-mid-turn opens its in-flight window with a
write-confirmation park — the one pause the corpus can express.
These go in RED on four servers. That is the deliverable: the audit's
systemic findings become CI-visible facts. Two real divergences, detailed
in the README:
* Rust emits interaction_required BEFORE the raise tool's stream_chunk;
Go/TS/Python/.NET emit the chunk first. The scenarios assert Rust's
order — it is the reference, and it is the order the corpus already
pins for the other park type (hitl-write-confirmation), which the
other four also honor, making them internally inconsistent between
their two park paths. Whether the spec adopts chunk-first instead is
a protocol decision, not a test fix.
* Go and .NET keep running a cancelled turn: it consumes one more LLM
response after the cancel and is merely gagged, not aborted.
Cancellation is a mute button there, not a stop button. Confirmed by
re-running with an extra mockLlmScript entry — both then pass, so the
entry is provably eaten by the cancelled turn.
Verified on all five: Rust 18/18. TS, Python 13/18 (the 5 park scenarios).
Go, .NET 12/18 (those 5 plus cancel-mid-turn).
There is no per-language skip/allowlist/xfail mechanism in any of the five
runners and none can be added in JSON, so these land on all five at once.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* th-eae69d: honour knownDivergences in all five parity runners
Per the ruling on #513: Rust's park ordering is correct (the four ports
change, not the spec), and this must not merge red. So the new scenarios
now carry an EXPIRING xfail marker that all five runners honour.
A scenario names the languages it fails on today, with the reason and the
pearl id right beside it:
"knownDivergences": ["go", "typescript", "python", "dotnet"],
"knownDivergencesReason": "th-eae69d — these four emit the raise tool's
toolCall chunk BEFORE interaction_required ..."
Both halves of the contract are implemented, and the second is the point:
* a listed language that FAILS is reported (reason + the real assertion)
and does not fail the build;
* a listed language that PASSES FAILS the build — "remove <lang> from
knownDivergences in <scenario> — it now passes".
Without the xpass half the markers rot silently and we recreate the exact
"green tests that prove nothing" problem this corpus exists to catch. A
marker is a tracked bug with an expiry, never an accepted difference.
Marked strictly to the measured matrix: the five interaction park scenarios
for go/typescript/python/dotnet, and cancel-mid-turn for go/dotnet. The
sixth interaction scenario (interaction-conversational-fallback) already
passes everywhere and is deliberately unmarked.
*testing.T and panics do not catch alike, so the five runners differ:
rust tokio::spawn, so the scenario's panic surfaces as a JoinError
go narrow *testing.T to a small parityT interface, then run a marked
scenario against a recorder whose Fatalf panics with the message
(testing.T's own Fatalf is terminal and cannot be un-failed)
ts/py catch the assertion (vitest / pytest.xfail)
dotnet catch XunitException
Verified in both directions, per language: with the real corpus all five are
green (Rust 18/18, Go/TS/.NET green with divergences logged, Python 13 passed
+ 5 xfailed), and with a passing scenario temporarily marked for all five,
every runner fails with its "it now passes" message. gofmt, cargo fmt,
clippy, tsc --noEmit and ruff are clean.
Note for anyone iterating locally: `go test` caches results and does NOT
invalidate on a scenario-JSON edit — use -count=1.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* th-eae69d: record the Go cancel-while-parked data race
CI runs `go test -race`, and cancel-mid-turn trips it: a DATA RACE in
core's MockLlmProvider.ChatStream, where the CANCELLED turn's goroutine and
the NEXT turn's goroutine pop the same unguarded FIFO script concurrently.
That is independent, mechanical proof of the divergence the scenario's
assertion already catches — the cancelled turn is genuinely still executing,
not merely mis-reporting. Reproduces deterministically, locally and in CI.
A knownDivergences marker cannot and must not suppress it: the race detector
fails the test outside the runner's assertion path. So vet-test (go/server)
stays red until Go actually aborts a cancelled turn. Documented rather than
worked around — guarding the mock's FIFO would silence the evidence and
leave the bug.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* th-eae69d: unmark Go on cancel-mid-turn so the port fix proves itself
The go marker on cancel-mid-turn bought nothing and would have backfired.
It covers only the assertion; the data race (th-f2ac48) fails the test
outside the runner's assertion path, so vet-test (go/server) was red with
the marker anyway. And once fix-cancel-unwind lands, Go would start PASSING
the assertion — firing the xpass guard and re-reding the build with 'remove
go from knownDivergences' instead of going green.
Unmarked, cancel-mid-turn turns green on its own the moment Go actually
unwinds a cancelled turn. That is the proof the port fix worked.
.NET stays marked: it has no race, so the marker is what keeps build-test
green today, and its xpass guard is what will tell us when it is fixed.
Also spells out in the README that hitl-write-confirmation — the scenario
proving the four ports are internally inconsistent with their own other
park path — is one ALL FIVE pass today, so the ordering ruling does not get
re-litigated by the next reader.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* th-eae69d: drop cancel-mid-turn's markers — the port fix landed
#514 made Go and .NET actually unwind a cancelled turn, and the corpus
proved it twice, unaided:
* Go was UNMARKED and went green on its own, scenario untouched, race
detector included. The corpus said Go was broken, the port was fixed,
and the corpus now agrees without anyone editing it.
* .NET was marked, so the xpass guard FIRED on its first real
opportunity: 'remove dotnet from knownDivergences in cancel-mid-turn
— it now passes'. Marker removed; .NET is back to 19/19.
That is the whole point of an expiring marker over a skip: it told us the
moment it became a lie, instead of rotting into a green check that proved
nothing.
The five interaction-ordering markers (th-ef78d0) are untouched — those
still genuinely fail and the ruling stands: Rust's order is correct.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Problem
Cancelling a turn is a mute button, not a stop button, in the Go and .NET servers (pearl
th-f2ac48, P0).After a client cancels, the runner walks away — it returns at its first cancellation check and the connection's sink is gagged, so
cancelledstays the terminal event. But the engine's agent loop keeps running. Both engines fold every tool failure back to the model as a tool result and iterate, and after a cancel that failure is either the context error a tool returns or the denial the write-confirmation gate returns once the park is unwound. The loop went straight on to another model call and acted on the answer, with every trace of it discarded.Rust, TypeScript and Python unwind correctly (Rust drops the turn future, which is preemptive). This brings the two ports in line.
Two independent signals, both on the shared conformance corpus's
cancel-mid-turnscenario (from #513):mockLlmScriptentry, so the next turn finds a drained script and streams nothing.go test -racereports a genuine DATA RACE in the engine'sMockLlmProvider.ChatStream— the cancelled turn's goroutine and the next turn's goroutine driving it concurrently. Two turns in flight at once is the bug.Fix
Neither engine's loop has a cancellation check of its own, and cancellation in Go/.NET is cooperative. The loop is stopped at the one place it re-enters shared state — the model call. The turn's chat client is wrapped (
cancelAwarein Go,CancelAwareChatClientin .NET) so a cancelled context fails the call rather than issuing it, which unwindsRunStream/RunStreamingAsyncand ends the turn. It covers every path into the next iteration, not just the confirmation gate.Worth being precise about the blast radius: in production the gateway client would have failed that next call on its own cancelled context anyway, so this is not a standing spend leak. What it fixes is that the servers were relying on the transport to stop a cancelled turn instead of stopping it themselves — which is exactly why the mock-driven conformance corpus catches it, and why the race keeps
vet-test (go/server)red.Verification
spec/conformance/scenarios/cancel-mid-turn.json(borrowed from Add cancel + Rich Interaction conformance scenarios (th-eae69d) #513 locally, not committed here) passes in both Go and .NET without an extra mock entry. It fails in both without the fix.go vet ./... && go test ./... -raceis clean ingo/serverandgo/— no DATA RACE..NET: 405 unit + 94 integration tests green, including all 12 scenarios.Regression tests, both asserting the model-call count at a settle point rather than on timing:
TestCancelledTurnMakesNoFurtherModelCall: after cancel, the next turn must still find its scripted response, and the mock must have been called exactly once per turn.ACancelledTurn_MakesNoFurtherModelCall_SoTheNextTurnKeepsItsResponse: the count is read after host shutdown. That is the settle point that makes it deterministic — a cancelled turn is orphaned (TryCancelActiveTurnclears the turn slot, soWaitForTurnsAsyncno longer tracks it) and teardown'sRejectPendingConfirmationsis what finally unparks it. Without the fix that unpark leads straight to a third model call.Both were confirmed to fail with the fix reverted (the .NET one 3/3, the Go one on both the assertion and the race detector).
This unblocks #513: with the fix, Go's
cancel-mid-turngoes green unaided, which is what that scenario's comment expects.It also makes .NET pass, so #513 needs to drop
"dotnet"from that scenario'sknownDivergencesor its xpass guard will red the build with "remove dotnet". I did not touch #513's scenarios or markers, as asked — flagging it so it can be sequenced.🤖 Generated with Claude Code