Review 5523
bhamodi · open, request changes proposed (Round 3) · reviewed at 6868c2e642c · view on GitHub
Verdict: request changes — runtime reload recovery is fixed; its browser regression test is still missing
The review asks for a one-line re-arm on the frame's own load event, because the offer loop stops for good at preview-ready and the preview can only ever adopt a port — it has no way to announce itself. It did not merge because that is a behaviour regression against main, measured both ways in one harness: previewed code that reloads or crashes its own frame leaves a playground where nothing anyone types reaches the preview again, nothing in the app notices, and the page has to be reloaded.
Anyone who opens a playground link someone else shared runs that link's code inside astryx.atmeta.com itself. The preview iframe carried sandbox="allow-scripts allow-same-origin" on a same-origin src, which is the sandbox no-op — Chrome logs it in as many words: "An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can escape its sandboxing." Previewed code could read the site's cookies and storage and reach straight into window.parent.document, so a crafted #code= hash is a stored-XSS delivery mechanism on the docs site.
(1 decision · ~118 runtime lines of 341)
-
allow-same-originis dropped from the preview iframe — the fix (PreviewStage.tsx:163).
Everything else in the diff is forced by that one decision rather than judged separately: origin equality can no longer anchor trust, so a MessagePort handshake replaces it in previewChannel.ts; the readiness ping becomes the port offer and preview-ping is retired (PlaygroundClient.tsx:457); and the popover's edit path routes through a module setter (targetingOverlay.tsx:131). Half the diff is the rewritten test.
Plainly: the preview page is moved out of the site's identity. It is still served from the same URL, but the browser is told to treat it as belonging to nobody, so code running in it has nothing of the site's to reach for. Because it now has no name, the two pages can no longer recognise each other by name — instead the playground hands the preview a private wire at startup, and everything after that travels down the wire, which only the two ends hold. Every change traces to the stated problem; nothing is piggybacked.
The title was tested against the diff and is honest. fix(docsite) claims a consumer-visible change and that is what this is — a same-origin escape closed, with channel behaviour changed alongside it. No changeset is the correct answer rather than an omission: apps/docsite is private: true. A chore or refactor here would have been the mislabel worth flagging; this is not one.
The trust check stays in the one module that owns it, previewChannel.ts, and both ends import it — no logic leaked into the components. The seams are the iframe itself and next.config.mjs's frame-ancestors/CSP for /playground/preview; that layer is untouched and still pins who the parent can be, which is what makes event.source === window.parent mean something. One drift comes with it: next.config.mjs:15-18 still says "The origin check on its postMessage channel is the actual guard (playground/previewChannel.ts); these headers are the layer underneath it." After this PR there is no origin check, and that comment is the only written record of why those headers exist. It is not in the diff and it is one line to fix.
The drift sweep ran and found one hand-maintained mirror: PreviewMessage at preview/page.tsx:37 enumerates the inbound message types and loses 'preview-ping' here. It documents rather than enforces — isPreviewMessage asserts only typeof data.type === 'string', so a type added on the playground side compiles and runs without ever appearing in the union, and nothing fails loudly when it goes stale. That is true on main and equally true after this diff: inherited, not the author's, and worth knowing rather than raising on this PR.
Everyone who opens the playground, immediately on deploy, and almost all of them notice nothing: the preview renders the same, the fonts are the same, theme and mode sync the same. What changes for a user is that a shared playground link can no longer touch the docs site's cookies or DOM. What changes for a builder poking at the playground is that previewed code calling localStorage now throws SecurityError — intended, and the PR says so.
What landing it newly exposes: apps/sandbox/src/app/(fullscreen)/PreviewShell.tsx:604 and :626 carry the identical allow-scripts allow-same-origin pair. Those frames render app pages rather than evaluating pasted source, so it is a misleading sandbox attribute rather than the same hole — but once the docsite is fixed, the sandbox is the last place the pattern lives, and #5527 (same author, open) touches that very file to harden its origin check rather than remove the flag. Worth one decision about the pair, not two.
No API change. apps/docsite is private: true, so nothing here is publishable surface and nothing ossifies. The app-internal module surface, for the record:
| change | public? | class | doc'd? | |
|---|---|---|---|---|
- |
trustedPreviewOrigin(): string |
no — app-internal | — | — |
- |
isTrustedPreviewMessage(event, origin, source): boolean |
no — app-internal | — | — |
+ |
PREVIEW_CONNECT: 'astryx-preview-connect' |
no — app-internal | — | file docblock |
+ |
connectToPreview(frame: Pick<Window,'postMessage'>): MessagePort |
no — app-internal | — | file docblock |
+ |
acceptPreviewConnect(event, parentWindow): MessagePort | null |
no — app-internal | — | file docblock |
+ |
setPostToParent(fn) in targetingOverlay.tsx
|
no — app-internal |
setActiveSiteMode, setCleanSource — same module-level setter pattern, same file |
inline comment |
n/a — structural only. The diff's added lines matching #hex|rgba?\(|hsla?\(|boxShadow|light-dark\(|stylex.create|xstyle return 0: no new theme targets, no token touched, no themeable surface removed.
Nothing freezes. The app is private, so neither removed export nor any of the three added ones is publishable surface, and both call sites of the removals are inside the diff. setPostToParent is the one addition that could have been a new idea and is not: it has a named class of two siblings in the same file — setActiveSiteMode and setCleanSource — and is called from the same render-body block that already calls createTargetingController, so it is a fourth instance of an existing shape. The trust boundary itself stays in previewChannel.ts, the module that already owned it, so nothing new is introduced for the repo to carry.
Consumers who can be broken: none by construction — apps/docsite is private: true.
- API — no. Nothing published; the two removed exports are app-internal and both call sites are in the diff.
- Visual — no. The diff writes no CSS, adds and removes no element, changes no class. Nothing already on screen moves, resizes or recolours.
- Theme — no. No target, token or override is touched.
- Behaviour — yes, and this is the finding.
| state | reachable by this diff | what the run found |
|---|---|---|
| first load / handshake | yes | completes on the first offer; measured |
slow compiler (offer before tsReady) |
yes | the offer loop supersedes cleanly; the preview announces on whichever port it holds when tsReady flips |
| a third window forges a connect | yes | rejected — measured |
| the preview document is replaced | yes | the channel never re-establishes; on main it does |
| empty / disabled / boundary | no | the diff adds no state, no default, no prop |
The last row, with its anchors and their text at this head:
PlaygroundClient.tsx:459 clearInterval(interval);
preview/page.tsx:211 const next = acceptPreviewConnect(event, window.parent);
The offer loop clears itself for good at preview-ready, and the preview can only ever adopt a port. On main the preview posted preview-ready to window.parent unprompted on mount, and the playground's permanent window listener heard it, so a replaced preview document healed on its own. Both arms were measured in one harness, same run, driving the real previewChannel.ts:
[OLD] handshake=true pong before reload=1 pong AFTER frame reload=1 -> channel RECOVERS
[NEW] handshake=true pong before reload=1 pong AFTER frame reload=0 -> channel DEAD
Reachability, stated narrowly because that is what it is: nothing in the shipped playground reloads the frame — PreviewStage renders one <iframe> at a fixed position with a fixed src and no key, and the fullscreen toggle only adds a preceding sibling, so React never remounts it. The trigger is the previewed code itself navigating or reloading its own frame, which a sandboxed frame may do, or the frame crashing. After that, nothing the playground sends arrives, and nothing in the playground notices: previewReady is still true, so no loading state and no error path fires.
+ useEffect [] preview/page.tsx:203 window 'message' listener, adopts a port
~ useEffect [tsReady, port, …] preview/page.tsx:222 was [tsReady, …]; now binds port.onmessage
~ useEffect [postCode, postToPreview] PlaygroundClient.tsx:417 was a window listener, now stores a ref
~ useEffect [] PlaygroundClient.tsx:456 was [postToPreview]; the handshake loop
- window 'message' listener (deleted) PlaygroundClient.tsx
Net one added effect, and it replaces a window listener that used to live in the other effect. None of them could be a handler — they are subscriptions to an inbound channel, which is what effects are for — and none sets state in a way the old code did not. One window listener is added in the preview and torn down on unmount; one is removed from the playground; every MessagePort the loop creates is close()d, both on supersede and in the effect's cleanup. No observers, no layout reads, no getComputedStyle, no new dependency, no bundle delta.
The one cost: each 300 ms tick now allocates a MessageChannel and closes the previous one instead of posting a message. The cadence is unchanged and the loop stops at handshake, so it is bounded by how long /vendor/typescript.js (9.1 MB) takes to load. It was not measured — that is the honest answer rather than "negligible".
No frames, for two reasons rather than one. Nothing rendered changes: the diff writes no CSS and adds or removes no element, and the style grep returns 0, so there is no before/after to shoot. And the docsite would not stand up at this head: next dev --webpack never finished its first compile of /playground across two waits of 15 minutes each, since the app disables SWC for a custom Babel config and its generated registries are each over 500 KB. Not a defect of this PR.
Because there is no frame, every finding sentence in the review is written in the vocabulary of the measurement instead. What was driven, in real headless Chromium, against a harness serving the real previewChannel.ts (esbuild'd, unmodified) behind /playground/preview's exact CSP header:
| claim | result |
|---|---|
| the frame's origin really is opaque | window.origin === "null" |
| previewed code can no longer reach the parent |
window.parent.document → SecurityError (was reachable) |
CSP 'self' still matches under an opaque origin |
inline script, <script src> and <link rel=stylesheet> from 'self' all load |
| the author's open font question |
document.fonts reports Figtree:loaded — Google Fonts answers Origin: null with ACAO: *
|
| a third window forging a connect | rejected — a sibling frame posting astryx-preview-connect with a real port to parent.frames[0] is refused on event.source !== parent
|
| round trip over the adopted port |
ping → pong
|
| after the preview document is replaced | dead on this PR, recovers on main (table above) |
The probe is reusable for any postMessage or iframe-isolation review and has not yet been banked in the review kit, which it should be.
Nothing here is touched, and the diff's added lines matching aria-|role=|useTranslator|t\('@astryx return 0.
- Strings — zero: the diff adds no user-visible or AT-facing string.
-
Busy never disables — checked:
isInteractionDisabledon the iframe is unchanged and still only a pointer-events style. - Announcements — checked: no live region added or moved.
-
Stranded state — checked, and this one is worth saying: when the channel dies,
previewReadystaystrue, so nothing is left announcing "loading" and no control is left disabled. The failure is silent rather than trapping — which is why it is a breaking finding and not an accessibility one. - Everything needing a browser is not applicable; the diff renders nothing.
request changes. The problem is real and the fix is the right one; the solution is a single decision with everything else forced by it, and the title is honest. Two things are noted and neither blocks: the next.config.mjs comment now describes a guard that no longer exists, and the sibling allow-scripts allow-same-origin pair in apps/sandbox is left in place while #5527 entrenches it. There is no public surface, no theming change, no performance or resource regression, and no accessibility exposure. The evidence is measurement rather than an image, and every sentence in the drafted comment is written to be checkable from a reading.
This is not a low-risk PR, and only one criterion misses it: no new API surface (private app, and the internal additions are enumerated above), no performance or resource regression, nothing grew — but a behaviour regression, which is the criterion that decides it. It was never a PR the loop could have posted unattended.
1. The offer loop clears for good at preview-ready, and the preview can only
adopt a port — never announce itself
→ previewed code that reloads its own frame (or a crashed frame) leaves a
playground where nothing you type reaches the preview again, and nothing
in the app notices; main recovers from the same event
· PlaygroundClient.tsx:459 · preview/page.tsx:211
No judgement call is involved — this is a defect fix against the existing contract with no new public surface, verified by running both arms. The author's own body says the PR needs a manual pass in a browser before merge, so the ask costs one push rather than a round trip.
Three things found and not spent on the author: Vercel is red on this PR and on every one of the author's other eight open PRs, including a pnpm-pin CI change, so it is environmental rather than this diff; the next.config.mjs:15-18 comment is a one-line fix outside the diff; and the PreviewShell.tsx:604,626 pair is the sibling pattern rather than this PR's to carry.
Not verified: the real playground end to end, since the docsite would not compile locally — theme and mode sync, the Properties targeting flow and error display are proved at the channel level rather than in the product. And whether the frozen-channel state is reachable by anything other than previewed code navigating or crashing its own frame.
Thanks — this is the right call, and the handshake is careful work.
One thing before merge. The offer loop stops for good at
preview-ready, and the preview can only adopt a port — it can't announce itself. So once the preview document is replaced (previewed code reloading its own frame, or a crash), nothing the playground sends arrives again, and the page has to be reloaded. Main recovers from the same event because the preview re-announced on mount; I measured both.Re-arming on the frame's own load event covers it:
<iframe onLoad={() => { readyRef.current = false; }} … />The browser pass you flagged is otherwise clean:
parent.documentthrows from inside the frame, a forged connect from another window is rejected, CSP'self'still matches under the opaque origin, and the webfonts still load.Is there a case the re-arm misses?
Inline: apps/docsite/src/app/playground/PlaygroundClient.tsx:459 — Clears for good. Nothing re-offers if the frame reloads.
One review, three gate passes, and the verdict never moved.
-
Gate 1 — failed on four counts, and the blocking one was in the comment. The draft asserted "the preview silently freezes" — a sentence about something a person would see, inside the blocking finding, with no frame attached and none obtainable. It was rewritten to the thing the measurement proves: nothing the playground sends arrives, backed by
pong 1 → 0. The comment also opened on 41 words of browser-pass evidence before the author reached the one thing to act on, so the evidence was compressed to a clause and moved behind the finding; the inline ran its consequence down to the line the summary already carried; and the time record was a placeholder. -
Gate 2 — three more, none of them in the comment. All three were checks that came back clean and were therefore never written down: the title-versus-diff test, the hand-maintained-mirror sweep, and the risk-class statement. A clean result that is silent reads exactly like a check that never ran, so each was added as a statement — the title is honest and no changeset is correct because the app is private; the sweep found
PreviewMessageatpreview/page.tsx:37and files it as inherited; and the PR is not low-risk, failing on the behaviour criterion. - Gate 3 — clean. No violations, and the answer to would she have posted it is yes. The drafted comment, the inline, the finding and the verdict are unchanged from round 2.
Two instrument errors were caught and re-measured rather than shipped. The first CSP probe read location.origin, derived from the URL, instead of window.origin, the document's actual origin, and so reported both frames as same-origin — caught because the result was surprising. The first handshake harness loaded the real module with <script type="module">; module imports are CORS-mode fetches, so from an opaque origin the header-less test server refused them and the preview script never ran, which looked exactly like the mechanism being broken. Rebuilt as a classic IIFE script, it worked first try. Suspect the instrument before the result.
The run's cost is worth recording for the harness rather than the review: 15 of its 18 wasted minutes went into a docsite compile that never finished, and every number in this review came from abandoning the product harness for an isolated one. warm-main.sh keeps main's checkout current but leaves its build stale, so the "before already exists" promise did not hold on this run and @astryxdesign/core had to be rebuilt.
Drafted, held for Cindy's judgement, not posted. Nothing is on the PR: no review, no comment, no inline. The opaque origin is the right call and the browser pass came back clean — but the offer loop stops for good at preview-ready and the preview can only adopt a port, so once the preview document is replaced the channel never re-establishes and the page has to be reloaded, where main recovers from the same event. Both arms were measured. A one-line re-arm on the frame's load event covers it.
The drafted comment carries no full-review link and no invitation to chat elsewhere; if it is posted as drafted, it goes up as written.
#5523 fix(docsite): give the playground preview an opaque origin by bhamodi (bucket: contributor)
6868c2e642c95661c21b333a58a147b47046cfac <- every claim below was verified at this commit
LOOP VERSION: 1.6.0 AUDIT RUBRIC: 1.13
LANE: full WHY: This is a security/trust-boundary change with a prior changes-requested review, changed Effects/listeners, and a browser-only iframe lifecycle.
WHY 1: A same-origin preview with allow-scripts allow-same-origin lets previewed playground code reach the docsite’s DOM, cookies, and storage.
WHY 2: Someone opening a shared playground link executes code they did not author with the docsite’s authority.
WHY 3: A playground cannot safely support shared, auto-executing examples if an example can escape its preview and act as the docs site.
USER-FACING PROBLEM: A person opening a shared playground cannot safely trust that the example is confined to the preview. PROBLEM SEVERITY: broken task — opening shared playground code is unsafe under the old same-origin sandbox.
The prior cixzhang review asked for recovery after the preview document is replaced and a focused production-browser regression. Current head fixes the runtime recovery; the new test stops below the iframe/React integration.
VERDICT: clear — the security problem and the remaining review scope are explicit
The browser gives the preview an opaque origin, so code inside it cannot use the docsite’s identity. The playground gives its own iframe a private channel, and the preview accepts that channel only from its parent. When the frame loads again, the playground reopens the channel and sends the current code and theme state to the replacement document.
SOLUTION (1 decision · ~252 runtime additions of 529 changed lines)
- Remove same-origin authority and replace origin-checked broadcasts with a parent-bound, reloadable MessagePort lifecycle. The test, port routing, load hook, and code/theme resend are consequences of that decision.
BURDEN: high — one timer-backed connector, three channel-related Effects, refs for current handlers/state, app-wide message routing, and a focused lifecycle test matrix. BURDEN MATCH: proportionate — the machinery closes a stored-XSS boundary and preserves the existing playground lifecycle.
VERDICT: clear — one security decision with proportionate lifecycle machinery
OWNER: previewChannel.ts owns the channel lifecycle; PlaygroundClient owns the iframe and current editor/theme state.
TIER 1: browser MessageChannel and iframe sandbox; no Astryx shared component system applies.
TIER 2: none.
SEAMS: initial iframe load, replaced iframe document, slow compiler adoption, code/theme resend, preview→parent targeting messages, and untrusted sibling windows.
BEHAVIOR UNIT: pure utility — createPreviewConnector now owns offer/ready/reset/stop and has focused MessagePort tests; the React/iframe wiring is not covered by a browser test.
| seam | driven result |
|---|---|
| initial production iframe | opaque origin, TypeScript and fonts loaded, default preview rendered |
| sibling-window forged connect | rejected |
| iframe reload → editor edit | recovered; current code rendered and parent reached “Build finished” |
| iframe reload with custom dark theme | custom dark provider restored before and after the edit |
| targeting after reload | preview→parent selection round trip completed |
The previous runtime finding is fixed. The remaining gap is the exact integration boundary that caused it: the new Vitest calls connector.reset() directly, so deleting onLoad={handleFrameLoad} or the parent’s code/theme resend leaves every focused test green.
VERDICT: BLOCKS — the repaired iframe lifecycle still lacks the focused browser regression requested in the prior review
Everyone opening the playground gets the intended safety boundary. At current head, reloading the preview document no longer strands later edits: the replacement receives current code and the active custom dark theme, returns to “Build finished,” and the targeting return path still works.
VERDICT: clear — the prior user-visible reload regression is fixed at exact head
No published API change. apps/docsite is private. App-internal changes add createPreviewConnector() and PreviewStage.onFrameLoad; both serve the existing channel lifecycle and create no package surface.
OSSIFICATION: none — no package barrel, prop contract, theme target, or released default changes.
VERDICT: clear
No theme target, token, variable, or style contract changes. The exact-head browser pass showed the custom dark provider state restored across reload.
VERDICT: clear
BEHAVIOR: intended security behavior changes; the prior unintended reload break is fixed. Initial load, slow adoption, reload, edit, theme resend, targeting return, and forged-connect rejection were rechecked. API: no — private app-internal surface only. VISUAL: no intended pixel change. The initial/recovered whole-page images differ in 20,960 pixels (1.6173%), all within the editor at x89–496; the preview starts at x515 and is pixel-identical. THEME: no — active custom dark state is restored after reload.
VERDICT: clear — no unintended current-head break found
EFFECTS:
-
+ useEffect [code]mirrors current code into a ref for the stable load handler; it sets no React state. -
~ useEffect [postCode, postToPreview]stores the current port message handler instead of attaching a window listener. -
~ useEffect []owns the connector lifecycle and cleans up timer plus active port. -
+ useEffect []on the preview page listens only for parent connect offers and removes the window listener on unmount. -
~ useEffect [tsReady, port, …]binds and clears the adopted port handler.
Measured across three exact-head production loads: 5 parent MessageChannels each; 15–16 parent React commits; 13–14 preview commits. The added [code] Effect performs one ref assignment per code state change and causes no render, listener, layout read, or bundle dependency. Every interval, listener, and port has a cleanup path; the connector stops offering after readiness and re-arms only on frame load.
VERDICT: clear — bounded lifecycle, no measured render/layout regression
VISUAL CHECK: manual frames required WHY: the repaired acceptance endpoint is visible after a preview reload, and the exact-head visual job was skipped because this path has no stable visual scope.
| Initial exact-head preview | Recovered after iframe reload + editor edit |
|---|---|
 |
 |
SENSOR RECEIPT: build 6868c2e642c95661c21b333a58a147b47046cfac; route /playground; theme astryx; light mode; LTR; 1440×900@1; forced colors off; reduced motion on; fine pointer/hover; one visible 908×834 preview iframe; fonts loaded; zero page errors. Initial state requires heading “Welcome” and action “Get started.” Recovered state additionally requires changed editor hash, /playground/preview, current heading/action, and “Build finished.” Preview pixels are identical; the 1.6173% whole-page delta is entirely in the editor region.
VERDICT: clear — current-head recovery is visible and sensored
REMEDY SEARCH: not triggered — no proven visual defect
No role, ARIA, focus, keyboard, user-visible string, translation, locale, direction, or rendered control change is in the diff. pr-a11y and pr-rtl were skipped by CI because no component scope changed. The browser pass exercised the existing Target element control after reload and completed its preview→parent state transition with zero page errors.
VERDICT: clear — affected interactive channel remained operable; no a11y/i18n surface changed
| slot | verdict |
|---|---|
| PROBLEM | clear |
| SOLUTION | clear |
| ARCHITECTURE | BLOCKS — browser integration regression absent |
| IMPACT | clear |
| API | clear |
| THEMING | clear |
| BREAKING | clear |
| PERFORMANCE | clear |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: met — exact-head production Chromium proves the opaque boundary and reload recovery, including current code/theme resend. DISPOSITION: missing browser regression → blocks now; the runtime reload finding from both earlier reviews is deleted as fixed. ADVICE: bounded outcome criteria — CI must load the real playground, reload its preview iframe, edit code, and prove the replacement renders with active theme state. AUTHOR CAN PROCEED: yes — the required behavior and browser assertion are fully specified; implementation remains theirs. WORST OUTCOME: “Removing the iframe load integration leaves every focused test green while the dead-preview regression returns.” → request changes
JUDGEMENT NEEDED: none — this is verification of an existing behavior contract.
request changes
- The focused test calls
reset()directly and never crosses the iframe load/React wiring → a future edit can remove the recovery integration while CI stays green and the dead-preview bug returns ·apps/docsite/src/__tests__/playground-preview-channel.test.ts:160
Thanks — the reload behavior is fixed. I verified the replacement document receives current code and active theme state, and targeting still round-trips.
I previously asked for a focused browser regression. The new test calls connector.reset() directly, so it remains green if <iframe onLoad={handleFrameLoad}> disappears or if code/theme resend stops. That integration is the exact failure fixed here, and this security boundary needs it pinned in CI.
Could you add the browser test from the previous review: load the playground, reload the preview frame, edit, and assert the new document renders with the active theme?
If you'd rather talk it through with someone, we're in Discord.
[Reviewed by Robohands]
-
apps/docsite/src/__tests__/playground-preview-channel.test.ts:160— This callsreset()directly, so it misses the iframe load wiring and code/theme resend.
- Exact-head focused Vitest passed 9/9; remote build, test, docsite-test, lint, dependency, and scope checks passed.
- Main changed only the
Object.hasOwn(themeByValue, rawThemeParam)hardening in a touched file since the old patch base; current head includes it and it does not alter this lifecycle. - Vercel deployment failed, but the exact-head local production build and production server succeeded.
TIME total 30m active setup/rules 5m full brief, critic, harness, rubric, prior reviews/wiki, worktree install/build/server 6m fast seed 35s; workspace build 67s; docsite build 224s; server 3s browser/a11y 7m guarded production probes, sensor captures, targeting and cleanup focused tests 0m focused Vitest 9/9 in 2s code/history analysis 7m old/new range diff, main movement, six-file review critique + wiki 5m four gates, final public round, index update, rebase/push CI wait 0m exact-head CI was already complete waste 3m bad compare expansion and two probe selector/race corrections
- WebKit and Firefox are unavailable on this managed Mac; Chromium is the only runnable engine.
- A renderer-process crash distinct from a document reload was not forced.
Not posted: this was a read-only re-review. Proposed event is REQUEST_CHANGES; Gate 4 passed clean with the runtime finding removed and only the previously requested browser regression remaining.