diff --git a/.changeset/workbench-app-preview-close-before-ready.md b/.changeset/workbench-app-preview-close-before-ready.md new file mode 100644 index 000000000..d28976dce --- /dev/null +++ b/.changeset/workbench-app-preview-close-before-ready.md @@ -0,0 +1,5 @@ +--- +"agent-bundle": patch +--- + +Close a Workbench MCP App preview immediately when its sandbox proxy has not loaded yet: closing the preview (or switching its profile, deactivating the MCP page, or ending the session) before the proxy signals readiness now releases the binding at once instead of posting a teardown no window can acknowledge and holding the preview in its closing state for the full five-second force-close budget (#435) diff --git a/docs/local-ci.md b/docs/local-ci.md index dd73a4cbe..829d647ff 100644 --- a/docs/local-ci.md +++ b/docs/local-ci.md @@ -187,6 +187,41 @@ write for a second epoch. Those edits now go through one atomic replacement, then a wait on the coordinator's published build attempt, so one edit is exactly one build. +## Infrastructure failures and their retry policy + +Two failure shapes in the hosted Release gates are registry or runner +infrastructure, not the tree under test. Neither gets a code-level retry, +and neither is a reason to weaken the gate; the policy is to re-run the job +once the cause has cleared, then treat a repeat as a real signal. + +- **`npm audit signatures` → `EATTESTATIONVERIFY`** (from + `scripts/audit-packed-release.mjs`, reached through `pnpm audit:release`). + Example (CI run 33584654855, 2026-09-02, Release gates on Node 22.19): + `@modelcontextprotocol/server@2.0.0 failed to verify attestation: + Unexpected end of JSON input`. npm fetched a truncated attestation bundle + from `registry.npmjs.org` for a dependency this repository does not + publish; the same pinned version and integrity verify on every later run + of the same gate without any lockfile change. The audit deliberately + installs against live registry metadata (no `--prefer-offline`), so a + registry-side transient reaches it unfiltered. Policy: read the JSON in the + step log first; if the `invalid` entry names a third-party package with an + unchanged pinned version and a parse-shaped message (`Unexpected end of + JSON input`, `Unexpected token`, a 5xx), re-run the failed job (`gh run + rerun --failed`). If the same package fails twice in a row, or the + message is a genuine signature mismatch (`EATTESTATIONSIGNATURE`, + `EINTEGRITY`), stop and investigate the dependency before merging: that is + the supply-chain check doing its job. Do not add retries around the audit + command and do not relax `--json` parsing to tolerate the error. +- **Runner network stalls during `npm install`** in the packed pool. The + pool's consumer installs are cache-backed per worker + (`rstest.worker-isolation.ts`): each worker pays for one cold download of + the packed dependency tree (about 180 MB), and `public-api-packed` warms + that cache in a `beforeAll` with its own budget so no per-test budget spans + a cold network. A remaining timeout inside that `beforeAll` on a hosted + runner is a registry or runner-network stall; re-run the job. A timeout in + a test body after the warm-up is not: read the failure text, it names the + step (install, `tsc`, CLI) that overran. + ## What is deliberately not covered - **dependency-review** runs as a GitHub-side action against the GitHub diff --git a/packages/agent-bundle/rslib.config.ts b/packages/agent-bundle/rslib.config.ts index 6298a57a6..4cec21a74 100644 --- a/packages/agent-bundle/rslib.config.ts +++ b/packages/agent-bundle/rslib.config.ts @@ -32,6 +32,18 @@ const esmNodeGlobalsPlugin = (rspack: typeof RspackInstance): Rspack.RspackPlugi }, }); +/** + * Rslib enables Rspack's persistent build cache by default and keys its + * directory by this config's root (`node_modules/.cache/rspack`), never by + * `--dist-path`. Two builds of this config running at once — the packed pool's + * packed-consumer and dev-workbench-packaging suites each rebuild it into an + * isolated dist from parallel workers — would then contend for a single cache + * lock ("Transaction already in progress by process … in directory …"). Test + * harnesses hand every spawned build its own directory through this variable + * (rstest.worker-isolation.ts); `pnpm build` keeps the default warm cache. + */ +const buildCacheDirectory = process.env['AGENT_BUNDLE_RSLIB_CACHE_DIRECTORY']; + export default defineConfig({ lib: [ { @@ -50,6 +62,9 @@ export default defineConfig({ legalComments: 'linked', target: 'node', }, + ...(buildCacheDirectory === undefined || buildCacheDirectory.length === 0 + ? {} + : { performance: { buildCache: { cacheDirectory: buildCacheDirectory } } }), // Suggestions stay informational; errors and warnings block publishing. plugins: [pluginPublint({ throwOn: 'warning' })], root: import.meta.dirname, diff --git a/packages/agent-bundle/tests/dev-workbench-packaging.test.ts b/packages/agent-bundle/tests/dev-workbench-packaging.test.ts index 02bf6a1c4..73b620f7a 100644 --- a/packages/agent-bundle/tests/dev-workbench-packaging.test.ts +++ b/packages/agent-bundle/tests/dev-workbench-packaging.test.ts @@ -60,11 +60,14 @@ it('prunes stale copied workbench assets without removing the package library ou await mkdir(join(workbench, 'static', 'js', 'async'), { recursive: true }); await writeFile(stale, 'obsolete workbench output\n'); await expect(access(stale)).resolves.toBeUndefined(); + // installedEnvironment() gives this build its own persistent-cache + // directory: packed-consumer rebuilds the same config from another + // worker, and Rslib keys the shared default by the config root. await execFile(join(workspaceRoot, 'node_modules', '.bin', 'rslib'), [ 'build', '--config', join(packageRoot, 'rslib.config.ts'), '--dist-path', isolatedDist, - ], { cwd: workspaceRoot }); + ], { cwd: workspaceRoot, env: installedEnvironment() }); await expect(access(stale)).rejects.toThrow(); await expect(access(join(isolatedDist, 'cli.js'))).resolves.toBeUndefined(); expect(await readdir(workbench, { recursive: true })).not.toContain('index.js.map'); diff --git a/packages/agent-bundle/tests/dev-workbench.test.ts b/packages/agent-bundle/tests/dev-workbench.test.ts index 12a9ef2ce..836a1d7c9 100644 --- a/packages/agent-bundle/tests/dev-workbench.test.ts +++ b/packages/agent-bundle/tests/dev-workbench.test.ts @@ -1,4 +1,4 @@ -import { access, mkdtemp, mkdir, readFile, rm, symlink, writeFile } from 'node:fs/promises'; +import { access, mkdtemp, mkdir, readdir, readFile, rm, symlink, writeFile } from 'node:fs/promises'; import { get as httpGet } from 'node:http'; import { tmpdir } from 'node:os'; import { join, relative } from 'node:path'; @@ -1110,17 +1110,28 @@ it('prepares the optional runtime once with the development config context befor }); let proxyCalls = 0; let surfaceCloseCalls = 0; + // Both fixture records live outside the watched project source. The dev + // watcher treats every non-ignored path under the project root as source + // (the project snapshot is broad by design), so a config that appended its + // calls inside the root, or a provider that wrote its start context there, + // would itself be a source change: the watcher rebuilds, the rebuild + // re-loads the config, the load appends again, and the dev-load count + // climbs until the test happens to read it. The provider uses its + // contract-provided storageRoot (`.agent-bundle/runtime/`, excluded + // from watching and snapshots); the config logs beside the workbench assets. + const configCallsPath = join(assetsRoot, 'config-calls.ndjson'); await mkdir(join(project.root, 'src', 'dev'), { recursive: true }); await Promise.all([ writeFile(join(assetsRoot, 'index.html'), 'Agent Bundle workbench'), writeFile(join(project.root, 'src', 'dev', 'provider.ts'), [ - "import { writeFile } from 'node:fs/promises';", + "import { mkdir, writeFile } from 'node:fs/promises';", "import { join } from 'node:path';", '', 'export const createDevRuntimeProvider = () => ({', " descriptor: { environmentVariables: [], id: 'fixture-runtime', label: 'Fixture runtime', schemaVersion: 1 },", ' start: async (context) => {', - " await writeFile(join(context.projectRoot, 'provider-context.json'), JSON.stringify({", + ' await mkdir(context.storageRoot, { recursive: true });', + " await writeFile(join(context.storageRoot, 'provider-context.json'), JSON.stringify({", ' artifact: context.artifactStatus(),', ' environment: context.environment,', ' preparedRuntime: context.preparedRuntime,', @@ -1142,11 +1153,10 @@ it('prepares the optional runtime once with the development config context befor ].join('\n')), writeFile(project.configPath, [ "import { appendFile } from 'node:fs/promises';", - "import { join } from 'node:path';", "import { defineConfig } from 'agent-bundle';", '', - 'export default defineConfig(async ({ command, mode, projectRoot }) => {', - " await appendFile(join(projectRoot, 'config-calls.ndjson'), JSON.stringify({ command, mode }) + '\\n');", + 'export default defineConfig(async ({ command, mode }) => {', + ` await appendFile(${JSON.stringify(configCallsPath)}, JSON.stringify({ command, mode }) + '\\n');`, ' return {', " dev: { runtime: { provider: './src/dev/provider.ts' } },", " plugin: { name: 'runtime-fixture', version: '1.0.0' },", @@ -1171,9 +1181,13 @@ it('prepares the optional runtime once with the development config context befor }, }); + const runtimeStorageRoot = join(project.root, '.agent-bundle', 'runtime'); + const [providerSessionDirectory, ...otherSessionDirectories] = await readdir(runtimeStorageRoot); + if (providerSessionDirectory === undefined) throw new Error('The fixture provider did not create its storage root.'); + expect(otherSessionDirectories).toEqual([]); const [calls, context, runtimeStatus, projectStatus] = await Promise.all([ - readFile(join(project.root, 'config-calls.ndjson'), 'utf8'), - readFile(join(project.root, 'provider-context.json'), 'utf8').then(JSON.parse) as Promise>, + readFile(configCallsPath, 'utf8'), + readFile(join(runtimeStorageRoot, providerSessionDirectory, 'provider-context.json'), 'utf8').then(JSON.parse) as Promise>, fetch(`${server.url}/api/runtime/status`).then((response) => response.json()), fetch(`${server.url}/api/project/status`).then((response) => response.json()), ]); @@ -1186,7 +1200,7 @@ it('prepares the optional runtime once with the development config context befor environment: {}, preparedRuntime: { provider: './src/dev/provider.ts' }, projectRoot: project.root, - storageRoot: expect.stringMatching(new RegExp(`^${join(project.root, '.agent-bundle', 'runtime').replace(/[.*+?^${}()|[\]\\]/gu, '\\$&')}/`)), + storageRoot: join(runtimeStorageRoot, providerSessionDirectory), }); expect(runtimeStatus).toMatchObject({ status: { descriptor: { id: 'fixture-runtime' }, state: 'active' } }); expect(projectStatus).toMatchObject({ status: { runtime: { state: 'configured' } } }); diff --git a/packages/agent-bundle/tests/public-api-packed.test.ts b/packages/agent-bundle/tests/public-api-packed.test.ts index f8d6b5250..9cdb2c105 100644 --- a/packages/agent-bundle/tests/public-api-packed.test.ts +++ b/packages/agent-bundle/tests/public-api-packed.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { promisify } from 'node:util'; -import { expect, it } from '@rstest/core'; +import { beforeAll, expect, it } from '@rstest/core'; import { isolatedCommandEnvironment } from '../../../rstest.worker-isolation.ts'; import { writeFixtureManifest } from './support/manifest.ts'; @@ -51,6 +51,31 @@ const producerFrom = async (output: string): Promise<{ readonly name: string; re return manifest.producer; }; +/** + * Warms this worker's npm cache with the packed tarball's full dependency + * tree once, so the three consumer installs below are cache-backed + * (`--prefer-offline` then serves every tarball and metadata record from + * disk). The download is the one network-bound step in this file: about + * 180 MB of registry tarballs, and a CI runner's npm cache is empty at job + * start (pnpm/setup caches only the pnpm store), so the Release gates and + * Verify jobs always pay it exactly once here, never inside a test's 30 s + * budget. The budget below covers that cold download on a slow runner + * network; a warm worker cache finishes in a few seconds. + */ +beforeAll(async () => { + const { tarball } = await sharedPackedTarball('agent-bundle'); + const warmRoot = await mkdtemp(join(tmpdir(), 'agent-bundle-packed-npm-warm-')); + try { + await writeFile(join(warmRoot, 'package.json'), '{"type":"module"}\n'); + await execFile( + 'npm', ['install', ...cachedNpmInstallArguments, tarball], + { cwd: warmRoot, env: isolatedCommandEnvironment() }, + ); + } finally { + await rm(warmRoot, { force: true, recursive: true }); + } +}, 180_000); + it('writes the package version as the producer of a packed CLI manifest', async () => { const { tarball } = await sharedPackedTarball('agent-bundle'); diff --git a/packages/workbench/src/mcp/mcp-app-frame.tsx b/packages/workbench/src/mcp/mcp-app-frame.tsx index 3b57cf2fe..7c84c2454 100644 --- a/packages/workbench/src/mcp/mcp-app-frame.tsx +++ b/packages/workbench/src/mcp/mcp-app-frame.tsx @@ -220,6 +220,16 @@ export class McpAppFrameRelay { if (this.#closePromise !== undefined) return this.#closePromise; this.#state = 'closing'; this.#closePromise = new Promise((resolve) => { this.#finishClose = resolve; }); + // Before the proxy signals readiness there is no app to tear down and no + // window that can acknowledge a teardown frame: the proxy document is + // still loading (or is the initial about:blank, whose origin never matches + // targetOrigin, so postMessage drops the frame silently). The graceful + // handshake could only wait out the force timer, so release the binding + // now instead of holding the closing state for the whole budget. + if (!this.#resourceProvided) { + void this.#forceClose(); + return this.#closePromise; + } this.#closeTimer = setTimeout(() => { void this.#forceClose(); }, this.#closeTimeoutMs); this.#enqueue(() => this.#beginClose(), true); return this.#closePromise; diff --git a/packages/workbench/tests/mcp-app-frame.test.ts b/packages/workbench/tests/mcp-app-frame.test.ts index 5141823f2..513b541be 100644 --- a/packages/workbench/tests/mcp-app-frame.test.ts +++ b/packages/workbench/tests/mcp-app-frame.test.ts @@ -385,11 +385,15 @@ describe('MCP App frame relay', () => { expect(messages).toHaveLength(1); }); - it('force-deletes a closing binding when the proxy never acknowledges the teardown frame', async () => { + it('force-deletes a closing binding when the ready proxy never acknowledges the teardown frame', async () => { const browser = fakeBrowser(); + let closeCalls = 0; let forceClosed = false; const routes: McpAppFrameRelayRoutes = { - close: async (_bindingId, options) => closeResult({ id: options.id, jsonrpc: '2.0', method: 'ui/resource-teardown', params: {} }), + close: async (_bindingId, options) => { + closeCalls += 1; + return closeResult({ id: options.id, jsonrpc: '2.0', method: 'ui/resource-teardown', params: {} }); + }, forceClose: async () => { forceClosed = true; return true; @@ -398,12 +402,44 @@ describe('MCP App frame relay', () => { }; const relay = createMcpAppFrameRelay({ bindingId: 'binding-weather', closeTimeoutMs: 1, frame, iframe: browser.iframe, resource, routes, window: browser.window }); relay.start(); + relay.receive({ data: proxyReady(), origin: frame.targetOrigin, source: browser.child }); await relay.close(); + expect(closeCalls).toBe(1); expect(forceClosed).toBe(true); }); + it('force-deletes immediately, without a teardown handshake or the timer, when the proxy never signaled readiness', async () => { + const browser = fakeBrowser(); + let closeCalls = 0; + let forceCloseCalls = 0; + const routes: McpAppFrameRelayRoutes = { + close: async (_bindingId, options) => { + closeCalls += 1; + return closeResult({ id: options.id, jsonrpc: '2.0', method: 'ui/resource-teardown', params: {} }); + }, + forceClose: async () => { + forceCloseCalls += 1; + return true; + }, + message: async () => messageResult(), + }; + // The full 30 s budget: a timer-driven fallback would fail this test. + const relay = createMcpAppFrameRelay({ bindingId: 'binding-weather', closeTimeoutMs: 30_000, frame, iframe: browser.iframe, resource, routes, window: browser.window }); + relay.start(); + + await relay.close(); + + expect(closeCalls).toBe(0); + expect(forceCloseCalls).toBe(1); + expect(relay.state).toBe('closed'); + expect(browser.child.posts).toEqual([]); + // A proxy that reports ready after the close began is a late arrival, not + // a reopened relay. + expect(relay.receive({ data: proxyReady(), origin: frame.targetOrigin, source: browser.child })).toBe(false); + }); + it('renders the exact server-issued sandbox URL and no inline document or credential-bearing attribute', () => { const markup = renderToStaticMarkup(createElement(McpAppFrame, { bindingId: 'binding-weather', diff --git a/packages/workbench/tests/mcp-app-real.e2e.test.ts b/packages/workbench/tests/mcp-app-real.e2e.test.ts index a2ad2bb0c..64919100b 100644 --- a/packages/workbench/tests/mcp-app-real.e2e.test.ts +++ b/packages/workbench/tests/mcp-app-real.e2e.test.ts @@ -424,6 +424,15 @@ e2e('runs a generated SDK-v2 App through the real foreground session and separat await page.getByRole('button', { name: 'Open App preview for mcp-page-1' }).click(); await expect(outerFrame).toBeVisible({ timeout: browserTimeout }); + // A visible iframe only proves the element mounted. The graceful /close + // handshake below needs a proxy that has loaded and an app that has + // initialized (a preview closed before its proxy signals readiness is + // released by DELETE, with nothing to acknowledge a teardown), so wait for + // the reopened binding's own `initialized` notification first. + await expect.poll(() => appRequests.filter((request) => { + const message = (request.body as { readonly message?: { readonly method?: string } } | undefined)?.message; + return request.path.endsWith('/messages') && message?.method === 'ui/notifications/initialized'; + }).length, { timeout: browserTimeout }).toBe(2); const secondClose = page.waitForRequest((request) => request.url().startsWith(`${foregroundOrigin}/api/mcp/apps/`) && request.url().endsWith('/close'), { timeout: 30_000 * timeScale }); const closedSession = page.waitForRequest((request) => request.url() === `${foregroundOrigin}/api/mcp/sessions/${openedSession.session.id}` && request.method() === 'DELETE', { timeout: 30_000 * timeScale }); @@ -595,14 +604,40 @@ e2e('opens the real RSC runtime timeline App from provider-owned run evidence', await page.getByLabel('Runtime target').selectOption('portable'); await page.getByRole('radio', { name: 'Raw JSON' }).check(); await page.locator('#runtime-input-raw').fill('{}'); + // The App preview create request is the last link of a chain: Run admits + // a runtime run, the run settles, and only a run that succeeded with App + // binding evidence makes the stage mount a preview (which then POSTs + // /api/runtime/apps). Wait on each link in order so a failed or + // evidence-less run reports its own diagnostics instead of surfacing as a + // create request that never arrives. + const runAdmitted = page.waitForResponse((response) => + response.url() === `${fixture.url}/api/runtime/runs` && response.request().method() === 'POST', { timeout: 30_000 * timeScale }); const createRequest = page.waitForRequest((request) => request.url() === `${fixture.url}/api/runtime/apps` && request.method() === 'POST', { timeout: 30_000 * timeScale }); const createResponse = page.waitForResponse((response) => response.url() === `${fixture.url}/api/runtime/apps` && response.request().method() === 'POST', { timeout: 30_000 * timeScale }); await page.getByRole('button', { name: 'Run', exact: true }).click(); - const [createdRequest, createdResponse] = await Promise.all([createRequest, createResponse]); + const admittedRun = await runAdmitted; + if (!admittedRun.ok()) throw new Error(`Runtime run admission failed with HTTP ${admittedRun.status()}: ${await admittedRun.text()}`); const history = page.getByRole('region', { name: 'Runtime run history' }).locator('ol > li'); await expect(history).toHaveCount(1, { timeout: 15_000 * timeScale }); + const runStatus = (): Promise => history.first().locator('button').first().textContent() + .then((label) => /· (succeeded|failed) ·/u.exec(label ?? '')?.[1]); + await expect.poll(runStatus, { timeout: 30_000 * timeScale }).toBeDefined(); + if (await runStatus() !== 'succeeded') { + throw new Error(`Runtime run did not succeed: ${JSON.stringify({ + console: browserConsole, + pageErrors: pageErrors.map((error) => error.message), + stage: await page.getByRole('region', { name: 'Runtime output stage' }).textContent(), + })}`); + } + const [createdRequest, createdResponse] = await Promise.all([createRequest, createResponse]).catch(async (error: unknown) => { + throw new Error(`Runtime App preview create request did not follow the succeeded run: ${JSON.stringify({ + console: browserConsole, + pageErrors: pageErrors.map((error) => error.message), + stage: await page.getByRole('region', { name: 'Runtime output stage' }).textContent(), + })}`, { cause: error }); + }); const runId = await history.first().getAttribute('data-runtime-run-id'); const expectedGenerationId = await runtimeIdentity.getAttribute('data-runtime-generation'); if (runId === null || expectedGenerationId === null) throw new Error('Expected selected Runtime run identity.'); diff --git a/packages/workbench/tests/mcp-page-app-browser.test.ts b/packages/workbench/tests/mcp-page-app-browser.test.ts index cd7d26d35..e722f3ff3 100644 --- a/packages/workbench/tests/mcp-page-app-browser.test.ts +++ b/packages/workbench/tests/mcp-page-app-browser.test.ts @@ -7,7 +7,7 @@ import { tmpdir } from 'node:os'; import { describe, expect, it } from '@rstest/core'; import { createRsbuild } from '@rsbuild/core'; import { pluginReact } from '@rsbuild/plugin-react'; -import { chromium } from 'playwright'; +import { chromium, type Page } from 'playwright'; import { closeServer } from './support/http.ts'; import { workbenchBrowserAliases } from './support/workbench-browser-modules.ts'; @@ -63,8 +63,11 @@ const proxyDocument = ` const mountedPageFixture = async (mode: 'artifact' | 'runtime' | 'runtime-direct' = 'artifact') => { const root = await mkdtemp(join(tmpdir(), 'agent-bundle-mcp-page-app-')); const sandboxRequests: string[] = []; + const sandboxRequestWaiters: Array<(url: string) => void> = []; const sandbox = createServer((request, response) => { - sandboxRequests.push(request.url ?? ''); + const url = request.url ?? ''; + sandboxRequests.push(url); + for (const waiter of sandboxRequestWaiters.splice(0)) waiter(url); response.writeHead(200, { 'content-type': 'text/html' }).end(proxyDocument); }); const sandboxOrigin = await listen(sandbox); @@ -185,12 +188,45 @@ const mountedPageFixture = async (mode: 'artifact' | 'runtime' | 'runtime-direct await closeServer(sandbox); await rm(root, { force: true, recursive: true }); }, + /** + * Resolves with the next sandbox document request the server accepts. + * Arm it before the action that provokes the load: an iframe's presence in + * the DOM only proves the element was inserted, not that its document + * request has reached this server yet. + */ + nextSandboxRequest: () => new Promise((resolvePromise) => { sandboxRequestWaiters.push(resolvePromise); }), outerOrigin, root, sandboxRequests: () => [...sandboxRequests], }; }; +type McpPageAppLifecycleStats = Readonly<{ + readonly closes: readonly { readonly bindingId: string; readonly type: 'close' | 'force' }[]; + readonly creates: readonly unknown[]; + readonly messages: readonly { readonly bindingId: string; readonly message: { readonly method?: string } }[]; +}>; + +/** + * Event-ordered lifecycle waits for the artifact fixture. Each waits for the + * fixture's own record of the route call rather than for DOM side effects: an + * iframe in the DOM says nothing about whether its proxy has loaded, and an + * iframe gone from the DOM says nothing about which route calls have landed. + */ +const lifecycleWaits = (page: Page) => ({ + /** The proxy loaded, handed the app its resource, and the app completed `ui/initialize`. */ + initialized: (bindingId: string) => page.waitForFunction((id) => (globalThis as typeof globalThis & { + __mcpPageAppFixture: { stats(): McpPageAppLifecycleStats }; + }).__mcpPageAppFixture.stats().messages.some((entry) => entry.bindingId === id && entry.message.method === 'ui/notifications/initialized'), bindingId), + /** The page released the binding through exactly one close route call (graceful or forced). */ + closed: (bindingId: string) => page.waitForFunction((id) => (globalThis as typeof globalThis & { + __mcpPageAppFixture: { stats(): McpPageAppLifecycleStats }; + }).__mcpPageAppFixture.stats().closes.some((entry) => entry.bindingId === id), bindingId), + stats: (): Promise => page.evaluate(() => (globalThis as typeof globalThis & { + __mcpPageAppFixture: { stats(): McpPageAppLifecycleStats }; + }).__mcpPageAppFixture.stats()), +}); + describe('MCP App page browser integration', () => { it('keeps the committed runtime evidence and preview request unchanged after caller mutation', async () => { const fixture = await mountedPageFixture('runtime'); @@ -451,6 +487,7 @@ describe('MCP App page browser integration', () => { const page = await browser.newPage({ viewport: { height: 800, width: 390 } }); const pageErrors: string[] = []; page.on('pageerror', (error) => { pageErrors.push(error.message); }); + const lifecycle = lifecycleWaits(page); try { await page.goto(`${fixture.outerOrigin}/page.html`); await page.waitForFunction(() => '__mcpPageAppFixture' in globalThis); @@ -463,7 +500,7 @@ describe('MCP App page browser integration', () => { expect(await frame.getAttribute('sandbox')).toBe('allow-scripts allow-same-origin'); expect(await frame.getAttribute('referrerpolicy')).toBe('no-referrer'); expect(await frame.contentFrame()?.locator('body').innerText()).not.toContain('foreground-token'); - await page.waitForFunction(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): { messages: readonly { readonly bindingId: string; readonly message: { readonly method?: string } }[] } } }).__mcpPageAppFixture.stats().messages.some(({ bindingId, message }) => bindingId === 'binding-1' && message.method === 'ui/notifications/initialized')); + await lifecycle.initialized('binding-1'); const first = await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): unknown } }).__mcpPageAppFixture.stats()) as { readonly creates: readonly { readonly request: { readonly host: { readonly displayMode: string; readonly locale: string; readonly theme: string }; readonly input: unknown; readonly previewProfile: string; readonly result: unknown; readonly toolName: string }; readonly sessionId: string }[]; readonly messages: readonly { readonly message: { readonly method?: string } }[]; @@ -496,6 +533,7 @@ describe('MCP App page browser integration', () => { trace.push(snapshot()); Object.assign(globalThis, { __mcpPageRemountTrace: { stop: () => observer.disconnect(), values: () => [...trace] } }); }); + const remountedDocumentRequest = fixture.nextSandboxRequest(); await page.getByRole('button', { name: 'Allow geolocation' }).click(); await page.locator('iframe[title="MCP App preview: weather"][data-mcp-app-document-revision="2"]').waitFor(); const remountTrace = await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageRemountTrace: { readonly stop: () => void; readonly values: () => readonly string[] } }).__mcpPageRemountTrace.values()); @@ -504,16 +542,30 @@ describe('MCP App page browser integration', () => { const refreshed = remountTrace.findIndex((value, index) => index > blank && value.startsWith('MCP App preview: weather|') && value.endsWith('|2')); expect(blank).toBeGreaterThanOrEqual(0); expect(refreshed).toBeGreaterThan(blank); + // The remounted document's request is a network event that trails the + // DOM insertion the locator above observed; wait for the server to + // accept it before asserting exactly one sandbox load. + expect(await remountedDocumentRequest).toBe('/'); expect(fixture.sandboxRequests().slice(sandboxRequestsBeforeRemount)).toEqual(['/']); + // The remounted proxy must reach the app before the graceful teardown + // below can be acknowledged by it (the initialize count doubles because + // the refreshed document runs the handshake again). + await page.waitForFunction(() => (globalThis as McpPageAppFixtureGlobal).__mcpPageAppFixture.stats().messages + .filter(({ bindingId, message }) => bindingId === 'binding-1' && message.method === 'ui/notifications/initialized').length === 2); await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { setActive(active: boolean): void } }).__mcpPageAppFixture.setActive(false)); await page.waitForFunction(() => document.querySelector('iframe[title="MCP App preview: weather"]') === null); - await page.waitForFunction(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): { closes: readonly unknown[] } } }).__mcpPageAppFixture.stats().closes.length === 1); + await lifecycle.closed('binding-1'); + expect((await lifecycle.stats()).closes).toEqual([{ bindingId: 'binding-1', options: expect.any(Object), type: 'close' }]); await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { setActive(active: boolean): void } }).__mcpPageAppFixture.setActive(true)); expect(await page.getByText('Select a completed tool call below to create an App preview.').count()).toBe(1); await page.getByRole('button', { name: 'Open App preview for weather-call' }).click(); await frame.waitFor(); + // Switching profiles tears the current binding down first; wait for its + // app to be initialized so the teardown is acknowledged by a proxy that + // exists, not posted into a document that is still loading. + await lifecycle.initialized('binding-2'); await page.selectOption('#mcp-app-profile', 'chatgpt'); await page.waitForFunction(() => { @@ -531,7 +583,7 @@ describe('MCP App page browser integration', () => { expect(await page.getByLabel('MCP App preview', { exact: true }).textContent()).toContain('claude'); await page.getByRole('button', { name: 'Close App preview' }).click(); - await page.waitForFunction(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): { closes: readonly unknown[] } } }).__mcpPageAppFixture.stats().closes.length >= 3); + await lifecycle.closed('binding-4'); await page.getByRole('button', { name: 'List tools' }).click(); await page.waitForFunction(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): { controllerEvents: readonly { readonly type: string }[] } } }).__mcpPageAppFixture.stats().controllerEvents.some(({ type }) => type === 'invoke')); @@ -539,21 +591,30 @@ describe('MCP App page browser integration', () => { await page.getByLabel('MCP App fallback').waitFor(); expect(await page.getByLabel('MCP App fallback').textContent()).toContain('unsupported-media-type'); await page.getByRole('button', { name: 'Close App preview' }).click(); + await lifecycle.closed('binding-5'); await page.getByRole('button', { name: 'Open App preview for legacy-template-call' }).click(); await page.getByLabel('MCP App fallback').waitFor(); expect(await page.getByLabel('MCP App fallback').textContent()).toContain('legacy-output-template'); await page.getByRole('button', { name: 'Close App preview' }).click(); + await lifecycle.closed('binding-6'); await page.getByRole('button', { name: 'Open App preview for weather-call' }).click(); await frame.waitFor(); + await lifecycle.initialized('binding-7'); await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { terminateAndClickClose(phase: 'error'): void } }).__mcpPageAppFixture.terminateAndClickClose('error')); await page.waitForFunction(() => document.querySelector('iframe[title="MCP App preview: weather"]') === null); expect(await page.locator('.mcp-page-phase').textContent()).toContain('Session error'); - const final = await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): unknown } }).__mcpPageAppFixture.stats()) as { - readonly closes: readonly unknown[]; + await lifecycle.closed('binding-7'); + const final = await lifecycle.stats() as McpPageAppLifecycleStats & { readonly creates: readonly { readonly request: Readonly> }[]; }; - expect(final.closes).toHaveLength(7); + // Every binding the page created was released through exactly one + // route call, in creation order: the canonical frames gracefully (the + // app acknowledged its teardown), the frameless fallbacks by force. + expect(final.creates).toHaveLength(7); + expect(final.closes.map(({ bindingId, type }) => `${bindingId}:${type}`)).toEqual([ + 'binding-1:close', 'binding-2:close', 'binding-3:close', 'binding-4:close', 'binding-5:force', 'binding-6:force', 'binding-7:close', + ]); expect(final.creates.every(({ request }) => !Object.hasOwn(request, 'toolMetadata') && !Object.hasOwn(request, 'resourceUri'))).toBe(true); expect(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true); @@ -561,11 +622,12 @@ describe('MCP App page browser integration', () => { await page.waitForFunction(() => '__mcpPageAppFixture' in globalThis); await page.getByRole('button', { name: 'Open App preview for weather-call' }).click(); await frame.waitFor(); + await lifecycle.initialized('binding-1'); await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { terminateAndClickClose(phase: 'closed'): void } }).__mcpPageAppFixture.terminateAndClickClose('closed')); await page.waitForFunction(() => document.querySelector('iframe[title="MCP App preview: weather"]') === null); expect(await page.locator('.mcp-page-phase').textContent()).toContain('Session closed'); - const closedTerminal = await page.evaluate(() => (globalThis as typeof globalThis & { __mcpPageAppFixture: { stats(): { closes: readonly unknown[] } } }).__mcpPageAppFixture.stats().closes); - expect(closedTerminal).toHaveLength(1); + await lifecycle.closed('binding-1'); + expect((await lifecycle.stats()).closes.map(({ bindingId, type }) => `${bindingId}:${type}`)).toEqual(['binding-1:close']); expect(pageErrors).toEqual([]); } finally { await browser.close(); diff --git a/packages/workbench/tests/packed-release.e2e.test.ts b/packages/workbench/tests/packed-release.e2e.test.ts index 3d6bd2eec..1dc856142 100644 --- a/packages/workbench/tests/packed-release.e2e.test.ts +++ b/packages/workbench/tests/packed-release.e2e.test.ts @@ -505,6 +505,36 @@ e2e('runs every Agent API tool from the installed tarball', { timeout: 360_000 * const response = await rebuilt; if (!response.ok()) throw new Error(`${label} rebuild returned HTTP ${response.status()}: ${await response.text()}`); }; + const buildStatusFrom = (toolResult: Awaited>, label: string) => { + const build = record(record(record(toolResult.structuredContent, `${label} result`).status, `${label} status`).build, `${label} build`); + const attemptIds = new Set(); + for (const key of ['activeAttempt', 'lastAttempt'] as const) { + const attempt = build[key]; + if (attempt !== undefined) attemptIds.add(string(record(attempt, `${label} ${key}`).id, `${label} ${key} id`)); + } + return { attemptIds, state: string(build.state, `${label} build state`) }; + }; + /** + * Replaces one watched source and waits for the packed dev server's own + * watcher-driven rebuild of that write to settle before the caller + * touches the Rebuild button. The coordinator publishes every attempt + * through project_status, so the first completed attempt the status did + * not report before the write is the watcher's build of it. Clicking + * Rebuild while that build is still pending (or not yet started: the + * watcher debounces and a loaded runner delivers the event late) races + * two builds of one edit; whichever lands last silently replaces the + * epoch every later phase pinned, which is the "epoch mismatch" this + * phase-labelled suite used to fail with. Same contract as + * replaceWatchedSourceAndAwaitRebuild for in-process dev servers. + */ + const replaceSourceAndAwaitWatcherRebuild = async (label: string, sourcePath: string, content: string): Promise => { + const known = buildStatusFrom(await call('project_status'), `${label} pre-edit`).attemptIds; + await replaceWatchedSource(project, sourcePath, content); + await expect.poll(async () => { + const build = buildStatusFrom(await call('project_status'), `${label} post-edit`); + return build.state !== 'building' && [...build.attemptIds].some((id) => !known.has(id)); + }, { timeout: browserTimeout }).toBe(true); + }; const settleNativeSelection = (): Promise => page.evaluate(async () => { await new Promise((resolvePromise) => requestAnimationFrame(() => requestAnimationFrame(() => resolvePromise()))); }); @@ -552,7 +582,7 @@ e2e('runs every Agent API tool from the installed tarball', { timeout: 360_000 * phase = 'good edit rebuild B'; const epochBMarker = 'Epoch B changed the packed review guidance.'; - await replaceWatchedSource(project, skillSource, `${originalSkill}\n\n${epochBMarker}\n`); + await replaceSourceAndAwaitWatcherRebuild('epoch B', skillSource, `${originalSkill}\n\n${epochBMarker}\n`); await page.getByRole('link', { name: 'Overview', exact: true }).click(); await expect(page.getByRole('heading', { name: 'Bundle dashboard' })).toBeVisible({ timeout: browserTimeout }); await rebuildFromOverview('epoch B'); @@ -605,7 +635,7 @@ e2e('runs every Agent API tool from the installed tarball', { timeout: 360_000 * await expectGeneratedSkill('last-good epoch B', lastGoodEpochB, epochBMarker); const invalidConfig = originalConfig.replace('ui://packed-release/dashboard.html', 'https://packed-release.example/dashboard.html'); if (invalidConfig === originalConfig) throw new Error('The packed fixture did not contain the resource URI used for the invalid rebuild.'); - await replaceWatchedSource(project, configSource, invalidConfig); + await replaceSourceAndAwaitWatcherRebuild('invalid epoch B', configSource, invalidConfig); await page.getByRole('link', { name: 'Overview', exact: true }).click(); await expect(page.getByRole('heading', { name: 'Bundle dashboard' })).toBeVisible({ timeout: browserTimeout }); await rebuildFromOverview('invalid epoch B'); @@ -620,10 +650,12 @@ e2e('runs every Agent API tool from the installed tarball', { timeout: 360_000 * phase = 'repaired edit rebuild C'; const epochCMarker = 'Epoch C repaired the packed review guidance.'; - await Promise.all([ - replaceWatchedSource(project, configSource, originalConfig), - replaceWatchedSource(project, skillSource, `${originalSkill}\n\n${epochCMarker}\n`), - ]); + // Two edits, each awaited: two renames inside one watcher debounce + // window usually coalesce into one build, but nothing guarantees it, and + // a second build landing after the manual rebuild below would replace + // epoch C after later phases pinned it. + await replaceSourceAndAwaitWatcherRebuild('epoch C config', configSource, originalConfig); + await replaceSourceAndAwaitWatcherRebuild('epoch C skill', skillSource, `${originalSkill}\n\n${epochCMarker}\n`); await rebuildFromOverview('epoch C'); const epochCStatus = activeEpochFrom(await call('project_status'), 'epoch C'); expect(epochCStatus.artifactStatus.state).toBe('active'); diff --git a/rstest.worker-isolation.ts b/rstest.worker-isolation.ts index c1fada37a..86ff22f51 100644 --- a/rstest.worker-isolation.ts +++ b/rstest.worker-isolation.ts @@ -91,6 +91,22 @@ export const isolateWorkerEnvironment = (): void => { let commandSerial = 0; +/** + * The worker's npm cache. It is shared by every command this worker spawns + * (not per command) on purpose: a packed consumer install pulls the package's + * full dependency tree (~180 MB of registry tarballs), and a per-command cache + * made every install in a file start cold — on a CI runner, whose npm cache is + * always empty at job start, that was the whole 30 s budget of each + * public-api-packed test. Sequential commands in one worker now hit the cache + * after the first install, and `--prefer-offline` then skips the registry + * round trips entirely. Concurrent installs within one worker (packed-consumer + * installs two consumers at once) share it safely: cacache is content + * addressed with atomic writes, the same property every developer machine + * relies on for parallel `npm install`s against ~/.npm. Workers never share a + * cache with each other. + */ +export const rstestWorkerNpmCacheDirectory = (): string => rstestWorkerCacheDirectory('npm'); + export const isolatedCommandEnvironment = (base: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv => { commandSerial += 1; const stamp = String(process.pid) + '-' + String(commandSerial); @@ -99,7 +115,12 @@ export const isolatedCommandEnvironment = (base: NodeJS.ProcessEnv = process.env mkdirSync(tmp, { recursive: true }); const { NODE_PATH: _nodePath, ...rest } = base; const environment: NodeJS.ProcessEnv = { ...rest }; - environment['npm_config_cache'] = cache; + environment['npm_config_cache'] = rstestWorkerNpmCacheDirectory(); + // Rslib's persistent Rspack build cache is keyed by the built config's + // root (`/node_modules/.cache/rspack`), not by `--dist-path`, so + // two workers rebuilding the same package into isolated dists would share + // one cache lock. packages/*/rslib.config.ts honor this override. + environment['AGENT_BUNDLE_RSLIB_CACHE_DIRECTORY'] = join(cache, 'rslib'); environment['TMPDIR'] = tmp; environment['TMP'] = tmp; environment['TEMP'] = tmp;