Studio: image replace from the media picker can fail silently, and the preview shows an image that was never staged

[agalin920]

Summary

Replacing an image from the Studio media picker can fail silently. The dialog closes, nothing is staged, no toast, no console line — and the preview still shows the new image, because the bridge command fires regardless. It looks applied and will never save.

Where

handleLayoutElementAttrUpdate (src/apps/studio/hooks/useLayoutReorderState.ts:995-1031) returns false on five preconditions — missing codeId/layoutId/attr, no cached template source for that codeId, no parse root, no [data-layout-id] match, no element at elementIndex. handleLayoutImageSrcUpdate (:1259) adds a sixth (!newSrc) and forwards the boolean.

Both call sites discard it (src/apps/studio/StudioWrapper.tsx:2510-2549):

handleLayoutImageSrcUpdate(...);      // boolean dropped
postCommandToBridge({ action: "updateImageSrc", ... });   // fires anyway
setImageEditState(null);

The inspector "Browse" branch above it has the same shape.

Why it matters beyond the UX

cypress/e2e/studio/studio-wrapper.spec.js:614 — saves updated image src after selecting from MediaDam — is flaky, and this is why the failure is unreadable: it lands 30s later on a missing save bar instead of at the precondition that failed.

Every run of that spec between 2026-09-15 and 2026-09-18: 10 pass, 2 fail. The two failures are 15 hours apart on unrelated branches — fix/4287-fontsize-typography-variants and fix/cv-verifier-deterministic-verdict, the latter a workflow-only diff — with a byte-identical error. retries: 1 does not rescue it; with testIsolation: false nothing is cleared between attempts, so both attempts fail.

Two candidate causes were ruled out by direct observation, not by reasoning:

• The save bar renders under the other data-cy. StudioWrapper.tsx:1019-1027 names the single bar StudioLayoutSaveBar only when hasPendingLayoutChanges && !hasPendingContentChanges. Probed locally with a content item dirty and a layout change staged: layoutBar=1 contentBar=0. Not the cause.
• Nothing was selected when Done was clicked. The Done button only renders inside selectedFiles?.length > 0 ? (src/apps/media/src/app/components/Header.tsx:217), so the click succeeding proves a file was selected.

Which of the six preconditions fails in CI is still unknown — precisely because the code says nothing when it gives up.

Acceptance criteria

1. When the image-replace update returns false, the user sees a warn notification and the bridge command is not posted, so the preview does not show an image that was never staged.
2. When it succeeds, behaviour is unchanged.
3. The new copy exists in all six locales under public/locales/<locale>/content.json.
4. The media picker's Done/Replace button and its selection-count element carry data-cy hooks; the Studio spec targets them instead of text-matching cy.contains("Done") (per CLAUDE.md, data-cy is the only selector strategy).
5. The two MediaDam tests assert the selection registered before clicking Done, so a selection that did not take fails at the selection rather than 30s later at the save bar.

Known and deliberately not in scope

studio-wrapper.spec.js asserts StudioLayoutSaveBar by that exact name in ~10 places while the product renames the bar based on combined pending state. That is a latent trap, it is not this failure, and rewriting ten assertions would bury the change that matters.

This work makes the next occurrence name its own cause. It does not by itself guarantee the flake stops.

[github-actions]

Classification: bug. The core defect (image-replace can silently no-op while the preview still updates) is a bug. Two of the five acceptance criteria (data-cy hooks, reordering the spec's assertions) are test-hardening/maintenance riding along with the fix rather than separate bugs, but the primary, user-facing issue is the silent failure.

Preliminary severity: Medium — a workaround exists (retry the replace, or notice the save bar never appeared and re-check), and the reporter frames this primarily as a data-integrity/observability gap rather than a reproduced total-loss-of-function report. It would be High if the six preconditions failed regularly in real usage, but the issue is explicit that this was found via code reading and CI-flake forensics, not a confirmed frequent user-facing occurrence.

Understanding: In Studio's layout editor, replacing an <img> via the media picker calls handleLayoutImageSrcUpdate (or, from the inspector "Browse" action, handleLayoutElementAttrUpdate) to patch the cached template source and stage it for save. Both functions return boolean and can legitimately return false on several preconditions (missing codeId/layoutId/attr, no cached source for that codeId, unparseable root, no matching [data-layout-id] element, out-of-range elementIndex, or — for the image-specific wrapper — an empty newSrc). Both call sites in StudioWrapper.tsx discard that return value and unconditionally call postCommandToBridge(...) to update the iframe preview and then close the dialog. So when a precondition fails, the source is never staged, but the live preview shows the new image anyway — it looks saved, has no staged changes to trigger the save bar, and nothing tells the user it didn't take. This is also why the related Cypress spec is described as flaky-and-unreadable: on failure it times out 30s later waiting for the save bar, instead of failing immediately at whichever of the six preconditions actually didn't hold.

Bug essentials:

• Steps to reproduce: MISSING — needed. The issue is derived from code reading, not a manually reproduced repro. No sequence of concrete UI actions that reliably trigger one of the six false-returning preconditions is given (e.g. it's unclear which of the six is hit in the flaky CI runs).
• Expected: Stated in the issue — on failure, show a warn notification and do not post the bridge command, so the preview doesn't reflect an unstaged change.
• Actual: Stated in the issue — dialog closes silently, no toast, no console output, but the preview updates as if the replace succeeded.
• Environment: Studio's layout-editing mode, image-replace via the media picker (canvas flow) and via the inspector "Browse" attribute flow. Not tied to a specific browser/OS; this is a logic bug independent of environment.

Relevant code:

• src/apps/studio/hooks/useLayoutReorderState.ts:993-1039 — handleLayoutElementAttrUpdate, returns false on 5 preconditions.
• src/apps/studio/hooks/useLayoutReorderState.ts:1257-1274 — handleLayoutImageSrcUpdate, thin wrapper adding the !newSrc guard.
• src/apps/studio/StudioWrapper.tsx:2505-2544 — both call sites (addImagesCallback inspector-Browse branch and canvas image-replace branch) discard the boolean and call postCommandToBridge unconditionally, then setImageEditState(null).
• src/apps/studio/StudioWrapper.tsx:796-852 — existing notify({ kind: "error"/"warn", message: t("content....") }) pattern to follow for the new warning.
• cypress/e2e/studio/studio-wrapper.spec.js:576-670 — the two MediaDam specs (saves updated image src after selecting from MediaDam, saves src for an img nested inside a layout leaf), both using cy.contains("Done").click() (lines 612, 661) with no pre-click assertion that the selection registered.
• src/apps/media/src/app/components/Header.tsx:213-312 — media picker header; no data-cy attributes exist anywhere in this file today. The Done/Replace button (t("common.replace")/t("common.done"), ~line 296-312) and the selection-count Typography (~line 217-229) both need one added.
• public/locales/en-US/content.json:635 — existing key-naming precedent (studioCannotSave) for where the new warning key should live.

Open questions:

• Which of the six preconditions is actually failing in the two observed CI flakes? The issue states this is unknown by design (that's the diagnosability gap being fixed) — not something answerable from the code alone, and not blocking since the fix is "handle all six," not "fix the specific one."
• Should the inspector "Browse" flow (handleLayoutElementAttrUpdate failure) get the same warn notification as the canvas replace flow (handleLayoutImageSrcUpdate failure), or does AC Shell #1 apply only to the canvas flow? The issue's prose says "The inspector 'Browse' branch above it has the same shape," implying both need the guard, but AC Shell #1 only explicitly names "the image-replace update."
• Exact copy for the new warn notification isn't specified — needs a decision (or PR-time judgment call) on wording, consistent with other content.studio* notification keys.

Acceptance criteria:

• In StudioWrapper.tsx, when handleLayoutImageSrcUpdate (canvas replace flow) returns false, dispatch a notify({ kind: "warn", ... }) and skip postCommandToBridge({ action: "updateImageSrc", ... }).
• Apply the same guard to the inspector "Browse" flow: when handleLayoutElementAttrUpdate returns false there, dispatch the warn notification and skip postCommandToBridge({ action: "updateElementAttr", ... }) and updateInspectorSlotValue(...).
• On success (true returned), behavior is unchanged — bridge command still posts, dialog still closes, no new notification appears.
• New notification copy is added as a content.* key in public/locales/en-US/content.json and mirrored (English placeholder acceptable) in es-ES, hi-IN, nl-NL, ru-RU, zh-CN under the same key.
• src/apps/media/src/app/components/Header.tsx gets a data-cy on the Done/Replace button and on the selection-count element (or its container).
• cypress/e2e/studio/studio-wrapper.spec.js's two MediaDam tests (saves updated image src after selecting from MediaDam, saves src for an img nested inside a layout leaf) target the new data-cy hooks instead of cy.contains("Done").
• Those same two tests assert the selection-count element reflects a registered selection before clicking Done/Replace, so a selection that silently failed to register fails at that assertion rather than 30s later at StudioLayoutSaveBar.
• npx tsc --noEmit passes (no test/type gate runs this in CI per CLAUDE.md, so it must be checked manually).

Scope: Single, well-bounded PR — one control-flow guard duplicated across two call sites, one new i18n key across 6 locales, two data-cy additions, and two spec updates. No need to split. The issue explicitly calls out the StudioLayoutSaveBar-naming latent trap (asserted by that literal name in ~10 places while the actual bar name depends on combined pending state) as known and out of scope — don't let review scope-creep that in. No duplicate found (searched for other open issues referencing handleLayoutImageSrcUpdate / MediaDam flake — none in this repo currently).

Readiness: ✅ Ready to pick up. The two open questions (scope of the inspector-Browse branch, exact notification copy) are minor enough to resolve during implementation/review rather than blocking start; everything else — reproduction context, affected code, and a concrete acceptance checklist — is grounded in the actual source.