Skip to content

feat(site): harden Agents embed frame communication and add theme sync#23574

Open
EhabY wants to merge 5 commits intomainfrom
fix/agents-embed-frame-communication
Open

feat(site): harden Agents embed frame communication and add theme sync#23574
EhabY wants to merge 5 commits intomainfrom
fix/agents-embed-frame-communication

Conversation

@EhabY
Copy link
Contributor

@EhabY EhabY commented Mar 25, 2026

🤖 This PR was written by Coder Agent on behalf of Ehab Younes 🤖

  • Move the scroll-to-bottom message handler from AgentEmbedPage into AgentDetail so it uses the existing scrollContainerRef instead of a brittle querySelector lookup.
  • Track the last signaled agent ID in the chat-ready effect so the coder:chat-ready notification re-fires when the user switches agents without a full remount.
  • Block in-app navigations that leave the embed route and forward the target URL to the parent frame via coder:navigate.
  • Add coder:set-theme message support so the parent frame (e.g. VS Code) can control the embed's color scheme. On mount, theme query is applied immediately to avoid a theme flash; explicit messages override it at any time. ThemeProvider skips its own class manipulation while the embed is active.

@EhabY EhabY self-assigned this Mar 25, 2026
@github-actions github-actions bot added the community Pull Requests and issues created by the community. label Mar 25, 2026
@EhabY EhabY changed the title fix(site/src/pages/AgentsPage): harden embed frame communication feat(site): harden Agents embed frame communication and add theme sync Mar 25, 2026
@EhabY EhabY force-pushed the fix/agents-embed-frame-communication branch from 5b19e4b to 28ed743 Compare March 25, 2026 12:09
Comment on lines +854 to +857
// When embedded, scroll to bottom once messages load and
// notify the parent frame that the chat is ready. Comparing
// against the last signaled agentId re-fires the notification
// when the user switches agents without a full remount.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this in the AgentEmbedPage itself rather than here? Conflating them is kinda unfortunate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I've added a callback that we can just listen to in AgentEmbedPage (onChatReady)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I can remove the whole embedding logic from here but then if I want to scroll to the bottom I'd have to do this:

document.querySelector<HTMLElement>(
					'[data-testid="scroll-container"]',
				);

instead of using refs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass a scroll ref instead and seems to work better now!

@EhabY EhabY force-pushed the fix/agents-embed-frame-communication branch from 28ed743 to 477fca0 Compare March 25, 2026 13:24
Copy link
Contributor

@DanielleMaywood DanielleMaywood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review by Pluto 🐕 and team — 3 verified findings (all P3 nits).

No blocking issues. The core implementation is solid: message validation is defensive, effect cleanup is correct for the primary flow, layout/effect timing is well-coordinated, and the useBlocker + postMessage forwarding pattern works cleanly.

🤖 This comment was written by Coder Agent on behalf of Danielle Maywood 🤖

EhabY added 5 commits March 25, 2026 17:36
Move the scroll-to-bottom message listener from AgentEmbedPage into
AgentDetail so it uses the existing scrollContainerRef instead of a
brittle querySelector lookup. Track the last signaled agentId in the
chat-ready effect so the notification re-fires when agents change
without a full remount. Include location hash in the navigation
blocker payload.
Let the parent frame control the embed's color scheme with
coder:set-theme messages. On mount the embed applies
prefers-color-scheme as an initial value, then listens for
explicit theme messages. ThemeProvider skips its own class
manipulation when the embed is active, preventing races.
Also adds a no-op guard to applyEmbedTheme so duplicate
messages skip DOM mutations.
…tDetail

Move all embed-specific logic out of AgentDetail into
AgentEmbedPage. Chat-ready notification uses the onChatReady
outlet context callback. Scroll-to-bottom handling moves to the
parent frame message listener in AgentEmbedPage. AgentDetail no
longer imports useEmbedContext.
- Remove useCallback from useBlocker (React Compiler handles memoization)
- Add classList.remove in theme cleanup to match setup symmetry
- Pass scrollContainerRef through outlet context so AgentEmbedPage
  uses the ref directly instead of querySelector for scroll-to-bottom
@EhabY EhabY force-pushed the fix/agents-embed-frame-communication branch from 580daeb to 66817fa Compare March 25, 2026 14:36
@EhabY EhabY requested a review from kylecarbs March 25, 2026 14:38
@coder-tasks
Copy link
Contributor

coder-tasks bot commented Mar 25, 2026

Documentation Check

New Documentation Needed

  • docs/ai-coder/agents/embed.md (or similar) — The /agents/:agentId/embed route exposes a postMessage protocol used by parent frames (e.g. VS Code). This PR adds coder:set-theme and coder:navigate messages alongside existing ones (coder:chat-ready, coder:scroll-to-bottom, coder:bootstrap). No documentation currently covers this integration API. Integrators building parent-frame embeddings have no reference for the supported message types, payloads, or the ?theme= query param.

Automated review via Coder Tasks

Copy link
Contributor

@DanielleMaywood DanielleMaywood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with this but wanna make sure Kyle is

@matifali matifali removed the community Pull Requests and issues created by the community. label Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants