Conversation
CORCTON
force-pushed
the
feat/support-local-image-reference-2786
branch
from
September 8, 2026 03:34
1c37b30 to
6d19c77
Compare
CORCTON
force-pushed
the
feat/support-local-image-reference-2786
branch
from
September 16, 2026 01:18
691b29a to
0eab58e
Compare
CORCTON
marked this pull request as ready for review
September 16, 2026 02:29
Collaborator
Author
|
/review -claude |
Contributor
There was a problem hiding this comment.
Review: local image reference support
Solid, well-tested PR. The selection state is centralized in reference-image.ts, validation is applied consistently at the construct/set/save/load boundaries, and generation flows thread a merged AbortSignal through cleanly. The path-traversal guard in loadReferenceImageFile and the raw-Kodo-URL check in toCostumeReferenceImageUrl are nicely defensive and covered by tests. No blocking or correctness issues found — the notes below are maintainability/consistency items.
Non-blocking observations (not inlined):
- The abort/cancel/merge-signal boilerplate in
genImages/generate/runGenerateVideo+cancel()is now duplicated across all four*-gen.tsfiles (backdrop, sprite, costume, animation). Each repeats: abort the priorAbortControllerwith anew Cancelled(...), cancel+dispose+null the prior task, create a fresh controller,mergeSignals(this.getSignal(), ctrl.signal), run the phase, and reset the field onlyif (this.xCtrl === ctrl). Only theCancelledmessage differs. Worth extracting a shared helper so the four copies (and the subtle=== ctrlguard) don't drift. reference-image.tsallows SVG as a reference upload (imgExtsincludessvg). No active XSS today since previews flow throughuseFileUrl→ object URL →<img>, but it's a latent stored-XSS surface if any future code renders the file viav-html/inline<svg>. Consider excluding SVG, or keep any rendering in a non-executing image context. Client-side type validation is extension/MIME-based only, so authoritative content validation should live server-side.
aofei
reviewed
Sep 20, 2026
This branch was successfully deployed
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
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
AIGCBackdropSettingswith optionalreferenceImageUrl.Closes #2786
Backend dependency
Backdrop reference-image generation requires https://github.com/goplus/builder-backend/pull/371. Deploy the backend change before enabling this frontend behavior; older backends ignore
referenceImageUrland fall back to text-only backdrop generation.Validation
vue-tsc --build --forcevitest --run src/models/spx/gen src/components/asset/gen/common/param-settings/ParamSelector.test.ts src/components/asset/gen/common/ReferenceImageButton.test.ts(139 tests)ImageService.Editsmoke test with a local reference image (4:3, one output); compared against the existing text-only path