fix(vm): silence reconcile log on sprite-import / backpack-paste#556
Merged
cwillisf merged 1 commit intohotfix/scratch-blocks-release-triagefrom Apr 30, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines scratch-vm’s “dangling reference reconciliation” logging so warnings only appear during whole-project load repair (where dangling refs indicate corruption), while keeping sprite import/backpack paste quiet (where creating stage broadcasts/vars is expected).
Changes:
- Add a
{logRepairs}option toTarget.reconcileVariableReferencesand gate alllog.warnrepair messages behind it. - Enable repair logging only for whole-project loads via
VirtualMachine.installTargets(..., wholeProject=true). - Add/adjust unit tests to assert the new logging behavior across sprite import vs. whole-project load paths.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/scratch-vm/src/engine/target.js | Adds {logRepairs} option and gates reconciliation warnings behind it. |
| packages/scratch-vm/src/virtual-machine.js | Passes {logRepairs: true} only on whole-project installs; keeps sprite import path silent. |
| packages/scratch-vm/test/unit/engine_target.js | Updates/extends tests to cover silent default and logging when {logRepairs: true}. |
| packages/scratch-vm/test/unit/virtual-machine.js | Adds VM-level tests asserting no warn on sprite import and warn on whole-project repair. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
reconcileVariableReferences was emitting log.warn whenever a sprite
imported via addSprite (the backpack-restore code path) referenced a
broadcast that wasn't defined in the new project. The text reads as a
corruption alert ("Reconciled dangling reference") but the case is
mundane: broadcasts live on the stage and naturally aren't carried with
a backpacked sprite, so reconcile must create one. The warning is
useful for the load-time-repair entry point added in #555 — where a
dangling reference does signal a corrupted saved project — but is
noise on import paths.
Add an opt-in {logRepairs} parameter, default false; only the
installTargets whole-project branch passes true. Behavior unchanged;
log emits in exactly one entry point now.
cwillisf
force-pushed
the
fix/silence-reconcile-log-on-sprite-import
branch
from
d703483 to
5da2332
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cwillisf
merged commit ea94b90
into
hotfix/scratch-blocks-release-triage
17 checks passed
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Resolves
Follow-up to #555: the load-time-repair helper's
log.warnwas firing on every sprite-import / backpack-paste that referenced a broadcast not defined in the destination project. The text reads as a corruption alert ("Reconciled dangling reference") but the case is mundane — broadcasts live on the stage and naturally aren't carried with a backpacked sprite, so reconcile must create one in the new project's stage.Confirmed via Playwright on scratch.ly:
Reconciled dangling reference on 'Sprite2': created stage variable 'S9YSR/)K(PHVjM-/xQTU' (name 'awefawef', type 'broadcast_msg').Proposed Changes
Target.reconcileVariableReferencesaccepts a{logRepairs}option (defaultfalse). All fourlog.warncall sites are gated on the option.VirtualMachine.installTargetspasses{logRepairs: true}only on the whole-project branch — where a dangling reference signals corruption from the missing-definitions bug.Target.fixUpVariableReferences(used for sprite import + backpack paste) callsreconcileVariableReferences()without options, so it stays silent.Reason for Changes
fixUpVariableReferencescallsreconcileVariableReferencesas its missing-definitions phase, so once #555 added logging there, every sprite-import path inherited the log. The warning's wording and severity are correct for project-load repair — those projects really are corrupted — but wrong for backpack-paste, which is normal cross-target reference reconciliation.The fix keeps all logging in exactly one entry point (
installTargetswhole-project) and leaves the sprite-import path silent, matching pre-#555 behavior on that path.Test Coverage
reconcileVariableReferences without {logRepairs} is silent even when it repairsfixUpVariableReferences does not log on sprite-import pathinstallTargets does not log on sprite import that creates a stage broadcastinstallTargets logs on whole-project repair{logRepairs: true}so they continue to assert the documented behavior of the load-repair entry point.Related