[PR #50] [MERGED] fix: expose jobId and expiresAt on the public wrappers #60

New issue

Closed

opened 2026-10-31 22:04:12 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:04:12 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-typescript-sdk/pull/50
Author: @wei-hai
Created: 8/13/2026
Status: ✅ Merged
Merged: 8/13/2026
Merged by: @wei-hai

Base: main ← Head: wei/expose-job-id

📝 Commits (1)

9e88b40 fix: expose jobId and expiresAt on the public wrappers

📊 Changes

5 files changed (+40 additions, -1 deletions)

View changed files

📝 src/sdk/assets.test.ts (+9 -0)
📝 src/sdk/assets.ts (+14 -0)
📝 src/sdk/outputs.test.ts (+9 -0)
📝 src/sdk/outputs.ts (+5 -0)
📝 test/support/stub-server.ts (+3 -1)

📄 Description

The recent spec sync added job_id and expires_at to the generated types, but the hand-written wrapper classes never surfaced them. A caller couldn't reach the field the change exists to provide.

How it was found

End-to-end testing against Comfy Cloud staging and a self-hosted comfy-api-proxy, driving a real workflow. The value was correct on the wire from both, and unreachable through the public API.

Existing tests missed it because they assert on request and response bodies, never on the wrapper's surface — so the field was verified at the HTTP layer while being invisible to users.

The fix

jobId on the output wrapper, plus jobId and expiresAt on the asset. Asset doesn't hold the full server model — only id/hash/createdNew are pulled out in adopt() — so these follow that same cached-field pattern rather than a pass-through.

Typed string | undefined: genuinely absent for an uploaded asset with no producing job, and before commit(). Not faked.

Tests assert the wrapper, not the wire

That's the gap that let this ship. New tests cover output.jobId after a real job poll, undefined when the model omits it, and an uploaded asset's jobId/expiresAt being undefined before and after commit(). The stub server now stamps job_id onto polled outputs, which it never carried — so the Job → Output wiring is exercised rather than only a hand-built Output.

Verification

oxlint, oxfmt, tsc, spec-drift, hygiene and build all clean; 169 tests pass.

The Python SDK carries the identical fix, keeping the two in lockstep, and was additionally re-verified end to end against Cloud staging and a local ComfyUI through the proxy.

One typing note for a reviewer

expiresAt returns the raw ISO string, not a Date. The Date conversion elsewhere (getDownloadUrl()) is computed from response headers rather than a model field, so extending it here would have been a wider change than this fix warrants — worth a look if Date consistency across the SDK is wanted.

🤖 Generated with Claude Code

_{🔄 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/50 **Author:** [@wei-hai](https://github.com/wei-hai) **Created:** 8/13/2026 **Status:** ✅ Merged **Merged:** 8/13/2026 **Merged by:** [@wei-hai](https://github.com/wei-hai) **Base:** `main` ← **Head:** `wei/expose-job-id` --- ### 📝 Commits (1) - [`9e88b40`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/9e88b40681af89b6ddb7cb06de587f3073a5af7d) fix: expose jobId and expiresAt on the public wrappers ### 📊 Changes **5 files changed** (+40 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/sdk/assets.test.ts` (+9 -0) 📝 `src/sdk/assets.ts` (+14 -0) 📝 `src/sdk/outputs.test.ts` (+9 -0) 📝 `src/sdk/outputs.ts` (+5 -0) 📝 `test/support/stub-server.ts` (+3 -1) </details> ### 📄 Description The recent spec sync added `job_id` and `expires_at` to the generated types, but the hand-written wrapper classes never surfaced them. A caller couldn't reach the field the change exists to provide. ## How it was found End-to-end testing against **Comfy Cloud staging** and a **self-hosted comfy-api-proxy**, driving a real workflow. The value was correct on the wire from both, and unreachable through the public API. Existing tests missed it because they assert on request and response **bodies**, never on the wrapper's surface — so the field was verified at the HTTP layer while being invisible to users. ## The fix `jobId` on the output wrapper, plus `jobId` and `expiresAt` on the asset. `Asset` doesn't hold the full server model — only `id`/`hash`/`createdNew` are pulled out in `adopt()` — so these follow that same cached-field pattern rather than a pass-through. Typed `string | undefined`: genuinely absent for an uploaded asset with no producing job, and before `commit()`. Not faked. ## Tests assert the wrapper, not the wire That's the gap that let this ship. New tests cover `output.jobId` after a real job poll, `undefined` when the model omits it, and an uploaded asset's `jobId`/`expiresAt` being `undefined` before and after `commit()`. The stub server now stamps `job_id` onto polled outputs, which it never carried — so the `Job` → `Output` wiring is exercised rather than only a hand-built `Output`. ## Verification oxlint, oxfmt, tsc, spec-drift, hygiene and build all clean; **169 tests pass**. The Python SDK carries the identical fix, keeping the two in lockstep, and was additionally re-verified end to end against Cloud staging and a local ComfyUI through the proxy. ## One typing note for a reviewer `expiresAt` returns the raw ISO `string`, not a `Date`. The `Date` conversion elsewhere (`getDownloadUrl()`) is computed from response headers rather than a model field, so extending it here would have been a wider change than this fix warrants — worth a look if `Date` consistency across the SDK is wanted. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 22:04:12 +00:00