From 607ef9481be1a20709d842a6efcd761b5aa1bc16 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Tue, 1 Sep 2026 21:44:14 +0000 Subject: [PATCH] fix(workbench): address route catalog review findings Correct CLI usage projections, retain externally packaged servers, and cover stale catalog repair in real Chrome. --- .changeset/routes-catalog-review-fixes.md | 7 ++ packages/workbench/src/routes/routes-model.ts | 16 ++++- packages/workbench/src/routes/routes-page.css | 1 + packages/workbench/src/routes/routes-page.tsx | 56 ++++++++++++++-- .../workbench/tests/examples-real.e2e.test.ts | 66 +++++++++++++------ packages/workbench/tests/routes-model.test.ts | 25 +++++++ packages/workbench/tests/routes-page.test.ts | 48 +++++++++++++- 7 files changed, 193 insertions(+), 26 deletions(-) create mode 100644 .changeset/routes-catalog-review-fixes.md diff --git a/.changeset/routes-catalog-review-fixes.md b/.changeset/routes-catalog-review-fixes.md new file mode 100644 index 000000000..2f4374043 --- /dev/null +++ b/.changeset/routes-catalog-review-fixes.md @@ -0,0 +1,7 @@ +--- +"agent-bundle": patch +--- + +Correct Workbench route usage summaries, retain externally packaged MCP server +surfaces in the Routes catalog, and cover the stale-manifest repair flow in +real-browser acceptance. diff --git a/packages/workbench/src/routes/routes-model.ts b/packages/workbench/src/routes/routes-model.ts index 64df35dd7..1330e6212 100644 --- a/packages/workbench/src/routes/routes-model.ts +++ b/packages/workbench/src/routes/routes-model.ts @@ -5,6 +5,7 @@ import type { RouteManifestConfigEntry, RouteManifestKind, RouteManifestRoute, + RouteManifestServerMode, } from '../../../agent-bundle/src/contracts/routes.ts'; /** @@ -55,6 +56,14 @@ export interface RouteCatalogProvider { readonly source: string; } +/** One declared MCP server, including externally packaged surfaces with no manifest routes. */ +export interface RouteCatalogServer { + readonly id: string; + readonly mode: RouteManifestServerMode; + readonly name: string; + readonly routeCount: number; +} + export interface RouteCatalog { readonly diagnostics: readonly Diagnostic[]; readonly digest: string; @@ -63,6 +72,7 @@ export interface RouteCatalog { readonly message?: string; readonly providers: readonly RouteCatalogProvider[]; readonly routeCount: number; + readonly servers: readonly RouteCatalogServer[]; readonly sourceRevision?: string; readonly state: RouteCatalogState; } @@ -147,6 +157,9 @@ export const routeCatalogFor = ( .map((provider) => Object.freeze({ id: provider.id, name: provider.name, source: provider.source })) .sort((left, right) => left.name.localeCompare(right.name))), routeCount: groups.reduce((total, group) => total + group.entries.length, 0), + servers: Object.freeze([...manifest.servers] + .map((server) => Object.freeze({ id: server.id, mode: server.mode, name: server.name, routeCount: server.routes.length })) + .sort((left, right) => left.name.localeCompare(right.name))), sourceRevision: manifest.sourceRevision, state: epochSourceRevision === undefined || epochSourceRevision === manifest.sourceRevision ? 'current' : 'stale', }); @@ -159,6 +172,7 @@ export const unavailableRouteCatalog = (message: string): RouteCatalog => Object message, providers: Object.freeze([]), routeCount: 0, + servers: Object.freeze([]), state: 'unavailable', }); @@ -167,4 +181,4 @@ export const routeCatalogHasKind = (catalog: RouteCatalog, kind: RouteManifestKi catalog.groups.some((group) => group.kind === kind && group.entries.length > 0); export const routeCatalogServerCount = (catalog: RouteCatalog): number => - new Set(catalog.groups.flatMap((group) => group.serverId === undefined ? [] : [group.serverId])).size; + catalog.servers.length; diff --git a/packages/workbench/src/routes/routes-page.css b/packages/workbench/src/routes/routes-page.css index f33f1b197..3e468019a 100644 --- a/packages/workbench/src/routes/routes-page.css +++ b/packages/workbench/src/routes/routes-page.css @@ -19,6 +19,7 @@ .route-group-heading { align-items: baseline; display: flex; flex-wrap: wrap; gap: 12px; justify-content: space-between; } .route-group-heading h2 { font-size: 17px; margin: 0; } .route-group-heading p { color: #596372; font-size: 13px; margin: 0; text-transform: lowercase; } +.route-server-summary { color: #596372; font-size: 13px; margin: 10px 0 0; max-width: 760px; } .route-table { border-collapse: collapse; margin-top: 14px; table-layout: fixed; width: 100%; } .route-table th, .route-table td { border-bottom: 1px solid #e4e8ef; padding: 11px 12px 11px 0; text-align: left; vertical-align: top; } .route-table thead th { color: #596372; font-size: 12px; font-weight: 750; letter-spacing: .03em; text-transform: uppercase; } diff --git a/packages/workbench/src/routes/routes-page.tsx b/packages/workbench/src/routes/routes-page.tsx index 1eb31fc00..761fc0c26 100644 --- a/packages/workbench/src/routes/routes-page.tsx +++ b/packages/workbench/src/routes/routes-page.tsx @@ -1,6 +1,11 @@ import React from 'react'; -import type { RouteCatalog, RouteCatalogEntry, RouteCatalogGroup } from './routes-model.ts'; +import type { + RouteCatalog, + RouteCatalogEntry, + RouteCatalogGroup, + RouteCatalogServer, +} from './routes-model.ts'; import './routes-page.css'; export interface RoutesPageProps { @@ -37,12 +42,49 @@ const commandSummary = (entry: RouteCatalogEntry): string | undefined => { if (command === undefined) return undefined; const positionals = command.options.filter((option) => option.positional !== undefined) .toSorted((left, right) => left.positional! - right.positional!) - .map((option) => option.repeated ? `[<${option.key}>…]` : `<${option.key}>`); + .map((option) => { + const name = option.repeated ? `${option.option}...` : option.option; + return option.required ? `<${name}>` : `[${name}]`; + }); const flags = command.options.filter((option) => option.positional === undefined) - .map((option) => option.required ? `--${option.option}` : `[--${option.option}]`); + .map((option) => { + const placeholder = option.kind === 'boolean' + ? '' + : ` <${option.choices === undefined ? option.kind : option.choices.join('|')}>`; + const flag = `--${option.option}${placeholder}`; + return option.required ? flag : `[${flag}]`; + }); return [...command.path, ...positionals, ...flags].join(' '); }; +const emptyServerSummary = (server: RouteCatalogServer): string => { + switch (server.mode) { + case 'command': + case 'custom': + case 'remote': + return `Routes are packaged externally in ${server.mode} mode, so the compiler manifest does not list route modules.`; + case 'generated': + return 'No conventional route modules were compiler-discovered for this generated server.'; + case 'conflict': + return 'No conventional route modules are listed while this server packaging mode remains in conflict.'; + default: { + const exhaustiveMode: never = server.mode; + return exhaustiveMode; + } + } +}; + +const EmptyServerSurface = ({ server }: { readonly server: RouteCatalogServer }) =>
+
+

{server.name}

+

{server.mode} mode

+
+

{emptyServerSummary(server)}

+
; + const RouteGroup = ({ group }: { readonly group: RouteCatalogGroup }) =>
This project declares no conventional route modules.

- : catalog.groups.map((group) => )} + : <> + {catalog.servers.filter((server) => server.routeCount === 0) + .map((server) => )} + {catalog.groups.map((group) => )} + } {catalog.providers.length === 0 ? undefined :

Context providers

diff --git a/packages/workbench/tests/examples-real.e2e.test.ts b/packages/workbench/tests/examples-real.e2e.test.ts index f3474d3d1..9518881a9 100644 --- a/packages/workbench/tests/examples-real.e2e.test.ts +++ b/packages/workbench/tests/examples-real.e2e.test.ts @@ -15,6 +15,7 @@ import { writeExampleReport, } from './support/example-acceptance.ts'; import { timeScale } from '../../agent-bundle/tests/support/time-scale.ts'; +import { replaceWatchedSource } from '../../agent-bundle/tests/support/watched-files.ts'; import { buildWorkbench, e2e, workbenchAssets, workbenchUrl } from './support/workbench-e2e.ts'; const browserTimeout = 15_000 * timeScale; @@ -39,6 +40,20 @@ const waitForExampleValue = async ( return value; }; +const rebuildFromCurrentPage = async (page: Parameters[0]): Promise => { + const status = await page.evaluate(async () => { + const sessionResponse = await fetch('/api/project/session'); + const session = await sessionResponse.json() as { readonly token: string }; + const response = await fetch('/api/project/rebuild', { + body: JSON.stringify({ paths: [] }), + headers: { 'content-type': 'application/json', 'x-agent-bundle-session': session.token }, + method: 'POST', + }); + return response.status; + }); + expect(status).toBe(200); +}; + e2e('drives the populated Skills Starter in real Chrome', { timeout: 90_000 }, async ({ page }) => { await buildWorkbench(); const server = await startDevServer({ @@ -113,19 +128,6 @@ e2e('reveals, retains, repairs, and removes capabilities without reloading Chrom root: project.root, }); const ledger = createExampleErrorLedger(page, server.url); - const rebuildFromCurrentPage = async (): Promise => { - const status = await page.evaluate(async () => { - const sessionResponse = await fetch('/api/project/session'); - const session = await sessionResponse.json() as { readonly token: string }; - const response = await fetch('/api/project/rebuild', { - body: JSON.stringify({ paths: [] }), - headers: { 'content-type': 'application/json', 'x-agent-bundle-session': session.token }, - method: 'POST', - }); - return response.status; - }); - expect(status).toBe(200); - }; try { await page.goto(workbenchUrl(server.url, 'hooks')); await waitForSettledWorkbench(page); @@ -134,7 +136,7 @@ e2e('reveals, retains, repairs, and removes capabilities without reloading Chrom await expect(page.getByRole('link', { name: 'Playground', exact: true })).toHaveCount(0, { timeout: browserTimeout }); await writeFile(configPath, hookConfig); - await rebuildFromCurrentPage(); + await rebuildFromCurrentPage(page); await expect(page.getByRole('link', { name: 'Hooks', exact: true })).toBeVisible({ timeout: browserTimeout }); await expect(page.getByRole('link', { name: 'Playground', exact: true })).toBeVisible({ timeout: browserTimeout }); await page.getByRole('link', { name: 'Hooks', exact: true }).click(); @@ -143,7 +145,7 @@ e2e('reveals, retains, repairs, and removes capabilities without reloading Chrom await captureExampleState(page, 'skills-starter', 'capability-revealed'); await writeFile(hookSource, 'export default () => ({\n'); - await rebuildFromCurrentPage(); + await rebuildFromCurrentPage(page); await page.getByRole('link', { name: 'Overview', exact: true }).click(); await waitForSettledWorkbench(page); await expect(page.getByRole('heading', { name: /Diagnostics \([1-9]/u })).toBeVisible({ timeout: browserTimeout }); @@ -153,7 +155,7 @@ e2e('reveals, retains, repairs, and removes capabilities without reloading Chrom await captureExampleState(page, 'skills-starter', 'capability-stale'); await writeFile(hookSource, healthyHook); - await rebuildFromCurrentPage(); + await rebuildFromCurrentPage(page); await expect(page.getByRole('heading', { name: 'Diagnostics (0)' })).toBeVisible({ timeout: browserTimeout }); await expect(page.locator('.build-health')).toContainText('Current build', { timeout: browserTimeout }); await captureExampleState(page, 'skills-starter', 'capability-repaired'); @@ -162,7 +164,7 @@ e2e('reveals, retains, repairs, and removes capabilities without reloading Chrom await waitForSettledWorkbench(page); await expect(page.locator('#hook-binding option')).not.toHaveCount(0, { timeout: browserTimeout }); await writeFile(configPath, originalConfig); - await rebuildFromCurrentPage(); + await rebuildFromCurrentPage(page); await expect(page).toHaveURL(new URL('#overview', server.url).href, { timeout: browserTimeout }); await expect(page.getByRole('link', { name: 'Hooks', exact: true })).toHaveCount(0, { timeout: browserTimeout }); await expect(page.getByRole('link', { name: 'Playground', exact: true })).toHaveCount(0, { timeout: browserTimeout }); @@ -571,6 +573,8 @@ e2e('drives every populated MCP App workflow surface in real Chrome', { timeout: e2e('renders the flagship compiled route catalog by server and kind in real Chrome', { timeout: 150_000 }, async ({ page }) => { await buildWorkbench(); const project = await copyExample('audiobook-curator'); + const conversionSource = join(project.root, 'src', 'conversion.ts'); + const healthyConversion = await readFile(conversionSource, 'utf8'); const server = await startDevServer({ assets: createWorkbenchAssetSource({ root: workbenchAssets }), open: false, @@ -610,9 +614,9 @@ e2e('renders the flagship compiled route catalog by server and kind in real Chro await expect(cli).toContainText('cli:library-audit', { timeout: browserTimeout }); await expect(cli).toContainText('src/cli/library-audit.tsx', { timeout: browserTimeout }); await expect(cli.locator('.route-command').filter({ hasText: 'library-audit' })) - .toHaveText('library-audit […] [--concurrency] --report [--strict]', { timeout: browserTimeout }); + .toHaveText('library-audit [--concurrency ] --report [--strict]', { timeout: browserTimeout }); await expect(cli.locator('.route-command').filter({ hasText: 'inspect' })) - .toHaveText('inspect [--max-files]', { timeout: browserTimeout }); + .toHaveText('inspect [--max-files ]', { timeout: browserTimeout }); // 17 MCP routes plus 15 CLI routes, and nothing invented: the curator // declares no conventional event routes, scripts, or context providers. @@ -623,6 +627,30 @@ e2e('renders the flagship compiled route catalog by server and kind in real Chro await expect(page.locator('.route-diagnostics')).toHaveCount(0); await captureExampleState(page, 'audiobook-curator', 'routes-catalog-by-server'); + // A prepared source revision can move ahead while a failed rebuild keeps + // the published epoch intact. Reloading the same browser page re-reads that + // prepared manifest and must identify it as stale until a repair publishes. + await replaceWatchedSource(project.root, conversionSource, `${healthyConversion}\nconst = ;\n`); + await rebuildFromCurrentPage(page); + await page.reload(); + await waitForSettledWorkbench(page); + await expect(page.locator('.route-state')).toHaveText('stale', { timeout: browserTimeout }); + await expect(page.locator('.routes-page-heading')).toContainText( + 'The dev server has compiled newer source than the published build. Rebuild to publish these routes.', + { timeout: browserTimeout }, + ); + await captureExampleState(page, 'audiobook-curator', 'routes-catalog-stale'); + + await replaceWatchedSource(project.root, conversionSource, healthyConversion); + await rebuildFromCurrentPage(page); + await waitForSettledWorkbench(page); + await expect(page.locator('.route-state')).toHaveText('current', { timeout: browserTimeout }); + await expect(page.locator('.routes-page-heading')).toContainText( + 'This catalog is the compiled route graph the published build was produced from.', + { timeout: browserTimeout }, + ); + await captureExampleState(page, 'audiobook-curator', 'routes-catalog-repaired'); + for (const preserved of ['Overview', 'Skills', 'Artifacts', 'Logs']) { await expect(page.getByRole('link', { name: preserved, exact: true })).toBeVisible({ timeout: browserTimeout }); } diff --git a/packages/workbench/tests/routes-model.test.ts b/packages/workbench/tests/routes-model.test.ts index 48a7800de..ab16385d9 100644 --- a/packages/workbench/tests/routes-model.test.ts +++ b/packages/workbench/tests/routes-model.test.ts @@ -130,6 +130,29 @@ it('carries the server packaging mode and the CLI surface mode as group metadata expect(catalog.groups.find((group) => group.kind === 'cli')?.server).toBeUndefined(); }); +it('retains empty externally packaged servers as catalog surfaces', () => { + const catalog = routeCatalogFor({ + diagnostics: [], + digest: 'e'.repeat(64), + events: [], + providers: [], + scripts: [], + servers: [ + { id: 'mcp:custom-library', mode: 'custom', name: 'custom-library', routes: [] }, + { id: 'mcp:command-catalog', mode: 'command', name: 'command-catalog', routes: [] }, + ], + sourceRevision: 'r'.repeat(64), + }); + + expect(routeCatalogServerCount(catalog)).toBe(2); + expect(catalog.servers).toEqual([ + { id: 'mcp:command-catalog', mode: 'command', name: 'command-catalog', routeCount: 0 }, + { id: 'mcp:custom-library', mode: 'custom', name: 'custom-library', routeCount: 0 }, + ]); + expect(catalog.groups).toEqual([]); + expect(catalog.routeCount).toBe(0); +}); + it('attaches the compiled command to its CLI route entry', () => { const catalog = routeCatalogFor(manifest); const entry = catalog.groups.find((group) => group.kind === 'cli')?.entries[0]; @@ -170,6 +193,7 @@ it('renders an empty compiled graph without groups', () => { }); expect(catalog.groups).toEqual([]); + expect(catalog.servers).toEqual([]); expect(catalog.routeCount).toBe(0); expect(catalog.state).toBe('current'); }); @@ -179,6 +203,7 @@ it('describes an unreadable manifest without inventing routes', () => { expect(catalog.state).toBe('unavailable'); expect(catalog.groups).toEqual([]); + expect(catalog.servers).toEqual([]); expect(catalog.sourceRevision).toBeUndefined(); expect(routeCatalogHasKind(catalog, 'tool')).toBe(false); }); diff --git a/packages/workbench/tests/routes-page.test.ts b/packages/workbench/tests/routes-page.test.ts index ca0485af5..368de37e3 100644 --- a/packages/workbench/tests/routes-page.test.ts +++ b/packages/workbench/tests/routes-page.test.ts @@ -101,7 +101,31 @@ it('leads the usage line with positionals in argv order regardless of option ord const markup = render(routeCatalogFor(reordered)); - expect(markup).toContain('library audit [<sources>…] [--concurrency] --report'); + expect(markup).toContain('library audit <sources...> [--concurrency <number>] --report <string>'); +}); + +it('formats optional positionals and enum flags like generated CLI help', () => { + const optionalInputs: RouteManifest = { + ...manifest, + cli: { + ...manifest.cli!, + commands: [{ + aliases: [], + exitCode: 'zero', + options: [ + { key: 'format', kind: 'enum', option: 'format', choices: ['mp3', 'opus'], repeated: false, required: false }, + { key: 'destination', kind: 'string', option: 'output-directory', positional: 0, repeated: false, required: false }, + { key: 'sources', kind: 'string', option: 'extra-source', positional: 1, repeated: true, required: false }, + ], + path: ['publish'], + routeId: 'cli:library/audit', + }], + }, + }; + + const markup = render(routeCatalogFor(optionalInputs)); + + expect(markup).toContain('publish [output-directory] [extra-source...] [--format <mp3|opus>]'); }); it('shows the canonical event beside an event route', () => { @@ -142,6 +166,28 @@ it('names the empty compiled graph rather than an error', () => { expect(markup).not.toContain('role="alert"'); }); +it('renders externally packaged empty servers instead of the no-routes state', () => { + const markup = render(routeCatalogFor({ + diagnostics: [], + digest: 'e'.repeat(64), + events: [], + providers: [], + scripts: [], + servers: [ + { id: 'mcp:custom-library', mode: 'custom', name: 'custom-library', routes: [] }, + { id: 'mcp:remote-catalog', mode: 'remote', name: 'remote-catalog', routes: [] }, + ], + sourceRevision: 'r'.repeat(64), + })); + + expect(markup).toContain('>custom-library<'); + expect(markup).toContain('>remote-catalog<'); + expect(markup).toContain('custom mode'); + expect(markup).toContain('remote mode'); + expect(markup).toContain('packaged externally'); + expect(markup).not.toContain('This project declares no conventional route modules.'); +}); + it('reports an unreadable manifest as an alert', () => { const markup = render(unavailableRouteCatalog('Route manifest is not available.'));