Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
AmanSwar
force-pushed
the
issue-80-shim-keepalive
branch
from
September 13, 2026 14:33
ded8a74 to
dd01aeb
Compare
…its operations reference (#81) The shim will call POST /v1/requests/{request_id}/cancel (InferenceInfra #440, PR #475) when the editor abandons a stream, so per the P0 contract-first rule the operation comes first: contracts/wally-cli-v1.openapi.json is re-extracted from that branch's control-plane-v1.openapi.json, src/account/console_contract.h regenerated (CancelRequestResponse, two strings -- the status is a const to the generator, not an enum), and the pin bumped; test_wally_contract holds the three together. Two things the extractor got wrong for as long as it has existed, fixed while here because the new operation would have made both worse: - It collapsed every $ref to its last segment and looked it up in components.schemas only, so `#/components/parameters/...` and `#/components/responses/...` references in a kept operation were carried into the extract unresolved -- thirteen dangling references in an artifact whose docstring calls itself "self-contained, valid OpenAPI". cancelRequest's path parameter lives under parameters, which would have been the fourteenth. The closure now follows every component section a kept operation references (and path-level parameters), and the extract refuses to be written with a dangling reference in it. - Its operation list said six while console.cpp has called GET /v1/models and GET /v1/models/catalog since #75 (FetchModels, FetchCatalog) and the committed artifact carried both: the artifact and the extractor had drifted, which a hash pin between the artifact and the HEADER cannot see. listModels and getModelCatalog are listed now; the extract is nine operations and reproduces the committed paths plus the new one. The source contract is a draft branch's, not main's: the operation is not deployed anywhere yet, and this commit says so where it is consumed. python3 contracts/extract-cli-contract.py <#475's contract>: 9 operations, 61 components python3 contracts/generate_console_binding.py --check: fresh ctest: 12/12
… transports honour (#81) The shim's abandon path (next commit) needs to tell the control plane to end a request, fire-and-forget: POST /v1/requests/{request_id}/cancel with the session's bearer, through the vendored binding rather than a hand-built call (AGENTS.md, the P0 contract-first rule). ConsoleClient::CancelRequest does that the way FetchUsage does -- the id escaped into the path, Send and ParseContract, HttpError on anything unexpected -- and maps the contract's three answers: 202 Cancelled (a node ended it; the body is informational), 404 NotFound (unknown, finished, or not this key's -- the server does not say which, by design), anything else Failed. An empty id or token is refused before any call. Fire-and-forget means the caller has already dropped the stream, so the call must never hold an exiting wrapper for the transport's 30 s default. HttpRequest gains timeout_ms (0 keeps today's 10 s connect / 30 s total); both transports honour it -- curl's CURLOPT_TIMEOUT_MS/CONNECTTIMEOUT_MS and WinHTTP's session, request and per-read deadlines -- with the connect phase never given more than its usual share. Every existing initializer stays valid: the field is trailing and defaulted. Guards bite. The contract test drives a mock transport through the four outcomes and the exact request shape (method, escaped path on the dev and production origins, bearer, empty body, timeout_ms). The timeout test stands up a real server that holds the request until released and asserts the real transport gives up within the request's own bound -- neutered (TotalTimeoutMs returning the default), it went red: [FAIL] a_request_timeout_bounds_the_real_transport - timeout_ms was not honoured: the call took 10013 ms restored: [PASS], 14 passed. ctest: 12/12 suites; test_wally_account 14 passed
…81) The Anthropic translator and the JetBrains proxy sit between an editor and the model endpoint. When the editor abandoned a stream (Esc in Claude Code, the app quitting) the endpoint used to see only a TCP close, and only once the next upstream chunk failed to write; during prefill nothing arrives so nothing failed, and the managed edge in front of the endpoint loses the close anyway (InferenceInfra #440 measured 22-41 s of paid decode after the client was gone). Two things fix that, and one call does both: net/upstream_call: PostWatched(lease, WatchedCall) is one upstream POST that watches the reader it streams to. A watch thread polls the server request's is_connection_closed (a peek on the editor's socket; the fd is captured by value and the join at the end of the call bounds its life). The response handler captures x-request-id and the status the moment the headers arrive, before any body byte. On abandon the call hands the id to on_abandoned -- once -- and stops the upstream socket, re-issuing stop() every poll until send() returns (a single stop with no request in flight only disconnects, after which httplib would reconnect and resend). A refusal (status >= 400) ran nothing and is never named as a cancel. The common abandon -- Esc while tokens are flowing -- is noticed by the RECEIVER, not the poll: the next write to the editor fails before the watch gets a turn. A receiver saying no is therefore an abandon too: the cancel is named right there, with the id already in hand, and the request aborted; without that the call would end as a plain Canceled that nobody follows up (found in review). The watch waits on a condition variable the call signals when send() returns, so a completed call is never held for the rest of a poll. on_abandoned also says whether the reader left before the headers (during_prefill), so the log line is true when the id arrived at the first token. The limit, stated in the header: the endpoint's gateway opens the response at the FIRST TOKEN, so the id is unknown during prefill. An abandon during prefill keeps the upstream open until the headers arrive (or id_wait = 120 s runs out, or the wrapper is stopping), cancels at that moment -- which ends the decode, the long part -- and discards everything after. Cancelling inside prefill needs the gateway to name the request earlier: InferenceInfra #440's follow-up, not this change. account/cancel_worker: one worker thread per wrapper sends the cancels off the request path (the abandon fires on the watch thread, inside the response handler or inside the receiver, none of which may block), in order, each bounded by timeout_ms = 3 s. Stop() drains the queue then joins, so the last abandon's cancel goes out before the process does. The bearer is read when each cancel goes OUT, through a supplier: the JetBrains proxy renews its token mid-session (RenewToken on a 401), and a cancel carrying the old one would be refused. The proxy's token now sits behind a small guard (Token()/SetToken()) that the renewal, the sinks and the worker all use. Outcomes go to the translator's log ("cancel id=... result=202|404|failed"), never the editor's terminal; the bearer is never logged. Both translators route every upstream POST through the watched call and veto both of their retries once the editor left: the stop that ended an abandoned call looks exactly like a stale keep-alive to the #80 rule, and re-sending the prompt for a reader that is gone is the waste this exists to end. An abandoned stream ends its sink; an abandoned buffered call answers 499 to nobody. Stop()/StopProxy() run in a fixed order: `stopping` first (an in-flight watch waiting for an id gives up on its next poll instead of holding the server thread until the first token), then the server (joins every handler), then the cancel queue. A local server has no console_url, so nothing is cancelled there: the dropped connection is enough, and the log says "cancel=skipped(local)". harness::Endpoint carries console_url (the session's control plane, base path included) beside base_url; docs/EDITORS.md describes the behaviour and names the two logs (shim.log under the state directory; the proxy's trace under --verbose), and AGENTS.md counts the nine contract operations. Tests. test_wally_net_call (new, 8) drives PostWatched against the fake with the reader as a flag: a completed stream is never abandoned; reader gone mid-body names the cancel and drops the socket within 1 s with nothing more reaching the sink; reader gone during prefill cancels at the headers (Error::Canceled from the handler); an id that never comes gives up after id_wait with an empty id and no cancel; stopping ends the wait within a poll; a 429 is not cancelled; a receiver that refuses the bytes names the cancel (the poll set to 30 s so only that path can); a completed call is not held for a poll (poll 2 s, returns in ms). test_wally_anthropic (+7) and test_wally_ide_proxy (+2) prove the wiring end to end through the real server request: an editor that leaves mid-stream produces exactly one POST /v1/requests/req-N/cancel with the session's bearer within 1 s and no second arrival of the prompt, both with the body withheld (the poll notices) and with the fake dripping a frame every 2 ms (the failed write notices; the drip then stops, which is the upstream socket dropped); a completed stream produces no cancel; a local endpoint is never cancelled; Stop() sends the last cancel before returning (fake answers after 500 ms); leaving during prefill cancels at the first token; stopping during prefill does not wait for the first token. The test editors CLOSE their sockets when they leave (stop, join, destroy), the way Claude Code and a quitting app do; stop() alone keeps the fd open and the shim's writes into it keep succeeding. test_wally_account (+1) drives CancelWorker on the mock transport: in order, the bearer read at send time (changed between two enqueues, the second carries the new one), every outcome reported, Stop() drains. The fake upstream gains per-arrival x-request-id, hold_headers/ release_headers (prefill) beside hold_streams_until (decode), drip(), and the cancel route, which records a cancel only once it is about to answer it -- so a Stop() that did not wait for the worker cannot get credit for a cancel it never sent. Its destructor releases every hold, which took the anthropic suite from 17.5 s to 4.9 s. Guards bite. Each neuter was applied, the suites run, and the change restored (dev/notes/issue-81-breaktests.md): veto removed (an abandoned call falls through to the stale retry): [FAIL] an_abandoned_stream_is_cancelled_by_name_and_never_resent stopping ignored by the watch: [FAIL] stopping_during_prefill_does_not_wait_for_the_first_token - Stop() waited for the first token: 4780 ms [FAIL] stopping_ends_the_wait_for_an_id - ... took 5016 ms a refusal cancelled too (< 400 -> < 1000): [FAIL] a_refusal_is_not_cancelled - a 4xx must not be cancelled: calls=1 reader never polled: 10 FAILs across the three suites, e.g. [FAIL] an_abandoned_stream_is_cancelled_by_name_and_never_resent - no cancel reached the endpoint within 1 s of the editor leaving Stop() without draining the cancel queue: [FAIL] stop_sends_the_last_cancel_before_returning - Stop() returned without sending the abandoned request's cancel a refusing receiver treated as a plain stop (the review's blocker): [FAIL] a_receiver_that_refuses_the_bytes_names_the_cancel - a refused write must be an abandon with the id in hand: abandoned=0 calls=0 [FAIL] leaving_while_tokens_flow_cancels_by_name - no cancel reached the endpoint within 1 s of the editor leaving (8/8 runs; proxy 6/8) the watch sleeping out its poll instead of waking on send(): [FAIL] a_completed_call_is_not_held_for_a_poll - ... took 2004 ms the worker reading the bearer once, at construction: [FAIL] the_cancel_worker_sends_in_order_with_the_current_bearer - the bearer must be read when the cancel goes out: first=old-token second=old-token restored: 8 / 15 / 14 / 4 passed, three runs each; ctest 13/13. The four threaded suites also ran under ThreadSanitizer (-fsanitize=thread, separate build tree, binaries newer than every source): 41 passed, no reports. Not tested against the real endpoint: the cancel route this speaks is InferenceInfra #475 (draft), and cancel-at-first-token against the pinned gateway is asserted from its source, not measured.
…call is never an abandon (#81) Three notes from a cross-read of the draft, all taken: The watch thread was joined only after client.send() returned. A callback throwing out of send() -- the receiver on a translator bug, on_abandoned on an allocation failure -- unwound past a joinable std::thread, whose destructor is std::terminate: the whole wrapper down for one bad chunk. An EndWatch guard now ends and joins the watch on every exit from PostWatched, exceptions included. `done` is an atomic stored the instant send() returns, before any lock, so a poll that lands in the gap between the reply completing and the call's own bookkeeping cannot mistake the reader's close-after-the-last- chunk for an abandon. What remains is httplib's own unwinding, and for the Anthropic shim not even that: a well-behaved editor cannot see message_stop, which is written after PostWatched returns, so it cannot close before send() does. For the JetBrains proxy, which forwards [DONE] byte for byte, the window is httplib's return -- microseconds -- and `a_completed_stream_is_never_abandoned` is proven for the shim and probabilistic for the proxy; the PR says so. The WatchedCall contract now says on_abandoned runs with the call's own lock held and must not block; Enqueue is a push and a notify. Guard: a_throwing_receiver_unwinds_with_the_watch_joined (net_call, 9 now) -- the receiver throws, the exception reaches the caller (httplib does not swallow it), the process is still here and a normal call on a fresh lease works. Neutered (the guard removed, join after send() as before): the suite dies at that test, exit 134 -- SIGABRT, "libc++abi: terminating". Restored: 9 passed. TSAN on net_call, anthropic, ide_proxy: 27 passed, no reports.
AmanSwar
force-pushed
the
issue-81-shim-cancel
branch
from
September 13, 2026 14:37
4e7569a to
ee4235e
Compare
Collaborator
Author
The approach, in plain words (for review without reading the code)The problem. When you press Esc in Claude Code (or quit the app), wally only closed its connection to the cloud. The cloud did not notice for up to 40 seconds and kept generating — and billing. Worse, during a long prefill nothing is flowing, so wally itself did not even notice you had left until the next chunk failed to deliver. What this PR does. wally now cancels by name. Four pieces:
Picture ordering food by phone: before, hanging up was the only way to cancel, and the kitchen kept cooking; now you say "cancel order #123" — and you can only do that once the kitchen has told you your order number. Why this way (and what we did not do). A cancel by name is the only signal that survives the load balancer. We did not add a thread per abandon (one worker per wrapper). The limit is stated plainly: the cloud names the request only at the first token, so an abandon during prefill is cancelled at the first token — that ends the decode, the long part — not inside prefill; the ask for an earlier id is filed on #440. How you can tell it works.
What it does not do / still needs.
|
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.
Closes #81. The client half of RunanywhereAI/InferenceInfra#440; the server half is RunanywhereAI/InferenceInfra#475 (draft). Stacked on #82 (
issue-80-shim-keepalive), which it builds on — the pool, the lease, the stale-retry rule — so the base is that branch and this PR retargetsmainwhen #82 lands. See "For the owner" for Sanchit's requested merge order with #103/#104.What changed, and why
When the editor abandoned a stream — Esc in Claude Code, the app quitting — the endpoint saw only a TCP close, and only once the next upstream chunk failed to write. During prefill nothing arrives, so nothing failed; and the managed edge in front of the endpoint loses the close anyway (#440 measured 22–41 s of paid decode after the client was gone). Two things fix that, and one call does both.
src/net/upstream_call.{h,cpp}—PostWatched: one upstream POST that watches the reader it streams to.is_connection_closedevery 100 ms — httplib's peek on the editor's socket, on an fd captured by value; the join at the end of the call bounds its life. The common abandon (Esc while tokens are flowing) is noticed even sooner, by the failed write to the editor: the receiver'sfalseis treated as an abandon too, not as a plain stop nobody follows up. That second path was missing in the first cut and found by the code review — it is the path most real abandons take.x-request-idthe moment the headers arrive (response_handler, before any body byte) — the name the cancel route wants — and on abandon hands that id toon_abandoned, exactly once, then stops the upstream socket.stop()is re-issued every poll untilsend()returns, because a single stop that lands with no request in flight only disconnects, after which httplib would reconnect and re-send the prompt.src/account/cancel_worker.{h,cpp}— one worker thread per wrapper sends the cancels off the request path, in order, each bounded by 3 s.Stop()drains the queue then joins, so the last abandon's cancel goes out before the process does. The bearer is read when each cancel goes out (the JetBrains proxy renews its token mid-session; a cancel carrying the old one would be refused). Outcomes are logged —abandoned during=stream id=… cancel=queued, thencancel id=… result=202|404|failed— toshim.logunder the state directory for the translator and to the proxy's--verbosetrace, never to the editor's terminal. The bearer is never logged.Contract first.
cancelRequestis vendored from InferenceInfra'scontrol-plane-v1.openapi.json(the #475 branch) intocontracts/wally-cli-v1.openapi.json;src/account/console_contract.his regenerated;ConsoleClient::CancelRequestbuilds nothing by hand beyond the path template. The extractor (contracts/extract-cli-contract.py) now follows every component section its operations reference, refuses dangling refs, and carrieslistModels/getModelCatalog, whichconsole.cppalready called but the artifact had drifted from.HttpRequestgains a per-requesttimeout_msthat both transports (curl, WinHTTP) honour — the cancel's 3 s bound — where before the 30 s default was the only choice.Both translators route every upstream POST through the watched call:
src/anthropic/messages.cpp(Claude Code / Claude Desktop) andsrc/ide/openai_proxy.cpp(JetBrains).Stop()/StopProxy()run in a fixed order:stoppingfirst (a watch still waiting for an id gives up on its next poll instead of holding a server thread until the first token), then the server (joins every handler), then the cancel queue. A local server has no console to tell: the log sayscancel=skipped(local)and the dropped connection is enough.The limit, stated
The gateway opens the response at the first token, so the id is unknown during prefill. An abandon during prefill keeps the upstream open until the first token arrives (or 120 s pass, or the wrapper is stopping), cancels at that moment — which ends the decode, the 22–41 s the issue measures — and discards everything after. Cancelling inside prefill needs the gateway to name the request earlier; that ask is filed on InferenceInfra #440 (comment of 2026-09-13), not solved here. #81's "including when abandoned during prefill" is therefore met as cancel-at-first-token, not cancel-during-prefill, and the live round should measure both.
Proof
Every test observes behaviour from the fake upstream's side — its cancel route, its arrival count, whether its drip stopped — through the real httplib server request and sink, never source text. Each guard was neutered and watched go red, then restored (
dev/notes/issue-81-breaktests.mdhas the full record; the commit message quotes it).an_abandoned_stream_is_cancelled_by_name_and_never_resent(shim;proxy_…in the proxy suite)POST /v1/requests/req-2/cancelwith the session's bearer within 1 s, warmed lease not retriedleaving_while_tokens_flow_cancels_by_name(shim and proxy)falsetreated as a plain stop → 8/8 runs red (proxy 6/8)a_receiver_that_refuses_the_bytes_names_the_cancel(net_call)abandoned=0 calls=0leaving_during_prefill_cancels_at_the_first_tokenstopping_during_prefill_does_not_wait_for_the_first_token/stopping_ends_the_wait_for_an_idStop()returns within 1.5 s while the id is still unknownstoppingignored →4780 ms/5016 msstop_sends_the_last_cancel_before_returningStop()waits for the queued cancel (fake answers after 500 ms)Stop() returned without sending the abandoned request's cancela_refusal_is_not_cancelled< 400→< 1000→calls=1a_completed_stream_is_not_cancelled,a_local_endpoint_is_never_cancelled,a_completed_stream_is_never_abandoned,an_id_that_never_comes_gives_up_after_the_waita_completed_call_is_not_held_for_a_poll2004 msthe_cancel_worker_sends_in_order_with_the_current_bearer(account)Stop()drainssecond=old-tokencancel_request_speaks_the_contract,a_request_timeout_bounds_the_real_transporttimeout_mshonoured by the real transport10013 msOne finding worth recording for #82's reviewer too: #82's body says an "editor abandons the stream" test was tried and removed because "the translator's writes to the closed reader kept succeeding". That was
httplib::Client::stop(), which only shuts the socket down and keeps the fd until the client is destroyed; a real editor closes (Claude Code aborts the fetch, a quitting app closes everything), and the next write fails. The test editors here stop, join and destroy, and the abort path is observable.The four threaded suites (net_call, anthropic, ide_proxy, account) ran under ThreadSanitizer in a separate build tree on the final sources: 41/41, no reports. The fake's destructor now releases its holds, which took
ctestfrom 44.6 s to 15.8 s.After a teammate's cross-read (commit 4,
4e7569a)Three notes, all taken: the watch thread is now joined by an RAII guard on every exit from
PostWatched— a callback throwing out ofsend()used to unwind past a joinablestd::thread, i.e.std::terminate(guard:a_throwing_receiver_unwinds_with_the_watch_joined; neutered, the suite dies with exit 134 /libc++abi: terminating);doneis an atomic stored the instantsend()returns, so a poll landing in the gap cannot mistake a completed call for an abandon — for the Anthropic shim the race is unreachable for a well-behaved editor (it cannot seemessage_stop, written afterPostWatchedreturns), for the JetBrains proxy ([DONE]forwarded byte for byte) the window is httplib's own return, soa_completed_stream_is_never_abandonedis proven for the shim and probabilistic for the proxy; and theWatchedCallcontract sayson_abandonedruns with the call's lock held and must not block. TSAN on the three threaded suites after the change: 27 passed, no reports.Review
Fresh-context review (Fable 5.1) of the plan before implementation and of the code after: the code review's one blocker — the receiver-refusal path above — is fixed with the guards in the table; its should-fixes (the poll-hold on every call, the stale bearer after renewal, the log paths in
docs/EDITORS.md, TSAN re-run on the final tree) are all in. Every cpp-httplib claim the design rests on was verified by the reviewer against the vendored 0.46.1 header (is_connection_closedcaptures the fd by value;Client::stop()from another thread only shuts the socket down while a request is in flight;response_handlerreturning false isError::Canceled;Server::stop()+ join runs the thread pool's shutdown, so no handler is alive when the cancel queue is drained).For the owner (not taken silently)
git merge-treeagainstcodex/issue84-stream-terminal: 9 conflict hunks / 275 lines insrc/anthropic/messages.cpp, 2 / 61 intests/test_wally_anthropic.cpp— a real rebase ofHandleStreaming, not a mechanical one. This PR is opened againstissue-80-shim-keepaliveas the plan on the issue said; I have not rebased it onto Preserve overload status and Retry-After before Anthropic streaming #103/Reject incomplete upstream streams before Anthropic success #104 and will not until you say which base you want.id_wait(an abandon during a prefill longer than that loses the cancel and just drops the socket — today's behaviour); 3 s per cancel, andStop()waits that long per queued cancel (typically one).wally … --serveis ended by Ctrl-C and never callsStop()(pre-existing, by design incmd_editors.cpp), so a cancel queued in the last moments of a--servesession is lost; the launch paths (claude-code,claude-desktop,clion,rustrover) drain. Documented indocs/EDITORS.md./api/v1relay a cancel would 404); andExpect: 100-continueabove 1 KiB on Reuse upstream connections in the Anthropic shim and the JetBrains proxy #82's live measurement.Validation
No CI on this PR yet:
ci.ymlruns pull-request checks only for basesmainandlaunch/**, so a PR stacked onissue-80-shim-keepalive(this one, and #103/#104 alike) reports nothing until it is retargeted tomain. The runs above are local (macOS arm64); Windows is not compiled here — the WinHTTPtimeout_mspath is code-read only.Docs:
docs/EDITORS.md(what happens when the tool stops listening; where the log is),AGENTS.md(the nine contract operations).