fix: stop the artifact frame blanking on iOS and nested dashboards - #6461
Conversation
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsFINDING -- website/src/components/ArtifactBody.tsx:307 -- after a successful report, reloading the same spent URL leaves False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Four independently measured root causes, each fixed at cause level with a mutation-verified guard and documented tradeoffs — sound, proportionate design throughout. Suggestions
[DESIGN-REVIEWED] 71611d7 |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of UX-Verdict: CONCERNS The new dead-frame retry never fires after a document has once reported — the exact eviction-to-404 case the PR names stays a silent empty box. Watch
Suggestions
[UX-REVIEWED] 71611d7 |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of Both files read, counts run. Assembling the review. First-Principles-Verdict: CONCERNS Every fix is measured and cause-level, but the new validated header builder skips one of the three sites that need it, and two new docstrings describe the reverted design. What this change shipsIntent: make artifact/widget frames actually render on iOS and in nested (Instances-embedded) dashboards — a FIX.
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 71611d7 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
184c126 to
75b8401
Compare
75b8401 to
0f35f35
Compare
|
GPT's I did not edit The branch is now rebuilt: my three commits replayed onto the current Verified after the rebuild rather than assumed:
|
Three independent causes made a widget or artifact frame render as an empty box. Each was reproduced and measured before being fixed, and each is invisible on a desktop Chromium dev loop, so every one carries a test. iOS WebKit skips the frame's FIRST paint. Measured on the reporting device: the document loaded, its injected scripts ran, it reported a correct 385px layout height, and it sat in a visible 385px frame while painting nothing. Four unrelated post-load invalidations each made it appear — a 1px resize, a transform toggle, an opacity flip, a display toggle — so the engine had laid the document out and simply never rasterized it. The frame is now promoted with translateZ(0), the one remedy of the four that needs no timing; anything scheduled off the load event is a race on a slow connection. Content height only ever CORRELATED with the symptom, because tall content happened to trigger one of those invalidations. It is fixed here too, on its own merits: the detail frame stood in a fixed calc(100vh - 240px) box with minHeight 480, which on a phone put a short artifact in a frame hundreds of pixels taller than itself and made the reader scroll a pane inside a scrolling page. It now takes the height its document reports, over the same mc-widget-height protocol the chat frame already uses, and carries no floor. That reported height is bounded at BOTH ends. It crosses a postMessage boundary from a document built out of model- or user-authored HTML, so an in-document script could otherwise post one huge finite number and expand the page by millions of pixels — and because the value is cached, the absurd box would be restored on every later open with nothing hostile still running. Both bounds live on one clamp that both readers of the protocol go through, on the way in from a report and on the way out of the cache; the chat frame had the same exposure and is fixed with it rather than left to drift. CSP frame-ancestors is matched against EVERY ancestor, not just the parent. The Instances embed nests a remote dashboard inside the local one, so a widget sits three levels down with a grandparent on another origin; 'self' alone left that unmatched and the browser refused the embed while the GET still returned 200. The value is now 'self' plus the ancestors 'self' cannot express. Both halves are load-bearing: 'self' is resolved by the browser against the frame's real URL, so it survives a TLS-terminating tunnel that rewrites Host and may not forward X-Forwarded-Proto — an intermediate version that derived the origin server-side named http://localhost:<port> while the phone was on https://<tunnel-host> and blanked every frame there. Extra ancestors are re-validated against a strict origin form, which also drops the http://[::1]:<port> entry that was being emitted: a bracketed IPv6 literal is not a valid CSP host-source, so engines refused the whole source expression and the grant never worked. Two hardening fixes fall out of the same investigation. The document URL is single-use, so a navigation the engine starts on its own re-requests a spent URL and lands the frame on a 404 that fires load like any other navigation, leaving a silent empty box while the failure notice stays hidden because the mint succeeded. Silence past a grace window now surfaces the existing retry, which re-mints. And /sandbox-doc/ joins the service worker's skip list: an iframe navigation has mode 'navigate', so the worker's offline fallback would otherwise serve the SPA shell INTO the widget frame.
0f35f35 to
71611d7
Compare
|
Dispositioning the
So the named driver does not reproduce, and the class of bug that does reproduce needs a multiplier above 1 — which takes deliberate construction, in documents this dashboard's own agent authors. The framing of the earlier finding in this PR ("untrusted artifacts can force an unbounded frame height") does not describe a threat model either: the same agent already writes arbitrary HTML into this page, so a height report is not the interesting capability. The source comments have been rewritten to say what is actually true instead of borrowing that framing. What this PR does about it: one shared What it deliberately does not do, and why the suggested remedy is declined:
Residual, stated plainly: a deliberately divergent document still leaves a too-tall height cached for that one artifact in that one browser. Catching it needs a viewport-relative sanity bound, i.e. the heuristic above with its own false positives. Not carried for a construction that takes effort to produce and that no agent-authored widget has been observed to emit.
|
Three independent causes made a widget or artifact frame render as an empty box.
Each was reproduced and measured before being fixed, and each is invisible in a
desktop Chromium dev loop — so every one carries a test that fails when the fix
is reverted.
iOS WebKit skips the frame's first paint
Measured on the reporting device, with a readout rendered in the parent page so
it stayed legible while the frame showed nothing:
The document loaded, its injected scripts ran, it reported a correct 385px layout
height, and it sat in a visible, correctly sized 385px frame — painting
nothing. Four unrelated invalidations applied after that state each made it
appear: a 1px resize, a transform toggle, an opacity flip, a display toggle. So
the engine had laid the document out and simply never rasterized it.
The frame is now promoted with
transform: translateZ(0). It is the one remedy ofthe four that needs no timing — the other three have to be fired after
load, andanything scheduled off a load event is a race on a slow connection.
Content height only ever correlated with this symptom. Tall content happened
to trigger one of those invalidations, which is why a taller copy of the same
artifact rendered while a byte-identical one did not. Six measurements were
consistent with a height threshold and all six were explained by invalidation
instead; the readout above is what separated them.
The detail frame stood in a fixed box
Independently worth fixing, and confirmed by the pair below. The frame was
calc(100vh - 240px)withminHeight: 480, so on a phone a short artifact sat ina frame hundreds of pixels taller than itself and the reader scrolled a pane
inside a scrolling page. It now takes the height its document reports — the same
mc-widget-heightprotocol the chat frame already uses, in its own measured-heightkey space — and carries no floor.
Measured at a 390px viewport with a 343px document:
min-height480pxBoth frames come from one script run against a live pod, with sha256 printed to
prove they differ — capturing them in two invocations is how a "before" silently
becomes a second copy of the "after". "Before" is the removed declarations
re-applied to the same live page with the same document in it, not a mock-up. The
sample artifact is invented for the capture.
frame-ancestorsis matched against every ancestor, not just the parentReproduced in a three-level harness. The Instances embed nests a remote dashboard
inside the local one, so a widget sits three levels down (local dashboard →
embedded dashboard → widget) with a grandparent on a different origin.
'self'alone left that grandparent unmatched, and the browser refused the embed while the
GETstill returned 200 — the failure looks like a blank frame with a healthynetwork tab.
The value is now
'self'plus the ancestors'self'cannot express, and bothhalves are load-bearing:
'self'is resolved by the BROWSER against the frame's real URL, so it stayscorrect behind a TLS-terminating tunnel that rewrites
Hostand may not forwardX-Forwarded-Proto. An intermediate version of this change derived the originserver-side instead; it named
http://localhost:<port>while the phone was onhttps://<tunnel-host>and blanked every frame on that path. Four tests now pinthat no rewritable
Hostvalue can reach the header.'self'alone is insufficient at nesting depth, which is the original bug.Extra ancestors come from
server._extra_frame_ancestorsand each is re-validatedagainst a strict origin form. That also drops an entry that was being emitted and
never worked:
http://[::1]:<port>. A CSP host-source admits only letters, digitsand hyphens, so a bracketed IPv6 literal is not expressible — engines refuse the
whole source expression and drop it, which was the source of a console warning per
frame.
The reported height is bounded at both ends
Not a threat model — these documents are agent-authored, and an agent that wanted
to wreck the page has easier ways than a height report. The bounds exist because
ordinary CSS lets a document's height depend on the frame's viewport, so a
self-sizing frame feeds its own measurement. Measured in Chromium:
min-height:100vh— the common idiommin-height:110vh— a deliberate multiplierSo the common case needs nothing, and the divergent case needs a deliberate
construction — but unbounded growth has no natural stopping point, and chat frames
have always been self-sizing off this same report with no bound at all. One
shared
clampFrameHeightnow bounds both readers at both ends, on the report pathand on the cache-read path. The ceiling sits far above any real document (a
20,000-word page lays out around 30,000px) and far below where engine layout
degrades, so it cannot truncate legitimate content.
Deliberately NOT fixed here: a divergent document still leaves a too-tall frame
cached for that artifact. Refusing to cache the ceiling value was tried and
measured — the runaway passes through large sub-ceiling values (98,066px) that
cache just the same, so the guard did not deliver what it promised and was removed
rather than kept half-working. Catching it properly needs a viewport-relative
sanity bound, which is a heuristic with its own false positives (a lazily-growing
document) and is not worth carrying for a construction that takes deliberate effort
to produce.
Two hardening fixes from the same investigation
URL is single-use, so a navigation the ENGINE starts on its own (memory
pressure, a back/forward cache eviction) re-requests a spent URL and lands the
frame on a 404 — which fires
loadlike any other navigation and leaves asilent empty box, while the failure notice stays hidden because the mint itself
succeeded. Every document this surface builds carries the height reporter, so
silence past a grace window surfaces the existing retry, which re-mints.
Deliberately not an automatic re-mint: a second
loadalso happens when alink inside an artifact navigates the frame, and silently pulling the reader
back would fight an action they took.
/sandbox-doc/joins the service worker's skip list. An iframe navigationhas
mode === 'navigate', so the worker's offline fallback would otherwiseserve the SPA shell INTO the widget frame — a dashboard rendered inside a
widget. The single-use credential is the second reason: any re-fetch the worker
performs resolves to a 404.
Tests
Twelve new frontend cases plus four backend guards, each mutation-verified —
every mutation turns exactly its own target red, and the suite is green when
restored:
translateZ(0)minHeight: 480on the frameincludeHeightReporterpostMessagesource check/sandbox-doc/from the worker's skip list'self'with a server-derived originwebsite/src/test/serviceWorkerSkipRules.test.tsexecutes the realsw.jsagainst a fake global rather than grepping it, so a rule that is present but
unreachable still fails.
What is not verified here
WebKit cannot launch on the build host, so no automated check in this repo
observes the paint fix — the screenshots above are Chromium and show only the
sizing change. The iOS behaviour was measured on a real device through the parent
page readout quoted at the top, and confirmed fixed on that device with the shipped
build. The
translateZ(0)test is therefore the whole guard for it: dropping theproperty looks completely harmless everywhere CI can see.
The side-panel path (a caller passing an explicit
heightStyle) still fits a fixedpane by design and keeps the same theoretical exposure; it is left alone rather
than changed without a way to verify it.
Gates
tscclean,eslint0 errors (2 pre-existing warnings in untouched branches ofthe same file),
vitest25267 passed / 0 failed,npm run buildclean,black/isort/flake8/mypy/docs-lintclean, backend 188 passedacross the touched modules.
docs/system-specs/modules/artifacts.mdis updated inthe same commit as the behaviour it documents.