[PR #187] feat: expose refusalSubject on RouterError and ComfyError from X-Comfy-Refusal-Subject #186

New issue

Open

opened 2026-10-31 22:05:29 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:05:29 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/187
Author: @mattmillerai
Created: 9/25/2026
Status: 🔄 Open

Base: main ← Head: be-17174-refusal-subject

📝 Commits (10+)

0cff534 feat(routerErrors): expose refusalSubject on RouterError from X-Comfy-Refusal-Subject
ee4fb2a fix(routerErrors): normalize refusalSubject, null when absent, and guard its unpinned wire names
916e75d fix(routerErrors): read a repeated identical refusal subject, and tighten its tests
d7c4706 Merge origin/main into be-17174-refusal-subject
64eb3af Merge origin/main into be-17174-refusal-subject
cdb627a docs(routerErrors): say status()/events() report a refused completion as data
a349941 feat(models): carry refusalSubject on ComfyError so models.run names the refused input/output (#191)
ffde40d Merge origin/main into be-17174-refusal-subject
6ee1919 fix(parity): refresh the Python snapshot and retire the QueueBacklogFull lead
fa834c0 test(router-spec): narrow the parsed Router spec instead of asserting its shape

📊 Changes

11 files changed (+594 additions, -19 deletions)

View changed files

📝 CHANGELOG.md (+19 -0)
📝 parity/python-surface.json (+6 -3)
📝 src/sdk/exceptions.test.ts (+9 -0)
📝 src/sdk/exceptions.ts (+25 -0)
📝 src/sdk/models.test.ts (+140 -0)
📝 src/sdk/models.ts (+17 -3)
📝 src/sdk/router-spec-contract.test.ts (+107 -0)
📝 src/sdk/routerErrors.test.ts (+156 -0)
📝 src/sdk/routerErrors.ts (+112 -0)
📝 src/sdk/surface-parity.test.ts (+0 -11)
📝 test/support/router-stub-server.ts (+3 -2)

📄 Description

What

A routerErrors.ContentPolicyViolation, and the ComfyError that comfy.models.run throws for content_policy_violation, now say which input or output the provider refused, so a caller can change that one part and retry rather than give up:

try {
  return await comfy.models.subscribe(model, input);
} catch (err) {
  if (err instanceof routerErrors.ContentPolicyViolation && err.refusalSubject === "output_audio") {
    return comfy.models.subscribe(model, { ...input, generate_audio: false });
  }
  throw err;
}

routerErrors.* is raised by the queued surface: submit, and a refused completion from subscribe or RequestHandle.get() (status() and events() report a completion as data on QueueUpdate and do not throw). comfy.models.run raises a ComfyError (code: "content_policy_violation"), which now carries the same value on ComfyError.refusalSubject (merged in from the stacked #191).

RouterErrorOptions.refusalSubject?: string | null and RouterError.refusalSubject: string | null. null means "not named", matching requestId, httpStatus and retryAfter. It sits on the base class, like retryAfter, so an error with a bucket this release doesn't recognize still keeps it.
toRouterError reads X-Comfy-Refusal-Subject first, then the body's refusal_subject. Both values are trimmed. A blank value names no subject and falls through, the same way an empty X-Comfy-Error-Type does. So does a value that joins two different subjects with a comma (Headers.get joins a repeated header that way). A repeat of the same subject (input_image, input_image) reads as that subject. A non-string body value is ignored. errorFromCompletion goes through the same builder, so a queued completion body that carries refusal_subject also sets it.
New exports: REFUSAL_SUBJECT_HEADER, REFUSAL_SUBJECTS (the ten values the Router contract documents: input, output, input_text, input_image, input_video, input_audio, output_text, output_image, output_video, output_audio) and type RefusalSubject. The property itself is not narrowed to that union. This follows the same policy as errorType: a newer server may send a value this release has never seen, so it passes through unchanged, and the doc tells callers to treat an unknown value as unspecified.
ComfyErrorOptions.refusalSubject?: string | null and ComfyError.refusalSubject: string | null, on the base class. models.ts's errorFromResponse reads it header first, body second, through the same readRefusalSubject normalizer toRouterError uses (exported from routerErrors.ts, as parseRetryAfter is). The read is not gated on code.
errorFromResponse now looks up the error class with Object.hasOwn(BY_ERROR_TYPE, code), as toRouterError already does. Before, an X-Comfy-Error-Type of constructor threw a String wrapper instead of a ComfyError, and toString/__proto__ raised a bare TypeError.
Changelog entries under Unreleased/Added.

Tests

routerErrors.test.ts (new refusalSubject block) covers:

the header wins over the body
body only
absent → null (both from the factory and from direct construction)
an unknown value passes through, from the header or the body
an empty or blank header falls through to the body, and a value is trimmed
a repeated header, which arrives comma-joined, names no subject when the parts differ, and names the subject when they repeat it
a non-string body value is ignored
a bucket this release doesn't recognize keeps it
the 422 detail[] validation shape never sets it
a queued completion body sets it

models.test.ts covers models.run against the Router stub server: header, body fallback, header precedence, absent → null, and a server-supplied bucket named after an Object.prototype key still yields a ComfyError. exceptions.test.ts covers the constructor default and pass-through. router-spec-contract.test.ts pins REFUSAL_SUBJECT_HEADER, the body's refusal_subject and REFUSAL_SUBJECTS against spec/router-openapi.yaml (see Residual).

Judgment calls / deviations

No surface-parity allowlist entry. surface-parity.test.ts compares class, method and error_type names, and the Python surface snapshot doesn't capture instance attributes. So the new property creates no parity difference, and nothing retires an entry once the Python twin lands. An earlier revision added an entry with only a why. Review dropped it, and the CHANGELOG now records the lead instead. I did not build attribute extraction into the parity machinery, because that would be a design change the ticket doesn't ask for.
Where the header is read. The ticket says to read the header "wherever the factory reads X-Comfy-Upstream-Status". This SDK never reads that header, so the read goes into toRouterError, the one place every Router error response is built.
The ten values come from the Router contract's prose. The contract lists them in the RouterRefusalSubjectHeader / RouterErrorResponse.refusal_subject descriptions and declares no enum, so the spec pin parses the backticked list after "closed vocabulary:" in both descriptions.

Residual

The spec sync landed on main first. The Router sync #199 merged before this PR and brought in RouterErrorResponse.refusal_subject and RouterRefusalSubjectHeader, which fired the rot guard this branch carried, as intended. Merging main replaced that guard with a real pin in src/sdk/router-spec-contract.test.ts. It checks that every response referencing RouterRefusalSubjectHeader names it REFUSAL_SUBJECT_HEADER, that RouterErrorResponse declares a string refusal_subject, and that REFUSAL_SUBJECTS matches the closed vocabulary in both descriptions, in order.
The Python SDK twin is not done. comfy-python-sdk has no refusal_subject yet. No parity entry tracks it (see above).
Not exercised against a live Router. The new field was verified only against stubbed responses (Response/Headers objects in unit tests). No real content_policy_violation carrying the header was triggered: that needs a provider that emits a machine-readable refusal code, and a real paid generation.
Not read by this PR: the parent epic about Router disclosing the refused subject, and the upstream investigation this work was scoped from. Only their titles were available.

Provenance

Authored by: agent-work loop
Verified: at head 6538606 (this branch merged with main at 5f65833, which cut 0.5.0; this PR's changelog entries now sit in the new Unreleased section):
- pnpm lint, pnpm format:check, pnpm typecheck and pnpm check:spec-drift pass locally.
- pnpm test: 818 passed, 5 skipped.
- The new refusal-subject spec pin was shown to fail when REFUSAL_SUBJECTS is altered; it narrows the parsed spec at runtime instead of asserting its shape.
- pnpm check:sdk-parity passed against the refreshed Python snapshot (locally at fa834c0; the sdk-parity CI job passed at 6538606).
- CI on 6538606 is green: Test (node 22 and 24), build-check, sdk-parity and public-repo-hygiene all pass.
Deviations:
- No parity entry for the lead over the Python SDK (see Judgment calls).
- The REFUSAL_SUBJECTS pin parses the contract's prose, because the contract declares no enum.
- Also re-syncs parity/python-surface.json and retires the QueueBacklogFull lead now that the Python SDK has it, which sdk-parity needed in order to pass.
- Scope grew past RouterError: #191's ComfyError.refusalSubject and the Object.hasOwn lookup guard were merged into this branch.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Router errors and models.run content-policy errors now expose a refusal subject, using the response header first and the response body as a fallback. The value is null when no usable subject is provided.
- Added documented refusal-subject values. Unknown values are preserved as strings and treated as unspecified.
Documentation
- Clarified that refusal subjects are available for both queued router errors and models.run content-policy errors.

_{🔄 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-typescript-sdk/pull/187 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/25/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `be-17174-refusal-subject` --- ### 📝 Commits (10+) - [`0cff534`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/0cff534421b6a01be0ce060be18732d86ef2717c) feat(routerErrors): expose refusalSubject on RouterError from X-Comfy-Refusal-Subject - [`ee4fb2a`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/ee4fb2abe63461d9df02d18108c5e76d4a636b27) fix(routerErrors): normalize refusalSubject, null when absent, and guard its unpinned wire names - [`916e75d`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/916e75d6f2b029e5d4743934e8196797f3317762) fix(routerErrors): read a repeated identical refusal subject, and tighten its tests - [`d7c4706`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/d7c4706458e88ddef42463f22f89767218d3cbda) Merge origin/main into be-17174-refusal-subject - [`64eb3af`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/64eb3afb04b059497a44074b2e408149d67e4d68) Merge origin/main into be-17174-refusal-subject - [`cdb627a`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/cdb627ab2d1c32074110e5720bcf368ffebf4566) docs(routerErrors): say status()/events() report a refused completion as data - [`a349941`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/a349941e60d8fef9a4b3de708ebb2842f28d00ab) feat(models): carry refusalSubject on ComfyError so models.run names the refused input/output (#191) - [`ffde40d`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/ffde40d49a57687acf4801ab14bdb9ccf9d6060e) Merge origin/main into be-17174-refusal-subject - [`6ee1919`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/6ee19190290b3b70fe358a194f8d540e2ae2128e) fix(parity): refresh the Python snapshot and retire the QueueBacklogFull lead - [`fa834c0`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/fa834c09eb0195c940185f9ac19c4d1a5d647057) test(router-spec): narrow the parsed Router spec instead of asserting its shape ### 📊 Changes **11 files changed** (+594 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+19 -0) 📝 `parity/python-surface.json` (+6 -3) 📝 `src/sdk/exceptions.test.ts` (+9 -0) 📝 `src/sdk/exceptions.ts` (+25 -0) 📝 `src/sdk/models.test.ts` (+140 -0) 📝 `src/sdk/models.ts` (+17 -3) 📝 `src/sdk/router-spec-contract.test.ts` (+107 -0) 📝 `src/sdk/routerErrors.test.ts` (+156 -0) 📝 `src/sdk/routerErrors.ts` (+112 -0) 📝 `src/sdk/surface-parity.test.ts` (+0 -11) 📝 `test/support/router-stub-server.ts` (+3 -2) </details> ### 📄 Description ## What A `routerErrors.ContentPolicyViolation`, and the `ComfyError` that `comfy.models.run` throws for `content_policy_violation`, now say **which** input or output the provider refused, so a caller can change that one part and retry rather than give up: ```ts try { return await comfy.models.subscribe(model, input); } catch (err) { if (err instanceof routerErrors.ContentPolicyViolation && err.refusalSubject === "output_audio") { return comfy.models.subscribe(model, { ...input, generate_audio: false }); } throw err; } ``` `routerErrors.*` is raised by the queued surface: `submit`, and a refused completion from `subscribe` or `RequestHandle.get()` (`status()` and `events()` report a completion as data on `QueueUpdate` and do not throw). `comfy.models.run` raises a `ComfyError` (`code: "content_policy_violation"`), which now carries the same value on `ComfyError.refusalSubject` (merged in from the stacked #191). - `RouterErrorOptions.refusalSubject?: string | null` and `RouterError.refusalSubject: string | null`. `null` means "not named", matching `requestId`, `httpStatus` and `retryAfter`. It sits on the **base** class, like `retryAfter`, so an error with a bucket this release doesn't recognize still keeps it. - `toRouterError` reads `X-Comfy-Refusal-Subject` first, then the body's `refusal_subject`. Both values are trimmed. A blank value names no subject and falls through, the same way an empty `X-Comfy-Error-Type` does. So does a value that joins two *different* subjects with a comma (`Headers.get` joins a repeated header that way). A repeat of the *same* subject (`input_image, input_image`) reads as that subject. A non-string body value is ignored. `errorFromCompletion` goes through the same builder, so a queued completion body that carries `refusal_subject` also sets it. - New exports: `REFUSAL_SUBJECT_HEADER`, `REFUSAL_SUBJECTS` (the ten values the Router contract documents: `input`, `output`, `input_text`, `input_image`, `input_video`, `input_audio`, `output_text`, `output_image`, `output_video`, `output_audio`) and `type RefusalSubject`. The property itself is **not** narrowed to that union. This follows the same policy as `errorType`: a newer server may send a value this release has never seen, so it passes through unchanged, and the doc tells callers to treat an unknown value as unspecified. - `ComfyErrorOptions.refusalSubject?: string | null` and `ComfyError.refusalSubject: string | null`, on the base class. `models.ts`'s `errorFromResponse` reads it header first, body second, through the same `readRefusalSubject` normalizer `toRouterError` uses (exported from `routerErrors.ts`, as `parseRetryAfter` is). The read is not gated on `code`. - `errorFromResponse` now looks up the error class with `Object.hasOwn(BY_ERROR_TYPE, code)`, as `toRouterError` already does. Before, an `X-Comfy-Error-Type` of `constructor` threw a `String` wrapper instead of a `ComfyError`, and `toString`/`__proto__` raised a bare `TypeError`. - Changelog entries under Unreleased/Added. ## Tests `routerErrors.test.ts` (new `refusalSubject` block) covers: - the header wins over the body - body only - absent → `null` (both from the factory and from direct construction) - an unknown value passes through, from the header or the body - an empty or blank header falls through to the body, and a value is trimmed - a repeated header, which arrives comma-joined, names no subject when the parts differ, and names the subject when they repeat it - a non-string body value is ignored - a bucket this release doesn't recognize keeps it - the 422 `detail[]` validation shape never sets it - a queued completion body sets it `models.test.ts` covers `models.run` against the Router stub server: header, body fallback, header precedence, absent → `null`, and a server-supplied bucket named after an `Object.prototype` key still yields a `ComfyError`. `exceptions.test.ts` covers the constructor default and pass-through. `router-spec-contract.test.ts` pins `REFUSAL_SUBJECT_HEADER`, the body's `refusal_subject` and `REFUSAL_SUBJECTS` against `spec/router-openapi.yaml` (see Residual). ## Judgment calls / deviations - **No surface-parity allowlist entry.** `surface-parity.test.ts` compares class, method and `error_type` **names**, and the Python surface snapshot doesn't capture instance attributes. So the new property creates no parity difference, and nothing retires an entry once the Python twin lands. An earlier revision added an entry with only a `why`. Review dropped it, and the CHANGELOG now records the lead instead. I did **not** build attribute extraction into the parity machinery, because that would be a design change the ticket doesn't ask for. - **Where the header is read.** The ticket says to read the header "wherever the factory reads `X-Comfy-Upstream-Status`". This SDK never reads that header, so the read goes into `toRouterError`, the one place every Router error response is built. - **The ten values come from the Router contract's prose.** The contract lists them in the `RouterRefusalSubjectHeader` / `RouterErrorResponse.refusal_subject` descriptions and declares no `enum`, so the spec pin parses the backticked list after "closed vocabulary:" in both descriptions. ## Residual - **The spec sync landed on main first.** The Router sync #199 merged before this PR and brought in `RouterErrorResponse.refusal_subject` and `RouterRefusalSubjectHeader`, which fired the rot guard this branch carried, as intended. Merging main replaced that guard with a real pin in `src/sdk/router-spec-contract.test.ts`. It checks that every response referencing `RouterRefusalSubjectHeader` names it `REFUSAL_SUBJECT_HEADER`, that `RouterErrorResponse` declares a string `refusal_subject`, and that `REFUSAL_SUBJECTS` matches the closed vocabulary in both descriptions, in order. - **The Python SDK twin is not done.** `comfy-python-sdk` has no `refusal_subject` yet. No parity entry tracks it (see above). - **Not exercised against a live Router.** The new field was verified only against stubbed responses (`Response`/`Headers` objects in unit tests). No real `content_policy_violation` carrying the header was triggered: that needs a provider that emits a machine-readable refusal code, and a real paid generation. - **Not read by this PR:** the parent epic about Router disclosing the refused subject, and the upstream investigation this work was scoped from. Only their titles were available. ## Provenance - **Authored by:** agent-work loop - **Verified:** at head 6538606 (this branch merged with main at 5f65833, which cut 0.5.0; this PR's changelog entries now sit in the new Unreleased section): - `pnpm lint`, `pnpm format:check`, `pnpm typecheck` and `pnpm check:spec-drift` pass locally. - `pnpm test`: 818 passed, 5 skipped. - The new refusal-subject spec pin was shown to fail when `REFUSAL_SUBJECTS` is altered; it narrows the parsed spec at runtime instead of asserting its shape. - `pnpm check:sdk-parity` passed against the refreshed Python snapshot (locally at fa834c0; the `sdk-parity` CI job passed at 6538606). - CI on 6538606 is green: Test (node 22 and 24), build-check, sdk-parity and public-repo-hygiene all pass. - **Deviations:** - No parity entry for the lead over the Python SDK (see Judgment calls). - The `REFUSAL_SUBJECTS` pin parses the contract's prose, because the contract declares no `enum`. - Also re-syncs `parity/python-surface.json` and retires the `QueueBacklogFull` lead now that the Python SDK has it, which `sdk-parity` needed in order to pass. - Scope grew past `RouterError`: #191's `ComfyError.refusalSubject` and the `Object.hasOwn` lookup guard were merged into this branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **New Features** * Router errors and `models.run` content-policy errors now expose a refusal subject, using the response header first and the response body as a fallback. The value is `null` when no usable subject is provided. * Added documented refusal-subject values. Unknown values are preserved as strings and treated as unspecified. * **Documentation** * Clarified that refusal subjects are available for both queued router errors and `models.run` content-policy errors.  --- <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:05:29 +00:00

bsakaguchi referenced this issue

2026-10-31 22:05:29 +00:00

[PR #186] [CLOSED] chore: sync vendored Comfy Router spec (cloud@bfad4ea) #188