Repository navigation
fix(shell): track renderer readiness per iframe so concurrent surfaces stop clobbering each other - #252
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the HostCommunication service to support multiple concurrent iframe renderers by tracking readiness per window and managing queued messages independently. The review feedback highlights two issues in host-communication.ts: a potential bug in isTargetReady where a nullish target could cause messages to be silently dropped if another iframe is already ready, and an unconditional call to set the renderer readiness signal to true in markWindowReady even when the source window is null.
mikzat
marked this pull request as ready for review
October 5, 2026 17:50
Contributor
⚡ A2UI Composer PR PreviewYour automated preview is successfully live (commit
|
jgindin
reviewed
Oct 5, 2026
jgindin
left a comment
jgindin
left a comment
Contributor
There was a problem hiding this comment.
Thanks, the diagnosis is spot on and per-window readiness is a clean way to fix it. Nothing blocking from me; a few suggestions are inline.
One general point, since it has no natural line to attach to: there's no Playwright coverage for the multi-frame scenario. The repo's testing guidelines ask for user journeys to be covered by Playwright (yarn e2e-headless in shell/). The headless-Chrome before/after in the description is convincing, but it's a manual check. A local stub A2A agent like the one you used for that check (one inline surface plus one Canvas card per turn) could turn the scenario into an e2e test that needs no GEMINI_API_KEY. The checks would be: open canvas #1, send turn 2 while it's open, open canvas #2, then confirm each inline frame keeps its own content. That would guard against this regressing.
…s stop clobbering each other The /a2a route mounts several renderer iframes at once: one per inline chat surface plus the side canvas. HostCommunication tracked readiness with a single global flag, reset it and wiped the whole outbound queue whenever any iframe registered, and sendMessage read that flag inside the components' effects. Opening the canvas therefore briefly flipped every frame back to "not ready", re-ran every RenderedFrame effect, and queued and replayed the inline surfaces' payloads. A2aChatView compounded this by also sending the canvas payload with no target, which goes to the most recently registered iframe: on first open that is still the inline frame, so the canvas content is drawn inline for roughly 600 ms and the inline surface is then recreated (losing its data model); when the canvas is already open and a newer turn has mounted its own inline frame, that inline surface is overwritten for good. Readiness is now recorded per guest window. A message is posted as soon as its own frame has announced RENDERER_READY, regardless of siblings that are still loading, and a frame's RENDERER_READY flushes only the messages addressed to it (plus untargeted ones, as before). Registering a frame discards only that frame's previous readiness and queued messages, and unregisterIframe releases the window and its queued messages even when the element already left the DOM. The duplicate untargeted sendRenderA2UI effects in A2aChatView are removed; the canvas RenderedFrame already sends its payload to its own iframe. The one spec that expected a frame's queued message to be delivered when a *different* frame became ready now expects delivery on that frame's own handshake, since a renderer that has not announced itself drops the message anyway.
…re sourceless RENDERER_READY Follow-up to the per-frame readiness tracking, addressing two review comments. An untargeted send resolves to the default (most recently registered) frame. When that frame's element is registered but momentarily has no window, `isTargetReady` previously fell through to the overall readiness flag, so with a ready sibling the message was handed to `sendMessage` with no window to post to and dropped. The default is now resolved up front and handled like an explicit target, so the message waits for the frame to announce itself. A RENDERER_READY without a source window can no longer flip the overall readiness flag, keeping it consistent with the per-window bookkeeping. Real guest frames always carry a source; only synthetic events lack one.
…ad readiness untracked in sendMessage Follow-ups from review: - RENDERER_READY now sends SET_THEME to the frame that announced itself instead of calling sendTheme(), which broadcast to every frame on every handshake (and, for the default frame, through the queue as well, so a second frame's handshake re-sent the theme to the first). sendTheme() itself now posts to each registered frame that is up and skips frames still handshaking, since those receive the current theme on their own RENDERER_READY; a queued copy would only arrive as a duplicate. The direct-postMessage loop is gone; everything goes through sendMessage. - MCP_REQUEST marks the source frame ready and flushes the queue, the same as a handshake, so a frame whose RENDERER_READY was missed does not wait for some other frame's handshake to receive its queued messages. - sendMessage reads readiness through untracked(). Callers run inside component effects (RenderedFrame's payload and theme effects); readiness must not become a dependency of those effects or every frame's handshake would re-run them and re-send their payloads. - Docs: isRendererReady is described as the aggregate it is, resolveTargetWindow names the default frame precisely, and the registerIframe/unregisterIframe comments say why untargeted queued messages are dropped on unregister. - Specs assert on what each window receives via postMessage instead of reaching into the private buffer, and cover the effect-dependency, per-frame theme, and MCP_REQUEST flush cases. The A2aChatView spec now asserts the canvas payload reaches the side-canvas iframe and that no canvas payload is ever sent without a target.
Adds e2e/a2a-multi-frame-surfaces.e2e.ts. A stubbed agent answers each turn with a surface whose root Column holds an inline Text and a Canvas card (autoOpen: false). The test sends turn one, opens its canvas, sends turn two, opens that canvas, and then checks every frame's document: each inline frame shows its own "INLINE SURFACE #n" and the side canvas shows "CANVAS CONTENT a2ui-project#2". On main the last step fails: the second inline frame shows "CANVAS CONTENT a2ui-project#2", because the canvas payload is dispatched to whichever frame registered last. On this branch it passes (3/3 repeats). connectMockAgent/setupMockA2aRoutes take an optional per-request responder so a test can script turn-specific replies; the existing keyword routing is unchanged when it is omitted. sseEvent is exported for building those replies.
mikzat
force-pushed
the
per-frame-renderer-readiness
branch
from
October 7, 2026 22:29
656faf2 to
9bcf649
Compare
Contributor
Author
|
Thanks for the review. Rebased onto On the Playwright point: added
Commits: The two items I left as follow-ups (double |
…s return-type convention - A RENDERER_READY without a source window already did not mark anything ready, but still went on to send the handshake theme untargeted, which queued a stray SET_THEME for the default frame. The whole branch now requires a source window. The spec for this case now drives the detached default frame through its real handshake afterwards and checks it receives the theme exactly once; without the guard it received two. - Reuse envelope.sourceWindow instead of computing the source a second time. - The private methods added in this PR carry explicit return types like the rest of the file. - The e2e journey's first-open assertions are documented as a steady-state check: the brief flash the old code showed until the canvas frame's handshake is not something a retrying assertion can pin down; the permanent variant of the misrouting is what the test guards.
Contributor
Author
|
One more small commit (
|
jgindin
approved these changes
Oct 8, 2026
The canvas payload dispatch specs located the side canvas iframe with a By.css query. A2aChatViewHarness now exposes the RenderedFrame inside the side canvas viewport, RenderedFrameHarness hands out its guest iframe, and the specs resolve the element from there (via TestbedHarnessEnvironment.getNativeElement, since the assertion is about which element the host posts to).
This was referenced Oct 9, 2026
jerelvelarde
added a commit
to jerelvelarde/a2ui-composer
that referenced
this pull request
Oct 9, 2026
…mo frames a2ui-project#252 tracks renderer readiness per guest window, which makes most of the HostCommunication additions this PR used to work around the global readiness flag unnecessary: - sendToFrame is removed. DemoCard sends its demo with sendRenderA2UI addressed to its own iframe; HostCommunication marks the frame ready before subscribers see its RENDERER_READY, so the payload is posted at once. DemosCatalog sends GET_DEMOS with sendMessage addressed to the coordinator, and only after the coordinator reported ready. A request sent earlier would now be held and posted on that handshake, in addition to the one the handshake sends. - messageStreamFor is removed. DemoCard subscribes to messageStream$ and already ignores envelopes from other windows. - unregisterSecondaryIframe is removed in favour of unregisterIframe, which also withdraws the frame's readiness and queued messages and never leaves the default target on an unregistered frame. - registerSecondaryIframe is replaced by registering with DefaultTarget.KEEP. Registering a frame otherwise makes it the default target: the frame that messages sent without a target go to. CatalogManagement answers every frame's RENDERER_READY with an untargeted GET_CATALOG, so with each card taking over as it mounted, the request would go to the newest card and be dropped when the next one registered, ending in the catalog watchdog error. KEEP leaves the coordinator as the default; everything else about the registration is unchanged. The origin-parsing helper this PR extracted is dropped too, so HostCommunication now differs from main only by DefaultTarget.
This was referenced Oct 9, 2026
jerelvelarde
added a commit
to jerelvelarde/a2ui-composer
that referenced
this pull request
Oct 16, 2026
…mo frames a2ui-project#252 tracks renderer readiness per guest window, which makes most of the HostCommunication additions this PR used to work around the global readiness flag unnecessary: - sendToFrame is removed. DemoCard sends its demo with sendRenderA2UI addressed to its own iframe; HostCommunication marks the frame ready before subscribers see its RENDERER_READY, so the payload is posted at once. DemosCatalog sends GET_DEMOS with sendMessage addressed to the coordinator, and only after the coordinator reported ready. A request sent earlier would now be held and posted on that handshake, in addition to the one the handshake sends. - messageStreamFor is removed. DemoCard subscribes to messageStream$ and already ignores envelopes from other windows. - unregisterSecondaryIframe is removed in favour of unregisterIframe, which also withdraws the frame's readiness and queued messages and never leaves the default target on an unregistered frame. - registerSecondaryIframe is replaced by registering with DefaultTarget.KEEP. Registering a frame otherwise makes it the default target: the frame that messages sent without a target go to. CatalogManagement answers every frame's RENDERER_READY with an untargeted GET_CATALOG, so with each card taking over as it mounted, the request would go to the newest card and be dropped when the next one registered, ending in the catalog watchdog error. KEEP leaves the coordinator as the default; everything else about the registration is unchanged. The origin-parsing helper this PR extracted is dropped too, so HostCommunication now differs from main only by DefaultTarget.
jerelvelarde
added a commit
to jerelvelarde/a2ui-composer
that referenced
this pull request
Oct 16, 2026
…mo frames a2ui-project#252 tracks renderer readiness per guest window, which makes most of the HostCommunication additions this PR used to work around the global readiness flag unnecessary: - sendToFrame is removed. DemoCard sends its demo with sendRenderA2UI addressed to its own iframe; HostCommunication marks the frame ready before subscribers see its RENDERER_READY, so the payload is posted at once. DemosCatalog sends GET_DEMOS with sendMessage addressed to the coordinator, and only after the coordinator reported ready. A request sent earlier would now be held and posted on that handshake, in addition to the one the handshake sends. - messageStreamFor is removed. DemoCard subscribes to messageStream$ and already ignores envelopes from other windows. - unregisterSecondaryIframe is removed in favour of unregisterIframe, which also withdraws the frame's readiness and queued messages and never leaves the default target on an unregistered frame. - registerSecondaryIframe is replaced by registering with DefaultTarget.KEEP. Registering a frame otherwise makes it the default target: the frame that messages sent without a target go to. CatalogManagement answers every frame's RENDERER_READY with an untargeted GET_CATALOG, so with each card taking over as it mounted, the request would go to the newest card and be dropped when the next one registered, ending in the catalog watchdog error. KEEP leaves the coordinator as the default; everything else about the registration is unchanged. The origin-parsing helper this PR extracted is dropped too, so HostCommunication now differs from main only by DefaultTarget.
jgindin
pushed a commit
that referenced
this pull request
Oct 19, 2026
* feat(demos): add an interactive bento demo wall Browse live examples from the selected renderer in a responsive bento grid, then open one in Composer to edit its A2UI. Renderers serve their demos over the bridge (GET_DEMOS / getDemos, returning a name, description, and A2UI messages for each), mirroring GET_COMPONENT_USAGES, so the shell stays renderer-agnostic. - Angular, Lit, and React provide the same four shared showcases and 43 pinned catalog examples; shared JSON sources are validated before generating renderer modules. - Bundled adapters report when their first surface has rendered, so the wall reveals real content without a fixed measurement wait; older external adapters keep the measurement fallback, and a renderer that never finishes reports an error. - Sandboxed frames mount lazily with a cap, replies are filtered by source window, and wide previews are fitted without clipping. - Open in Composer loads the example through the shared-design route. * fix(bridge): coalesce SURFACE_RESIZE bursts and drop repeated reports The demo protocol's readiness reports (contentReady false, then true) add to the reports a renderer sends while its first surface settles. With React's layout passes that burst exceeded the host's feedback-loop cadence bound. The bridge now sends the first report of a burst at once, coalesces the rest into one trailing report per 16ms carrying the latest size, and drops a report identical to the last one sent. A readiness change still differs from the last report, so it is always delivered. The demo e2e now waits for the workspace preview to settle before editing an opened demo, and compares card row positions to within half a pixel instead of exactly. * refactor(demos): pass loggers through the validator and URL helpers, as #245 does #245 routes shell logging through ErrorLogger and makes CrossFrameValidator take a logger. The demo card, the demo frame dispatch, and the new message checks now pass theirs, and buildRendererUrl takes the caller's logger. * refactor(bridge): name the resize steps and pass sandbox options as one object Addresses review feedback on the bridge: - Rename flushSurfaceResize to sendSurfaceResize, and document the three steps a SURFACE_RESIZE report goes through (measure, coalesce, send) and what `force` is for. - Explain the 16ms coalescing window (one 60 Hz frame), why the measurement after every render isn't forced while the one after a reset is, and why renderers without the readiness hook still get measured. - Rename RendererConfig.whenSurfaceRendered to onInitialRender, and describe renderers without it instead of calling them "legacy". - A2uiSandboxConnection takes the AngularSandboxOptions object rather than four positional parameters, so argument order no longer matters. - Fix the type errors in the new bridge specs. * refactor(demos): share one generated demo list across the sample renderers Addresses review feedback on the sample demos: - The generator writes one samples/shared/demos/demos.json instead of three identical demos.ts modules, and the Angular, Lit, and React samples import it through samples/shared/demos/index.ts. - JSON carries no license header, so the generator no longer copies one out of bridge-message.ts; index.ts has an ordinary header that addlicense checks like any other file. - The README explains how the four hand-written showcase demos relate to the generated list: they lead it, followed by the 43 upstream basic-catalog examples. * refactor(demos): use enums for card state and rewrite the comments that were hard to follow DemoCardState and MeasurementPhase are enums now, and the template reads a hasError flag instead of comparing strings. Fields no longer carry their type in the name (stateSignal, cardHeightSignal, coordinatorSignal, mountedKeysSignal). Rewrote the demo catalog, card measurement and bento layout comments in plain terms: what the coordinator iframe is, what a height report is, which window or iframe each sentence means, and the bug the 32px floor guards against. * refactor(demos): use per-frame readiness from #252 for demo frames #252 tracks renderer readiness per guest window, which makes most of the HostCommunication additions this PR used to work around the global readiness flag unnecessary: - sendToFrame is removed. DemoCard sends its demo with sendRenderA2UI addressed to its own iframe; HostCommunication marks the frame ready before subscribers see its RENDERER_READY, so the payload is posted at once. DemosCatalog sends GET_DEMOS with sendMessage addressed to the coordinator, and only after the coordinator reported ready. A request sent earlier would now be held and posted on that handshake, in addition to the one the handshake sends. - messageStreamFor is removed. DemoCard subscribes to messageStream$ and already ignores envelopes from other windows. - unregisterSecondaryIframe is removed in favour of unregisterIframe, which also withdraws the frame's readiness and queued messages and never leaves the default target on an unregistered frame. - registerSecondaryIframe is replaced by registering with DefaultTarget.KEEP. Registering a frame otherwise makes it the default target: the frame that messages sent without a target go to. CatalogManagement answers every frame's RENDERER_READY with an untargeted GET_CATALOG, so with each card taking over as it mounted, the request would go to the newest card and be dropped when the next one registered, ending in the catalog watchdog error. KEEP leaves the coordinator as the default; everything else about the registration is unchanged. The origin-parsing helper this PR extracted is dropped too, so HostCommunication now differs from main only by DefaultTarget. * test(demos): drop try blocks and fixed waits from the demo tests - The Lit and Angular bootstrap tests register their cleanup with onTestFinished instead of try/finally. - The demos e2e init script only touches the Composer page's storage, so it no longer needs a try/catch for the renderer iframes. - The bridge's GET_DEMOS tests wait for the DEMOS reply with vi.waitFor instead of sleeping 10ms. - The demo wall spec's test IntersectionObserver queues the targets it observes and delivers them when the test asks, instead of on a 0ms timer. * fix(demos): show one Flight status card on the demo wall The wall showed two Flight cards: the "Flight status" showcase and the specification's own "Flight Status" basic-catalog example. The generator now drops a specification example when a showcase has the same name, ignoring case, so a showcase replaces the example it's modelled on. The wall has 46 demos instead of 47. * feat(demos): fill the demo wall's rows without empty cards The wall paired neighboring demos two to a row in reading order, which left cards mostly empty in two ways. A row is as tall as its taller card, so a short demo beside a tall one was stretched to the tall one's height: Interactive Button sat in a card three times its height beside Incremental's list, and four other rows did the same. Now, when a pair's shorter tile leaves a gap, the tiles that follow stack under it, in its column, while each brings the column closer to the tall tile's height, up to three tiles; the tall tile spans their rows. Sparse grid auto-placement puts each stacked card in the next row beside the tall one, so reading order and keyboard order are unchanged. A demo that overflowed the narrower half of a row took a full row, even when it fit the wider half, and so did the neighbor left without a partner: Live Invitation Builder and Task Card need a little more than the narrower half, and Movie Card and Music Player were left alone next to them, so all four filled the full width with demos far narrower. A demo now takes the wider half when it needs it, and a full row only when it needs more than that. DemoCard reports the narrowest card that shows its demo unscaled (minimumCardWidth, replacing needsWideLayout), and the wall reads its own width and gap from the resize observer it already has. A tile still left without a partner, before a demo that needs a full row or at the end of the wall, joins the row before it, three to a row, when that row is a plain pair and all three demos fit a third of it. Otherwise it takes a full row as before. A tile that hasn't been measured yet is paired as before and never stacked. On the Angular sample at 1440px, the emptiest card went from about 30% filled to about 60%, and the only demo on a full row is Task Card, whose content is wider than any card, as before. * fix(demos): stop a demo that is always wider than its frame from shrinking A card scales a demo that overflows its frame and widens the frame to the content width it recorded, so the demo fits. A demo that is always wider than its frame by the same amount then reported a wider width from the widened frame, and the card recorded that too: the sample Task Card is about 65px wider than any frame, and its recorded width grew from 1,347px to 1,919px in one session while its preview shrank. Once a frame is scaled, the card now records a wider width only while the demo converges: while its overflow shrinks, or repeats within 3px (widths are rounded up to whole pixels), up to 20 times in a row. React's invitation builder, which lays out wider in a wider frame, still settles, overflowing by 45, 34, 26 and so on down to 3px. Task Card is scaled once and stays put.
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.
Description
The
/a2aroute mounts several renderer iframes at once: one per inline chat surface plus the side canvas.HostCommunicationtracked renderer readiness with a single global flag, reset that flag and wiped the whole outbound queue whenever any iframe registered, andsendMessageread the flag inside the components' effects. Opening the canvas therefore briefly flipped every frame back to "not ready", re-ran everyRenderedFrameeffect, and queued and replayed the inline surfaces' payloads.A2aChatViewcompounded this by also sending the canvas payload with no target, which goes to the most recently registered iframe.Observable symptoms on
/a2aagainst an agent whose surface contains aCanvascard:DATA_MODEL_CHANGEresets to{}).Canvascard, no new canvas iframe mounts, the readiness flag never toggles, and the newer inline surface is overwritten with the canvas content for good.RENDER_A2UI/SET_THEMEdispatches to frames that did not change.What changes
Readiness is now recorded per guest window:
RENDERER_READY(or sent anMCP_REQUEST), regardless of siblings that are still loading, and a frame'sRENDERER_READYflushes only the messages addressed to it (plus untargeted ones, as before). Messages for frames that are still handshaking stay queued instead of being replayed at the newest frame.registerIframediscards only that frame's previous readiness and queued messages; frames that are already up keep both.registerIframe(null)still clears everything.unregisterIframewithdraws the frame's readiness and drops its queued messages, and does so even when the element has already left the DOM (itscontentWindowis recorded while it is attached).sendMessagereads readiness throughuntracked(), so a frame's handshake cannot become a dependency of the component effects that call it.sendTheme()(theme switches) posts to each registered frame that is up and skips frames still handshaking, which receive the current theme on their ownRENDERER_READY; the directpostMessageloop is gone. AnMCP_REQUESTfrom a frame marks it ready and flushes its queued messages, the same as a handshake.sendRenderA2UIeffects inA2aChatVieware removed; the canvasRenderedFramealready sends the payload to its own iframe and re-sends it on that iframe'sRENDERER_READY/A2UI_CATALOG.One existing spec changed meaning: it expected a message queued for frame A to be delivered when a different frame B became ready. It now expects delivery on frame A's own handshake, since a renderer that has not announced itself drops the message anyway. The specs assert on what each guest window receives via
postMessagerather than on the service's private queue.No issue was filed for this beforehand.
Verification
Rebased onto
mainat5b10985.yarn vitest runinshell/: 80 files, 1708 tests pass.host-communication.spec.ts68,a2a-chat-view.spec.ts36,rendered-frame.spec.ts35../scripts/fix_format.sh --checkandeslint .are clean.New Playwright journey
shell/e2e/a2a-multi-frame-surfaces.e2e.ts(runs with the regularyarn e2esetup against the local dev server; the agent is a Playwright route stub, no API key needed). Per turn the stub returns a surface with an inlineText("INLINE SURFACE #n") and aCanvascard ("CANVAS CONTENT #n",autoOpen: false). Scenario: turn 1 → open its canvas → turn 2 while the canvas stays open → open turn 2's canvas, then read every frame's document. On this branch it passes (3/3 repeats). Withmain'shost-communication.tsanda2a-chat-view.tsswapped in it fails at the last step: inline frame Phase 1: Shell Layout & Complete Startup Resolution Scaffolding #2 shows "CANVAS CONTENT Phase 1: Shell Layout & Complete Startup Resolution Scaffolding #2".e2e/theme-consistency.e2e.tsstill passes.Before/after in a headless Chrome against the local dev server (
yarn start,ng-basic-catalogrenderer), once onmain(8e7a477) and once on this branch, with the same scenario driven by a small A2A agent. Observed via the bridge messages received by the host page and aResizeObserveron the iframes (inline content renders at 179 px, canvas content at 88/136 px):mainDATA_MODEL_CHANGEresets, and is re-rendered after the canvas frame'sRENDERER_READYRENDER_SUCCESS×2, data-model resets); inline Add project & github boilerplate #1 ends up clipped at 131 pxRENDER_SUCCESStotal for 3 framesThe same sequence reproduces on the current GitHub Pages deployment of
main(https://a2ui-project.github.io/composer/a2awith an agent URL pointing at that agent).Pre-launch Checklist
GEMINI_API_KEY, which I do not have in this environment. The Playwright suite including the new journey was run directly.)If you need help, consider asking for advice on the discussion board.