feat: add local asset file actions - #14831
Open
adobex wants to merge 4 commits into
Open
Conversation
🎨 Storybook: 🚧 Building...🎭 Playwright: ❌ 1788 passed, 5 failed · 4 flaky❌ Failed Tests📊 Browser Reports
📦 Bundle: 8.69 MB gzip 🟢 -159 kBDetailsSummary
Category Glance App Entry Points — 3.67 kB (baseline 3.71 kB) • 🟢 -32 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.36 MB (baseline 1.37 MB) • 🟢 -10.1 kBGraph editor runtime, canvas, workflow orchestration
Status: 2 added / 2 removed / 1 unchanged Views & Navigation — 124 kB (baseline 124 kB) • 🟢 -399 BTop-level views, pages, and routed surfaces
Status: 16 added / 16 removed / 1 unchanged Panels & Settings — 565 kB (baseline 565 kB) • 🔴 +3 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 27 kB (baseline 27.7 kB) • 🟢 -705 BAuthentication, profile, and account management bundles
Status: 8 added / 9 removed / 2 unchanged Editors & Dialogs — 125 kB (baseline 125 kB) • 🟢 -2 BModals, dialogs, drawers, and in-app editors
Status: 8 added / 8 removed UI Components — 67.1 kB (baseline 67.1 kB) • 🟢 -18 BReusable component library chunks
Status: 14 added / 14 removed Data & Services — 3.51 MB (baseline 3.52 MB) • 🟢 -11.9 kBStores, services, APIs, and repositories
Status: 17 added / 17 removed Utilities & Hooks — 550 kB (baseline 550 kB) • 🔴 +36 BHelpers, composables, and utility bundles
Status: 29 added / 29 removed / 9 unchanged Vendor & Third-Party — 16.3 MB (baseline 16.8 MB) • 🟢 -525 kBExternal libraries and shared vendor chunks
Status: 7 added / 8 removed / 10 unchanged Other — 14 MB (baseline 14.2 MB) • 🟢 -128 kBBundles that do not match a named category
Status: 145 added / 144 removed / 141 unchanged ⚡ Performance
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds local asset location and deletion actions. It extends asset-service APIs, updates deletion orchestration, adds context-menu controls, refines loopback detection and loader-path handling, and expands localization and test coverage. ChangesAsset actions
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AssetContextMenu
participant MediaAssetActions
participant assetService
participant AssetBackend
AssetContextMenu->>MediaAssetActions: Request deletion or file-location opening
MediaAssetActions->>assetService: Execute the selected asset action
assetService->>AssetBackend: Send deletion or location request
AssetBackend-->>assetService: Return operation result
assetService-->>MediaAssetActions: Return success or error
MediaAssetActions-->>AssetContextMenu: Show notification and emit asset-deleted
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/assets/components/MediaAssetContextMenu.test.ts (1)
107-135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd negative coverage for
show-delete-button=false.
mountComponentnow sets:show-delete-button="true"for every case in this test file, while the component hides delete-related menu items whenshowDeleteButtonisfalse. Add a test mounting the component with:show-delete-button="false"and asserting that the delete actions do not appear.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/components/MediaAssetContextMenu.test.ts` around lines 107 - 135, Add a negative test using the existing mountComponent helper configured with show-delete-button set to false, then assert that all delete-related menu actions are absent from the rendered output. Keep the existing default true behavior and verify the component’s hidden-delete path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/sidebar/tabs/AssetsSidebarTab.vue`:
- Line 166: Review the unconfirmed delete behavior controlled by
show-delete-button and align it with the current Figma slip-prevention guidance
for destructive actions. Preserve confirmation or add an undo affordance through
the success toast as required, ensuring deleteAssets and its skipConfirmation
path remain safe. Also verify whether shouldShowSelectionDeleteButton
intentionally keeps bulk deletion hidden for the OSS input tab, and adjust the
two paths consistently if not.
In `@src/locales/en/main.json`:
- Line 3703: Remove the unused deletingImportedFilesCloudOnly localization key
from the locale files, confirming no remaining cloud-only deletion path requires
it; do not change deleteAssetApi or assetService.deleteLocalInputAsset.
- Around line 3699-3701: Replace the hardcoded “asset(s)” text in
removeSelectedGeneratedAssetsDescription and
selectedGeneratedAssetsRemovedSuccessfully with vue-i18n singular/plural pipe
forms using the count parameter. Update the
selectedGeneratedAssetsRemovedSuccessfully call in useMediaAssetActions.ts to
pass the count as the third translation argument, matching the existing
exportStarted usage.
In `@src/platform/assets/components/MediaAssetContextMenu.test.ts`:
- Around line 283-299: Update the test around “orders local generated file
actions after download” to stub the imported isLoopbackHost function and
explicitly return true before mounting the component, rather than relying on the
Vitest hostname. Reuse the resulting mock so loopback-dependent menu behavior is
deterministic and ready for the companion non-loopback case.
In `@src/platform/assets/components/MediaAssetContextMenu.vue`:
- Around line 244-258: Move the skip-confirmation policy out of the context
menu’s delete command and into useMediaAssetActions.ts: export or reuse a
predicate based on the existing isPersistentLocalOutputAsset logic, including
local inputs, and have deleteAssets derive the value itself. Remove the
duplicated isCloud/assetType/loader_path condition from the component while
preserving confirmation behavior for all other assets.
In `@src/platform/assets/composables/useMediaAssetActions.test.ts`:
- Around line 1248-1264: Add a Vitest case in the copyJobId persistent-output
tests for an asset with loader_path set but neither job_id nor prompt_id. Assert
that the warning toast is shown and mockCopyToClipboard is not called, while
preserving the existing provenance-success coverage.
In `@src/platform/assets/composables/useMediaAssetActions.ts`:
- Around line 742-744: Separate the store refresh calls from the delete
operation’s error handling in the surrounding action flow. Ensure failures from
assetsStore.updateFlatOutputs, updateHistory, or updateInputs are caught and
reported as refresh failures, while the existing delete catch only handles
delete failures and accurately preserves the successful delete result.
- Around line 842-845: Update performDeleteAssets to return true only when no
deletions fail and false from its catch path, then have the skipConfirmation
branch in useMediaAssetActions forward that returned outcome instead of always
returning true.
- Around line 51-53: Update isPersistentLocalOutputAsset to require the asset’s
generated/output type as well as loader_path, then export a companion predicate
for skipConfirmation that uses the same type-aware logic. In
MediaAssetContextMenu.vue lines 244-258, use that exported predicate for
confirmation handling, and update deleteAssets in useMediaAssetActions.ts to use
the same predicate so UI confirmation and deletion dispatch consistently
distinguish local outputs from local inputs.
In `@src/platform/assets/services/assetService.test.ts`:
- Around line 477-484: Update the deleteLocalInputAsset tests to separately
cover missing, ambiguous, and empty-path inputs: retain the zero-record
missing-path assertion, add two private input assets sharing the same normalized
loader_path and assert rejection with no DELETE request, and add an assertion
that an empty path is rejected. Use the existing asset fixtures and fetchApiMock
setup while verifying the ambiguous case does not issue a DELETE request.
In `@src/platform/assets/services/assetService.ts`:
- Around line 229-234: Update the local asset path validation in
deleteLocalInputAsset to reject every drive-qualified path by applying a
/^[A-Za-z]:/ check after slash normalization, covering C:/file, C:file, and C:.
Preserve the existing rejection rules and add regression coverage for the
slashless drive-relative forms.
---
Outside diff comments:
In `@src/platform/assets/components/MediaAssetContextMenu.test.ts`:
- Around line 107-135: Add a negative test using the existing mountComponent
helper configured with show-delete-button set to false, then assert that all
delete-related menu actions are absent from the rendered output. Keep the
existing default true behavior and verify the component’s hidden-delete path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8b71eeba-a519-4f02-a6d2-77da080ceb85
📒 Files selected for processing (13)
src/components/sidebar/tabs/AssetsSidebarTab.vuesrc/locales/en/main.jsonsrc/platform/assets/components/MediaAssetContextMenu.test.tssrc/platform/assets/components/MediaAssetContextMenu.vuesrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/schemas/assetSchema.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.tssrc/platform/assets/utils/assetMetadataUtils.test.tssrc/platform/assets/utils/assetMetadataUtils.tssrc/utils/hostWhitelist.test.tssrc/utils/hostWhitelist.ts
adobex
force-pushed
the
agent/local-asset-file-actions
branch
from
August 6, 2026 06:04
99f7720 to
3072a4d
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
src/platform/assets/composables/useMediaAssetActions.test.ts (1)
1248-1264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the missing-provenance case for a persistent output.
This describe block covers only the
job_idpath. The new branch atuseMediaAssetActions.tslines 332-334 suppresses the asset-id fallback whenloader_pathis set. Add a case where a persistent output has neitherjob_idnorprompt_id, and assert the warn toast plus no clipboard write.As per path instructions, cover "critical success, failure, confirmation, feature-gating, path, empty/null, and deletion-order edge cases".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/composables/useMediaAssetActions.test.ts` around lines 1248 - 1264, Add a test in the “copyJobId - persistent output provenance” describe block using an output asset with loader_path set but neither job_id nor prompt_id; verify the warning toast is shown and mockCopyToClipboard is not called, covering the missing-provenance branch in useMediaAssetActions.copyJobId.Source: Path instructions
src/platform/assets/composables/useMediaAssetActions.ts (3)
742-744: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winIsolate the store refreshes from the delete error path.
The three refresh calls still run inside the same
tryas the deletions. IfupdateFlatOutputs,updateHistory, orupdateInputsrejects, control jumps to thecatchat line 816 and the user sees "Failed to delete asset" although every delete succeeded. Wrap the refreshes in their owntry/catchand log the refresh failure separately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/composables/useMediaAssetActions.ts` around lines 742 - 744, Separate the refresh calls after deletion from the deletion error path by wrapping updateFlatOutputs, updateHistory, and updateInputs in their own try/catch block. Log refresh failures with a distinct refresh-specific error message, while ensuring refresh rejections do not reach the deletion catch or report “Failed to delete asset” after successful deletions.
842-845: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn the real deletion outcome.
performDeleteAssetscatches every failure and returns nothing. Line 844 therefore returnstrueeven when every delete rejects, and the confirm path at line 891 resolvestruefor the same reason.MediaAssetContextMenu.vuethen emitsasset-deletedandAssetsSidebarTab.vueclears the selection while the error toast is visible. Returnfailed.length === 0fromperformDeleteAssets,falsefrom itscatch, and forward that value from both branches ofdeleteAssets.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/composables/useMediaAssetActions.ts` around lines 842 - 845, Update performDeleteAssets to return whether deletion fully succeeded by returning failed.length === 0 after processing and false from its catch handler; then have both skipConfirmation and confirmation branches in deleteAssets return/forward that result instead of unconditionally returning true, so callers only receive true when all assets are deleted.
51-53: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOne local-delete policy is expressed twice and the two copies disagree. The composable predicate checks only
loader_path, while the component recomputes a wider condition that also includes input assets. Local input assets carryloader_pathtoo, asgetAssetStoredFilenameshows, so the two definitions classify the same asset differently. The result is a confirmation dialog and a success toast that state the file on disk is kept, whiledeleteAssetApicallsdeleteLocalInputAssetand removes the file. Define one type-aware predicate and use it at both sites.
src/platform/assets/composables/useMediaAssetActions.ts#L51-L53: add the asset-type check toisPersistentLocalOutputAssetso it matches only local output assets, and export a companion predicate that decidesskipConfirmationfor local input and local generated assets.src/platform/assets/components/MediaAssetContextMenu.vue#L250-L264: replace the inline!isCloud && (assetType === 'input' || (assetType === 'output' && Boolean(asset.loader_path)))expression in the delete command with a call to that exported predicate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/composables/useMediaAssetActions.ts` around lines 51 - 53, Unify the local-delete policy with type-aware predicates: in src/platform/assets/composables/useMediaAssetActions.ts lines 51-53, update isPersistentLocalOutputAsset to require a local output asset with loader_path and export a companion predicate for local input or generated assets used to skip confirmation; in src/platform/assets/components/MediaAssetContextMenu.vue lines 250-264, replace the inline skipConfirmation condition with that exported predicate.src/platform/assets/components/MediaAssetContextMenu.test.ts (1)
300-316: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStub
isLoopbackHostso the precondition is explicit.
shouldShowOpenLocationButtonrequiresisLoopbackHost(window.location.hostname). This test assertsmediaAsset.actions.openFileLocationis present, so it depends on the Vitest environment URL resolving to a loopback hostname. If that environment URL changes, the test fails for a reason unrelated to the component. Mock@/utils/hostWhitelistwith a hoisted spy, and add a companion case that returnsfalseand asserts the item is absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/platform/assets/components/MediaAssetContextMenu.test.ts` around lines 300 - 316, Update the tests around “orders local generated file actions after download” to mock `@/utils/hostWhitelist` with a hoisted isLoopbackHost spy, explicitly returning true for the existing ordering case. Add a companion test with the spy returning false and assert mediaAsset.actions.openFileLocation is absent from the captured menu.Source: Path instructions
src/locales/en/main.json (1)
3693-3701: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse vue-i18n pluralization for the count-bearing messages.
Lines 3693, 3695, 3699, and 3701 hardcode the plural, either as a fixed plural noun (
{count} files) or asasset(s). The coding guidelines require vue-i18n pluralization. Line 3742 in this same file already uses the correct pipe form.Each call site must also pass the count as the third
targument, asexportStarteddoes. The affected calls inuseMediaAssetActions.tsare lines 786, 873, 879, and 884.As per coding guidelines, "use pluralization rather than hardcoded template pluralization".
♻️ Proposed fix
- "deleteSelectedGeneratedSourceFilesDescription": "{count} files will be permanently deleted from disk and removed from Generated. This action cannot be undone.", + "deleteSelectedGeneratedSourceFilesDescription": "{count} file will be permanently deleted from disk and removed from Generated. This action cannot be undone. | {count} files will be permanently deleted from disk and removed from Generated. This action cannot be undone.", "deleteSelectedTitle": "Delete selected assets?", - "deleteSelectedDescription": "{count} asset(s) will be permanently removed.", + "deleteSelectedDescription": "{count} asset will be permanently removed. | {count} assets will be permanently removed.", "removeGeneratedAssetTitle": "Remove this asset from Generated?", "removeGeneratedAssetDescription": "This asset will be removed from Generated. The file on disk will be kept.", "removeSelectedGeneratedAssetsTitle": "Remove selected assets from Generated?", - "removeSelectedGeneratedAssetsDescription": "{count} asset(s) will be removed from Generated. The files on disk will be kept.", + "removeSelectedGeneratedAssetsDescription": "{count} asset will be removed from Generated. The file on disk will be kept. | {count} assets will be removed from Generated. The files on disk will be kept.", "generatedAssetRemovedSuccessfully": "Removed from Generated; file kept on disk", - "selectedGeneratedAssetsRemovedSuccessfully": "{count} asset(s) removed from Generated; files kept on disk", + "selectedGeneratedAssetsRemovedSuccessfully": "{count} asset removed from Generated; file kept on disk | {count} assets removed from Generated; files kept on disk",In
src/platform/assets/composables/useMediaAssetActions.ts, pass the count as the third argument at each affected call, for example:- : t('mediaAsset.selectedGeneratedAssetsRemovedSuccessfully', { - count: succeeded - }) + : t( + 'mediaAsset.selectedGeneratedAssetsRemovedSuccessfully', + { count: succeeded }, + succeeded + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/locales/en/main.json` around lines 3693 - 3701, Update the count-bearing locale messages deleteSelectedGeneratedSourceFilesDescription, deleteSelectedDescription, removeSelectedGeneratedAssetsDescription, and selectedGeneratedAssetsRemovedSuccessfully to use vue-i18n pipe pluralization instead of fixed plurals or “asset(s)”. Update the corresponding calls in useMediaAssetActions.ts to pass the count as the third t argument, matching the existing exportStarted pattern.Source: Coding guidelines
src/components/sidebar/tabs/AssetsSidebarTab.vue (1)
166-166: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winConfirm the single-versus-bulk delete asymmetry for OSS input assets.
This line now always enables the context-menu delete item.
shouldShowSelectionDeleteButtonat line 271 still hides bulk delete for the OSS input tab. A user can therefore delete imported files one at a time, without confirmation, but cannot delete them in bulk. Confirm that this split is intended and align the two paths if it is not.The literal
truealso duplicates the component default, becauseMediaAssetContextMenu.vueline 125 already appliesshowDeleteButton ?? true.As per path instructions, consult the Figma standards for "destructive-action confirmation" before finalizing the unconfirmed delete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/sidebar/tabs/AssetsSidebarTab.vue` at line 166, Review the delete behavior in AssetsSidebarTab, specifically the show-delete-button binding and shouldShowSelectionDeleteButton, and align single-item and bulk deletion for OSS input assets so both follow the intended confirmation policy. Consult the Figma destructive-action confirmation standards and add or preserve confirmation consistently; remove the redundant explicit enablement where MediaAssetContextMenu’s default already provides it.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/assets/services/assetService.ts`:
- Around line 905-908: Update the candidate matching logic around
normalizeLoaderPath in the asset deletion flow to catch normalization failures
for individual asset.loader_path values and skip those malformed records, while
retaining strict validation for the requested path. Ensure a malformed candidate
does not prevent a valid matching asset from being selected, and add coverage
containing both records.
---
Duplicate comments:
In `@src/components/sidebar/tabs/AssetsSidebarTab.vue`:
- Line 166: Review the delete behavior in AssetsSidebarTab, specifically the
show-delete-button binding and shouldShowSelectionDeleteButton, and align
single-item and bulk deletion for OSS input assets so both follow the intended
confirmation policy. Consult the Figma destructive-action confirmation standards
and add or preserve confirmation consistently; remove the redundant explicit
enablement where MediaAssetContextMenu’s default already provides it.
In `@src/locales/en/main.json`:
- Around line 3693-3701: Update the count-bearing locale messages
deleteSelectedGeneratedSourceFilesDescription, deleteSelectedDescription,
removeSelectedGeneratedAssetsDescription, and
selectedGeneratedAssetsRemovedSuccessfully to use vue-i18n pipe pluralization
instead of fixed plurals or “asset(s)”. Update the corresponding calls in
useMediaAssetActions.ts to pass the count as the third t argument, matching the
existing exportStarted pattern.
In `@src/platform/assets/components/MediaAssetContextMenu.test.ts`:
- Around line 300-316: Update the tests around “orders local generated file
actions after download” to mock `@/utils/hostWhitelist` with a hoisted
isLoopbackHost spy, explicitly returning true for the existing ordering case.
Add a companion test with the spy returning false and assert
mediaAsset.actions.openFileLocation is absent from the captured menu.
In `@src/platform/assets/composables/useMediaAssetActions.test.ts`:
- Around line 1248-1264: Add a test in the “copyJobId - persistent output
provenance” describe block using an output asset with loader_path set but
neither job_id nor prompt_id; verify the warning toast is shown and
mockCopyToClipboard is not called, covering the missing-provenance branch in
useMediaAssetActions.copyJobId.
In `@src/platform/assets/composables/useMediaAssetActions.ts`:
- Around line 742-744: Separate the refresh calls after deletion from the
deletion error path by wrapping updateFlatOutputs, updateHistory, and
updateInputs in their own try/catch block. Log refresh failures with a distinct
refresh-specific error message, while ensuring refresh rejections do not reach
the deletion catch or report “Failed to delete asset” after successful
deletions.
- Around line 842-845: Update performDeleteAssets to return whether deletion
fully succeeded by returning failed.length === 0 after processing and false from
its catch handler; then have both skipConfirmation and confirmation branches in
deleteAssets return/forward that result instead of unconditionally returning
true, so callers only receive true when all assets are deleted.
- Around line 51-53: Unify the local-delete policy with type-aware predicates:
in src/platform/assets/composables/useMediaAssetActions.ts lines 51-53, update
isPersistentLocalOutputAsset to require a local output asset with loader_path
and export a companion predicate for local input or generated assets used to
skip confirmation; in src/platform/assets/components/MediaAssetContextMenu.vue
lines 250-264, replace the inline skipConfirmation condition with that exported
predicate.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 246f5636-1025-4310-bff1-3467f70ebfa0
📒 Files selected for processing (13)
src/components/sidebar/tabs/AssetsSidebarTab.vuesrc/locales/en/main.jsonsrc/platform/assets/components/MediaAssetContextMenu.test.tssrc/platform/assets/components/MediaAssetContextMenu.vuesrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/schemas/assetSchema.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.tssrc/platform/assets/utils/assetMetadataUtils.test.tssrc/platform/assets/utils/assetMetadataUtils.tssrc/utils/hostWhitelist.test.tssrc/utils/hostWhitelist.ts
adobex
force-pushed
the
agent/local-asset-file-actions
branch
from
August 10, 2026 02:35
3072a4d to
c3ae2f1
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Author
|
Updated onto current |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/assets/components/MediaAssetContextMenu.test.ts`:
- Around line 327-385: Split the test around persistentOutput into four
independent cases covering menu ordering, open-location execution, delete
arguments, and delete-source arguments, sharing a fixture setup. Remove the
redundant mockIsLoopbackHost.mockReturnValue(true) from these cases because
beforeEach already establishes it, and keep each case focused with only the
mocks and command invocation it needs.
In `@src/platform/assets/components/MediaAssetContextMenu.vue`:
- Line 125: Make hasLocalAssetApi a computed value so it reactively re-evaluates
api.getServerFeature('assets', false) after serverFeatureFlags.value is
populated. Update the delete and open-location computed checks to read
hasLocalAssetApi.value, preserving the existing isCloud condition.
In `@src/platform/assets/composables/useMediaAssetActions.ts`:
- Around line 866-907: Extract the title and message translation-key selection
from the confirmation dialog in the asset deletion flow into a small helper or
lookup structure. Derive a shared copy variant from deletingGeneratedSourceFiles
and allPersistentLocalOutputs, then select single versus plural keys using
isSingle; use the selected description key with the existing count interpolation
and pluralization arguments.
In `@src/platform/assets/services/assetService.test.ts`:
- Around line 451-454: Replace vi.clearAllMocks() with vi.resetAllMocks() in the
beforeEach hooks for both test suites:
src/platform/assets/services/assetService.test.ts lines 451-454 and 1538-1541.
Ensure each test configures any required mock responses independently.
In `@src/platform/assets/services/assetService.ts`:
- Around line 226-235: Replace the raw error messages in normalizeLoaderPath and
the related open-location and unresolved-record error paths with existing
localization-helper usage, adding the required English keys to main.json. Ensure
all three user-facing errors resolve through vue-i18n while preserving their
current contextual details and behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e8b9cfe7-f6e6-4916-a06f-8c7d76b652d3
📒 Files selected for processing (13)
src/components/sidebar/tabs/AssetsSidebarTab.vuesrc/locales/en/main.jsonsrc/platform/assets/components/MediaAssetContextMenu.test.tssrc/platform/assets/components/MediaAssetContextMenu.vuesrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/schemas/assetSchema.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.tssrc/platform/assets/utils/assetMetadataUtils.test.tssrc/platform/assets/utils/assetMetadataUtils.tssrc/utils/hostWhitelist.test.tssrc/utils/hostWhitelist.ts
adobex
force-pushed
the
agent/local-asset-file-actions
branch
from
August 11, 2026 08:35
c3ae2f1 to
68f6171
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Author
|
Updated once onto current |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/platform/assets/components/MediaAssetContextMenu.test.ts`:
- Around line 382-420: Update the MediaAssetContextMenu test helper
mountComponent to expose an asset-deleted listener, then assert onAssetDeleted
is called exactly once after each delete command invocation in the local
generated asset tests. Keep the existing deleteAssets argument assertions and
apply the emit assertion to both the local-only and source-file deletion cases.
In `@src/platform/assets/components/MediaAssetContextMenu.vue`:
- Around line 247-279: Insert a menu separator before the destructive action
group beginning with the openFileLocation item in the context-menu construction,
keeping Download separate from openFileLocation, delete, and deleteSourceFile.
Update the ordering assertion in the context-menu test to expect the added
separator.
In `@src/platform/assets/composables/useMediaAssetActions.test.ts`:
- Around line 1338-1353: Add a test case in the copyJobId persistent output
provenance suite for an output asset with prompt_id set and job_id absent, then
assert copyJobId passes prompt_id to mockCopyToClipboard. Reuse the existing
mock setup and createMockAsset pattern.
In `@src/platform/assets/services/assetService.test.ts`:
- Around line 496-504: Add a traversal input such as a parent-directory path to
the parameterized cases for deleteLocalInputAsset, ensuring it reaches the
requested-path validation and rejects with “Invalid local input asset path.”
Keep the existing fetchApiMock-not-called assertion so the test verifies loading
is skipped.
- Around line 67-81: Update the i18n mock’s t function in the asset service
tests to return the translation key followed by the serialized interpolation
payload instead of duplicating English messages. Revise the affected assertions
around invalidLocalInputAssetPath, failedToOpenAssetLocation, and
failedToResolveLocalInputAsset to verify the exact key and payload values,
including path, id, status, and count.
In `@src/platform/assets/services/assetService.ts`:
- Around line 908-930: The local input deletion flow repeatedly lists all input
assets through deleteLocalInputAsset; update performDeleteAssets and
deleteLocalInputAsset to resolve selected loader paths from a single
getAllAssetsByTag(INPUT_TAG, false) result, then delete the resolved asset IDs
in the batch. Preserve the existing normalization, invalid-path handling, and
exact-match error behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5255934c-df1f-4dd2-b977-a7aba14834ee
📒 Files selected for processing (13)
src/components/sidebar/tabs/AssetsSidebarTab.vuesrc/locales/en/main.jsonsrc/platform/assets/components/MediaAssetContextMenu.test.tssrc/platform/assets/components/MediaAssetContextMenu.vuesrc/platform/assets/composables/useMediaAssetActions.test.tssrc/platform/assets/composables/useMediaAssetActions.tssrc/platform/assets/schemas/assetSchema.tssrc/platform/assets/services/assetService.test.tssrc/platform/assets/services/assetService.tssrc/platform/assets/utils/assetMetadataUtils.test.tssrc/platform/assets/utils/assetMetadataUtils.tssrc/utils/hostWhitelist.test.tssrc/utils/hostWhitelist.ts
Author
|
Addressed all six current follow-up threads in ca59a4c with one update. Successful delete commands now assert the |
adobex
marked this pull request as ready for review
August 11, 2026 09:17
dosubot
Bot
added
the
size:XL
christian-byrne
left a comment
christian-byrne
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the contribution. We are working on these features, can you watch for some recent / upcoming PRs to check if they align with your proposed changes here?
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #14831 +/- ##
==========================================
+ Coverage 80.90% 80.93% +0.02%
==========================================
Files 1872 1872
Lines 122197 122302 +105
Branches 37896 37943 +47
==========================================
+ Hits 98864 98980 +116
+ Misses 22784 22765 -19
- Partials 549 557 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add clear local asset actions for removing gallery entries, deleting source files, and opening generated files in the host file manager.
Changes
Review Focus
Tests
pnpm typecheckpnpm lintpnpm format:checkpnpm knippnpm build