From a2a5c2ddb857e4e9707bd00ed4ee3b6de9075aa5 Mon Sep 17 00:00:00 2001 From: dante01yoon Date: Thu, 28 May 2026 15:10:07 +0900 Subject: [PATCH] fix(assets): invalidate caches and exclude missing assets on per-output delete Codex adversarial review on PR #12503 surfaced two real follow-up gaps: 1. `findOutputAssetIdByHash` queried without `exclude_tags=missing`, so a soft-deleted asset that still carries the same hash could shadow the live output we want to delete. Mirror the exclusion every other asset query uses. 2. After a successful per-output cloud delete, only `assetsStore.updateHistory()` refreshes. The `jobOutputCache` (job-detail + task LRU) and the `useOutputStacks` per-job children cache both still served the deleted child, leaving the row visible and selectable in the sidebar until the user collapsed the stack. Expose `invalidateJobOutputs(jobId)` from the cache module, call it from `deleteAssets` for every fulfilled output delete, and have `useOutputStacks` cross-check cached children against the cover's refreshed `allOutputs` so stale children disappear from the rendered list immediately. Adds two regression tests: cache invalidation is asserted on the cloud delete path, and `useOutputStacks` prunes a child whose filename drops off the cover's allOutputs. --- .../composables/useMediaAssetActions.test.ts | 10 +++ .../composables/useMediaAssetActions.ts | 11 ++++ .../composables/useOutputStacks.test.ts | 61 +++++++++++++++++++ .../assets/composables/useOutputStacks.ts | 23 ++++++- src/platform/assets/services/assetService.ts | 1 + src/services/jobOutputCache.ts | 9 +++ 6 files changed, 114 insertions(+), 1 deletion(-) diff --git a/src/platform/assets/composables/useMediaAssetActions.test.ts b/src/platform/assets/composables/useMediaAssetActions.test.ts index c5a3cddd9b..462e5b7c6f 100644 --- a/src/platform/assets/composables/useMediaAssetActions.test.ts +++ b/src/platform/assets/composables/useMediaAssetActions.test.ts @@ -174,6 +174,11 @@ vi.mock('@/scripts/api', () => ({ } })) +const mockInvalidateJobOutputs = vi.hoisted(() => vi.fn()) +vi.mock('@/services/jobOutputCache', () => ({ + invalidateJobOutputs: mockInvalidateJobOutputs +})) + const mockAppGraph = vi.hoisted(() => ({ value: { _nodes: [] as unknown[] } })) vi.mock('@/scripts/app', () => ({ app: { @@ -1084,6 +1089,7 @@ describe('useMediaAssetActions', () => { it('routes cloud output deletion through assetService.deleteAsset using the resolved UUID', async () => { mockIsCloud.value = true + mockGetOutputAssetMetadata.mockReturnValue({ jobId: 'job-abc' }) mockFindOutputAssetIdByHash.mockResolvedValue( 'real-output-asset-uuid-1234' ) @@ -1111,6 +1117,10 @@ describe('useMediaAssetActions', () => { const { api } = await import('@/scripts/api') expect(api.deleteItem).not.toHaveBeenCalled() + + // Stack/folder/job-detail caches must be invalidated so the deleted + // child does not linger in the rendered list after success. + expect(mockInvalidateJobOutputs).toHaveBeenCalledWith('job-abc') }) it('falls back to history delete in OSS mode (no per-asset endpoint available)', async () => { diff --git a/src/platform/assets/composables/useMediaAssetActions.ts b/src/platform/assets/composables/useMediaAssetActions.ts index 8827c3ce7b..9de1a9fbde 100644 --- a/src/platform/assets/composables/useMediaAssetActions.ts +++ b/src/platform/assets/composables/useMediaAssetActions.ts @@ -14,6 +14,7 @@ import { app } from '@/scripts/app' import { useLitegraphService } from '@/services/litegraphService' import { useNodeDefStore } from '@/stores/nodeDefStore' import { getOutputAssetMetadata } from '../schemas/assetMetadataSchema' +import { invalidateJobOutputs } from '@/services/jobOutputCache' import { useAssetsStore } from '@/stores/assetsStore' import { useDialogStore } from '@/stores/dialogStore' import { useNodeOutputStore } from '@/stores/nodeOutputStore' @@ -684,6 +685,16 @@ export function useMediaAssetActions() { ) if (hasOutputAssets) { + // Drop cached job-detail/task entries so the next stack + // expand or folder open sees the post-delete output list. + assetArray.forEach((asset, index) => { + if (results[index].status !== 'fulfilled') return + if (getAssetType(asset) !== 'output') return + const jobId = + getOutputAssetMetadata(asset.user_metadata)?.jobId ?? + asset.id + if (jobId) invalidateJobOutputs(jobId) + }) await assetsStore.updateHistory() } if (hasInputAssets) { diff --git a/src/platform/assets/composables/useOutputStacks.test.ts b/src/platform/assets/composables/useOutputStacks.test.ts index 3b89952622..4ebb835ea3 100644 --- a/src/platform/assets/composables/useOutputStacks.test.ts +++ b/src/platform/assets/composables/useOutputStacks.test.ts @@ -202,4 +202,65 @@ describe('useOutputStacks', () => { child.id ]) }) + + it('drops cached children whose filename is no longer in the cover.allOutputs (FE-814)', async () => { + const childKept = createAsset({ + id: 'child-keep', + name: 'keep.png', + user_metadata: undefined + }) + const childDeleted = createAsset({ + id: 'child-gone', + name: 'gone.png', + user_metadata: undefined + }) + + vi.mocked(mocks.resolveOutputAssetItems).mockResolvedValue([ + childKept, + childDeleted + ]) + + const initialCover = createAsset({ + id: 'parent', + name: 'parent.png', + user_metadata: { + jobId: 'job-1', + nodeId: 'node-1', + subfolder: 'outputs', + allOutputs: [ + { filename: 'parent.png' }, + { filename: 'keep.png' }, + { filename: 'gone.png' } + ] + } + }) + const assets = ref([initialCover]) + + const { assetItems, toggleStack } = useOutputStacks({ assets }) + await toggleStack(initialCover) + + expect(assetItems.value.map((i) => i.asset.id)).toEqual([ + initialCover.id, + childKept.id, + childDeleted.id + ]) + + // Simulate a successful per-output delete: updateHistory() refreshes the + // cover with a shorter allOutputs. The previously cached child whose + // filename is no longer present must vanish from the rendered list. + assets.value = [ + createAsset({ + ...initialCover, + user_metadata: { + ...initialCover.user_metadata!, + allOutputs: [{ filename: 'parent.png' }, { filename: 'keep.png' }] + } + }) + ] + + expect(assetItems.value.map((i) => i.asset.id)).toEqual([ + 'parent', + childKept.id + ]) + }) }) diff --git a/src/platform/assets/composables/useOutputStacks.ts b/src/platform/assets/composables/useOutputStacks.ts index 0651cc0f86..823fef8747 100644 --- a/src/platform/assets/composables/useOutputStacks.ts +++ b/src/platform/assets/composables/useOutputStacks.ts @@ -7,6 +7,7 @@ import { getOutputKey, resolveOutputAssetItems } from '@/platform/assets/utils/outputAssetUtil' +import type { ResultItemImpl } from '@/stores/queueStore' export type OutputStackListItem = { key: string @@ -38,7 +39,8 @@ export function useOutputStacks({ assets }: UseOutputStacksOptions) { } const children = stackChildrenByJobId.value[jobId] ?? [] - for (const child of children) { + const filteredChildren = filterChildrenAgainstCover(asset, children) + for (const child of filteredChildren) { items.push({ key: `asset-${child.id}`, asset: child, @@ -59,6 +61,25 @@ export function useOutputStacks({ assets }: UseOutputStacksOptions) { return metadata?.jobId ?? null } + // Drops cached children whose underlying output is no longer present on the + // cover's refreshed `allOutputs` list. Lets per-output deletes disappear + // immediately after `updateHistory()` re-fetches the cover, without waiting + // for the user to collapse/expand the stack. + function filterChildrenAgainstCover( + cover: AssetItem, + children: AssetItem[] + ): AssetItem[] { + const metadata = getOutputAssetMetadata(cover.user_metadata) + const allOutputs = metadata?.allOutputs as + | readonly ResultItemImpl[] + | undefined + if (!allOutputs?.length) return children + const validFilenames = new Set( + allOutputs.map((output) => output.filename).filter(Boolean) + ) + return children.filter((child) => validFilenames.has(child.name)) + } + function isStackExpanded(asset: AssetItem): boolean { const jobId = getStackJobId(asset) if (!jobId) return false diff --git a/src/platform/assets/services/assetService.ts b/src/platform/assets/services/assetService.ts index b7e09500f8..ae1a10db76 100644 --- a/src/platform/assets/services/assetService.ts +++ b/src/platform/assets/services/assetService.ts @@ -601,6 +601,7 @@ function createAssetService() { const queryParams = new URLSearchParams({ asset_hash: hash, include_tags: OUTPUT_TAG, + exclude_tags: MISSING_TAG, limit: '1', include_public: 'false' }) diff --git a/src/services/jobOutputCache.ts b/src/services/jobOutputCache.ts index cd8386d550..569cea13ca 100644 --- a/src/services/jobOutputCache.ts +++ b/src/services/jobOutputCache.ts @@ -114,3 +114,12 @@ export async function getJobWorkflow( const detail = await getJobDetail(jobId) return await extractWorkflow(detail) } + +/** + * Drops cached task and detail entries for a job so subsequent reads pick up + * server-side changes (e.g. an output deleted via DELETE /assets/{id}). + */ +export function invalidateJobOutputs(jobId: string): void { + taskCache.delete(jobId) + jobDetailCache.delete(jobId) +}