perf(artifacts): mount the shell frame with the detail route so the shell downloads once, before the row arrives

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 <p data-testid="artifact-marker">${marker}</p>; }`,
+      },
+    });
+
+    yield* Effect.gen(function* () {
+      const detailPath = orgSlug ? `/${orgSlug}/artifacts/${saved.id}` : `/artifacts/${saved.id}`;
+
+      yield* browser.session(identity, async ({ page, step }) => {
+        const shellRequestTimes: Array<number> = [];
+        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),
+      ),
+    );
+  }),
+);

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<HTMLIFrameElement | null>(null);
   const containerRef = useRef<HTMLDivElement | null>(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<string | null>(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<AppBridge | null>(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"
     >
       <iframe
-        // Keyed on the source: opening a different artifact remounts the
-        // element, which detaches the old bridge and attaches a fresh one to a
-        // fresh document rather than reusing a shell that has already rendered
-        // something else.
-        key={props.code}
+        // Keyed on the source, once there is one: a different source remounts
+        // the element, which detaches the old bridge and attaches a fresh one to
+        // a fresh document rather than reusing a shell that has already rendered
+        // something else. The first source does not remount: the frame was
+        // mounted before it, so the shell document is already downloading.
+        key={frameGenerationRef.current}
         ref={attachFrame}
         data-testid="artifact-shell-frame"
         title="Artifact"

packages/react/src/api/artifact-renderer.test.tsx

@@ -1,4 +1,4 @@
-import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest";
+import { describe, expect, it } from "@effect/vitest";
 
 import { preloadArtifactRenderer, type ArtifactRendererLoader } from "./artifact-renderer";
 
@@ -91,44 +91,3 @@ describe("preloadArtifactRenderer", () => {
     expect(second.calls()).toBe(1);
   });
 });
-
-describe("preloadArtifactRenderer shell document prefetch", () => {
-  // The suite runs without a DOM, so a minimal `document` records what is
-  // appended to the head.
-  const appended: Array<{ rel: string; as: string; href: string }> = [];
-  const settle = async () => {
-    for (let i = 0; i < 4; i += 1) await Promise.resolve();
-  };
-
-  beforeEach(() => {
-    appended.length = 0;
-    vi.stubGlobal("document", {
-      createElement: () => ({ rel: "", as: "", href: "" }),
-      head: { appendChild: (node: (typeof appended)[number]) => appended.push(node) },
-    });
-  });
-  afterEach(() => {
-    vi.unstubAllGlobals();
-  });
-
-  it("adds one prefetch link for the shell document, however many loaders announce it", async () => {
-    const url = "/assets/executor-mcp-apps-shell-test.html";
-    const loaderFor = (): ArtifactRendererLoader => async () => ({
-      default: (() => null) as never,
-      shellHtmlUrl: url,
-    });
-
-    preloadArtifactRenderer(loaderFor());
-    preloadArtifactRenderer(loaderFor());
-    await settle();
-
-    expect(appended).toEqual([{ rel: "prefetch", as: "document", href: url }]);
-  });
-
-  it("adds no link when the renderer does not announce a document", async () => {
-    preloadArtifactRenderer(countingLoader().loader);
-    await settle();
-
-    expect(appended).toEqual([]);
-  });
-});

packages/react/src/api/artifact-renderer.tsx

@@ -29,8 +29,13 @@ import * as React from "react";
 
 /** What the page hands the shell: the stored source, and the live host. */
 export interface ArtifactRendererProps {
-  /** The artifact's stored JSX source. */
-  readonly code: string;
+  /**
+   * The artifact's stored JSX source, or `null` while the row is still being
+   * fetched. The renderer mounts its frame right away so the shell document
+   * downloads and boots in parallel with the row, and delivers the source once
+   * it arrives.
+   */
+  readonly code: string | null;
   /**
    * The server's precompile of `code` (JS and Tailwind CSS), when the host has
    * one. The shell trusts it only if its fingerprint matches the shell's own
@@ -78,9 +83,6 @@ export type ArtifactRenderer = React.ComponentType<ArtifactRendererProps>;
  */
 export type ArtifactRendererLoader = () => Promise<{
   readonly default: ArtifactRenderer;
-  /** URL of the document the renderer loads into its frame. When present,
-   *  preloading prefetches it alongside the chunk. */
-  readonly shellHtmlUrl?: string;
 }>;
 
 const ArtifactRendererContext = React.createContext<ArtifactRendererLoader | null>(null);
@@ -140,23 +142,6 @@ export function useArtifactRenderer(): ArtifactRenderer | null {
  */
 const preloaded = new WeakSet<ArtifactRendererLoader>();
 
-/**
- * Shell document URLs already hinted. The document is several megabytes and the
- * iframe only mounts after the artifact row arrives, so the hint lets the two
- * fetches overlap. One `<link>` per URL for the life of the page.
- */
-const prefetchedDocuments = new Set<string>();
-
-function prefetchDocument(url: string): void {
-  if (typeof document === "undefined" || prefetchedDocuments.has(url)) return;
-  prefetchedDocuments.add(url);
-  const link = document.createElement("link");
-  link.rel = "prefetch";
-  link.as = "document";
-  link.href = url;
-  document.head.appendChild(link);
-}
-
 /**
  * Start fetching the renderer chunk before anything needs to render it.
  *
@@ -185,9 +170,7 @@ export function preloadArtifactRenderer(loader: ArtifactRendererLoader | null):
   if (!loader || preloaded.has(loader)) return;
   preloaded.add(loader);
   void loader().then(
-    (renderer) => {
-      if (renderer.shellHtmlUrl) prefetchDocument(renderer.shellHtmlUrl);
-    },
+    () => undefined,
     () => {
       // Let a failed speculative fetch be tried again for real. Without this a
       // transient network blip during the preload would mark the loader done

packages/react/src/pages/artifact-detail.tsx

@@ -34,7 +34,6 @@ import {
 import { ArtifactStageSkeleton } from "../components/artifact-stage-skeleton";
 import { Button } from "../components/button";
 import { ErrorState } from "../components/error-state";
-import { isAsyncResultLoading } from "../lib/async-result";
 import { useExecutorDocumentTitle } from "../lib/document-title";
 import { formatRelativeTime } from "../lib/relative-time";
 import { RenameArtifactDialog } from "./artifact-rename-dialog";
@@ -202,32 +201,35 @@ export function ArtifactDetailPage(props: { readonly artifactId: ArtifactId }) {
         with the route and stays until the artifact has actually painted, with
         every one of those boundaries crossed underneath it.
 
-        `isAsyncResultLoading` and `onInitial` therefore collapse into the SAME
-        surface the stage shows, rather than each having its own.
+        The stage mounts with the route too, so the loading row, the chunk and
+        the shell document all run in parallel under that one skeleton.
       */}
       <div className="min-h-0 flex-1 overflow-hidden">
-        {isAsyncResultLoading(artifact) ? (
-          <ArtifactStageSkeleton />
-        ) : (
-          AsyncResult.match(artifact, {
-            onInitial: () => <ArtifactStageSkeleton />,
-            // A deleted or foreign id lands here. The message says which of the
-            // two it is as far as this viewer can tell, and offers the way back.
-            // An error REPLACES the skeleton rather than stacking on it: the
-            // wait is over, and what it produced was an answer.
-            onFailure: () => (
-              <div className="p-6">
-                <ErrorState
-                  message="This artifact isn't available. It may have been deleted."
-                  onRetry={refresh}
-                />
-              </div>
-            ),
-            onSuccess: ({ value }) => (
-              <ArtifactStage code={value.code} compiled={value.compiled} artifactId={value.id} />
-            ),
-          })
-        )}
+        {
+          // A failed row REPLACES the stage rather than stacking on it: the
+          // wait is over, what it produced was an answer, and unmounting the
+          // stage takes the early-booted frame with it so no blank frame is
+          // left behind the error. A deleted or foreign id lands here; the
+          // message says which of the two it is as far as this viewer can tell,
+          // and offers the way back.
+          AsyncResult.isFailure(artifact) ? (
+            <div className="p-6">
+              <ErrorState
+                message="This artifact isn't available. It may have been deleted."
+                onRetry={refresh}
+              />
+            </div>
+          ) : (
+            // Mounted while the row is still in flight (`code` null): the shell
+            // document is the largest download on this page, and the frame
+            // itself is what starts it, so it must not wait for the row.
+            <ArtifactStage
+              code={AsyncResult.isSuccess(artifact) ? artifact.value.code : null}
+              compiled={AsyncResult.isSuccess(artifact) ? artifact.value.compiled : undefined}
+              artifactId={props.artifactId}
+            />
+          )
+        }
       </div>
     </div>
   );
@@ -248,7 +250,7 @@ export function ArtifactDetailPage(props: { readonly artifactId: ArtifactId }) {
  * instead of crashing.
  */
 function ArtifactStage(props: {
-  readonly code: string;
+  readonly code: string | null;
   readonly compiled: ArtifactRendererProps["compiled"];
   readonly artifactId: string;
 }) {