[PR #68] [MERGED] feat(router): add client.models.run with idempotency, timeout and collect-loop semantics #69

New issue

Closed

opened 2026-10-26 22:34:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:34:37 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-10180-router-models-run

📝 Commits (10+)

a7015f5 feat(router): add client.models.run with idempotency, timeout and collect-loop semantics
a67299c fix(router): keep response-controlled text out of logs and tighten two edges
a5d6521 fix(router): bound the whole call, and refuse inputs that cannot bound it
7311f63 fix(router): refuse redirects on the run route, and stop re-sending into a budget too small to answer in
c5f858d fix(router): refuse userinfo in the base URL, and bound the request built after applyAuth
7a27246 fix(router): close the six open review findings on the run surface
9faebc0 fix(router): make the attempt cap a per-run budget and close the remaining retention gaps
213dcfc fix(router): bound retained validation content, and hold the attempt cap on the 401 path
2c90aac fix(router): stop the description trapping, and make the retention bounds true on every axis
6d7bffd fix(router): close two process-kill paths and finish the log-safety bounds

📊 Changes

11 files changed (+3382 additions, -42 deletions)

View changed files

📝 README.md (+38 -9)
📝 Scripts/contract/sdk-endpoints.yml (+7 -0)
📝 Sources/ComfySwiftSDK/Internal/RouterErrorMapping.swift (+281 -24)
➕ Sources/ComfySwiftSDK/Internal/RouterTransport.swift (+838 -0)
📝 Sources/ComfySwiftSDK/Internal/SDKLog.swift (+67 -0)
📝 Sources/ComfySwiftSDK/Internal/Transport.swift (+30 -3)
📝 Sources/ComfySwiftSDK/Public/ComfyCloudClient.swift (+30 -1)
📝 Sources/ComfySwiftSDK/Public/RouterError.swift (+91 -5)
➕ Sources/ComfySwiftSDK/Public/RouterModels.swift (+240 -0)
📝 Tests/ComfySwiftSDKTests/RouterErrorMappingTests.swift (+279 -0)
➕ Tests/ComfySwiftSDKTests/RouterRunTests.swift (+1481 -0)

📄 Description

ELI-5

The SDK could already run a ComfyUI workflow graph on Comfy Cloud. It could not run a partner model — "give me a cat picture from bfl/flux-2-pro" — even though Comfy Router exposes exactly that as a single HTTP call. This PR adds client.models.run("bfl/flux-2-pro", input: [...]). One request goes out, it waits (up to 11 minutes) for the model to answer, and it hands you back the model's own JSON. The tricky part is that image generation costs money, so every call is stamped with an idempotency key: if the connection drops or Comfy stops holding the line, asking again with the same key collects the generation you already paid for instead of starting — and billing — a second one. Most of the code here is about getting that "ask again" rule exactly right, including the three specific server answers that mean "still running, ask me again" and the many that mean "stop".

What changed

New public surface — Sources/ComfySwiftSDK/Public/RouterModels.swift

RouterRunResult: data (the model's native JSON output byte-for-byte), output (a read-only RouterJSON view — result.output["images"][0]["url"].stringValue), requestId, idempotencyKey, replayed, and decode(_:using:).
RouterModels: baseURL, run(_:input:idempotencyKey:timeout:), and defaultTimeout = 660 — deliberately a minute above Router's own 10-minute server deadline, so the client never gives up first and turn a 504 deadline_exceeded the collect loop handles into a client-side timeout whose outcome nobody knows.
ComfyCloudClient gains public let models plus an optional routerBaseURL: on its designated init. The existing signature keeps working (every pre-existing test compiles and passes unchanged).

New transport — Sources/ComfySwiftSDK/Internal/RouterTransport.swift (actor)

Posts once and collects. It re-sends the same bytes under the same key for exactly the three status/bucket pairings the vendored contract says a re-send collects — 429, 409 concurrency_limit_exceeded, 504 deadline_exceeded — and each only when the response carried a Retry-After that still fits inside the caller's deadline. It never re-sends on 409 invalid_input (the key is consumed and unreplayable — re-sending just loops on the same refusal), on a 4xx with no Retry-After, on a 5xx outside that pairing, or after a transport failure or client-side timeout, whose outcome is unknown and where re-sending blind is the caller's call to make.

Small Transport refactor. applyAuth(to:) and withAuthRetry(perform:) widen from private to internal (translate(_:) was already internal) so RouterTransport reuses the one implementation of credential injection, proactive refresh and the 401-refresh-retry. No behaviour changed and nothing was duplicated; the widening is additive and the existing RefreshOn401Tests still pass untouched.

One defect found and fixed during self-review

The ticket's flow — persist the key, relaunch, call run(..., idempotencyKey:) again to collect — was broken as originally written, and not subtly.

Swift seeds Dictionary hashing per process, so serialising the same [String: Any] emits a different key order in every launch. Measured on this machine across three runs of the same input:

plain  : {"seed":7,"steps":30,"prompt":"a cat","width":1024,"cfg":3.5}
plain  : {"steps":30,"prompt":"a cat","width":1024,"seed":7,"cfg":3.5}
plain  : {"width":1024,"steps":30,"seed":7,"cfg":3.5,"prompt":"a cat"}

Router refuses a re-used key whose request differs with 409 invalid_input rather than replaying it — so the recovery flow would have presented the same key with reordered bytes and been refused, in precisely the case it exists for. Serialisation is now .sortedKeys, which makes the bytes stable across launches. JSON object key order carries no meaning, so no provider can observe the difference. Locked in by serialisation_is_key_order_stable.

Negative-claim falsification — the three-segment model ID

This diff adds a capability denial ("a/b/c" is refused), so per the falsification discipline I went looking for the capability rather than asserting its absence. Every path reachable from this repo:

The vendored contract's run route (spec/router-openapi.yaml): /v2/models/{provider}/{model}, operationId: runRouterModel — exactly two path parameters. grep -n variant spec/router-openapi.yaml returns 4 hits, all inside human-readable description prose; there is no variant path parameter, no variant schema and no variant field on any catalog entry.
The discovery path — GET /v2/models — cannot emit one. RouterModelId is pattern: ^[a-z0-9]+([._-][a-z0-9]+)*/[a-z0-9]+([._-][a-z0-9]+)*$, maxLength: 193 (= the provider segment's 64 + the model segment's 128 + one separator). A three-segment ID is structurally unrepresentable there, so the catalog can never hand a caller one to pass back in.
The segment schemas (RouterModelSegment, RouterProviderSegment) forbid / inside a segment, so a variant cannot be smuggled into either one.
The SDK's other product surface, client.submit(_:), is the ComfyUI workflow contract on a different host and does not address models by Router ID at all — it is not an alternative route to the same capability.

The [/{variant}] form in the spec's prose is therefore not addressable on this route today. Critically, the denial redirects rather than dead-ends: it names the variant form and points at the two-segment ID the catalog lists. And it is self-invalidating — if Router ever adds a variant path parameter, check_router_contract.py's route pin fails CI, which is the signal to revisit this branch.

Limit on that evidence: I have no Router credential and no network path to api.comfy.org from this environment, so no live call was made. The falsification above is against the vendored contract, which is the only authority this SDK has and the same one CI pins to — but it is a document, not the running service.

Judgment calls (deviations from the plan as written)

ComfySDKInfo.version → 0.5.0, not 0.4.0. The plan asked for 0.4.0; the tree was already at 0.4.1, and 0.4.0 is an existing released tag, so that would have been a downgrade onto a shipped version. This PR adds public API, so a minor bump is the right move. The README install pin moves with it.
The three-segment ID gets its own identifier. The plan asked for .other("invalid_model_id") and for the variant case to be "refused with a message that names the variant form", which conflicts — ServerRejectionReason.other is documented as carrying a stable machine identifier, so a prose sentence does not belong in it. The variant case throws .other("invalid_model_id_variant_unsupported"), which names the form in a machine-stable way and is prefixed with the general identifier so a caller branching with hasPrefix("invalid_model_id") still matches both. Every other malformed shape throws .other("invalid_model_id") exactly as specified.
routerBaseURL: defaults to RouterModels.defaultBaseURL, not RouterConstants.defaultBaseURL. Swift forbids an internal declaration in a public function's default argument. RouterModels.defaultBaseURL is a public re-export of the internal constant, so check_router_contract.py still pins the single source of truth to the spec's servers[0]; a test asserts the two are equal.
Collect-loop tests stub Retry-After: 1, not 0. The plan's Retry-After: 0 would not exercise a re-send at all: the contract declares a minimum of 1, and the already-shipped RouterErrorMapping deliberately reads anything below it as "no advice" (retryAfter == nil) so an unusable value can never become an immediate retry. 1 is the smallest value that exercises the path, which costs those three tests about a second each of real sleeping. That is the sleep under test.

Verification

swift build — clean.
swift test --no-parallel — 305 tests in 32 suites passed, 0 failed (24 of them new in RouterRunTests). ImportBoundaryTests and LoggingPrivacyTests both still pass; the new SDKLog router category never takes the key, the body or a credential as a parameter, so none can reach a log line.
python3 Scripts/contract/check_router_contract.py — OK, 15 error types and the bound route in sync with the spec.
python3 Scripts/contract/check_contract.py --spec <ComfyUI master openapi.yaml> --manifest Scripts/contract/sdk-endpoints.yml — 6 ok, 0 warnings, 0 errors, so the new excluded: entry did not disturb the ComfyUI gate.
The acceptance snippet was compiled, not eyeballed: swiftc -typecheck against the built module, clean. One note — the snippet as specified needs import Foundation alongside import ComfySwiftSDK for ProcessInfo, which the SDK does not re-export. The README copy includes it so a reader can paste it and have it build.

Residual

No live Router call was made. Everything here is verified against the vendored spec/router-openapi.yaml and a URLProtocol stub. The contract is the same artifact CI pins to, but it is a document: the response shapes are asserted, the running service's behaviour is not. Anyone with a Router credential should run the acceptance snippet against api.comfy.org once before this is relied on in the iOS app — in particular to confirm that a real 504 deadline_exceeded carries the Retry-After the collect loop requires, since a 504 without it is deliberately terminal here and would surface to users as a failed run rather than a collected one.
The TypeScript SDK's parseModelId was not read. The plan says the validation "mirrors" it; that repo is not in this sandbox, so the rules here were derived from the vendored spec's own RouterModelId / RouterModelSegment / RouterProviderSegment patterns instead. If the two implementations disagree, the spec is the tiebreaker and this is the one to re-check — most likely around whether the TS side rejects or silently truncates a third segment, and whether it enforces the schemas' lowercase-only ^[a-z0-9]+([._-][a-z0-9]+)*$ pattern, which this implementation deliberately does not (it percent-encodes and lets the server reject, so a catalog addition that widens the pattern is not blocked client-side by a stale copy of it).
One new strict-concurrency warning, invisible in the default build. Under -strict-concurrency=complete the transport.withAuthRetry { ... } call in RouterTransport.swift warns "sending value of non-Sendable type … risks causing data races". The package is swift-tools-version: 5.9 (Swift 5 language mode), so it is not emitted by swift build or CI. It is safe in fact — the closure captures only value types plus an actor reference, and its body does nothing but call one actor-isolated method — but it is an error in Swift 6 language mode. There is exactly one other such warning in the package, pre-existing, on Transport.swift:109 (normalizeToken), and both sit on the same helper. Whoever migrates this package to Swift 6 mode has to restructure the two together; doing it here would mean reshaping a shared helper every existing endpoint calls, which is out of scope for this PR.
RouterInputSerializationError is internal but escapes through a public API via ComfyError.unknown(underlying:). A caller can read its localizedDescription but cannot as?-match the type. That is fine for the case it reports — a non-JSON input is a programming error fixed in the caller, not a condition to branch on — but if it ever needs branching, promoting it to public is the change.

Provenance

Authored by: agent-work loop
Verified: swift build clean; swift test --no-parallel 305 passed / 0 failed in 32 suites; check_router_contract.py OK (15 error types, route in sync); check_contract.py 6 ok / 0 warnings / 0 errors; acceptance snippet swiftc -typecheck clean against the built module
Deviations: four, each stated with its reason under "Judgment calls" above — version bumped to 0.5.0 rather than the plan's already-released 0.4.0; the three-segment model ID throws a prefixed variant-specific identifier rather than the bare one; routerBaseURL: defaults to the public RouterModels.defaultBaseURL because Swift forbids an internal declaration in a public default argument; collect-loop tests use Retry-After: 1 because the contract's minimum is 1 and 0 exercises no re-send at all

_{🔄 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/68 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/11/2026 **Status:** ✅ Merged **Merged:** 9/13/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-10180-router-models-run` --- ### 📝 Commits (10+) - [`a7015f5`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/a7015f594fa3c8f3a8fd975d2d282ab9273d1f17) feat(router): add client.models.run with idempotency, timeout and collect-loop semantics - [`a67299c`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/a67299cea340fade74b5e09a88f14f11d79699d6) fix(router): keep response-controlled text out of logs and tighten two edges - [`a5d6521`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/a5d65210118301a4a1fcd89b1b8ce315c61f1a7a) fix(router): bound the whole call, and refuse inputs that cannot bound it - [`7311f63`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/7311f63835b6a1d6ff0a69163985adee7d789dd4) fix(router): refuse redirects on the run route, and stop re-sending into a budget too small to answer in - [`c5f858d`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/c5f858db942fd5beee20894a03aa7e81514b69cb) fix(router): refuse userinfo in the base URL, and bound the request built after applyAuth - [`7a27246`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/7a27246c4b341e5f6db269a5f64d1a5ff7fe5514) fix(router): close the six open review findings on the run surface - [`9faebc0`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/9faebc0aa63a8f784aa79dbde9a84304d5303752) fix(router): make the attempt cap a per-run budget and close the remaining retention gaps - [`213dcfc`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/213dcfc8c17e432d31a1cb9216abc664b452ae37) fix(router): bound retained validation content, and hold the attempt cap on the 401 path - [`2c90aac`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/2c90aacdb8d6122ae1a2f94bbf0a734336d6e13e) fix(router): stop the description trapping, and make the retention bounds true on every axis - [`6d7bffd`](https://github.com/Comfy-Org/comfy-swift-sdk/commit/6d7bffdbaa9d7d2ac5efff2a23efa020a21503ec) fix(router): close two process-kill paths and finish the log-safety bounds ### 📊 Changes **11 files changed** (+3382 additions, -42 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+38 -9) 📝 `Scripts/contract/sdk-endpoints.yml` (+7 -0) 📝 `Sources/ComfySwiftSDK/Internal/RouterErrorMapping.swift` (+281 -24) ➕ `Sources/ComfySwiftSDK/Internal/RouterTransport.swift` (+838 -0) 📝 `Sources/ComfySwiftSDK/Internal/SDKLog.swift` (+67 -0) 📝 `Sources/ComfySwiftSDK/Internal/Transport.swift` (+30 -3) 📝 `Sources/ComfySwiftSDK/Public/ComfyCloudClient.swift` (+30 -1) 📝 `Sources/ComfySwiftSDK/Public/RouterError.swift` (+91 -5) ➕ `Sources/ComfySwiftSDK/Public/RouterModels.swift` (+240 -0) 📝 `Tests/ComfySwiftSDKTests/RouterErrorMappingTests.swift` (+279 -0) ➕ `Tests/ComfySwiftSDKTests/RouterRunTests.swift` (+1481 -0) </details> ### 📄 Description ## ELI-5 The SDK could already run a ComfyUI *workflow graph* on Comfy Cloud. It could not run a *partner model* — "give me a cat picture from `bfl/flux-2-pro`" — even though Comfy Router exposes exactly that as a single HTTP call. This PR adds `client.models.run("bfl/flux-2-pro", input: [...])`. One request goes out, it waits (up to 11 minutes) for the model to answer, and it hands you back the model's own JSON. The tricky part is that image generation costs money, so every call is stamped with an idempotency key: if the connection drops or Comfy stops holding the line, asking again *with the same key* collects the generation you already paid for instead of starting — and billing — a second one. Most of the code here is about getting that "ask again" rule exactly right, including the three specific server answers that mean "still running, ask me again" and the many that mean "stop". ## What changed **New public surface** — `Sources/ComfySwiftSDK/Public/RouterModels.swift` - `RouterRunResult`: `data` (the model's native JSON output byte-for-byte), `output` (a read-only `RouterJSON` view — `result.output["images"][0]["url"].stringValue`), `requestId`, `idempotencyKey`, `replayed`, and `decode(_:using:)`. - `RouterModels`: `baseURL`, `run(_:input:idempotencyKey:timeout:)`, and `defaultTimeout = 660` — deliberately a minute *above* Router's own 10-minute server deadline, so the client never gives up first and turn a `504 deadline_exceeded` the collect loop handles into a client-side timeout whose outcome nobody knows. - `ComfyCloudClient` gains `public let models` plus an optional `routerBaseURL:` on its designated init. The existing signature keeps working (every pre-existing test compiles and passes unchanged). **New transport** — `Sources/ComfySwiftSDK/Internal/RouterTransport.swift` (actor) Posts once and collects. It re-sends the **same bytes under the same key** for exactly the three status/bucket pairings the vendored contract says a re-send collects — `429`, `409 concurrency_limit_exceeded`, `504 deadline_exceeded` — and each only when the response carried a `Retry-After` that still fits inside the caller's deadline. It never re-sends on `409 invalid_input` (the key is consumed and unreplayable — re-sending just loops on the same refusal), on a `4xx` with no `Retry-After`, on a `5xx` outside that pairing, or after a transport failure or client-side timeout, whose outcome is *unknown* and where re-sending blind is the caller's call to make. **Small `Transport` refactor.** `applyAuth(to:)` and `withAuthRetry(perform:)` widen from `private` to `internal` (`translate(_:)` was already internal) so `RouterTransport` reuses the one implementation of credential injection, proactive refresh and the 401-refresh-retry. No behaviour changed and nothing was duplicated; the widening is additive and the existing `RefreshOn401Tests` still pass untouched. ## One defect found and fixed during self-review The ticket's flow — persist the key, relaunch, call `run(..., idempotencyKey:)` again to collect — was **broken as originally written**, and not subtly. Swift seeds `Dictionary` hashing *per process*, so serialising the same `[String: Any]` emits a different key order in every launch. Measured on this machine across three runs of the same input: ``` plain : {"seed":7,"steps":30,"prompt":"a cat","width":1024,"cfg":3.5} plain : {"steps":30,"prompt":"a cat","width":1024,"seed":7,"cfg":3.5} plain : {"width":1024,"steps":30,"seed":7,"cfg":3.5,"prompt":"a cat"} ``` Router refuses a re-used key whose request *differs* with `409 invalid_input` rather than replaying it — so the recovery flow would have presented the same key with reordered bytes and been refused, in precisely the case it exists for. Serialisation is now `.sortedKeys`, which makes the bytes stable across launches. JSON object key order carries no meaning, so no provider can observe the difference. Locked in by `serialisation_is_key_order_stable`. ## Negative-claim falsification — the three-segment model ID This diff adds a capability denial (`"a/b/c"` is refused), so per the falsification discipline I went looking for the capability rather than asserting its absence. Every path reachable from this repo: 1. **The vendored contract's run route** (`spec/router-openapi.yaml`): `/v2/models/{provider}/{model}`, `operationId: runRouterModel` — exactly two path parameters. `grep -n variant spec/router-openapi.yaml` returns 4 hits, **all** inside human-readable `description` prose; there is no `variant` path parameter, no `variant` schema and no `variant` field on any catalog entry. 2. **The discovery path** — `GET /v2/models` — cannot emit one. `RouterModelId` is `pattern: ^[a-z0-9]+([._-][a-z0-9]+)*/[a-z0-9]+([._-][a-z0-9]+)*$`, `maxLength: 193` (= the provider segment's 64 + the model segment's 128 + one separator). A three-segment ID is structurally unrepresentable there, so the catalog can never hand a caller one to pass back in. 3. **The segment schemas** (`RouterModelSegment`, `RouterProviderSegment`) forbid `/` inside a segment, so a variant cannot be smuggled into either one. 4. **The SDK's other product surface**, `client.submit(_:)`, is the ComfyUI workflow contract on a different host and does not address models by Router ID at all — it is not an alternative route to the same capability. The `[/{variant}]` form in the spec's prose is therefore not addressable on this route today. Critically, the denial **redirects rather than dead-ends**: it names the variant form and points at the two-segment ID the catalog lists. And it is self-invalidating — if Router ever adds a variant path parameter, `check_router_contract.py`'s route pin fails CI, which is the signal to revisit this branch. **Limit on that evidence:** I have no Router credential and no network path to `api.comfy.org` from this environment, so no *live* call was made. The falsification above is against the vendored contract, which is the only authority this SDK has and the same one CI pins to — but it is a document, not the running service. ## Judgment calls (deviations from the plan as written) - **`ComfySDKInfo.version` → `0.5.0`, not `0.4.0`.** The plan asked for `0.4.0`; the tree was already at `0.4.1`, and `0.4.0` is an existing released tag, so that would have been a downgrade onto a shipped version. This PR adds public API, so a minor bump is the right move. The README install pin moves with it. - **The three-segment ID gets its own identifier.** The plan asked for `.other("invalid_model_id")` *and* for the variant case to be "refused with a message that names the variant form", which conflicts — `ServerRejectionReason.other` is documented as carrying a *stable machine identifier*, so a prose sentence does not belong in it. The variant case throws `.other("invalid_model_id_variant_unsupported")`, which names the form in a machine-stable way and is **prefixed** with the general identifier so a caller branching with `hasPrefix("invalid_model_id")` still matches both. Every other malformed shape throws `.other("invalid_model_id")` exactly as specified. - **`routerBaseURL:` defaults to `RouterModels.defaultBaseURL`, not `RouterConstants.defaultBaseURL`.** Swift forbids an internal declaration in a *public* function's default argument. `RouterModels.defaultBaseURL` is a public re-export of the internal constant, so `check_router_contract.py` still pins the single source of truth to the spec's `servers[0]`; a test asserts the two are equal. - **Collect-loop tests stub `Retry-After: 1`, not `0`.** The plan's `Retry-After: 0` would not exercise a re-send at all: the contract declares a minimum of 1, and the already-shipped `RouterErrorMapping` deliberately reads anything below it as "no advice" (`retryAfter == nil`) so an unusable value can never become an immediate retry. `1` is the smallest value that exercises the path, which costs those three tests about a second each of real sleeping. That is the sleep under test. ## Verification - `swift build` — clean. - `swift test --no-parallel` — **305 tests in 32 suites passed, 0 failed** (24 of them new in `RouterRunTests`). `ImportBoundaryTests` and `LoggingPrivacyTests` both still pass; the new `SDKLog` router category never takes the key, the body or a credential as a parameter, so none can reach a log line. - `python3 Scripts/contract/check_router_contract.py` — OK, 15 error types and the bound route in sync with the spec. - `python3 Scripts/contract/check_contract.py --spec <ComfyUI master openapi.yaml> --manifest Scripts/contract/sdk-endpoints.yml` — 6 ok, 0 warnings, 0 errors, so the new `excluded:` entry did not disturb the ComfyUI gate. - **The acceptance snippet was compiled**, not eyeballed: `swiftc -typecheck` against the built module, clean. One note — the snippet as specified needs `import Foundation` alongside `import ComfySwiftSDK` for `ProcessInfo`, which the SDK does not re-export. The README copy includes it so a reader can paste it and have it build. ## Residual - **No live Router call was made.** Everything here is verified against the vendored `spec/router-openapi.yaml` and a `URLProtocol` stub. The contract is the same artifact CI pins to, but it is a document: the response *shapes* are asserted, the running service's behaviour is not. Anyone with a Router credential should run the acceptance snippet against `api.comfy.org` once before this is relied on in the iOS app — in particular to confirm that a real `504 deadline_exceeded` carries the `Retry-After` the collect loop requires, since a `504` *without* it is deliberately terminal here and would surface to users as a failed run rather than a collected one. - **The TypeScript SDK's `parseModelId` was not read.** The plan says the validation "mirrors" it; that repo is not in this sandbox, so the rules here were derived from the vendored spec's own `RouterModelId` / `RouterModelSegment` / `RouterProviderSegment` patterns instead. If the two implementations disagree, the spec is the tiebreaker and this is the one to re-check — most likely around whether the TS side rejects or silently truncates a third segment, and whether it enforces the schemas' lowercase-only `^[a-z0-9]+([._-][a-z0-9]+)*$` pattern, which **this implementation deliberately does not** (it percent-encodes and lets the server reject, so a catalog addition that widens the pattern is not blocked client-side by a stale copy of it). - **One new strict-concurrency warning, invisible in the default build.** Under `-strict-concurrency=complete` the `transport.withAuthRetry { ... }` call in `RouterTransport.swift` warns "sending value of non-Sendable type … risks causing data races". The package is `swift-tools-version: 5.9` (Swift 5 language mode), so it is not emitted by `swift build` or CI. It is safe in fact — the closure captures only value types plus an actor reference, and its body does nothing but call one actor-isolated method — but it *is* an error in Swift 6 language mode. There is exactly one other such warning in the package, pre-existing, on `Transport.swift:109` (`normalizeToken`), and both sit on the same helper. Whoever migrates this package to Swift 6 mode has to restructure the two together; doing it here would mean reshaping a shared helper every existing endpoint calls, which is out of scope for this PR. - **`RouterInputSerializationError` is internal but escapes through a public API** via `ComfyError.unknown(underlying:)`. A caller can read its `localizedDescription` but cannot `as?`-match the type. That is fine for the case it reports — a non-JSON `input` is a programming error fixed in the caller, not a condition to branch on — but if it ever needs branching, promoting it to public is the change. ## Provenance - **Authored by:** agent-work loop - **Verified:** `swift build` clean; `swift test --no-parallel` 305 passed / 0 failed in 32 suites; `check_router_contract.py` OK (15 error types, route in sync); `check_contract.py` 6 ok / 0 warnings / 0 errors; acceptance snippet `swiftc -typecheck` clean against the built module - **Deviations:** four, each stated with its reason under "Judgment calls" above — version bumped to `0.5.0` rather than the plan's already-released `0.4.0`; the three-segment model ID throws a prefixed variant-specific identifier rather than the bare one; `routerBaseURL:` defaults to the public `RouterModels.defaultBaseURL` because Swift forbids an internal declaration in a public default argument; collect-loop tests use `Retry-After: 1` because the contract's minimum is 1 and `0` exercises no re-send at all --- <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-26 22:34:37 +00:00

bsakaguchi referenced this issue

2026-10-26 22:34:37 +00:00

[PR #69] [MERGED] fix(router): resolve the four deferred review findings on the error surface #70

bsakaguchi closed this issue

2026-10-26 22:34:37 +00:00