From 45ed99aae53f09b8ff100017a1e477213d47f71b Mon Sep 17 00:00:00 2001 From: fforres Date: Sun, 18 Oct 2026 01:23:55 -0700 Subject: [PATCH] perf(artifacts): mount the shell frame with the detail route so the shell downloads once, before the row arrives The artifact page used to add a prefetch hint for the shell document and then mount the frame after the row loaded, which made two downloads of the largest asset. The frame now mounts as soon as the route renders, boots while the row is fetched, and receives the source over the existing bridge. A failed row replaces the stage with the error state, so no booted frame stays behind. Adds a scenario that asserts one shell request on a cold cache, started before the row response completes. Claude-Session: https://claude.ai/code/session_019ZE3j9YzRKxT4LaiDCBv6S --- .../artifact-shell-single-download.test.ts | 104 ++++++++++++++++++ .../src/shell/artifact-renderer.tsx | 49 +++++++-- .../react/src/api/artifact-renderer.test.tsx | 43 +------- packages/react/src/api/artifact-renderer.tsx | 33 ++---- packages/react/src/pages/artifact-detail.tsx | 54 ++++----- 5 files changed, 180 insertions(+), 103 deletions(-) create mode 100644 e2e/scenarios/artifact-shell-single-download.test.ts diff --git a/e2e/scenarios/artifact-shell-single-download.test.ts b/e2e/scenarios/artifact-shell-single-download.test.ts new file mode 100644 index 0000000000..1f60c705f8 --- /dev/null +++ b/e2e/scenarios/artifact-shell-single-download.test.ts @@ -0,0 +1,104 @@ +// On a cold cache the artifact's shell document is downloaded exactly once, and +// that download starts before the artifact row has come back. +// +// The shell is the largest thing the page loads. A hint from the top frame does +// not make the iframe's own navigation free: the frame issues its own request, +// so a prefetch plus a frame is two downloads. The frame therefore mounts with +// the route and starts the one download itself, while the row is still in +// flight; the source reaches it over the bridge afterwards. +// +// The row response is held for a moment so "the shell request started first" is +// a property of the page rather than of how fast the API happened to answer. +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { AccountHttpApi } from "@executor-js/api"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; + +const api = composePluginApi([] as const); + +const ROW_DELAY_MS = 1_500; + +scenario( + "Artifacts · a cold open downloads the shell once, before the row arrives", + { timeout: 180_000 }, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: apiClient } = yield* Api; + + const identity = yield* target.newIdentity(); + const client = yield* apiClient(api, identity); + const accountClient = yield* apiClient(AccountHttpApi, identity); + const orgSlug = (yield* accountClient.account.me()).organization?.slug; + + const marker = `single-download-${randomBytes(4).toString("hex")}`; + const saved = yield* client.artifacts.save({ + payload: { + title: `Single download ${marker}`, + code: `function App() { return

${marker}

; }`, + }, + }); + + yield* Effect.gen(function* () { + const detailPath = orgSlug ? `/${orgSlug}/artifacts/${saved.id}` : `/artifacts/${saved.id}`; + + yield* browser.session(identity, async ({ page, step }) => { + const shellRequestTimes: Array = []; + let rowFinishedAt: number | null = null; + + await step( + "Open the artifact by URL with an empty cache, row response delayed", + async () => { + const isRow = (url: string, resourceType: string) => + resourceType === "fetch" && url.includes(`/artifacts/${saved.id}`); + + await page.route( + (url) => url.pathname.includes(`/artifacts/${saved.id}`), + async (route) => { + if (isRow(route.request().url(), route.request().resourceType())) { + await new Promise((resolve) => setTimeout(resolve, ROW_DELAY_MS)); + } + await route.continue(); + }, + ); + page.on("request", (request) => { + // Every request for the shell document counts, whatever started + // it: a prefetch hint is a second download just like a second frame. + if (/mcp-apps-shell.*\.html/.test(new URL(request.url()).pathname)) { + shellRequestTimes.push(Date.now()); + } + }); + page.on("requestfinished", (request) => { + if (isRow(request.url(), request.resourceType())) rowFinishedAt = Date.now(); + }); + + await page.goto(`${target.baseUrl}${detailPath}`, { waitUntil: "commit" }); + await page + .frameLocator('[data-testid="artifact-shell-frame"]') + .frameLocator("iframe") + .getByTestId("artifact-marker") + .waitFor({ timeout: 60_000 }); + }, + ); + + await step("The shell document was requested once, before the row finished", async () => { + expect(shellRequestTimes.length, "the shell document is downloaded exactly once").toBe(1); + expect(rowFinishedAt, "the artifact row response was observed").not.toBeNull(); + expect( + (rowFinishedAt ?? 0) - (shellRequestTimes[0] ?? 0), + "the shell request started before the row response completed", + ).toBeGreaterThan(0); + }); + }); + }).pipe( + Effect.ensuring( + client.artifacts.remove({ params: { artifactId: saved.id } }).pipe(Effect.ignore), + ), + ); + }), +); diff --git a/packages/hosts/mcp-apps-shell/src/shell/artifact-renderer.tsx b/packages/hosts/mcp-apps-shell/src/shell/artifact-renderer.tsx index 95a5ecaf09..a4ac4fb75d 100644 --- a/packages/hosts/mcp-apps-shell/src/shell/artifact-renderer.tsx +++ b/packages/hosts/mcp-apps-shell/src/shell/artifact-renderer.tsx @@ -136,10 +136,6 @@ const hostContextFor = (theme: "light" | "dark", viewport: number | null): McpUi [RENDER_SIGNAL_HOST_CONTEXT_KEY]: true, }); -// Read by `preloadArtifactRenderer` so the document download starts with the -// chunk instead of when the iframe mounts. -export { shellHtmlUrl }; - export default function ArtifactShell(props: ArtifactRendererProps) { const frameRef = useRef(null); const containerRef = useRef(null); @@ -170,6 +166,20 @@ export default function ArtifactShell(props: ArtifactRendererProps) { hostRef.current = props.host as HttpShellHost; const codeRef = useRef(props.code); codeRef.current = props.code; + // The frame is mounted before the source exists (`props.code` is `null` while + // the row is fetched), so it must not be remounted when the first source + // arrives, only when a LATER source replaces it. `claimedCodeRef` is the + // source the current frame is responsible for; `frameGenerationRef` is the + // iframe key. + const claimedCodeRef = useRef(null); + const frameGenerationRef = useRef(0); + if (props.code !== null && props.code !== claimedCodeRef.current) { + if (claimedCodeRef.current !== null) frameGenerationRef.current += 1; + claimedCodeRef.current = props.code; + } + // Sends the source to the current bridge once it is both initialized and has + // a source; registered by `attachFrame`, called again when the source lands. + const deliverRef = useRef<(() => void) | null>(null); const compiledRef = useRef(props.compiled); compiledRef.current = props.compiled; const artifactIdRef = useRef(props.artifactId); @@ -187,6 +197,10 @@ export default function ArtifactShell(props: ArtifactRendererProps) { const bridgeRef = useRef(null); + useEffect(() => { + deliverRef.current?.(); + }, [props.code, props.compiled]); + // Track the console's theme and forward it as host context, which is what // drives the shell's own `applyTheme` — inside the iframe's document, where it // belongs, rather than on the console's `documentElement`. @@ -310,8 +324,16 @@ export default function ArtifactShell(props: ArtifactRendererProps) { // exact shape `renderedInAppResult` builds in the MCP host. The shell // already handles both, so it takes the same path here as it does under any // other MCP-Apps client, with no artifact-page-only branch. - bridge.oninitialized = () => { + // + // The frame can initialize before the row has arrived, so delivery waits for + // both: whichever of "initialized" and "source known" happens last sends it, + // exactly once per frame. + let initialized = false; + let delivered = false; + const deliver = () => { const code = codeRef.current; + if (!initialized || delivered || code === null) return; + delivered = true; const compiled = compiledRef.current; const artifactId = artifactIdRef.current; // `compiled` rides on both messages: the shell renders on the input and @@ -328,6 +350,11 @@ export default function ArtifactShell(props: ArtifactRendererProps) { console.error("[executor-console] Failed to deliver the artifact to the shell:", error); }); }; + bridge.oninitialized = () => { + initialized = true; + deliver(); + }; + deliverRef.current = deliver; /** * The app keeps reporting its content height. In fill mode this host @@ -396,6 +423,7 @@ export default function ArtifactShell(props: ArtifactRendererProps) { // null out its successor. return () => { void bridge.close(); + if (deliverRef.current === deliver) deliverRef.current = null; if (bridgeRef.current === bridge) bridgeRef.current = null; if (frameRef.current === frame) frameRef.current = null; }; @@ -413,11 +441,12 @@ export default function ArtifactShell(props: ArtifactRendererProps) { className="h-full min-h-0 w-full" >