[PR #158] feat!: subscribe's timeout detaches from an in-flight run instead of erroring #158

New issue

Open

opened 2026-10-31 22:01:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:01:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-python-sdk/pull/158
Author: @mattmillerai
Created: 9/16/2026
Status: 🔄 Open

Base: main ← Head: matt/be-14561-subscribe-timeout-detach

📝 Commits (6)

b6f253e feat!: subscribe's timeout detaches from an in-flight run instead of erroring
21a7b0b Merge origin/main into matt/be-14561-subscribe-timeout-detach
c76ebd4 fix(models): read the cancel's answer before calling a subscribe timeout cancelled
e077856 fix(models): read the cancel route's 202 CANCELLATION_REQUESTED per contract (#172)
2ba9e5d Merge origin/main into matt/be-14561-subscribe-timeout-detach
e3aadfd fix(tests): reconcile the credits_used tripwire now that the spec declares it

📊 Changes

9 files changed (+2024 additions, -94 deletions)

View changed files

📝 CHANGELOG.md (+70 -22)
📝 README.md (+83 -6)
📝 src/comfy_sdk/__init__.py (+16 -1)
📝 src/comfy_sdk/model_requests.py (+698 -10)
📝 src/comfy_sdk/models.py (+120 -25)
📝 tests/conftest.py (+64 -15)
📝 tests/test_models_queue.py (+930 -3)
📝 tests/test_router_spec_contract.py (+19 -9)
📝 tests/test_sync_async_parity.py (+24 -3)

📄 Description

ELI-5

When models.subscribe(..., timeout=N) runs out of time it quietly asks the server to cancel the run. The queue only accepts that cancel while the request is still waiting in line — once it has been dispatched, the run is served to the end and billed whatever you do, and the cancel is refused. Until now the SDK swallowed that refusal along with every other cancel failure and raised a bare TimeoutError, which reads as "the run stopped" — the one thing that did not happen. Now a timeout on an in-flight run is reported as what it is: a detach. You stop waiting, the generation carries on, and you get back the ids and a live handle to collect the thing you are being billed for.

What changed

models.subscribe(timeout=N) now has three endings, told apart by type and never by a message string:

ending	what happened	how it surfaces
cancelled	the request was still queued, the cancel was accepted, nothing ran and nothing is billed	raises `SubscribeTimeout` (a `TimeoutError` subclass) with `.cancelled` `True`
detached	the cancel did not stop the run — it was refused because the run was in flight, or it was accepted while reporting a live status — and a confirming poll found it still going. It continues, completes and is billed	returns a `DetachedRequest` / `AsyncDetachedRequest` carrying `.request_id`, `.model`, `.status` and a live `.handle`
completed during teardown	the same refusal, but the run had just finished. The result exists and is paid for	collects it and returns it like any other result

A cancel that fails for any other reason — a transport failure, a 401, a 500 — is no longer swallowed. SubscribeTimeout is raised with .cancelled False and the cancel's own failure on .cancel_error and __cause__, because a cancel that never landed means the run may still be going. Before this change all three of those looked identical to a successful cancellation.

Sync and async, with the same names and the same arguments; the async side returns AsyncDetachedRequest so the handle it carries is the awaitable one.

New public names, all exported from comfy_sdk: DetachedRequest, AsyncDetachedRequest, SubscribeTimeout.

How the refusal is recognised

One predicate, _refused_on_state, and it asks the typed question first. Rebasing onto main picked up #157, which types the cancel route's refusals as CancelRefused / AlreadyCompleted — so the "branching on a typed class is strictly better than branching on a status" this originally deferred is now simply what it does:

if isinstance(exc, CancelRefused):
    return True
if getattr(exc, "http_status", None) != _CANCEL_REFUSED_STATUS:
    return False
bucket = error_bucket_of(exc)
return bucket is None or bucket == _UNIDENTIFIED_REFUSAL_CODE

The status clause is the fallback for the one refusal the contract has not named yet — the in-flight one, which today carries no bucket on the header or in the body. It fails closed: a 409 that names a bucket the contract does document (invalid_input, concurrency_limit_exceeded with its Retry-After) is not the state refusal and surfaces on .cancel_error rather than being reported as a detach. That matches two nearby precedents — retry.is_collectable gates 409 on status and bucket, and comfy_low.errors._CODE_BY_STATUS omits 409 outright as ambiguous.

"Names no bucket" has two spellings, because the layer below fills the gap rather than leaving it: comfy_low.errors synthesises code = f"http_{http_status}" for a response that identified itself with nothing, and documents that shape as unable to collide with a wire code or a Router bucket. So a bucket-less 409 arrives carrying http_409, not None, and both are read as "named nothing".

No body-text match anywhere.

How an ACCEPTED cancel is read

A 2xx says the route took the message, not that the run stopped — the binding accepts 200, 202 and 204 alike. The cleanup cancel now hands back the QueueUpdate it was answered with, and that body decides:

the cancel answered	reading
a terminal `COMPLETED` with an `error_type`	stopped. This queue has no `CANCELED` status; it expresses a stop as `COMPLETED` plus a bucket
a terminal `COMPLETED` without a bucket	it finished on its own, and is billed — so the result is collected, not discarded
any live status (`202`/`CANCELING`, or a `200` on a request that won the race into flight)	not stopped — confirmed against the server and reported as a detach
no body at all (the ordinary `204`)	stopped, unconfirmed. Nothing in an empty body contradicts the accept, and charging every timeout an extra round trip to re-confirm the common case is the wrong trade

Because the refusal names nothing about the request's state, the detach is confirmed against the server: one unretried, 10s-bounded status poll, which is what separates "still running" from "finished while we were tearing down". That follows the rule the surface already states — the authoritative state of a request is the next status read, never what a cancel answered. A poll that itself fails does not undo what the refusal established (the request was not cancelled), so it still reports a detach, with status left empty to say the state was unconfirmed.

Compatibility

SubscribeTimeout subclasses TimeoutError, so every existing except TimeoutError around subscribe keeps catching the cases that still raise, with the same message plus a clause naming the outcome.
Breaking: models.subscribe is now annotated dict[str, Any] | DetachedRequest. A typed caller that indexes the result directly has to check the type first. This is the behaviour change the detach is — the alternative is telling a caller their run was cancelled when it was not — and the queued surface shipped two days ago behind a server-side gate, so the blast radius is small. Recorded under ### Changed in the changelog.
SubscribeTimeout pickles and copies with every field intact (__reduce__), so .request_id survives reaching another process — which is the workflow the queued surface exists for.
timeout bounds the wait, not the teardown. Deciding which of the three endings happened costs up to three further round trips past the deadline (the cancel, the confirming poll, and on the completed path the result fetch), each bounded at 10s — so a subscribe(timeout=N) can return up to ~30s after N in the worst case. Now stated on the method and in the README; it was previously ~10s and undocumented.
handle.cancel(), handle.get() and handle.iter_events() are untouched. iter_events still never cancels — a for loop with a break must not be destructive.

Verification of the negative claim

This diff's user-facing outcome does not deny a capability, and the empirical check says so rather than the prose: the cleanup cancel is still attempted on every timeout via exactly the path it used before, and the tests assert queue_cancel_count == 1 on each of the three endings. Nothing that could be cancelled before is refused here — a cancel the queue accepts still cancels, and handle.cancel() is unchanged. What the diff removes is a raise, not a capability: the previously-dead-ended timeout now hands back a working handle, and the re-attach path it points at is exercised (models.handle(model, request_id).get() returns the completed result, sync and async).

Judgment calls

Detach returns, cancelled raises. Mixing a return and a raise across one method's outcomes is unusual, and it is the honest split: a detached run is healthy and collectable (there is something to hand back), while a cancelled one is gone (there is not). It is also what makes the two impossible to confuse without reading a message.
Cancelled now raises SubscribeTimeout rather than a bare TimeoutError. A subclass, so no handler breaks; it is what carries .request_id, .model and .cancelled for the outcomes that still raise.
A non-benign cancel failure is chained onto the timeout rather than replacing it. The existing code's reasoning still holds — the timeout is the failure worth reporting, and masking it sends the caller looking in the wrong place — but it is no longer silent: the failure rides out on .cancel_error and __cause__.
Completed-during-teardown collects the result rather than raising, overrunning the caller's deadline by one bounded fetch. Discarding a generation that has already been generated and billed is the worse trade.
One test-guard exemption: the sync/async parity walk fails a discovered pair with zero public methods as "introspection broke?". DetachedRequest/AsyncDetachedRequest are frozen dataclasses whose whole surface is fields, so the exemption is declared in a new _DATA_ONLY table with its reason, alongside the file's existing _RENAMES / _ALLOWED_ASYMMETRY / _SYNC_ON_BOTH tables, and it is itself checked for staleness (it fails if the pair ever grows a method the exemption would then hide).

Residual

The in-flight refusal's wire shape is still not pinned by any contract, and the status clause remains a guess. The typed half is now real (CancelRefused / AlreadyCompleted, from #157), but the in-flight refusal has not shipped server side; its 409 is the documented state-conflict status the cancel route already uses for cancel-after-completion, not a value read off the new refusal. If the server ships it with a different status, every timeout raises as it does today — no regression, but the detach never fires — and if it ships a named bucket, that bucket becomes a CancelRefused subclass and the status clause can be deleted outright. _refused_on_state in src/comfy_sdk/model_requests.py is the single predicate to reconcile when the server PR lands. This is the follow-up: re-pin the predicate against the shipped refusal.
A confirming poll that reports the request as still QUEUED is reported as a detach anyway, which claims a run that may never have started is running and billed. Distinguishing it needs a status vocabulary the contract does not pin (QueueUpdate.status is a deliberately open string; COMPLETED is the only value the SDK names) and a behaviour decision about what subscribe should do instead. Raised in review, deferred to a follow-up alongside the re-pin above, and tracked on that thread.
The cancel endpoint the report names — PUT /v2/models/{provider}/{model}/requests/{id}/cancel — was exercised only against this repo's in-tree stub, never a live deployment. The test suite has no network dependency by design and this environment has no credentials for the router; the refusal is therefore a stub the SDK was written against, not a recorded response.
The production request the report cites as having produced this path was not examined. It lives on an internal deployment this environment cannot reach, and its identifiers are internal, so it is named here only as an unexercised artifact. The same applies to the linked decision issue and the two linked end-to-end coverage issues, whose bodies were not available to this change.
One of the three cancel-swallow sites on this surface is deliberately left as it was. Sweeping src/ for places that suppress a cleanup cancel's failure finds three: the sync subscribe timeout and the async subscribe timeout (both fixed here), and the async subscribe's asyncio.CancelledError path (src/comfy_sdk/models.py), which still swallows every cancel failure alike, refusal included. It cannot report a detach — the caller is receiving no value from that call, and replacing their CancelledError would break the cancellation they asked for — so a task cancelled from outside still loses the request id of a run that may be billing. Reaching it needs submit rather than subscribe. Worth its own decision; a comment in the code now says so. The workflow-jobs surface has no equivalent site (Comfy.run does not cancel on timeout), so the count there is zero.
Out of scope, per the ticket: the server-side refusal itself, the TypeScript and Go SDKs (which carry the same default and will need matching changes once the wire shape is settled), and any change to the default timeout or to making cancel-on-timeout opt-out.

Provenance

Authored by: agent-work loop
Verified: uv run --extra dev pytest -q: 1004 passed, 9 skipped, 0 failed (12 new tests this round); ruff check .: all checks passed; ruff format --check .: 57 files already formatted; mypy src: no issues in 21 source files; python3 scripts/check_public_repo_hygiene.py: no internal-only references; uv run --extra codegen python scripts/check_drift.py: models in sync, all 18 router error types covered, run route matches the spec. All re-run on the merged tree (main merged in at 21a7b0b), not on the branch alone.
Deviations: the ticket's first acceptance criterion asks that the detached timeout "returns normally, does not raise" — met — while its second asks that an accepted cancel "keeps today's cancelled semantics"; that one now raises SubscribeTimeout rather than a bare TimeoutError. The subclass keeps every existing except TimeoutError working and is what carries the request id, but it is a narrowing of the exact class raised. The review round narrowed it further: an accepted cancel is no longer assumed to have stopped the run, so a 2xx answering a live status now returns a DetachedRequest where it previously raised with cancelled=True — closer to the criterion's intent (only a real cancellation reports one), but a second change to the exact outcome. No criterion was skipped; see ## Residual for what could not be exercised.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-python-sdk/pull/158 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `matt/be-14561-subscribe-timeout-detach` --- ### 📝 Commits (6) - [`b6f253e`](https://github.com/Comfy-Org/comfy-python-sdk/commit/b6f253e8a9492d2c1d73168910dba002182d4ec1) feat!: subscribe's timeout detaches from an in-flight run instead of erroring - [`21a7b0b`](https://github.com/Comfy-Org/comfy-python-sdk/commit/21a7b0b9932e3ea551cc92d241f7dcb3e9e5f136) Merge origin/main into matt/be-14561-subscribe-timeout-detach - [`c76ebd4`](https://github.com/Comfy-Org/comfy-python-sdk/commit/c76ebd494b02bee1404a40fbd4bf9b7212d5eca5) fix(models): read the cancel's answer before calling a subscribe timeout cancelled - [`e077856`](https://github.com/Comfy-Org/comfy-python-sdk/commit/e07785691c90bff4b4beec776b252877130dec1b) fix(models): read the cancel route's 202 CANCELLATION_REQUESTED per contract (#172) - [`2ba9e5d`](https://github.com/Comfy-Org/comfy-python-sdk/commit/2ba9e5d41c7eafeb0ab1053571e68f05736ae63d) Merge origin/main into matt/be-14561-subscribe-timeout-detach - [`e3aadfd`](https://github.com/Comfy-Org/comfy-python-sdk/commit/e3aadfd1daa6004d0e5a6588b8f2cd0d168bbeed) fix(tests): reconcile the credits_used tripwire now that the spec declares it ### 📊 Changes **9 files changed** (+2024 additions, -94 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+70 -22) 📝 `README.md` (+83 -6) 📝 `src/comfy_sdk/__init__.py` (+16 -1) 📝 `src/comfy_sdk/model_requests.py` (+698 -10) 📝 `src/comfy_sdk/models.py` (+120 -25) 📝 `tests/conftest.py` (+64 -15) 📝 `tests/test_models_queue.py` (+930 -3) 📝 `tests/test_router_spec_contract.py` (+19 -9) 📝 `tests/test_sync_async_parity.py` (+24 -3) </details> ### 📄 Description ## ELI-5 When `models.subscribe(..., timeout=N)` runs out of time it quietly asks the server to cancel the run. The queue only accepts that cancel while the request is still *waiting in line* — once it has been dispatched, the run is served to the end and billed whatever you do, and the cancel is refused. Until now the SDK swallowed that refusal along with every other cancel failure and raised a bare `TimeoutError`, which reads as "the run stopped" — the one thing that did not happen. Now a timeout on an in-flight run is reported as what it is: a **detach**. You stop waiting, the generation carries on, and you get back the ids and a live handle to collect the thing you are being billed for. ## What changed `models.subscribe(timeout=N)` now has three endings, told apart by **type** and never by a message string: | ending | what happened | how it surfaces | |---|---|---| | **cancelled** | the request was still queued, the cancel was accepted, nothing ran and nothing is billed | raises `SubscribeTimeout` (a `TimeoutError` subclass) with `.cancelled` `True` | | **detached** | the cancel did not stop the run — it was refused because the run was in flight, or it was *accepted* while reporting a live status — and a confirming poll found it still going. It continues, completes and **is billed** | **returns** a `DetachedRequest` / `AsyncDetachedRequest` carrying `.request_id`, `.model`, `.status` and a live `.handle` | | **completed during teardown** | the same refusal, but the run had just finished. The result exists and is paid for | collects it and returns it like any other result | A cancel that fails for any **other** reason — a transport failure, a `401`, a `500` — is no longer swallowed. `SubscribeTimeout` is raised with `.cancelled` `False` and the cancel's own failure on `.cancel_error` and `__cause__`, because a cancel that never landed means the run may still be going. Before this change all three of those looked identical to a successful cancellation. Sync and async, with the same names and the same arguments; the async side returns `AsyncDetachedRequest` so the handle it carries is the awaitable one. New public names, all exported from `comfy_sdk`: `DetachedRequest`, `AsyncDetachedRequest`, `SubscribeTimeout`. ## How the refusal is recognised One predicate, `_refused_on_state`, and it asks the **typed** question first. Rebasing onto `main` picked up #157, which types the cancel route's refusals as `CancelRefused` / `AlreadyCompleted` — so the "branching on a typed class is strictly better than branching on a status" this originally deferred is now simply what it does: ```python if isinstance(exc, CancelRefused): return True if getattr(exc, "http_status", None) != _CANCEL_REFUSED_STATUS: return False bucket = error_bucket_of(exc) return bucket is None or bucket == _UNIDENTIFIED_REFUSAL_CODE ``` The status clause is the fallback for the one refusal the contract has **not** named yet — the in-flight one, which today carries no bucket on the header or in the body. It **fails closed**: a `409` that names a bucket the contract *does* document (`invalid_input`, `concurrency_limit_exceeded` with its `Retry-After`) is not the state refusal and surfaces on `.cancel_error` rather than being reported as a detach. That matches two nearby precedents — `retry.is_collectable` gates `409` on status *and* bucket, and `comfy_low.errors._CODE_BY_STATUS` omits `409` outright as ambiguous. "Names no bucket" has two spellings, because the layer below fills the gap rather than leaving it: `comfy_low.errors` synthesises `code = f"http_{http_status}"` for a response that identified itself with nothing, and documents that shape as unable to collide with a wire `code` or a Router bucket. So a bucket-less `409` arrives carrying `http_409`, not `None`, and both are read as "named nothing". No body-text match anywhere. ## How an ACCEPTED cancel is read A 2xx says the route took the message, not that the run stopped — the binding accepts `200`, `202` and `204` alike. The cleanup cancel now hands back the `QueueUpdate` it was answered with, and that body decides: | the cancel answered | reading | |---|---| | a terminal `COMPLETED` **with** an `error_type` | stopped. This queue has no `CANCELED` status; it expresses a stop as `COMPLETED` plus a bucket | | a terminal `COMPLETED` **without** a bucket | it finished on its own, and is billed — so the result is collected, not discarded | | any live status (`202`/`CANCELING`, or a `200` on a request that won the race into flight) | **not stopped** — confirmed against the server and reported as a detach | | no body at all (the ordinary `204`) | stopped, unconfirmed. Nothing in an empty body contradicts the accept, and charging every timeout an extra round trip to re-confirm the common case is the wrong trade | Because the refusal names nothing about the request's state, the detach is **confirmed against the server**: one unretried, 10s-bounded status poll, which is what separates "still running" from "finished while we were tearing down". That follows the rule the surface already states — the authoritative state of a request is the next status read, never what a cancel answered. A poll that itself fails does not undo what the refusal established (the request was *not* cancelled), so it still reports a detach, with `status` left empty to say the state was unconfirmed. ## Compatibility - `SubscribeTimeout` subclasses `TimeoutError`, so every existing `except TimeoutError` around `subscribe` keeps catching the cases that still raise, with the same message plus a clause naming the outcome. - **Breaking:** `models.subscribe` is now annotated `dict[str, Any] | DetachedRequest`. A typed caller that indexes the result directly has to check the type first. This is the behaviour change the detach *is* — the alternative is telling a caller their run was cancelled when it was not — and the queued surface shipped two days ago behind a server-side gate, so the blast radius is small. Recorded under `### Changed` in the changelog. - `SubscribeTimeout` pickles and copies with every field intact (`__reduce__`), so `.request_id` survives reaching another process — which is the workflow the queued surface exists for. - **`timeout` bounds the wait, not the teardown.** Deciding which of the three endings happened costs up to three further round trips past the deadline (the cancel, the confirming poll, and on the completed path the result fetch), each bounded at 10s — so a `subscribe(timeout=N)` can return up to ~30s after `N` in the worst case. Now stated on the method and in the README; it was previously ~10s and undocumented. - `handle.cancel()`, `handle.get()` and `handle.iter_events()` are untouched. `iter_events` still never cancels — a `for` loop with a `break` must not be destructive. ## Verification of the negative claim This diff's user-facing outcome does **not** deny a capability, and the empirical check says so rather than the prose: the cleanup cancel is still attempted on **every** timeout via exactly the path it used before, and the tests assert `queue_cancel_count == 1` on each of the three endings. Nothing that could be cancelled before is refused here — a cancel the queue accepts still cancels, and `handle.cancel()` is unchanged. What the diff removes is a *raise*, not a capability: the previously-dead-ended timeout now hands back a working handle, and the re-attach path it points at is exercised (`models.handle(model, request_id).get()` returns the completed result, sync and async). ## Judgment calls - **Detach returns, cancelled raises.** Mixing a return and a raise across one method's outcomes is unusual, and it is the honest split: a detached run is healthy and collectable (there is something to hand back), while a cancelled one is gone (there is not). It is also what makes the two impossible to confuse without reading a message. - **Cancelled now raises `SubscribeTimeout` rather than a bare `TimeoutError`.** A subclass, so no handler breaks; it is what carries `.request_id`, `.model` and `.cancelled` for the outcomes that still raise. - **A non-benign cancel failure is chained onto the timeout rather than replacing it.** The existing code's reasoning still holds — the timeout is the failure worth reporting, and masking it sends the caller looking in the wrong place — but it is no longer *silent*: the failure rides out on `.cancel_error` and `__cause__`. - **Completed-during-teardown collects the result** rather than raising, overrunning the caller's deadline by one bounded fetch. Discarding a generation that has already been generated and billed is the worse trade. - One test-guard exemption: the sync/async parity walk fails a discovered pair with zero public methods as "introspection broke?". `DetachedRequest`/`AsyncDetachedRequest` are frozen dataclasses whose whole surface is fields, so the exemption is declared in a new `_DATA_ONLY` table with its reason, alongside the file's existing `_RENAMES` / `_ALLOWED_ASYMMETRY` / `_SYNC_ON_BOTH` tables, and it is itself checked for staleness (it fails if the pair ever grows a method the exemption would then hide). ## Residual - **The in-flight refusal's wire shape is still not pinned by any contract, and the status clause remains a guess.** The typed half is now real (`CancelRefused` / `AlreadyCompleted`, from #157), but the *in-flight* refusal has not shipped server side; its `409` is the documented state-conflict status the cancel route already uses for cancel-after-completion, not a value read off the new refusal. If the server ships it with a different status, every timeout raises as it does today — no regression, but the detach never fires — and if it ships a named bucket, that bucket becomes a `CancelRefused` subclass and the status clause can be deleted outright. `_refused_on_state` in `src/comfy_sdk/model_requests.py` is the single predicate to reconcile when the server PR lands. **This is the follow-up: re-pin the predicate against the shipped refusal.** - **A confirming poll that reports the request as still QUEUED is reported as a detach anyway**, which claims a run that may never have started is running and billed. Distinguishing it needs a status vocabulary the contract does not pin (`QueueUpdate.status` is a deliberately open string; `COMPLETED` is the only value the SDK names) *and* a behaviour decision about what `subscribe` should do instead. Raised in review, deferred to a follow-up alongside the re-pin above, and tracked on that thread. - **The cancel endpoint the report names — `PUT /v2/models/{provider}/{model}/requests/{id}/cancel` — was exercised only against this repo's in-tree stub, never a live deployment.** The test suite has no network dependency by design and this environment has no credentials for the router; the refusal is therefore a stub the SDK was written against, not a recorded response. - **The production request the report cites as having produced this path was not examined.** It lives on an internal deployment this environment cannot reach, and its identifiers are internal, so it is named here only as an unexercised artifact. The same applies to the linked decision issue and the two linked end-to-end coverage issues, whose bodies were not available to this change. - **One of the three cancel-swallow sites on this surface is deliberately left as it was.** Sweeping `src/` for places that suppress a cleanup cancel's failure finds three: the sync `subscribe` timeout and the async `subscribe` timeout (both fixed here), and the async `subscribe`'s `asyncio.CancelledError` path (`src/comfy_sdk/models.py`), which still swallows every cancel failure alike, refusal included. It cannot report a detach — the caller is receiving no value from that call, and replacing their `CancelledError` would break the cancellation they asked for — so a task cancelled from outside still loses the request id of a run that may be billing. Reaching it needs `submit` rather than `subscribe`. Worth its own decision; a comment in the code now says so. The workflow-jobs surface has no equivalent site (`Comfy.run` does not cancel on timeout), so the count there is zero. - **Out of scope, per the ticket:** the server-side refusal itself, the TypeScript and Go SDKs (which carry the same default and will need matching changes once the wire shape is settled), and any change to the default `timeout` or to making cancel-on-timeout opt-out. ## Provenance - **Authored by:** agent-work loop - **Verified:** `uv run --extra dev pytest -q`: 1004 passed, 9 skipped, 0 failed (12 new tests this round); `ruff check .`: all checks passed; `ruff format --check .`: 57 files already formatted; `mypy src`: no issues in 21 source files; `python3 scripts/check_public_repo_hygiene.py`: no internal-only references; `uv run --extra codegen python scripts/check_drift.py`: models in sync, all 18 router error types covered, run route matches the spec. All re-run on the merged tree (`main` merged in at `21a7b0b`), not on the branch alone. - **Deviations:** the ticket's first acceptance criterion asks that the detached timeout "returns normally, does not raise" — met — while its second asks that an accepted cancel "keeps today's cancelled semantics"; that one now raises `SubscribeTimeout` rather than a bare `TimeoutError`. The subclass keeps every existing `except TimeoutError` working and is what carries the request id, but it is a narrowing of the exact class raised. The review round narrowed it further: an accepted cancel is no longer *assumed* to have stopped the run, so a 2xx answering a live status now returns a `DetachedRequest` where it previously raised with `cancelled=True` — closer to the criterion's intent (only a real cancellation reports one), but a second change to the exact outcome. No criterion was skipped; see `## Residual` for what could not be exercised. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 22:01:52 +00:00

bsakaguchi referenced this issue

2026-10-31 22:02:01 +00:00

[PR #172] [MERGED] fix(models): read the cancel route's 202 CANCELLATION_REQUESTED per contract #172