diff --git a/.changeset/workbench-gate-diagnostic-code.md b/.changeset/workbench-gate-diagnostic-code.md new file mode 100644 index 000000000..2f94b8cc0 --- /dev/null +++ b/.changeset/workbench-gate-diagnostic-code.md @@ -0,0 +1,5 @@ +--- +'agent-bundle': patch +--- + +Workbench connection gate and Overview rebuild alert: show the foreground diagnostic code and message — `AB8003 — Origin http://localhost:3000 is not allowed by the foreground server at http://127.0.0.1:3100. Open http://127.0.0.1:3100 instead, or start agent-bundle dev with --workbench-dev-origin http://localhost:3000 to allow this origin.` — with an `(HTTP )` suffix only when the foreground response itself failed, instead of the misleading `Workbench request failed with HTTP 200.` (#589) diff --git a/packages/workbench/src/main.tsx b/packages/workbench/src/main.tsx index 59a84840f..0579282cd 100644 --- a/packages/workbench/src/main.tsx +++ b/packages/workbench/src/main.tsx @@ -65,7 +65,7 @@ import { RoutesPage } from './routes/routes-page.tsx'; import { overviewFor } from './overview-model.ts'; import { downloadBlob, errorMessage as messageFrom } from './client-helpers.ts'; import { BundleWorkflow, HostAdoptionSection, StateMark } from './overview-page.tsx'; -import { ProjectClient, type ProjectConnectionState } from './project-client.ts'; +import { projectFailureText, ProjectClient, type ProjectConnectionState } from './project-client.ts'; import { SkillClient } from './skill-client.ts'; import { SkillsPage } from './skills-page.tsx'; import { @@ -73,7 +73,7 @@ import { loadWorkbenchCapabilities, type WorkbenchCapabilities, } from './workbench-capabilities.ts'; -import type { WorkbenchPage as GeneralWorkbenchPage } from './workbench-screen.tsx'; +import { ConnectionGate, type WorkbenchPage as GeneralWorkbenchPage } from './workbench-screen.tsx'; import { RuntimeClient, type RuntimeBootstrap } from './runtime-client.ts'; import { createRuntimeEventBuffer, @@ -101,6 +101,9 @@ const sourceFor = (diagnostic: Diagnostic): string => const errorMessage = (reason: unknown): string => messageFrom(reason, 'Foreground project state could not be refreshed.'); +const connectionFailure = (reason: unknown): string => + projectFailureText(reason, 'Foreground project state could not be refreshed.'); + const activeEpochFor = (status: ProjectStatus) => status.artifact.state === 'missing' ? undefined : status.artifact.activeEpoch; @@ -388,7 +391,7 @@ const Overview = ({ capabilities, changedFiles, client, connectionError, onNavig try { onStatus(await client.rebuild()); } catch (reason) { - setError(messageFrom(reason, 'Rebuild request could not be completed.')); + setError(projectFailureText(reason, 'Rebuild request could not be completed.')); } finally { setRebuilding(false); } @@ -866,7 +869,7 @@ const Workbench = () => { mcpControllerRef.current?.close() ?? Promise.resolve(), ]); const failure = results.find((result) => result.status === 'rejected'); - if (failure?.status === 'rejected') setConnectionError(errorMessage(failure.reason)); + if (failure?.status === 'rejected') setConnectionError(connectionFailure(failure.reason)); mcpAppClient.current?.resetRuntimeForForegroundReplacement(); handoffCoordinator.current = undefined; mcpPreviewDeparture.current = undefined; @@ -1243,11 +1246,11 @@ const Workbench = () => { } }, (reason) => { - if (mounted) setConnectionError(errorMessage(reason)); + if (mounted) setConnectionError(connectionFailure(reason)); }, (event) => { runtimeEvents.receive(event); }, ).catch((reason: unknown) => { - if (mounted) setConnectionError(errorMessage(reason)); + if (mounted) setConnectionError(connectionFailure(reason)); }); return () => { mounted = false; @@ -1345,11 +1348,7 @@ const Workbench = () => { }, [mcpController]); useEffect(() => mcpController.subscribe(setMcpModel), [mcpController]); - const connectionGate = connection.state === 'connected' ? undefined :
-

{connection.state === 'unavailable' ? 'Foreground connection unavailable' : 'Foreground connection reconnecting'}

-

{connection.state === 'unavailable' ? 'Waiting for the foreground server to recover.' : 'Connecting to the foreground server.'}

- {connectionError === undefined ? undefined :

{connectionError}

} -
; + const connectionGate = connection.state === 'connected' ? undefined : ; const withConnectionGate = (content: ReactNode): ReactNode => <>
{content}
{connectionGate} diff --git a/packages/workbench/src/mcp/mcp-route-client.ts b/packages/workbench/src/mcp/mcp-route-client.ts index ac58e9ac2..737b6626c 100644 --- a/packages/workbench/src/mcp/mcp-route-client.ts +++ b/packages/workbench/src/mcp/mcp-route-client.ts @@ -651,7 +651,11 @@ export class ForegroundRouteClient implements ForegroundRequestAuthority { browserOrigin !== undefined && browserOrigin !== 'null' && browserOrigin !== body.origin && devOrigins?.includes(browserOrigin) !== true ) { - throw new ForegroundRouteClientError('AB8003', 'Foreground session bootstrap origin does not match this browser.', response.status); + throw new ForegroundRouteClientError( + 'AB8003', + `Origin ${browserOrigin} is not allowed by the foreground server at ${body.origin}. Open ${body.origin} instead, or start agent-bundle dev with --workbench-dev-origin ${browserOrigin} to allow this origin.`, + response.status, + ); } const previous = this.#snapshot; const generation = previous === undefined @@ -699,20 +703,36 @@ export class ForegroundRouteClientError extends Error { readonly code: string; readonly details: unknown | undefined; readonly phase: string | undefined; + /** + * HTTP status of the failed foreground response (`fromResponse`); `undefined` + * when the client constructed the failure itself — a refused 200 body, a + * superseded or invalidated session — and `status` is only nominal. + */ + readonly responseStatus: number | undefined; readonly status: number; - constructor(code: string, message: string, status: number, options: Readonly<{ readonly details?: unknown; readonly phase?: string }> = {}) { + constructor( + code: string, + message: string, + status: number, + options: Readonly<{ readonly details?: unknown; readonly phase?: string; readonly responseStatus?: number }> = {}, + ) { super(message); this.name = 'ForegroundRouteClientError'; this.code = code; this.details = options.details; this.phase = options.phase; + this.responseStatus = options.responseStatus; this.status = status; } static fromResponse(body: unknown, status: number): ForegroundRouteClientError { const detail = diagnostic(body, status); - return new ForegroundRouteClientError(detail.code, detail.message, status, detail); + return new ForegroundRouteClientError(detail.code, detail.message, status, { + details: detail.details, + phase: detail.phase, + responseStatus: status, + }); } } diff --git a/packages/workbench/src/project-client.ts b/packages/workbench/src/project-client.ts index b632282f7..d5eb054b5 100644 --- a/packages/workbench/src/project-client.ts +++ b/packages/workbench/src/project-client.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import type { Diagnostic } from '../../agent-bundle/src/contracts/diagnostics.ts'; -import { exactKeys, isRecord } from './client-helpers.ts'; +import { errorMessage, exactKeys, isRecord } from './client-helpers.ts'; import { freezeJsonValue, type ArtifactEpoch, @@ -68,14 +68,35 @@ interface QueuedProjectEvent { export class ProjectClientError extends Error { readonly code: string | undefined; - - constructor(message: string, code?: string) { + /** + * Status of the failed foreground response (4xx/5xx); absent when the client + * constructed the failure itself — it refused an HTTP 200 bootstrap body, or + * the session was superseded or invalidated while a request was in flight. + */ + readonly status: number | undefined; + + constructor(message: string, code?: string, status?: number) { super(message); this.name = 'ProjectClientError'; this.code = code; + this.status = status; } } +/** + * The one-line account of a project client failure the connection gate, the + * topbar, and the Overview rebuild alert show: ` (HTTP )`, + * omitting the parts a failure lacks. Other errors keep their message; a + * hostile reason falls back. + */ +export const projectFailureText = (reason: unknown, fallback: string): string => { + const message = errorMessage(reason, fallback); + if (!(reason instanceof ProjectClientError)) return message; + const code = reason.code === undefined ? '' : `${reason.code} — `; + const status = reason.status === undefined ? '' : ` (HTTP ${reason.status})`; + return `${code}${message}${status}`; +}; + const projectEventTypes = [ 'artifact.available', 'artifact.status', @@ -241,7 +262,7 @@ const projectStatusResponse = (value: unknown): ProjectStatusResponse => { const projectError = (error: unknown): ProjectClientError | unknown => error instanceof ForegroundRouteClientError - ? new ProjectClientError(`Workbench request failed with HTTP ${error.status}.`, error.code) + ? new ProjectClientError(error.message, error.code, error.responseStatus) : error; const isSequence = (value: unknown): value is number => @@ -344,6 +365,7 @@ export class ProjectClient { #eventRefreshQueued = false; #highestQueuedEventId = -1; #lastEventId = 0; + #lastReportedFailure: string | undefined; #lastSourceChangeSequence = -1; #errorListener: ProjectClientErrorListener | undefined; #listener: ((status: ProjectStatus) => void) | undefined; @@ -533,8 +555,9 @@ export class ProjectClient { source.addEventListener('open', () => { void this.#refreshRecoveredSource(version); }); this.#setConnection({ generation: snapshot.generation, instanceId: snapshot.instanceId, state: 'connecting' }); return; - } catch { + } catch (error) { if (this.#closed || recoveryVersion !== this.#recoveryVersion) return; + this.#reportRecoveryFailure(projectError(error)); await this.#retryDelay(retryDelayMilliseconds); } } @@ -555,7 +578,7 @@ export class ProjectClient { this.#eventSource = undefined; source.close(); this.#setConnection({ ...this.#connection, state: 'unavailable' }); - this.#reportError(error); + this.#reportError(projectError(error)); this.#startRecovery(version); } } @@ -718,6 +741,7 @@ export class ProjectClient { #reportError(reason: unknown): void { if (this.#closed) return; + this.#lastReportedFailure = projectFailureText(reason, ''); try { this.#errorListener?.(reason); } catch { @@ -725,6 +749,17 @@ export class ProjectClient { } } + /** + * Recovery retries every {@link retryDelayMilliseconds}; a failed attempt is + * reported only when its line differs from the last report, so the gate moves + * from `Foreground project event stream disconnected.` to the refusal that + * keeps recovery from completing (e.g. `AB8003`) without flooding the listener. + */ + #reportRecoveryFailure(reason: unknown): void { + if (projectFailureText(reason, '') === this.#lastReportedFailure) return; + this.#reportError(reason); + } + #publishEvent(event: ProjectEventMessage): void { const listeners = [ ...(this.#eventListener === undefined ? [] : [this.#eventListener]), diff --git a/packages/workbench/src/workbench-screen.tsx b/packages/workbench/src/workbench-screen.tsx index 7354fefbe..b1910d67a 100644 --- a/packages/workbench/src/workbench-screen.tsx +++ b/packages/workbench/src/workbench-screen.tsx @@ -1,5 +1,7 @@ import React, { type ReactNode } from 'react'; +import type { ProjectConnectionPhase } from './project-client.ts'; + export type WorkbenchPage = 'artifacts' | 'comparisons' | 'evals' | 'hooks' | 'hosts' | 'lifecycles' | 'logs' | 'mcp' | 'overview' | 'playground' | 'routes' | 'skills'; interface NavigationItem { @@ -68,6 +70,16 @@ export const Topbar = ({ connectionError }: { readonly connectionError?: string ; +/** Overlays the Workbench while the foreground connection is not `connected`; `error` is the `projectFailureText` line. */ +export const ConnectionGate = ({ error, state }: { + readonly error?: string; + readonly state: Exclude; +}) =>
+

{state === 'unavailable' ? 'Foreground connection unavailable' : 'Foreground connection reconnecting'}

+

{state === 'unavailable' ? 'Waiting for the foreground server to recover.' : 'Connecting to the foreground server.'}

+ {error === undefined ? undefined :

{error}

} +
; + export const Navigation = ({ onNavigate, page, pages }: { readonly onNavigate: (page: WorkbenchPage) => void; readonly page: WorkbenchPage; diff --git a/packages/workbench/tests/contributor-hmr.e2e.test.ts b/packages/workbench/tests/contributor-hmr.e2e.test.ts index 5811bf4c4..606e23aa0 100644 --- a/packages/workbench/tests/contributor-hmr.e2e.test.ts +++ b/packages/workbench/tests/contributor-hmr.e2e.test.ts @@ -3,6 +3,7 @@ import { join } from 'node:path'; import { expect } from '@rstest/playwright'; import { createRsbuild, type Rspack, type StartDevServerResult } from '@rsbuild/core'; +import type { StartDevServerOptions } from '../../agent-bundle/src/dev/workbench-server.ts'; import { createProjectFixture, removeProjectFixture, type ProjectFixture } from '../../agent-bundle/tests/helpers/project-fixture.ts'; import { availablePort } from '../../agent-bundle/tests/support/available-port.ts'; import { within } from '../../agent-bundle/tests/support/eventually.ts'; @@ -39,6 +40,15 @@ import { * reaches the dev server with a foreign loopback origin is still refused * by the foreground with AB8003, while the allowlisted origin is disclosed * as `devOrigins` next to the unchanged foreground `origin`. + * 4. The documented no-flag failure, live: with the page open, the foreground + * restarts on its port WITHOUT `--workbench-dev-origin`; the event stream + * drops and every recovery bootstrap succeeds as HTTP 200 yet names only the + * foreground origin, so the client refuses it as AB8003 and the "Foreground + * connection unavailable" gate's alert settles on the code, the foreground + * URL to open instead, and the exact flag to add — not on the disconnect line. + * 5. The same failure on a fresh load of the dev origin, with the 200 bootstrap + * and its Origin-less request observed; the foreground refuses the page's + * `Origin` outright. * * Deliberately not covered: MCP App preview and runtime client-surface * iframes stay bound to the foreground origin (their sandbox and proxy @@ -56,7 +66,17 @@ const devHost = 'localhost'; interface ContributorLoop { readonly dev: StartDevServerResult; readonly devOrigin: string; + /** The foreground behind the dev server's `/api` proxy: the replacement once `restartForeground` has run. */ readonly foreground: WorkbenchServer; + /** Closes the running foreground; a no-op while a restart left none running. */ + readonly closeForeground: () => Promise; + /** + * Closes the foreground and starts another on the SAME port with the e2e + * defaults plus `overrides`, so the already-compiled dev server keeps proxying + * `/api` to it — an `agent-bundle dev` restart that drops the allowlist unless + * `overrides` names `workbenchDevOrigins` again. + */ + readonly restartForeground: (overrides?: Partial) => Promise; } const startContributorLoop = async (project: ProjectFixture): Promise => { @@ -66,6 +86,13 @@ const startContributorLoop = async (project: ProjectFixture): Promise => { + const closing = current; + current = undefined; + await closing?.close(); + }; try { const documented = createWorkbenchConfig(foreground.url); if (!('server' in documented)) throw new Error('The documented Workbench config did not configure the /api proxy.'); @@ -94,31 +121,46 @@ const startContributorLoop = async (project: ProjectFixture): Promise => { + await closeForeground(); + current = await startWorkbenchDevServer(project, { ...overrides, port: foregroundPort }); + return current; + }, + }; } catch (error) { - await Promise.allSettled([foreground.close()]); + await Promise.allSettled([closeForeground()]); throw error; } }; /** The dev server closes first: its proxy holds upstream connections into the foreground. */ -const closeContributorLoop = async ({ dev, foreground }: ContributorLoop): Promise => { +const closeContributorLoop = async ({ closeForeground, dev }: ContributorLoop): Promise => { const [devClosed] = await Promise.allSettled([dev.server.close()]); - await foreground.close(); + await closeForeground(); if (devClosed?.status === 'rejected') throw devClosed.reason; }; const sessionThroughProxy = (devOrigin: string, origin?: string): Promise => fetch(`${devOrigin}/api/project/session`, origin === undefined ? {} : { headers: { origin } }); -e2e('completes a Workbench session through the documented contributor HMR proxy', { timeout: 180_000 }, async ({ page }) => { +// 180 s covered one foreground start plus the compile and browser budgets; the no-flag step adds a second foreground start. +e2e('completes a Workbench session through the documented contributor HMR proxy and gates a foreground started without the allowlist', { timeout: 210_000 }, async ({ page }) => { await buildWorkbench(); await withWorkbenchServer({ close: closeContributorLoop, createProject: () => createProjectFixture(), dispose: (project) => removeProjectFixture(project.root), start: startContributorLoop, - }, async ({ devOrigin, foreground }) => { + }, async (loop) => { + const { devOrigin, foreground } = loop; expect(devOrigin).not.toBe(foreground.url); const pageErrors: Error[] = []; page.on('pageerror', (error) => pageErrors.push(error)); @@ -164,5 +206,65 @@ e2e('completes a Workbench session through the documented contributor HMR proxy' const admitted = await sessionThroughProxy(devOrigin, devOrigin); expect(admitted.status).toBe(200); expect(await admitted.json()).toMatchObject({ devOrigins: [devOrigin], origin: foreground.url }); + + // 4. The documented no-flag failure, live: the foreground restarts on its + // port without `--workbench-dev-origin` while the page stays open, so the + // proxy target is unchanged and only the allowlist is gone. The page's + // event stream drops, recovery re-bootstraps through the proxy, and the + // client refuses the body it gets back — the gate must end on that + // refusal, not on the disconnect line the recovery loop used to hide it behind. + const unlisted = await loop.restartForeground(); + expect(unlisted.url).toBe(foreground.url); + const noFlagLine = `AB8003 — Origin ${devOrigin} is not allowed by the foreground server at ${unlisted.url}. ` + + `Open ${unlisted.url} instead, or start agent-bundle dev with --workbench-dev-origin ${devOrigin} to allow this origin.`; + await expect(page.getByRole('heading', { name: 'Foreground connection unavailable' })).toBeVisible({ timeout: browserTimeout }); + await expect(page.getByRole('alert')).toHaveText(noFlagLine, { timeout: browserTimeout }); + expect(new URL(page.url()).origin).toBe(devOrigin); + expect(pageErrors).toEqual([]); + + // 5. The same failure on a fresh load — the documented "open + // http://localhost:" moment — with the bootstrap itself observed. + // The page leaves first: a `goto` to the URL it already shows is a + // same-document navigation, and the recovery loop's own bootstraps + // would otherwise be the responses observed below. + await page.goto('about:blank'); + const bootstrap = page.waitForResponse((candidate) => + candidate.request().method() === 'GET' && candidate.url() === `${devOrigin}/api/project/session`); + await page.goto(workbenchUrl(devOrigin, 'overview')); + // The bootstrap itself succeeds — the browser's same-origin GET carries no + // `Origin` — but its body names only the foreground origin and no `devOrigins`, + // so the refusal below is the client's own, not an HTTP failure. + const bootstrapResponse = await bootstrap; + expect((await bootstrapResponse.request().allHeaders())['origin']).toBeUndefined(); + expect(bootstrapResponse.status()).toBe(200); + const bootstrapBody: unknown = await bootstrapResponse.json(); + expect(bootstrapBody).toMatchObject({ origin: unlisted.url }); + expect(bootstrapBody).not.toHaveProperty('devOrigins'); + try { + await expect(page.getByRole('heading', { name: 'Foreground connection unavailable' })).toBeVisible({ timeout: browserTimeout }); + } catch (reason) { + throw new Error( + `The connection gate did not appear on ${page.url()} without the dev-origin allowlist.\n${await page.locator('body').innerText()}`, + { cause: reason }, + ); + } + expect(new URL(page.url()).origin).toBe(devOrigin); + // The gate's alert is the diagnostic itself: the code, the foreground URL to + // open instead, and the exact flag that is missing — not a bare HTTP status. + const alert = page.getByRole('alert'); + await expect(alert).toContainText('AB8003', { timeout: browserTimeout }); + await expect(alert).toContainText(`--workbench-dev-origin ${devOrigin}`); + await expect(alert).toHaveText(noFlagLine); + await expect(alert).not.toContainText('HTTP 200'); + await expect(alert).not.toContainText('Workbench request failed'); + await expect(page.getByRole('heading', { name: 'Bundle dashboard' })).toHaveCount(0); + expect(pageErrors).toEqual([]); + + // The server-side half of the same sentence: the Origin admitted in step 3 is + // refused outright once the allowlist is gone, so no mutation from this page + // could be admitted either. + const refused = await sessionThroughProxy(devOrigin, devOrigin); + expect(refused.status).toBe(403); + expect(await refused.json()).toMatchObject({ diagnostic: { code: 'AB8003' } }); }); }); diff --git a/packages/workbench/tests/mcp-app-preview-browser.test.ts b/packages/workbench/tests/mcp-app-preview-browser.test.ts index 51e8a2b65..fd5168ee8 100644 --- a/packages/workbench/tests/mcp-app-preview-browser.test.ts +++ b/packages/workbench/tests/mcp-app-preview-browser.test.ts @@ -8,6 +8,9 @@ import { describe, expect, it } from '@rstest/core'; import { createRsbuild } from '@rsbuild/core'; import { chromium } from 'playwright'; +import { within } from '../../agent-bundle/tests/support/eventually.ts'; +import { timeScale } from '../../agent-bundle/tests/support/time-scale.ts'; +import { requestRecorder } from './support/http.ts'; import { createWorkbenchFixtureConfig } from './support/workbench-fixture-config.ts'; import { browserLaunchOptions } from './support/workbench-e2e.ts'; @@ -17,9 +20,9 @@ const runtimeClientSource = join(workspaceRoot, 'packages', 'workbench', 'src', const runtimeRouteClientSource = join(workspaceRoot, 'packages', 'workbench', 'src', 'mcp', 'mcp-route-client.ts'); const mountedPreviewFixture = async () => { - const bootstrapRequests: string[] = []; + const bootstrapRequests = requestRecorder(); const bootstrap = createServer((request, response) => { - bootstrapRequests.push(request.url ?? '/'); + bootstrapRequests.record(request.url ?? '/'); response.writeHead(200, { 'content-type': 'text/html' }); response.end('Runtime App
Runtime App
'); }); @@ -229,7 +232,7 @@ describe('MCP App preview browser', () => { expect(policyTrace.map((entry) => entry.name)).toEqual(['src', 'allow', 'referrerpolicy', 'sandbox', 'src']); expect(policyTrace[0]?.value).toBe('about:blank'); expect(policyTrace.at(-1)?.value).toBeDefined(); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap']); + expect(await within(fixture.bootstrapRequests.arrived(1), 5_000 * timeScale)).toEqual(['/runtime-bootstrap']); const simulatedProfile = page.getByLabel('Simulated MCP App profile'); expect(await simulatedProfile.isVisible()).toBe(true); @@ -277,7 +280,7 @@ describe('MCP App preview browser', () => { const traced = await stats(); expect(lifecycleEvents(traced.events)).toEqual(lifecycleEvents(beforeOperationTrace.events)); expect(traced.iframeNodes).toBe(beforeOperationTrace.iframeNodes); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap']); + expect(fixture.bootstrapRequests.paths).toEqual(['/runtime-bootstrap']); await runtime('publishOperationTraceWithWrongEpoch'); expect(await implementationEvidence.textContent()).toContain('operation-2'); await runtime('publishOperationTrace'); @@ -289,7 +292,7 @@ describe('MCP App preview browser', () => { await page.waitForTimeout(100); const same = await stats(); expect(lifecycleEvents(same.events)).toEqual(lifecycleEvents(initial.events)); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap']); + expect(fixture.bootstrapRequests.paths).toEqual(['/runtime-bootstrap']); await runtime('changeRegistrar'); await page.waitForTimeout(100); @@ -314,7 +317,7 @@ describe('MCP App preview browser', () => { expect(newCreate).toBeGreaterThan(oldUnregister); expect(replaced.events.filter((entry) => entry === 'register:first')).toHaveLength(1); expect(replaced.events.filter((entry) => entry === 'register:second')).toHaveLength(1); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap', '/runtime-bootstrap']); + expect(await within(fixture.bootstrapRequests.arrived(2), 5_000 * timeScale)).toEqual(['/runtime-bootstrap', '/runtime-bootstrap']); expect(await implementationEvidence.count()).toBe(0); await runtime('publishOperationTraceWithUnexpectedEpoch'); expect(await implementationEvidence.count()).toBe(0); @@ -336,7 +339,7 @@ describe('MCP App preview browser', () => { const held = await stats(); expect(held.events.filter((entry) => entry === 'factory')).toHaveLength(factoriesBeforeHeldCreate); expect(held.events).not.toContain('policy:runtime-binding-c'); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap', '/runtime-bootstrap']); + expect(fixture.bootstrapRequests.paths).toEqual(['/runtime-bootstrap', '/runtime-bootstrap']); expect(held.events.lastIndexOf('unregister:second')).toBeGreaterThan(held.events.lastIndexOf('backend:runtime-binding-c')); expect(browserErrors).toEqual([]); } finally { @@ -428,7 +431,7 @@ describe('MCP App preview browser', () => { const abandoned = await stats(); expect(lifecycleEvents(abandoned.events)).toEqual(lifecycleEvents(stable.events)); expect(abandoned.iframeNodes).toBe(stable.iframeNodes); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap']); + expect(await within(fixture.bootstrapRequests.arrived(1), 5_000 * timeScale)).toEqual(['/runtime-bootstrap']); await runtime('failClose'); await runtime('throwUnregister'); @@ -440,7 +443,7 @@ describe('MCP App preview browser', () => { expect(failedReplacement.currentHandle).toBe('present'); expect(failedReplacement.events).not.toContain('register:second'); expect(failedReplacement.events).not.toContain('create:runtime-binding-b'); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap']); + expect(fixture.bootstrapRequests.paths).toEqual(['/runtime-bootstrap']); await runtime('mutateCommittedSource'); await runtime('retryCurrent'); @@ -458,7 +461,7 @@ describe('MCP App preview browser', () => { expect(await page.getByLabel('Runtime App result').textContent()).toContain('22'); expect(await page.getByLabel('Runtime App result').textContent()).not.toContain('Mutated'); expect(await page.getByLabel('Runtime App result').textContent()).not.toContain('999'); - expect(fixture.bootstrapRequests).toEqual(['/runtime-bootstrap', '/runtime-bootstrap']); + expect(await within(fixture.bootstrapRequests.arrived(2), 5_000 * timeScale)).toEqual(['/runtime-bootstrap', '/runtime-bootstrap']); await runtime('failClose'); await runtime('unmountRuntime'); diff --git a/packages/workbench/tests/mcp-route-client.test.ts b/packages/workbench/tests/mcp-route-client.test.ts index b0d120f1c..06a744e9a 100644 --- a/packages/workbench/tests/mcp-route-client.test.ts +++ b/packages/workbench/tests/mcp-route-client.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from '@rstest/core'; import { ForegroundRouteClient, McpRouteClient, McpRouteClientError } from '../src/mcp/mcp-route-client.ts'; +import { withBrowserOrigin } from './support/browser-origin.ts'; const json = (body: unknown, status = 200): Response => new Response(JSON.stringify(body), { headers: { 'content-type': 'application/json' }, @@ -196,18 +197,6 @@ const sessionBootstrap = (body: unknown): ForegroundRouteClient => new Foregroun }, }); -/** Stubs the browser origin the Node unit pool lacks, then removes the stub (or restores a prior descriptor). */ -const withBrowserOrigin = async (origin: string, run: () => Promise): Promise => { - const previous = Object.getOwnPropertyDescriptor(globalThis, 'location'); - Object.defineProperty(globalThis, 'location', { configurable: true, value: { origin } }); - try { - await run(); - } finally { - if (previous === undefined) Reflect.deleteProperty(globalThis, 'location'); - else Object.defineProperty(globalThis, 'location', previous); - } -}; - it('admits a browser served from an allowlisted contributor dev-server origin', async () => { await withBrowserOrigin('http://localhost:3000', async () => { const foreground = sessionBootstrap(devSessionBody); @@ -225,13 +214,21 @@ it('admits a browser served from an allowlisted contributor dev-server origin', it('rejects a browser origin outside the contributor dev-server allowlist', async () => { await withBrowserOrigin('http://localhost:3001', async () => { - await expect(sessionBootstrap(devSessionBody).sessionSnapshot()).rejects.toMatchObject({ code: 'AB8003' }); + await expect(sessionBootstrap(devSessionBody).sessionSnapshot()).rejects.toMatchObject({ + code: 'AB8003', + message: 'Origin http://localhost:3001 is not allowed by the foreground server at http://127.0.0.1:4100. ' + + 'Open http://127.0.0.1:4100 instead, or start agent-bundle dev with --workbench-dev-origin http://localhost:3001 to allow this origin.', + }); }); }); it('still rejects a foreign browser origin when the bootstrap carries no dev-server allowlist', async () => { await withBrowserOrigin('http://localhost:3000', async () => { - await expect(sessionBootstrap(sessionBody).sessionSnapshot()).rejects.toMatchObject({ code: 'AB8003' }); + await expect(sessionBootstrap(sessionBody).sessionSnapshot()).rejects.toMatchObject({ + code: 'AB8003', + message: 'Origin http://localhost:3000 is not allowed by the foreground server at http://127.0.0.1:4100. ' + + 'Open http://127.0.0.1:4100 instead, or start agent-bundle dev with --workbench-dev-origin http://localhost:3000 to allow this origin.', + }); }); }); diff --git a/packages/workbench/tests/project-client.test.ts b/packages/workbench/tests/project-client.test.ts index e01f28af7..a67c62d8e 100644 --- a/packages/workbench/tests/project-client.test.ts +++ b/packages/workbench/tests/project-client.test.ts @@ -1,6 +1,7 @@ import { expect, it } from '@rstest/core'; import { + projectFailureText, ProjectClient, ProjectClientError, type EventSourceFactory, @@ -11,6 +12,7 @@ import { HookClient } from '../src/hooks/hook-client.ts'; import { LogClient } from '../src/logs/log-client.ts'; import { ForegroundRouteClient } from '../src/mcp/mcp-route-client.ts'; import { PlaygroundClient } from '../src/playground/playground-client.ts'; +import { withBrowserOrigin } from './support/browser-origin.ts'; interface Listener { readonly listener: (event: { readonly data: string; readonly lastEventId: string }) => void; @@ -103,6 +105,10 @@ const flushEvents = async (): Promise => { await new Promise((resolvePromise) => setImmediate(resolvePromise)); }; +/** What the route client says when a contributor dev origin reaches a foreground server that was not started with the flag. */ +const noFlagRefusal = 'Origin http://localhost:3000 is not allowed by the foreground server at http://127.0.0.1:3100. ' + + 'Open http://127.0.0.1:3100 instead, or start agent-bundle dev with --workbench-dev-origin http://localhost:3000 to allow this origin.'; + it('calls the default browser fetch with its global receiver', async () => { const originalFetch = globalThis.fetch; const browserFetch = async function (this: typeof globalThis, input: RequestInfo | URL): Promise { @@ -889,7 +895,7 @@ it('reports one failed event refresh without an unhandled rejection and retains expect(observed).toEqual(['active']); expect(errors).toHaveLength(1); - expect(errors[0]).toMatchObject({ message: 'Workbench request failed with HTTP 500.' }); + expect(errors[0]).toMatchObject({ code: 'AB8007', message: 'Request could not be completed.', status: 500 }); expect(unhandled).toEqual([]); } finally { process.off('unhandledRejection', onUnhandled); @@ -1187,3 +1193,179 @@ it('invalidates every shared foreground admission when ProjectClient closes duri expect(eventSources).toBe(0); expect(stream.closed).toBe(false); }); + +it('surfaces the client-side refusal of an HTTP 200 bootstrap with its code and message but no status', async () => { + await withBrowserOrigin('http://localhost:3000', async () => { + const errors: unknown[] = []; + const client = new ProjectClient({ + events: () => new RecordingEventSource(), + fetch: withForegroundSession(async () => Response.json({ status: status() })), + retryDelay: () => new Promise((resolvePromise) => setImmediate(resolvePromise)), + }); + try { + const rejection: unknown = await client.connect(() => undefined, (reason) => errors.push(reason)) + .then(() => undefined, (reason: unknown) => reason); + + expect(rejection).toBeInstanceOf(ProjectClientError); + if (!(rejection instanceof ProjectClientError)) return; + expect(rejection.code).toBe('AB8003'); + expect(rejection.status).toBeUndefined(); + expect(rejection.message).toBe(noFlagRefusal); + expect(errors).toEqual([rejection]); + expect(client.connection.state).toBe('unavailable'); + } finally { + client.close(); + } + }); +}); + +it('rejects a refused rebuild with the foreground diagnostic code, message, and HTTP status', async () => { + const client = new ProjectClient({ + events: () => new RecordingEventSource(), + fetch: withForegroundSession(async (input) => String(input) === '/api/project/rebuild' + ? Response.json({ diagnostic: { code: 'AB8003', message: 'Request origin is not this foreground server.' } }, { status: 403 }) + : Response.json({ status: status() })), + }); + try { + await client.connect(() => undefined); + const rejection: unknown = await client.rebuild().then(() => undefined, (reason: unknown) => reason); + + expect(rejection).toBeInstanceOf(ProjectClientError); + if (!(rejection instanceof ProjectClientError)) return; + expect(rejection.code).toBe('AB8003'); + expect(rejection.status).toBe(403); + expect(rejection.message).toBe('Request origin is not this foreground server.'); + expect(projectFailureText(rejection, 'Rebuild request could not be completed.')) + .toBe('AB8003 — Request origin is not this foreground server. (HTTP 403)'); + } finally { + client.close(); + } +}); + +it('reports a refused session bootstrap during recovered-source refresh as a ProjectClientError with code and status', async () => { + const firstStream = new RecordingEventSource(); + const secondStream = new RecordingEventSource(); + const thirdStream = new RecordingEventSource(); + const streams = [firstStream, secondStream, thirdStream]; + const errors: unknown[] = []; + let sessionRequests = 0; + const foreground = new ForegroundRouteClient({ + fetch: async (input) => { + if (String(input) !== '/api/project/session') return Response.json({ status: status() }); + sessionRequests += 1; + return sessionRequests === 3 + ? Response.json({ diagnostic: { code: 'AB8003', message: 'Request origin is not this foreground server.' } }, { status: 403 }) + : Response.json(foregroundSession); + }, + }); + const client = new ProjectClient({ + events: () => streams.shift()!, + foreground, + retryDelay: () => new Promise((resolvePromise) => setImmediate(resolvePromise)), + }); + try { + await client.connect(() => undefined, (reason) => errors.push(reason)); + firstStream.emit('error', { data: '', lastEventId: '' }); + await flushEvents(); + await flushEvents(); + expect(errors).toHaveLength(1); + expect(sessionRequests).toBe(2); + + // The recovered stream opens while the session is forgotten, so the status + // read succeeds and the snapshot re-bootstrap is what the foreground refuses. + foreground.forgetAuthentication(); + secondStream.emit('open', { data: '', lastEventId: '' }); + await flushEvents(); + await flushEvents(); + + expect(sessionRequests).toBeGreaterThanOrEqual(3); + const refusal = errors[1]; + expect(refusal).toBeInstanceOf(ProjectClientError); + if (!(refusal instanceof ProjectClientError)) return; + expect(refusal.code).toBe('AB8003'); + expect(refusal.status).toBe(403); + expect(refusal.message).toBe('Request origin is not this foreground server.'); + } finally { + client.close(); + } +}); + +it('carries no HTTP status for a session the client itself invalidated mid-bootstrap', async () => { + const session = deferred(); + const foreground = new ForegroundRouteClient({ + fetch: async (input) => String(input) === '/api/project/session' ? session.promise : Response.json({ status: status() }), + }); + const client = new ProjectClient({ events: () => new RecordingEventSource(), foreground, retryDelay: async () => undefined }); + try { + const connecting = client.connect(() => undefined); + await new Promise((resolvePromise) => setImmediate(resolvePromise)); + client.close(); + foreground.forgetAuthentication(); + session.resolve(Response.json(foregroundSession)); + const rejection: unknown = await connecting.then(() => undefined, (reason: unknown) => reason); + + expect(rejection).toBeInstanceOf(ProjectClientError); + if (!(rejection instanceof ProjectClientError)) return; + expect(rejection.code).toBe('AB8019'); + expect(rejection.status).toBeUndefined(); + expect(projectFailureText(rejection, 'fallback')).toBe(`AB8019 — ${rejection.message}`); + } finally { + client.close(); + } +}); + +it('reports a recovery attempt the foreground keeps refusing once, then the line that replaces it', async () => { + const firstStream = new RecordingEventSource(); + const secondStream = new RecordingEventSource(); + const streams = [firstStream, secondStream]; + const errors: unknown[] = []; + const fourthSession = deferred(); + let sessionRequests = 0; + const client = new ProjectClient({ + events: () => streams.shift()!, + fetch: async (input) => { + if (String(input) !== '/api/project/session') return Response.json({ status: status() }); + sessionRequests += 1; + if (sessionRequests === 4) fourthSession.resolve(); + return sessionRequests === 2 || sessionRequests === 3 + ? Response.json({ diagnostic: { code: 'AB8003', message: 'Request origin is not this foreground server.' } }, { status: 403 }) + : Response.json(foregroundSession); + }, + retryDelay: () => new Promise((resolvePromise) => setImmediate(resolvePromise)), + }); + try { + await client.connect(() => undefined, (reason) => errors.push(reason)); + firstStream.emit('error', { data: '', lastEventId: '' }); + await fourthSession.promise; + await flushEvents(); + + // Two identical refusals produce one report after the disconnect line; the + // successful fourth bootstrap ends recovery without a further report. + expect(errors).toHaveLength(2); + expect(errors[0]).toMatchObject({ message: 'Foreground project event stream disconnected.' }); + expect(errors[1]).toBeInstanceOf(ProjectClientError); + expect(errors[1]).toMatchObject({ code: 'AB8003', message: 'Request origin is not this foreground server.', status: 403 }); + expect(client.connection.state).toBe('connecting'); + + secondStream.emit('open', { data: '', lastEventId: '' }); + await flushEvents(); + expect(client.connection.state).toBe('connected'); + expect(errors).toHaveLength(2); + } finally { + client.close(); + } +}); + +it('formats a project client failure as its code, message, and HTTP status, omitting the parts it lacks', () => { + expect(projectFailureText(new ProjectClientError('Request origin is not this foreground server.', 'AB8003', 403), 'fallback')) + .toBe('AB8003 — Request origin is not this foreground server. (HTTP 403)'); + expect(projectFailureText(new ProjectClientError(noFlagRefusal, 'AB8003'), 'fallback')).toBe(`AB8003 — ${noFlagRefusal}`); + expect(projectFailureText(new ProjectClientError('Foreground project event stream disconnected.'), 'fallback')) + .toBe('Foreground project event stream disconnected.'); + expect(projectFailureText(new Error('x'), 'fallback')).toBe('x'); + expect(projectFailureText({ get message(): string { throw new Error('hostile'); } }, 'fallback')).toBe('fallback'); + expect(projectFailureText(Object.defineProperty(new Error('x'), 'message', { get(): string { throw new Error('hostile'); } }), 'fallback')) + .toBe('fallback'); + expect(projectFailureText('not an error', 'fallback')).toBe('fallback'); + expect(projectFailureText(undefined, 'fallback')).toBe('fallback'); +}); diff --git a/packages/workbench/tests/support/browser-origin.ts b/packages/workbench/tests/support/browser-origin.ts new file mode 100644 index 000000000..9eb3a3d77 --- /dev/null +++ b/packages/workbench/tests/support/browser-origin.ts @@ -0,0 +1,15 @@ +/** + * Stubs the browser origin the Node unit pool lacks — `ForegroundRouteClient` + * reads `globalThis.location.origin` during the session bootstrap — then + * removes the stub (or restores a prior descriptor) even when `run` throws. + */ +export const withBrowserOrigin = async (origin: string, run: () => Promise): Promise => { + const previous = Object.getOwnPropertyDescriptor(globalThis, 'location'); + Object.defineProperty(globalThis, 'location', { configurable: true, value: { origin } }); + try { + await run(); + } finally { + if (previous === undefined) Reflect.deleteProperty(globalThis, 'location'); + else Object.defineProperty(globalThis, 'location', previous); + } +}; diff --git a/packages/workbench/tests/support/http.ts b/packages/workbench/tests/support/http.ts index 20c0a9115..f3d52f251 100644 --- a/packages/workbench/tests/support/http.ts +++ b/packages/workbench/tests/support/http.ts @@ -17,3 +17,46 @@ export const closeServer = async (server: Server): Promise => { server.closeAllConnections(); await closed; }; + +/** + * Records the request paths a loopback fixture server receives and lets a test + * await the Nth arrival through a promise the route handler settles. + * + * The browser issues a fixture request (an iframe fetching its bootstrap + * document) only after the in-page event a test waits on, so asserting the + * recorded paths synchronously after that wait races the network, and polling + * the array on a timer trades the race for a budget. Awaiting `arrived(n)` + * does neither: it settles inside the handler that records the nth request, + * or on the microtask queue when `n` requests have already been recorded. + * Checks that a step made *no* new request stay synchronous reads of `paths`. + */ +export interface RequestRecorder { + /** Resolves with the paths recorded so far once at least `count` have arrived — immediately if they already have. */ + readonly arrived: (count: number) => Promise; + /** Every recorded path, in arrival order (live view). */ + readonly paths: readonly string[]; + /** Called by the route handler with the request path. */ + readonly record: (path: string) => void; +} + +/** Creates a {@link RequestRecorder}; pending `arrived` waiters settle in FIFO order as their counts are reached. */ +export const requestRecorder = (): RequestRecorder => { + const paths: string[] = []; + let waiters: readonly Readonly<{ count: number; resolve: (paths: readonly string[]) => void }>[] = []; + return { + arrived: (count) => new Promise((resolve) => { + if (paths.length >= count) { + resolve([...paths]); + return; + } + waiters = [...waiters, { count, resolve }]; + }), + paths, + record: (path) => { + paths.push(path); + const settled = waiters.filter((waiter) => waiter.count <= paths.length); + waiters = waiters.filter((waiter) => waiter.count > paths.length); + for (const waiter of settled) waiter.resolve([...paths]); + }, + }; +}; diff --git a/packages/workbench/tests/workbench-screen.test.ts b/packages/workbench/tests/workbench-screen.test.ts index 3f015aedf..9297eddd6 100644 --- a/packages/workbench/tests/workbench-screen.test.ts +++ b/packages/workbench/tests/workbench-screen.test.ts @@ -3,10 +3,15 @@ import { renderToStaticMarkup } from 'react-dom/server'; import { expect, it } from '@rstest/core'; -import { Navigation, pageForHash, type WorkbenchPage } from '../src/workbench-screen.tsx'; +import { projectFailureText, ProjectClientError } from '../src/project-client.ts'; +import { ConnectionGate, Navigation, pageForHash, type WorkbenchPage } from '../src/workbench-screen.tsx'; const pages = (...values: WorkbenchPage[]): ReadonlySet => new Set(values); +/** What the route client says when a contributor dev origin reaches a foreground server that was not started with the flag. */ +const noFlagRefusal = 'Origin http://localhost:3000 is not allowed by the foreground server at http://127.0.0.1:3100. ' + + 'Open http://127.0.0.1:3100 instead, or start agent-bundle dev with --workbench-dev-origin http://localhost:3000 to allow this origin.'; + it('renders only available routes in grouped navigation', () => { const markup = renderToStaticMarkup(createElement(Navigation, { onNavigate: () => undefined, @@ -38,3 +43,33 @@ it('resolves unsupported and unknown hashes to Overview', () => { expect(pageForHash('#unknown', available)).toBe('overview'); expect(pageForHash('', available)).toBe('overview'); }); + +it('gates an unavailable connection with the failure code, message, and HTTP status', () => { + const markup = renderToStaticMarkup(createElement(ConnectionGate, { + error: projectFailureText(new ProjectClientError('Request origin is not this foreground server.', 'AB8003', 403), 'fallback'), + state: 'unavailable', + })); + + expect(markup).toContain('
'); + expect(markup).toContain('

Foreground connection unavailable

'); + expect(markup).toContain('

Waiting for the foreground server to recover.

'); + expect(markup).toContain('

AB8003 — Request origin is not this foreground server. (HTTP 403)

'); +}); + +it('gates a client-side refusal of an HTTP 200 bootstrap without presenting the 200 as the failure', () => { + const markup = renderToStaticMarkup(createElement(ConnectionGate, { + error: projectFailureText(new ProjectClientError(noFlagRefusal, 'AB8003'), 'fallback'), + state: 'unavailable', + })); + + expect(markup).toContain(`

AB8003 — ${noFlagRefusal}

`); + expect(markup).not.toContain('HTTP'); +}); + +it('gates a reconnecting connection without an alert', () => { + const markup = renderToStaticMarkup(createElement(ConnectionGate, { state: 'connecting' })); + + expect(markup).toContain('

Foreground connection reconnecting

'); + expect(markup).toContain('

Connecting to the foreground server.

'); + expect(markup).not.toContain('role="alert"'); +}); diff --git a/website/docs/en/guide/development/workbench.mdx b/website/docs/en/guide/development/workbench.mdx index 9be7c43fc..e60529bf9 100644 --- a/website/docs/en/guide/development/workbench.mdx +++ b/website/docs/en/guide/development/workbench.mdx @@ -268,9 +268,17 @@ still sees the browser's real `Origin`. Without `--workbench-dev-origin`, the UI at `http://localhost:3000` refuses its own bootstrap as `AB8003` (the session body names the foreground origin, not the page's) and stays on the -"Foreground connection unavailable" gate, reporting "Workbench request failed with HTTP 200."; the -foreground refuses mutations carrying that `Origin` with `AB8003` as well. The foreground URL keeps -working regardless. +"Foreground connection unavailable" gate, reporting `AB8003 — Origin http://localhost:3000 is not +allowed by the foreground server at http://127.0.0.1:3100. Open http://127.0.0.1:3100 instead, or +start agent-bundle dev with --workbench-dev-origin http://localhost:3000 to allow this origin.` +The gate appends `(HTTP )` only when the foreground response itself failed — a 403 +`AB8003` from the foreground renders as `AB8003 — Request origin is not this foreground server. +(HTTP 403)` — so the no-flag line carries no status: the foreground answered 200 and the UI +refused the body. Restarting the foreground without the flag while the page is open ends on the same +status-less `AB8003 — Origin …` line: the gate first reports `Foreground project event stream +disconnected.`, then the refusal once the reconnect's own 200 bootstrap is turned away by the UI. The +foreground still refuses mutations carrying +that `Origin` with `AB8003`. The foreground URL keeps working regardless. ## Next diff --git a/website/docs/zh/guide/development/workbench.mdx b/website/docs/zh/guide/development/workbench.mdx index 7fd4dd411..3ebf348e6 100644 --- a/website/docs/zh/guide/development/workbench.mdx +++ b/website/docs/zh/guide/development/workbench.mdx @@ -235,8 +235,15 @@ MCP App 沙箱 iframe 在独立的 loopback 端口上运行第三方插件代码 不带 `--workbench-dev-origin` 时,位于 `http://localhost:3000` 的界面会以 `AB8003` 拒绝自己的引导 (会话响应体中写的是前台 origin,而不是页面的 origin),并停留在"Foreground connection unavailable" -门控页上,显示"Workbench request failed with HTTP 200.";前台也同样会以 `AB8003` 拒绝携带该 `Origin` -的变更请求。前台 URL 本身无论如何都照常工作。 +门控页上,显示 `AB8003 — Origin http://localhost:3000 is not allowed by the foreground server at +http://127.0.0.1:3100. Open http://127.0.0.1:3100 instead, or start agent-bundle dev with +--workbench-dev-origin http://localhost:3000 to allow this origin.`。门控页只在前台响应本身失败时才会 +追加 `(HTTP )`——例如前台回以 403 的 `AB8003` 会显示为 +`AB8003 — Request origin is not this foreground server. (HTTP 403)`——因此不带标志时这一行没有状态码: +前台回应的是 200,是界面拒绝了该响应体。页面保持打开时不带标志重启前台,最终也会停在同一行不带状态码的 +`AB8003 — Origin …`:门控页先报告 `Foreground project event stream disconnected.`,待重连自身的 200 引导被界面 +拒绝后再显示这条拒绝信息。前台仍会以 +`AB8003` 拒绝携带该 `Origin` 的变更请求。前台 URL 本身无论如何都照常工作。 ## 下一步