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" >