[PR #77] [MERGED] feat(router): add client.models.submit / subscribe / handle for queued delivery #78

Closed
opened 2026-10-26 22:34:42 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-swift-sdk/pull/77
Author: @mattmillerai
Created: 9/16/2026
Status: ✅ Merged
Merged: 9/18/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-14639-router-queue-surface


📝 Commits (5)

  • 18d2b71 feat(router): add client.models.submit / subscribe / handle for queued delivery
  • febb80b fix(router): guard the acknowledgement status, keep the failure signal, and stop wasting the last poll pause
  • 1698b1f Merge origin/main into matt/be-14639-router-queue-surface
  • 7cc5079 fix(router): retry transient status polls and give the result fetch a download-sized budget
  • 9bef73d Merge branch 'main' into matt/be-14639-router-queue-surface

📊 Changes

11 files changed (+3363 additions, -30 deletions)

View changed files

📝 README.md (+87 -0)
📝 Scripts/contract/check_router_contract.py (+160 -2)
📝 Sources/ComfySwiftSDK/Internal/RouterConstants.swift (+24 -8)
📝 Sources/ComfySwiftSDK/Internal/RouterErrorMapping.swift (+25 -1)
➕ Sources/ComfySwiftSDK/Internal/RouterQueueTransport.swift (+968 -0)
📝 Sources/ComfySwiftSDK/Internal/RouterTransport.swift (+61 -17)
📝 Sources/ComfySwiftSDK/Internal/SDKLog.swift (+46 -0)
📝 Sources/ComfySwiftSDK/Public/RouterModels.swift (+200 -1)
➕ Sources/ComfySwiftSDK/Public/RouterRequests.swift (+366 -0)
➕ Tests/ComfySwiftSDKTests/RouterQueueTests.swift (+1425 -0)
📝 spec/README.md (+1 -1)

📄 Description

ELI-5

Comfy Router can run a partner model two ways: hold the connection open until it finishes (client.models.run, which this SDK already had), or queue it — the server takes the request, hands back an id, and you check back later. Only the second survives a dropped network, a backgrounded app, or a relaunch. The Python and TypeScript SDKs have had the queued form for a while; Swift was the only Router SDK without it. This adds it: client.models.submit, client.models.subscribe, client.models.handle, and the handle those hand back.

What this adds

  • client.models.submit(_:input:idempotencyKey:timeout:) — POST …/requests, returns a RouterRequestHandle with requestId, model, status(), result(), cancel() and an AsyncThrowingStream from events().
  • client.models.subscribe(_:input:onQueueUpdate:timeout:idempotencyKey:) — submit + poll + collect in one call, returning the same RouterRunResult that run does.
  • client.models.handle(_:requestId:) — rebuilds a handle from the two ids with no request made; both validated locally, so a bad row in a caller's database throws before anything reaches the wire.
  • RouterRequestState / RouterRequestStatus / RouterCancelOutcome — the queue's own types, alongside the existing RouterError taxonomy, which already carried notEnabled and needed no change.
  • README: a queued-delivery section mirroring the Python and TypeScript READMEs.
  • Scripts/contract/check_router_contract.py: pins the four queue operations — path and HTTP method — against the vendored spec.

No new dependencies (Foundation only), and no client-side feature flag: the gate is server-side (403 not_enabled), matching the decision recorded on the TypeScript side.

Design notes worth a reviewer's time

One transport, not two. The queue routes live in RouterQueueTransport.swift as an extension RouterTransport, sharing the run route's base-URL validation, redirect refusal, credential injection, proactive-OAuth/401-refresh, attempt cap and withWallClockDeadline. That required widening a handful of private members in RouterTransport.swift to internal — the whole visible cost of not forking a second transport that could drift on auth or on which base URLs are safe. runURL was extracted into a parameterised routeURL(baseURL:template:path:requestId:); the run route's own behaviour is byte-identical (its existing tests cover it unchanged).

URLs are composed, never taken from the response. The submit response carries status_url / response_url / cancel_url. This SDK ignores all three and builds every route from the contract-pinned templates. Each of those calls stamps the caller's credential onto the request, so a URL read out of a response body is a place the server could have it sent. A test asserts that an acknowledgement naming evil.test for all three still produces requests only to api.comfy.org. The server-supplied request_id gets the same treatment: it is run through the same path-segment validation a caller's would be, before it reaches a URL.

Poll-authoritative. The status route decides when a request is done — not the result route's 202, not the submit acknowledgement. The schedule starts at 0.5 s, backs off ×1.5 to a 10 s ceiling, and a server Retry-After beats it outright, capped at 60 s (RouterRequestHandle.maximumRetryAfter) before it is slept on. The pause is also clamped to the remaining budget, so a 60 s hint with 2 s left does not report the timeout 58 s late.

Terminal is not successful, and who throws depends on who is asking. A provider failure, a content refusal, and a cancellation that took effect all read COMPLETED with an error_type. The collecting calls (subscribe, handle.result()) throw the matching RouterError bucket — a 200 from the result route is never returned as success when the completion reported a failure. The watching calls (handle.status(), handle.events()) report it as data on the observation. That split is not invented here: it is exactly what the Python and TypeScript SDKs do, and it exists because reading a status is how a caller discovers a failure in the first place.

Giving up cancels, once, best-effort. subscribe's elapsed timeout and a Task.cancel() each issue exactly one PUT cancel, sent once with no retries and a 5 s bound, before throwing .timeout / .cancelled. The cancel's own failure is swallowed — the error the caller can act on is the one they get. It runs in a detached task because on the cancellation path every inline await would throw before reaching the wire, which is the path it matters most on. handle.result() and handle.events() deliberately do not cancel: they did not submit the request and do not own it.

Verifying against the sibling implementations

The two SDK PRs the ticket names are public, so I read them rather than assuming the shape. Four divergences turned up in my first pass and are fixed in this branch — each one would have made the Swift surface behave differently from the other two:

  1. submit accepted only 201; both siblings accept 200, 201 or 202 as an acceptance, with the body's request_id as the actual proof. Widened, with the request_id still required (a 2xx without one throws rather than producing a handle that addresses nothing).
  2. handle.result() was a single fetch that threw on a 202; both siblings' handle.get() poll to completion, then collect. Changed.
  3. events() threw on a completion carrying an error_type; both siblings deliberately yield it (TypeScript's README spells out why). Changed, along with the stream element — it now yields the full RouterRequestStatus, mirroring QueueUpdate, so errorType is reachable from an observation.
  4. timeout: 0 was rejected; TypeScript documents timeoutMs: 0 as "look once". The polling entry points now take a separate validatePollTimeout that admits zero — a poll is a cheap GET, so the run route's 1-second floor (which exists because a sub-second budget buys one billable POST too short to answer in) does not transfer. submit and subscribe, which do dispatch a billable request, keep the existing floor.

The vendored spec, and what the contract check does about it

spec/router-openapi.yaml is a one-way vendored copy (spec/README.md, AGENTS.md) and I did not hand-edit it. When this branch was opened its sync predated the queue paths, so the drift check was written to arm itself. Merging origin/main brought the sync that declares them (via #80): the spec now carries all seven paths, and check_router_contract.py runs the queue half for real and passes — all four operation IDs, their paths, and their methods (the cancel PUT included) match RouterConstants.

The stand-down is still in the check, and still matters for the next spec sync that might drop the operations: it asserts each of submitRouterModelRequest / getRouterModelRequestStatus / getRouterModelRequestResult / cancelRouterModelRequest against RouterConstants, path and method (the cancel is a PUT, the one route here a reader will assume is a POST) — but stands down with a ::notice:: and a pass when the spec declares none of the four, and fails hard on a partial set. Failing today would make the job red for something no PR in this repo can fix, which is how a check gets disabled.

Verified by temporarily patching a local copy of the spec (reverted, not committed): all four declared → pass; cancelRouterModelRequest moved to post → fail naming the method; one operation removed → fail naming the partial set.

The {request_id} path-parameter spelling in the four new templates is my assumption. If upstream spells it differently, the drift check fails loudly on the sync that brings it in and the fix is the four constants — which is the two-step change spec/README.md already describes.

Verification

swift build and swift test --no-parallel on the macOS target, after merging origin/main (which brought #80's run-route query parameters into RouterTransport). 442 tests in 34 suites, 0 failures, of which RouterQueueTests contributes 52. All run against the recorded-server TestURLProtocol harness in the style of RouterRunTests — never a live server. python3 Scripts/contract/check_router_contract.py exits 0.

Residual

The queue routes have not been exercised against a live Comfy Router. Every test here runs against a TestURLProtocol stub that answers what I believe the contract says, so what is proven is that the client behaves correctly given that contract — not that the contract is what the server serves. Two things follow, and both need a human with preview access before this is cut into a release:

  • The four route templates, the {request_id} parameter spelling, the 201 acknowledgement body's field names (request_id, queue_position, status), the status body's (status, queue_position, error_type), and the ALREADY_COMPLETED spelling on the cancel 400 are all transcribed from the ticket's prose and the sibling SDKs, not observed. The drift check catches the route half on the next spec sync; the body field names are pinned by nothing — a mismatch there surfaces as RouterInvalidResponseError or a silently-absent queuePosition, not as a build failure. Worth one live smoke run against the preview before tagging.
  • 403 not_enabled outside the preview is reproduced from the contract, not observed. The code path is tested (it throws .notEnabled and is not retried), but no call has been made to a server that actually answers it.

The vendored spec/router-openapi.yaml still carries only three paths, so the queue half of the contract check is dormant on this branch. Resolved by the merge from origin/main. The sync that #80 carried declares all four queue operations, so the check is armed and green on this branch, and the four route constants are now pinned by CI.

Artifacts I could not open. The ticket's parent, BE-12682 (Epic: Comfy Router Queueing POC), was named to me by identifier and title only — its body and comments were not available, so any acceptance detail recorded there and nowhere else is unexercised. The three attachments were reachable and I read all three (comfy-python-sdk#137, comfy-typescript-sdk#133, docs#1688); the first two produced the four parity fixes above.

A subscribe that gives up while the polls themselves are failing does not cancel. subscribe issues its one best-effort cancel on ComfyError.timeout and .cancelled only. The poll loop now throws the last swallowed transient failure at the deadline rather than a bare .timeout, so a watch that ran out of budget against a persistent 503 or a down link rethrows that failure and issues no cancel: the request stays queued and may still be billed. Deliberate — a cancel over a link that is evidently down either cannot be delivered (and costs the caller bestEffortCancelTimeout on the way out) or abandons a generation that was probably fine. Call handle.cancel() explicitly if your app would rather give up the generation than pay for it.

The request id is not attached to a non-terminal transport failure out of subscribe. A .network, .offline or .authExpired thrown mid-poll is rethrown unchanged, so a caller who reached for subscribe rather than submit + handle.result() has no id in hand to collect or cancel the queued run with. Both remedies were considered and rejected as changes to contracted behaviour: attaching the id changes the public ComfyError shape (a new associated value on cases every existing catch pattern-matches, or re-wrapping a transport failure as a RouterError it is not), and extending the best-effort cancel to those three cases cancels requests that are probably fine over a link that is probably down. submit returns the handle before any of this can happen and is the documented flow for anything that has to survive a dropped connection; subscribe is the convenience call that trades that for one await.

The result fetch deliberately overruns the caller's timeout. It is floored at RouterTransport.resultFetchMinimumBudget (= RouterModels.defaultRequestTimeout, 60 s) rather than at the 1-second attempt floor, because that leg is a transfer of the provider's payload rather than a wait — and result(timeout: 0) would otherwise give a multi-megabyte download exactly one second on every call, reporting a timeout for a generation that had completed and been billed. The overrun is bounded by that constant and only applies to the one fetch; a timeout above 60 s bounds it as stated. Documented on result(timeout:), on subscribe, and in the README.

RouterRunResult.idempotencyKey changed from String to String? — a source-breaking change on a pre-1.0 public property. A result collected through a handle that handle(_:requestId:) rebuilt has no submit and therefore no key, and an invented one would be actively harmful because the documented recovery flow is to re-send that value. run and subscribe still always populate it. Flagging it because it is the only break in this PR and it needs to be in release notes.

Out of scope, as the ticket states, and untouched here: the docs PR adding Swift tabs to the queued-delivery page; the Swift tab on the 209 generated per-model code pages; the Go SDK; webhooks, SSE, priority and queue list/flush; any client-side queue behaviour; any server contract change; and the version bump and tag, which stay a human step once the production server serves these routes outside the preview.

Provenance

  • Authored by: agent-work loop
  • Verified: swift build: clean, 0 warnings in changed files; swift test --no-parallel: 442 tests in 34 suites, 0 failures; python3 Scripts/contract/check_router_contract.py: exit 0, queue half armed and verifying all four routes against the post-merge spec, plus three synthetic-spec runs proving it arms, catches a moved method, and catches a partial set
  • Deviations: submit/subscribe keep the run route's 1-second minimum timeout where the TypeScript SDK allows zero — they dispatch a billable request; result/events, which only poll, accept zero. submit takes a defaulted timeout: the ticket's signature does not name. The stream element is RouterRequestStatus rather than a slimmer event enum, to match the siblings' QueueUpdate and keep errorType reachable from an observation.

🔄 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-swift-sdk/pull/77 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/16/2026 **Status:** ✅ Merged **Merged:** 9/18/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-14639-router-queue-surface` --- ### 📝 Commits (5) - [`18d2b71`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/18d2b71ccae4dc713655c1ddd3d9b463532ec24f) feat(router): add client.models.submit / subscribe / handle for queued delivery - [`febb80b`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/febb80b834063091bdb61ed28d7d38c55c624d6e) fix(router): guard the acknowledgement status, keep the failure signal, and stop wasting the last poll pause - [`1698b1f`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/1698b1f4aea59c9133368ad6e8c99ce625a6dbfb) Merge origin/main into matt/be-14639-router-queue-surface - [`7cc5079`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/7cc50794d348503e1b0bb8863716125f10018971) fix(router): retry transient status polls and give the result fetch a download-sized budget - [`9bef73d`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/9bef73d289da90f5aef0696ab7ddf2bda69393da) Merge branch 'main' into matt/be-14639-router-queue-surface ### 📊 Changes **11 files changed** (+3363 additions, -30 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+87 -0) 📝 `Scripts/contract/check_router_contract.py` (+160 -2) 📝 `Sources/ComfySwiftSDK/Internal/RouterConstants.swift` (+24 -8) 📝 `Sources/ComfySwiftSDK/Internal/RouterErrorMapping.swift` (+25 -1) ➕ `Sources/ComfySwiftSDK/Internal/RouterQueueTransport.swift` (+968 -0) 📝 `Sources/ComfySwiftSDK/Internal/RouterTransport.swift` (+61 -17) 📝 `Sources/ComfySwiftSDK/Internal/SDKLog.swift` (+46 -0) 📝 `Sources/ComfySwiftSDK/Public/RouterModels.swift` (+200 -1) ➕ `Sources/ComfySwiftSDK/Public/RouterRequests.swift` (+366 -0) ➕ `Tests/ComfySwiftSDKTests/RouterQueueTests.swift` (+1425 -0) 📝 `spec/README.md` (+1 -1) </details> ### 📄 Description ## ELI-5 Comfy Router can run a partner model two ways: hold the connection open until it finishes (`client.models.run`, which this SDK already had), or **queue it** — the server takes the request, hands back an id, and you check back later. Only the second survives a dropped network, a backgrounded app, or a relaunch. The Python and TypeScript SDKs have had the queued form for a while; Swift was the only Router SDK without it. This adds it: `client.models.submit`, `client.models.subscribe`, `client.models.handle`, and the handle those hand back. ## What this adds - **`client.models.submit(_:input:idempotencyKey:timeout:)`** — `POST …/requests`, returns a `RouterRequestHandle` with `requestId`, `model`, `status()`, `result()`, `cancel()` and an `AsyncThrowingStream` from `events()`. - **`client.models.subscribe(_:input:onQueueUpdate:timeout:idempotencyKey:)`** — submit + poll + collect in one call, returning the same `RouterRunResult` that `run` does. - **`client.models.handle(_:requestId:)`** — rebuilds a handle from the two ids with **no request made**; both validated locally, so a bad row in a caller's database throws before anything reaches the wire. - **`RouterRequestState` / `RouterRequestStatus` / `RouterCancelOutcome`** — the queue's own types, alongside the existing `RouterError` taxonomy, which already carried `notEnabled` and needed no change. - **README**: a queued-delivery section mirroring the Python and TypeScript READMEs. - **`Scripts/contract/check_router_contract.py`**: pins the four queue operations — path *and* HTTP method — against the vendored spec. No new dependencies (Foundation only), and no client-side feature flag: the gate is server-side (`403 not_enabled`), matching the decision recorded on the TypeScript side. ## Design notes worth a reviewer's time **One transport, not two.** The queue routes live in `RouterQueueTransport.swift` as an `extension RouterTransport`, sharing the run route's base-URL validation, redirect refusal, credential injection, proactive-OAuth/401-refresh, attempt cap and `withWallClockDeadline`. That required widening a handful of `private` members in `RouterTransport.swift` to `internal` — the whole visible cost of *not* forking a second transport that could drift on auth or on which base URLs are safe. `runURL` was extracted into a parameterised `routeURL(baseURL:template:path:requestId:)`; the run route's own behaviour is byte-identical (its existing tests cover it unchanged). **URLs are composed, never taken from the response.** The submit response carries `status_url` / `response_url` / `cancel_url`. This SDK ignores all three and builds every route from the contract-pinned templates. Each of those calls stamps the caller's credential onto the request, so a URL read out of a response body is a place the server could have it sent. A test asserts that an acknowledgement naming `evil.test` for all three still produces requests only to `api.comfy.org`. The server-supplied `request_id` gets the same treatment: it is run through the same path-segment validation a caller's would be, before it reaches a URL. **Poll-authoritative.** The status route decides when a request is done — not the result route's `202`, not the submit acknowledgement. The schedule starts at 0.5 s, backs off ×1.5 to a 10 s ceiling, and a server `Retry-After` beats it outright, capped at 60 s (`RouterRequestHandle.maximumRetryAfter`) before it is slept on. The pause is also clamped to the remaining budget, so a 60 s hint with 2 s left does not report the timeout 58 s late. **Terminal is not successful, and who throws depends on who is asking.** A provider failure, a content refusal, and a cancellation that took effect all read `COMPLETED` with an `error_type`. The *collecting* calls (`subscribe`, `handle.result()`) throw the matching `RouterError` bucket — a `200` from the result route is never returned as success when the completion reported a failure. The *watching* calls (`handle.status()`, `handle.events()`) report it as data on the observation. That split is not invented here: it is exactly what the Python and TypeScript SDKs do, and it exists because reading a status is how a caller discovers a failure in the first place. **Giving up cancels, once, best-effort.** `subscribe`'s elapsed timeout and a `Task.cancel()` each issue exactly one `PUT` cancel, sent once with no retries and a 5 s bound, before throwing `.timeout` / `.cancelled`. The cancel's own failure is swallowed — the error the caller can act on is the one they get. It runs in a detached task because on the cancellation path every inline `await` would throw before reaching the wire, which is the path it matters most on. `handle.result()` and `handle.events()` deliberately do **not** cancel: they did not submit the request and do not own it. ## Verifying against the sibling implementations The two SDK PRs the ticket names are public, so I read them rather than assuming the shape. Four divergences turned up in my first pass and are fixed in this branch — each one would have made the Swift surface behave differently from the other two: 1. `submit` accepted only `201`; both siblings accept **`200`, `201` or `202`** as an acceptance, with the body's `request_id` as the actual proof. Widened, with the `request_id` still required (a `2xx` without one throws rather than producing a handle that addresses nothing). 2. `handle.result()` was a single fetch that threw on a `202`; both siblings' `handle.get()` **poll to completion, then collect**. Changed. 3. `events()` threw on a completion carrying an `error_type`; both siblings deliberately *yield* it (TypeScript's README spells out why). Changed, along with the stream element — it now yields the full `RouterRequestStatus`, mirroring `QueueUpdate`, so `errorType` is reachable from an observation. 4. `timeout: 0` was rejected; TypeScript documents `timeoutMs: 0` as "look once". The polling entry points now take a separate `validatePollTimeout` that admits zero — a poll is a cheap `GET`, so the run route's 1-second floor (which exists because a sub-second budget buys one *billable* POST too short to answer in) does not transfer. `submit` and `subscribe`, which do dispatch a billable request, keep the existing floor. ## The vendored spec, and what the contract check does about it `spec/router-openapi.yaml` is a one-way vendored copy (`spec/README.md`, `AGENTS.md`) and I did not hand-edit it. When this branch was opened its sync predated the queue paths, so the drift check was written to arm itself. **Merging `origin/main` brought the sync that declares them** (via #80): the spec now carries all seven paths, and `check_router_contract.py` runs the queue half for real and passes — all four operation IDs, their paths, and their methods (the cancel `PUT` included) match `RouterConstants`. The stand-down is still in the check, and still matters for the next spec sync that might drop the operations: it asserts each of `submitRouterModelRequest` / `getRouterModelRequestStatus` / `getRouterModelRequestResult` / `cancelRouterModelRequest` against `RouterConstants`, path **and method** (the cancel is a `PUT`, the one route here a reader will assume is a `POST`) — but stands down with a `::notice::` and a pass when the spec declares **none** of the four, and fails hard on a **partial** set. Failing today would make the job red for something no PR in this repo can fix, which is how a check gets disabled. Verified by temporarily patching a local copy of the spec (reverted, not committed): all four declared → pass; `cancelRouterModelRequest` moved to `post` → fail naming the method; one operation removed → fail naming the partial set. The `{request_id}` path-parameter *spelling* in the four new templates is my assumption. If upstream spells it differently, the drift check fails loudly on the sync that brings it in and the fix is the four constants — which is the two-step change `spec/README.md` already describes. ## Verification `swift build` and `swift test --no-parallel` on the macOS target, after merging `origin/main` (which brought #80's run-route query parameters into `RouterTransport`). **442 tests in 34 suites, 0 failures**, of which `RouterQueueTests` contributes 52. All run against the recorded-server `TestURLProtocol` harness in the style of `RouterRunTests` — never a live server. `python3 Scripts/contract/check_router_contract.py` exits 0. ## Residual **The queue routes have not been exercised against a live Comfy Router.** Every test here runs against a `TestURLProtocol` stub that answers what I believe the contract says, so what is proven is that the client behaves correctly *given that contract* — not that the contract is what the server serves. Two things follow, and both need a human with preview access before this is cut into a release: - The four route templates, the `{request_id}` parameter spelling, the `201` acknowledgement body's field names (`request_id`, `queue_position`, `status`), the status body's (`status`, `queue_position`, `error_type`), and the `ALREADY_COMPLETED` spelling on the cancel `400` are all transcribed from the ticket's prose and the sibling SDKs, not observed. The drift check catches the route half on the next spec sync; the **body field names are pinned by nothing** — a mismatch there surfaces as `RouterInvalidResponseError` or a silently-absent `queuePosition`, not as a build failure. Worth one live smoke run against the preview before tagging. - `403 not_enabled` outside the preview is reproduced from the contract, not observed. The code path is tested (it throws `.notEnabled` and is not retried), but no call has been made to a server that actually answers it. ~~**The vendored `spec/router-openapi.yaml` still carries only three paths**, so the queue half of the contract check is dormant on this branch.~~ **Resolved by the merge from `origin/main`.** The sync that #80 carried declares all four queue operations, so the check is armed and green on this branch, and the four route constants are now pinned by CI. **Artifacts I could not open.** The ticket's parent, `BE-12682` (Epic: Comfy Router Queueing POC), was named to me by identifier and title only — its body and comments were not available, so any acceptance detail recorded there and nowhere else is unexercised. The three attachments *were* reachable and I read all three (`comfy-python-sdk#137`, `comfy-typescript-sdk#133`, `docs#1688`); the first two produced the four parity fixes above. **A `subscribe` that gives up while the polls themselves are failing does not cancel.** `subscribe` issues its one best-effort cancel on `ComfyError.timeout` and `.cancelled` only. The poll loop now throws the *last swallowed transient failure* at the deadline rather than a bare `.timeout`, so a watch that ran out of budget against a persistent `503` or a down link rethrows that failure and issues no cancel: the request stays queued and may still be billed. Deliberate — a cancel over a link that is evidently down either cannot be delivered (and costs the caller `bestEffortCancelTimeout` on the way out) or abandons a generation that was probably fine. Call `handle.cancel()` explicitly if your app would rather give up the generation than pay for it. **The request id is not attached to a non-terminal transport failure out of `subscribe`.** A `.network`, `.offline` or `.authExpired` thrown mid-poll is rethrown unchanged, so a caller who reached for `subscribe` rather than `submit` + `handle.result()` has no id in hand to collect or cancel the queued run with. Both remedies were considered and rejected as changes to contracted behaviour: attaching the id changes the public `ComfyError` shape (a new associated value on cases every existing `catch` pattern-matches, or re-wrapping a transport failure as a `RouterError` it is not), and extending the best-effort cancel to those three cases cancels requests that are probably fine over a link that is probably down. `submit` returns the handle *before* any of this can happen and is the documented flow for anything that has to survive a dropped connection; `subscribe` is the convenience call that trades that for one `await`. **The result fetch deliberately overruns the caller's `timeout`.** It is floored at `RouterTransport.resultFetchMinimumBudget` (= `RouterModels.defaultRequestTimeout`, 60 s) rather than at the 1-second attempt floor, because that leg is a *transfer* of the provider's payload rather than a wait — and `result(timeout: 0)` would otherwise give a multi-megabyte download exactly one second on every call, reporting a timeout for a generation that had completed and been billed. The overrun is bounded by that constant and only applies to the one fetch; a `timeout` above 60 s bounds it as stated. Documented on `result(timeout:)`, on `subscribe`, and in the README. **`RouterRunResult.idempotencyKey` changed from `String` to `String?`** — a source-breaking change on a pre-1.0 public property. A result collected through a handle that `handle(_:requestId:)` rebuilt has no submit and therefore no key, and an invented one would be actively harmful because the documented recovery flow is to *re-send* that value. `run` and `subscribe` still always populate it. Flagging it because it is the only break in this PR and it needs to be in release notes. **Out of scope, as the ticket states, and untouched here:** the docs PR adding Swift tabs to the queued-delivery page; the Swift tab on the 209 generated per-model code pages; the Go SDK; webhooks, SSE, priority and queue list/flush; any client-side queue behaviour; any server contract change; and the version bump and tag, which stay a human step once the production server serves these routes outside the preview. ## Provenance - **Authored by:** agent-work loop - **Verified:** `swift build`: clean, 0 warnings in changed files; `swift test --no-parallel`: 442 tests in 34 suites, 0 failures; `python3 Scripts/contract/check_router_contract.py`: exit 0, queue half armed and verifying all four routes against the post-merge spec, plus three synthetic-spec runs proving it arms, catches a moved method, and catches a partial set - **Deviations:** `submit`/`subscribe` keep the run route's 1-second minimum `timeout` where the TypeScript SDK allows zero — they dispatch a billable request; `result`/`events`, which only poll, accept zero. `submit` takes a defaulted `timeout:` the ticket's signature does not name. The stream element is `RouterRequestStatus` rather than a slimmer event enum, to match the siblings' `QueueUpdate` and keep `errorType` reachable from an observation. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-26 22:34:42 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Comfy-Org/comfy-swift-sdk#78
No description provided.