[PR #48] [CLOSED] feat: add asset deletion #57

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/48
Author: @wei-hai
Created: 8/13/2026
Status: ❌ Closed

Base: main ← Head: wei/asset-delete

📝 Commits (1)

01552f0 feat: add asset deletion

📊 Changes

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

View changed files

📝 src/low/transport.test.ts (+24 -0)
📝 src/low/transport.ts (+16 -0)
📝 src/sdk/assets.test.ts (+29 -1)
📝 src/sdk/assets.ts (+19 -0)
📝 test/support/stub-server.ts (+40 -0)

📄 Description

DELETE /api/v2/assets/{id} is live server-side and the Python SDK is adding support, but this SDK had none — a gap between two clients kept deliberately in lockstep.

Surface, mirroring Python


`ComfyLow.deleteAsset(id, { signal })`	the transport call
`Asset.delete(signal?)`	delete the asset a handle already points to
`assets.delete(id, signal?)`	delete by id, no fetch first

The sdk-layer methods take signal positionally, matching the existing commit(signal?) / asReference(signal?) convention rather than the low layer's options object. Deleting an uncommitted handle throws without touching the network, and a successful delete clears the handle's id.

Error mapping — existing types only

Checked the canonical contract rather than guessing: 204 on success, 404 asset_not_found for an unknown or repeat delete, 409 asset_in_use when a workflow version still references it.

404 already maps to NotFound. 409 has no dedicated class, and I didn't add one — it surfaces through the existing BY_CODE[code] ?? ApiError fallback as a typed ApiError with code === "asset_in_use" and httpStatus === 409, which is inspectable without inventing a class for a single status.

Why hand-written

This repo's vendored spec/openapi.yaml doesn't describe the operation yet — the sync carrying it is open as #47. Registering deleteAsset in OPERATION_IDS now would break spec-coverage.test.ts, which asserts exact parity with the current vendored spec. So the call is hand-written, spec/ is untouched, and a comment marks it to move onto the generated client on the next sync — same shape as the job-workflow call.

Verification

oxlint, oxfmt --check, tsc --noEmit, spec-drift, public-repo hygiene, and build all clean; 159 tests pass.

Tests use the repo's stub server rather than mocking the transport, and cover a successful delete (with the follow-up GET then 404ing), a repeat delete surfacing NotFound, an in-use delete surfacing the typed 409, deleting by id without fetching, and the uncommitted-handle guard.

🤖 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/48 **Author:** [@wei-hai](https://github.com/wei-hai) **Created:** 8/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `wei/asset-delete` --- ### 📝 Commits (1) - [`01552f0`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/01552f0a811304f86a2434f0e2a3505ba75e0adf) feat: add asset deletion ### 📊 Changes **5 files changed** (+128 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/low/transport.test.ts` (+24 -0) 📝 `src/low/transport.ts` (+16 -0) 📝 `src/sdk/assets.test.ts` (+29 -1) 📝 `src/sdk/assets.ts` (+19 -0) 📝 `test/support/stub-server.ts` (+40 -0) </details> ### 📄 Description `DELETE /api/v2/assets/{id}` is live server-side and the Python SDK is adding support, but this SDK had none — a gap between two clients kept deliberately in lockstep. ## Surface, mirroring Python | | | |---|---| | `ComfyLow.deleteAsset(id, { signal })` | the transport call | | `Asset.delete(signal?)` | delete the asset a handle already points to | | `assets.delete(id, signal?)` | delete by id, no fetch first | The `sdk`-layer methods take `signal` positionally, matching the existing `commit(signal?)` / `asReference(signal?)` convention rather than the low layer's options object. Deleting an uncommitted handle throws without touching the network, and a successful delete clears the handle's id. ## Error mapping — existing types only Checked the canonical contract rather than guessing: `204` on success, `404 asset_not_found` for an unknown or repeat delete, `409 asset_in_use` when a workflow version still references it. `404` already maps to `NotFound`. `409` has no dedicated class, and I didn't add one — it surfaces through the existing `BY_CODE[code] ?? ApiError` fallback as a typed `ApiError` with `code === "asset_in_use"` and `httpStatus === 409`, which is inspectable without inventing a class for a single status. ## Why hand-written This repo's vendored `spec/openapi.yaml` doesn't describe the operation yet — the sync carrying it is open as #47. Registering `deleteAsset` in `OPERATION_IDS` now would break `spec-coverage.test.ts`, which asserts exact parity with the *current* vendored spec. So the call is hand-written, `spec/` is untouched, and a comment marks it to move onto the generated client on the next sync — same shape as the job-workflow call. ## Verification `oxlint`, `oxfmt --check`, `tsc --noEmit`, spec-drift, public-repo hygiene, and build all clean; **159 tests pass**. Tests use the repo's stub server rather than mocking the transport, and cover a successful delete (with the follow-up `GET` then 404ing), a repeat delete surfacing `NotFound`, an in-use delete surfacing the typed 409, deleting by id without fetching, and the uncommitted-handle guard. 🤖 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