From bdb92c845e846d97a00e4ea1bec2d8fb9d2bbb22 Mon Sep 17 00:00:00 2001 From: Dante Date: Thu, 14 May 2026 13:13:16 +0900 Subject: [PATCH] fix: include share_id when importing published assets (FE-603) (#12055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *PR Created by the Glary-Bot Agent* https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778150003248989 FE-603 --- ## Summary Send `share_id` alongside `published_asset_ids` from `workflowShareService.importPublishedAssets`, and type / zod-validate the body against `ImportPublishedAssetsRequest` + `zImportPublishedAssetsRequest` from `@comfyorg/ingest-types`. This is a **parameter-schema change**, not a live bug fix. The original `BAD_REQUEST: share_id is required` failure is **no longer reproducible in production** — the backend rolled back the required-field enforcement in [BE-855](https://linear.app/comfyorg/issue/BE-855) (cloud [#3587](https://github.com/Comfy-Org/cloud/pull/3587) / [#3588](https://github.com/Comfy-Org/cloud/pull/3588)). Today the import succeeds without `share_id`. - Closes FE-603 ## Why we still ship this Frontend goes first so the backend can later re-tighten the contract without breaking shared-workflow imports a second time. Agreed sequence in [Slack](https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1778535349236419) — BE-6 was split into two sub-issues to make the chain explicit: 1. **[BE-898](https://linear.app/comfyorg/issue/BE-898)** (blocks FE-603) — BE makes `share_id` **optional**, validating when present. [cloud PR #3633](https://github.com/Comfy-Org/cloud/pull/3633), In Review. 2. **FE-603** (this PR) — FE sends `share_id`. 3. **[BE-899](https://linear.app/comfyorg/issue/BE-899)** (blocked by FE-603) — BE flips `share_id` back to **required** after FE-603 deploys. Steps 1 and 2 ship in order: BE-898 → FE-603 → BE-899. ## Changes - `workflowShareService.importPublishedAssets` now takes `shareId: string` and types the request body with `ImportPublishedAssetsRequest`. The body is parsed through `zImportPublishedAssetsRequest` before the network call so future contract drift surfaces as a typecheck or zod parse failure rather than a silent runtime 400. - `useSharedWorkflowUrlLoader.ts` threads `payload.shareId` (already in scope from `SharedWorkflowPayload`) into the import call. - Unit tests assert `share_id` is sent and that an empty `share_id` is rejected before fetch. ## Verification - `pnpm typecheck` — clean - `pnpm lint` — 0 errors / 0 warnings on changed files (3 pre-existing warnings in unrelated files) - `pnpm format` — applied - `pnpm vitest run` on both affected files — 34/34 passing ### Live in-browser smoke test (Vite dev server) Loaded the built module in the browser, intercepted `fetch`, and exercised the new code path. The new `importPublishedAssets` produces the correct wire format and zod rejects bad input before fetch: ```json // Happy path — sent to /api/assets/import (POST) { "published_asset_ids": ["pa-1", "pa-2", "pa-3"], "share_id": "share-abc" } ``` ```text // Empty share_id — zod rejects, fetch is never called [ { code: "too_small", minimum: 1, path: ["share_id"], message: "String must contain at least 1 character(s)" } ] fetchCallsBetweenAttempts: 0 ``` ## Why typecheck didn't catch the original miss `api.fetchApi(route: string, options?: RequestInit)` accepts the standard DOM `RequestInit`, so `body` is just `BodyInit | null`. Once the call site does `JSON.stringify({ ... })`, the inline object is erased into a string and TS has no schema to enforce. There was no `@ts-ignore` or `as any` — the generated types were simply never imported. This PR plugs that one call site; the same pattern should be applied wherever the frontend hits ingest endpoints (the broader hey-zod migration gap mentioned in #bug-dump). Reported in #bug-dump. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12055-fix-include-share_id-when-importing-published-assets-3596d73d365081c0a1c7e69102f5cfcc) by [Unito](https://www.unito.io) --------- Co-authored-by: Glary-Bot Co-authored-by: Alexander Brown --- .../useSharedWorkflowUrlLoader.test.ts | 4 +- .../composables/useSharedWorkflowUrlLoader.ts | 3 +- .../services/workflowShareService.test.ts | 41 ++++++++++++++++--- .../sharing/services/workflowShareService.ts | 14 ++++++- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.test.ts b/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.test.ts index ebaff54b6c..67e75af908 100644 --- a/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.test.ts +++ b/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.test.ts @@ -244,7 +244,7 @@ describe('useSharedWorkflowUrlLoader', () => { const { loadSharedWorkflowFromUrl } = useSharedWorkflowUrlLoader() await loadSharedWorkflowFromUrl() - expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1']) + expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1'], 'share-id-1') }) it('does not call import when user chooses open-only', async () => { @@ -348,7 +348,7 @@ describe('useSharedWorkflowUrlLoader', () => { const { loadSharedWorkflowFromUrl } = useSharedWorkflowUrlLoader() await loadSharedWorkflowFromUrl() - expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1']) + expect(mockImportPublishedAssets).toHaveBeenCalledWith(['a1'], 'share-id-1') }) it('restores preserved share query before loading', async () => { diff --git a/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.ts b/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.ts index f840cf4a9b..e7acdb13b1 100644 --- a/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.ts +++ b/src/platform/workflow/sharing/composables/useSharedWorkflowUrlLoader.ts @@ -161,7 +161,8 @@ export function useSharedWorkflowUrlLoader() { if (result.action === 'copy-and-open' && nonOwnedAssets.length > 0) { try { await workflowShareService.importPublishedAssets( - nonOwnedAssets.map((a) => a.id) + nonOwnedAssets.map((a) => a.id), + payload.shareId ) } catch (importError) { console.error( diff --git a/src/platform/workflow/sharing/services/workflowShareService.test.ts b/src/platform/workflow/sharing/services/workflowShareService.test.ts index 67371d78fb..98a2f3ff6a 100644 --- a/src/platform/workflow/sharing/services/workflowShareService.test.ts +++ b/src/platform/workflow/sharing/services/workflowShareService.test.ts @@ -334,16 +334,45 @@ describe(useWorkflowShareService, () => { ) }) - it('imports published assets via POST /assets/import', async () => { + it('imports published assets via POST /assets/import with share_id', async () => { mockFetchApi.mockResolvedValue(mockJsonResponse({}, true, 200)) const service = useWorkflowShareService() - await service.importPublishedAssets(['pa-1', 'pa-2']) + await service.importPublishedAssets(['pa-1', 'pa-2'], 'share-id-1') expect(mockFetchApi).toHaveBeenCalledWith('/assets/import', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ published_asset_ids: ['pa-1', 'pa-2'] }) + body: JSON.stringify({ + published_asset_ids: ['pa-1', 'pa-2'], + share_id: 'share-id-1' + }) + }) + }) + + it('omits share_id from the payload when not provided', async () => { + mockFetchApi.mockResolvedValue(mockJsonResponse({}, true, 200)) + + const service = useWorkflowShareService() + await service.importPublishedAssets(['pa-1']) + + expect(mockFetchApi).toHaveBeenCalledWith('/assets/import', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ published_asset_ids: ['pa-1'] }) + }) + }) + + it('omits share_id from the payload when shareId is an empty string', async () => { + mockFetchApi.mockResolvedValue(mockJsonResponse({}, true, 200)) + + const service = useWorkflowShareService() + await service.importPublishedAssets(['pa-1'], '') + + expect(mockFetchApi).toHaveBeenCalledWith('/assets/import', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ published_asset_ids: ['pa-1'] }) }) }) @@ -352,9 +381,9 @@ describe(useWorkflowShareService, () => { const service = useWorkflowShareService() - await expect(service.importPublishedAssets(['bad-id'])).rejects.toThrow( - 'Failed to import assets: 400' - ) + await expect( + service.importPublishedAssets(['bad-id'], 'share-id-1') + ).rejects.toThrow('Failed to import assets: 400') }) it('throws when shared workflow payload is invalid', async () => { diff --git a/src/platform/workflow/sharing/services/workflowShareService.ts b/src/platform/workflow/sharing/services/workflowShareService.ts index 284a860e19..f0cb9d81af 100644 --- a/src/platform/workflow/sharing/services/workflowShareService.ts +++ b/src/platform/workflow/sharing/services/workflowShareService.ts @@ -1,3 +1,5 @@ +import type { ImportPublishedAssetsRequest } from '@comfyorg/ingest-types' + import type { PublishPrefill, SharedWorkflowPayload, @@ -255,11 +257,19 @@ export function useWorkflowShareService() { return workflow } - async function importPublishedAssets(assetIds: string[]): Promise { + async function importPublishedAssets( + assetIds: string[], + shareId?: string + ): Promise { + const body: ImportPublishedAssetsRequest = { + published_asset_ids: assetIds, + ...(shareId ? { share_id: shareId } : {}) + } + const response = await api.fetchApi('/assets/import', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ published_asset_ids: assetIds }) + body: JSON.stringify(body) }) if (!response.ok) {