From c8fa9795eca2920400c8a7238631195ff8369609 Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 17:57:53 -0400 Subject: [PATCH 1/9] hub-mcp get_errors v2: plan + carried-over pieces (local WASM validation) Pivot per review guidance: drop the CRDT diagnostics sidecar entirely; get_errors will render the files the MCP already holds using the same wasm-quarto-hub-client module the browser preview runs (feasibility spike in plan doc), plus execution errors from the existing captures sidecar. Carries over from v1: the local-prod WS proxy crash fix and the two get_errors test files (to be reworked onto the new backing). Co-Authored-By: Claude Fable 5 --- .../plans/2026-07-28-hub-mcp-get-errors-v2.md | 118 ++++++++ scripts/local-prod-server.mjs | 16 ++ .../src/get-errors-handler.test.ts | 264 ++++++++++++++++++ .../src/get-errors-live.test.ts | 131 +++++++++ 4 files changed, 529 insertions(+) create mode 100644 claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md create mode 100644 ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts create mode 100644 ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts diff --git a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md new file mode 100644 index 000000000..289aef2aa --- /dev/null +++ b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md @@ -0,0 +1,118 @@ +# hub-mcp `get_errors` v2: validate locally with the QuartoHub WASM pipeline + +## Overview + +Agents using the Quarto Hub MCP server can read and write project files but +cannot see render errors. **v1** (branch `feature/hub-mcp-get-errors`, +plan `claude-notes/plans/2026-07-16-hub-mcp-get-errors.md`) had the browser +preview publish its diagnostics into an automerge index-doc sidecar that the +MCP read back with a content-hash staleness flag. It was implemented and +verified end-to-end, but review guidance from Carlos killed the architecture: + +> Don't try to chase synchronization with the CRDT. Just grab the content of +> the file/project you care about and have an API entry point to check for +> the validity. You're never going to be able to know if the document you +> just changed ends up looking exactly how you expected it to, because it's +> a distributed system. + +**v2** (this branch): `get_errors` renders the project files the MCP already +holds — using the *same WASM module the browser preview runs* +(`wasm-quarto-hub-client`) hosted in the MCP's Node process — and reports the +diagnostics of exactly what it rendered. Deterministic, no cross-peer +choreography, no schema change, hub-client untouched. The only cross-peer +data still read is the existing `captures` sidecar (execution errors happen +elsewhere and cannot be recomputed locally). + +Feasibility proven 2026-07-28 in a Node spike: esbuild-bundle the +wasm-bindgen JS with three aliases (`/src/wasm-js-bridge/{cache,fetch,sass}.js` +→ `ts-packages/wasm-js-bridge/src/*`), `sass` external (never needed for +diagnostics), init from bytes (`init(await readFile(wasmPath))`), then +`vfs_add_file` + `render_page_in_project('index.qmd')` returned the identical +structured diagnostic the browser shows (`[Q-2-13] Unclosed Strong Star +Emphasis`, line 5 col 24) for a broken fixture. + +## Design + +New module `ts-packages/quarto-hub-mcp/src/local-render.ts`: + +- `initRenderer(wasmBytes | wasmPath)` — one-time init (lazy, on first + `get_errors` call; keeps server startup fast). +- `renderDiagnostics(files: Map, path: string)` — + `vfs_clear()`, `vfs_add_file('/project/' + p, text)` for every text file + (`vfs_add_binary_file` for binaries), then `render_page_in_project(path)`; + returns `{ diagnostics, warnings, pass1Failures, error }` mapped from the + WASM `RenderResponse`. Serialize renders with a promise chain (the VFS is + a module-global in the WASM instance). + +`get_errors` tool (kept name, args `{ project, path? }`, read-only mode): +- `path` given → render that file; omitted → render every `.qmd` in the + project (pass-1 failures attribute sibling errors to their own paths, so a + single `index.qmd` render already surfaces most project-wide breakage — + render each remaining `.qmd` for completeness, capped and noted). +- Output per file: `{ path, checkedContentSha256, errors, warnings }` plus + `execution: { state, lastError }` from the `captures` sidecar. No `stale` + flag — the response describes exactly the bytes that were rendered. +- Tool description teaches the loop: read → fix via patch_file → call + get_errors again (it validates the new content immediately; no waiting). + +Bundling (two consumers): +- `tsc` dev build (`dist/`, used by tests): a Node loader in + `local-render.ts` resolves the wasm-bindgen JS + `.wasm` from + `hub-client/wasm-quarto-hub-client/` via an env override + (`QUARTO_HUB_MCP_WASM_DIR`) falling back to a path probe. +- esbuild bundle (`dist-bundle/`, embedded in `q2 mcp`): extend + `crates/xtask`'s build-hub-mcp-bundle with the three bridge aliases + + `sass` external, and copy `wasm_quarto_hub_client_bg.wasm` (~38 MB) into + `dist-bundle/`. Note: the q2 binary already embeds a second copy of this + WASM for the preview SPA — dedupe is a follow-up, not v1. + +## Work items (TDD) + +### Phase 1 — local renderer +- [ ] Tests first (`src/local-render.test.ts`, real WASM, no mocks): broken + YAML → error diagnostic with line/col; clean doc → empty; sibling + pass-1 failure attributed to sibling path; binary files tolerated; + sequential renders don't interleave +- [ ] Implement `local-render.ts` (loader, VFS fill, render, mapping) + +### Phase 2 — get_errors tool rework +- [ ] Rework `src/get-errors-handler.test.ts` (ported from v1): shapes, + captures surfacing (error surfaced / idle suppressed / running + surfaced), path filter, checkedContentSha256 present, renderer mocked + at the module seam +- [ ] Rework `src/get-errors-live.test.ts`: real server binary + test hub + + real WASM — create broken project via MCP, `get_errors` returns the + diagnostic; `patch_file` fix; `get_errors` immediately returns clean +- [ ] `tools.ts`: reimplement handler on local render + captures; + keep `onCapturesChange` wiring in connection-manager (v1's + `sidecars.captures`), drop everything diagnostics-sidecar +- [ ] Tool lists in both modes (`hub-mcp.test.ts`) include `get_errors` + +### Phase 3 — bundling +- [ ] `cargo xtask build-hub-mcp-bundle`: bridge aliases, `sass` external, + wasm copy into dist-bundle; `q2 mcp --launcher-info` freshness check +- [ ] `bundle.test.ts` covers the wasm asset presence + +### Phase 4 — verification +- [ ] Package suites green; `cargo xtask verify` legs; e2e against local-prod + hub (create broken project via MCP → get_errors → patch_file → + get_errors clean), recorded here per the end-to-end policy + +## Carried over from v1 (independent of architecture) +- [x] `scripts/local-prod-server.mjs`: WS proxy no longer crashes on client + ECONNRESET (unhandled socket 'error') — verified by hard-killing a + live WS client +- Strand backlog (braid still awaiting the q2 skein doc id on this machine): + 1. samod wedge: connection close with pending sync state busy-loops the + hub and stops all doc exchange until restart (see v1 plan's BLOCKER + section for log signatures + repro; p0/p1) + 2. deleteFile/renameFile leave stale `captures` sidecar entries + 3. preview red-bars `date-modified: last-modified` (keyword unresolvable + in browser VFS) as if it were a document error + 4. dedupe the two embedded copies of wasm_quarto_hub_client_bg.wasm in q2 + +## v1 disposition +`feature/hub-mcp-get-errors` (sidecar publish/read, fully implemented and +tested) is preserved as a branch. If a human-facing "see collaborators' +preview state" feature is ever wanted, that work is a starting point — but +it is intentionally NOT part of this PR. diff --git a/scripts/local-prod-server.mjs b/scripts/local-prod-server.mjs index 30d934811..2dccb50ba 100755 --- a/scripts/local-prod-server.mjs +++ b/scripts/local-prod-server.mjs @@ -89,9 +89,25 @@ function handleUpgrade(req, socket, head) { headers: req.headers, }; + // A client that drops without a closing handshake emits 'error' + // (ECONNRESET) on this socket. Unhandled, that event crashes the + // whole proxy process — tear down just this connection instead. + socket.on('error', (err) => { + console.error(`WebSocket client socket error: ${err.message}`); + socket.destroy(); + }); + const proxyReq = http.request(options); proxyReq.on('upgrade', (proxyRes, proxySocket, proxyHead) => { + // Same hazard on the hub side of the pipe. + proxySocket.on('error', (err) => { + console.error(`WebSocket hub socket error: ${err.message}`); + socket.destroy(); + }); + socket.on('close', () => proxySocket.destroy()); + proxySocket.on('close', () => socket.destroy()); + socket.write('HTTP/1.1 101 Switching Protocols\r\n'); Object.keys(proxyRes.headers).forEach(key => { socket.write(`${key}: ${proxyRes.headers[key]}\r\n`); diff --git a/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts b/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts new file mode 100644 index 000000000..26dfa3da6 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts @@ -0,0 +1,264 @@ +/** + * Handler-level tests for the `get_errors` tool (handleGetErrors). + * + * Same harness pattern as wait-for-change-handler.test.ts: the REAL + * `registerTools` dispatch runs against a fake ConnectionManager whose + * `connect` returns a fabricated project state. Coverage: + * + * - the no-publisher note when the diagnostics sidecar is empty + * - `stale` false when the stored contentHash matches the current text + * - `stale` true on hash mismatch, missing file, and binary payloads + * - errors/warnings split by diagnostic kind + * - execution errors surfaced from the captures sidecar (state 'error' + * with lastError; 'running' surfaced; 'idle' suppressed) + * - the optional `path` filter + */ + +import { createHash } from 'node:crypto'; +import { describe, it, expect } from 'vitest'; +import { CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js'; +import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import type { FilePayload, CaptureRef, FileDiagnostics } from '@quarto/quarto-sync-client'; +import { registerTools } from './tools.js'; +import type { ConnectionManager } from './connection-manager.js'; + +function sha256(text: string): string { + return `sha256:${createHash('sha256').update(text, 'utf8').digest('hex')}`; +} + +interface FakeStateInit { + files?: Record; + captures?: Record; + diagnostics?: Record; +} + +/** + * Register the real tool handlers against a fake manager whose `connect` + * resolves to a state fabricated from `init`. + */ +function harness(init: FakeStateInit): { + call: (args: Record) => Promise; +} { + const state = { + client: {} as never, + files: new Map(Object.entries(init.files ?? {})), + waiters: new Set(), + sidecars: { + captures: init.captures ?? {}, + diagnostics: init.diagnostics ?? {}, + }, + }; + const manager = { + async connect(_project: string) { + return state; + }, + } as unknown as ConnectionManager; + + let callToolHandler: + | ((req: { params: { name: string; arguments?: Record } }, extra: unknown) => Promise) + | undefined; + const server = { + setRequestHandler(schema: unknown, cb: unknown) { + if (schema === CallToolRequestSchema) { + callToolHandler = cb as typeof callToolHandler; + } + }, + } as unknown as Server; + + registerTools(server, manager, false); + if (!callToolHandler) throw new Error('CallTool handler was not registered'); + + const call = (args: Record) => + callToolHandler!({ params: { name: 'get_errors', arguments: args } }, {}); + + return { call }; +} + +function parse(result: CallToolResult): Record { + const block = result.content[0]; + if (block.type !== 'text') throw new Error('expected a text result block'); + return JSON.parse(block.text) as Record; +} + +const ERROR_ITEM = { + kind: 'error' as const, + title: 'YAML parse error', + hints: ['close the bracket'], + start_line: 2, + start_column: 8, + details: [], +}; + +const WARNING_ITEM = { + kind: 'warning' as const, + title: 'unknown option', + hints: [], + details: [], +}; + +function fileDiagnostics( + content: string, + items: FileDiagnostics['items'], +): FileDiagnostics { + return { + contentHash: sha256(content), + asOf: '2026-07-16T12:00:00.000Z', + source: 'hub-client-preview', + items, + }; +} + +describe('handleGetErrors — no publisher yet', () => { + it('returns a note explaining diagnostics appear when a preview is open', async () => { + const h = harness({ files: { 'index.qmd': { type: 'text', text: '# hi' } } }); + const out = parse(await h.call({ project: 'idx' })); + expect(out.note).toContain('preview'); + expect(out.files).toEqual([]); + }); + + it('still reports execution errors when only the captures sidecar exists', async () => { + const h = harness({ + files: { 'index.qmd': { type: 'text', text: '# hi' } }, + captures: { + 'index.qmd': { captureDocId: 'cap-1', state: 'error', lastError: 'kernel died' }, + }, + }); + const out = parse(await h.call({ project: 'idx' })); + expect(out.note).toContain('preview'); + const files = out.files as Array>; + expect(files).toHaveLength(1); + expect(files[0].path).toBe('index.qmd'); + expect(files[0].execution).toEqual({ state: 'error', lastError: 'kernel died' }); + }); +}); + +describe('handleGetErrors — staleness', () => { + it('reports stale: false when the stored hash matches the current file text', async () => { + const content = '---\ntitle: [broken\n---\n'; + const h = harness({ + files: { 'index.qmd': { type: 'text', text: content } }, + diagnostics: { 'index.qmd': fileDiagnostics(content, [ERROR_ITEM]) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const files = out.files as Array>; + expect(files).toHaveLength(1); + const preview = files[0].preview as Record; + expect(preview.stale).toBe(false); + expect(preview.asOf).toBe('2026-07-16T12:00:00.000Z'); + expect(preview.source).toBe('hub-client-preview'); + }); + + it('reports stale: true when the file has changed since the render', async () => { + const h = harness({ + files: { 'index.qmd': { type: 'text', text: 'edited since the render' } }, + diagnostics: { 'index.qmd': fileDiagnostics('what was rendered', [ERROR_ITEM]) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const files = out.files as Array>; + expect((files[0].preview as Record).stale).toBe(true); + }); + + it('reports stale: true when the file is missing from the project', async () => { + const h = harness({ + diagnostics: { 'gone.qmd': fileDiagnostics('old content', [ERROR_ITEM]) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const files = out.files as Array>; + expect((files[0].preview as Record).stale).toBe(true); + }); + + it('reports stale: true when the payload is binary', async () => { + const h = harness({ + files: { 'img.png': { type: 'binary', data: new Uint8Array([1]), mimeType: 'image/png' } }, + diagnostics: { 'img.png': fileDiagnostics('old text', [ERROR_ITEM]) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const files = out.files as Array>; + expect((files[0].preview as Record).stale).toBe(true); + }); +}); + +describe('handleGetErrors — content shape', () => { + it('splits items into errors and warnings by kind', async () => { + const content = 'x'; + const h = harness({ + files: { 'a.qmd': { type: 'text', text: content } }, + diagnostics: { 'a.qmd': fileDiagnostics(content, [ERROR_ITEM, WARNING_ITEM]) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const preview = (out.files as Array>)[0] + .preview as Record; + expect(preview.errors).toEqual([ERROR_ITEM]); + expect(preview.warnings).toEqual([WARNING_ITEM]); + }); + + it('an empty items array reports a clean render (no errors, no warnings)', async () => { + const content = '# fine'; + const h = harness({ + files: { 'a.qmd': { type: 'text', text: content } }, + diagnostics: { 'a.qmd': fileDiagnostics(content, []) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const preview = (out.files as Array>)[0] + .preview as Record; + expect(preview.errors).toEqual([]); + expect(preview.warnings).toEqual([]); + expect(preview.stale).toBe(false); + expect(out.note).toBeUndefined(); + }); + + it('surfaces a running capture but suppresses idle ones', async () => { + const content = 'x'; + const h = harness({ + files: { + 'running.qmd': { type: 'text', text: content }, + 'idle.qmd': { type: 'text', text: content }, + }, + captures: { + 'running.qmd': { captureDocId: 'cap-r', state: 'running' }, + 'idle.qmd': { captureDocId: 'cap-i', state: 'idle' }, + }, + diagnostics: { + 'running.qmd': fileDiagnostics(content, []), + 'idle.qmd': fileDiagnostics(content, []), + }, + }); + const out = parse(await h.call({ project: 'idx' })); + const files = out.files as Array>; + const running = files.find((f) => f.path === 'running.qmd')!; + const idle = files.find((f) => f.path === 'idle.qmd')!; + expect(running.execution).toEqual({ state: 'running' }); + expect(idle.execution).toBeUndefined(); + }); + + it('filters to a single path when `path` is given', async () => { + const content = 'x'; + const h = harness({ + files: { + 'a.qmd': { type: 'text', text: content }, + 'b.qmd': { type: 'text', text: content }, + }, + diagnostics: { + 'a.qmd': fileDiagnostics(content, [ERROR_ITEM]), + 'b.qmd': fileDiagnostics(content, [WARNING_ITEM]), + }, + }); + const out = parse(await h.call({ project: 'idx', path: 'b.qmd' })); + const files = out.files as Array>; + expect(files).toHaveLength(1); + expect(files[0].path).toBe('b.qmd'); + }); + + it('lists paths sorted and unions diagnostics + capture paths', async () => { + const content = 'x'; + const h = harness({ + files: { 'b.qmd': { type: 'text', text: content } }, + captures: { 'a.qmd': { captureDocId: 'cap', state: 'error', lastError: 'boom' } }, + diagnostics: { 'b.qmd': fileDiagnostics(content, [ERROR_ITEM]) }, + }); + const out = parse(await h.call({ project: 'idx' })); + const files = out.files as Array>; + expect(files.map((f) => f.path)).toEqual(['a.qmd', 'b.qmd']); + }); +}); diff --git a/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts new file mode 100644 index 000000000..96d9e53bd --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts @@ -0,0 +1,131 @@ +/** + * MCP-level integration test for `get_errors` against the in-process + * test hub: the diagnostics sidecar is planted by mutating the project + * index through the hub's own repo handle (standing in for the + * hub-client preview publisher), then a fresh MCP server instance + * reads it back through the real tool surface — including the + * staleness flip after the agent edits the file via `write_file`. + * + * Same harness as dangling-entries.test.ts: drives the real server + * binary (dist/index.js) over stdio; no external network. + */ + +import { createHash } from 'node:crypto'; +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { DocumentId } from '@automerge/automerge-repo'; +import type { IndexDocument } from '@quarto/quarto-automerge-schema'; + +import { McpTestClient } from './mcp-test-client.js'; +import { startTestHub, type TestHub } from './test-hub.js'; + +const BROKEN_CONTENT = '---\ntitle: [broken\n---\n'; + +function sha256(text: string): string { + return `sha256:${createHash('sha256').update(text, 'utf8').digest('hex')}`; +} + +describe('get_errors at the MCP tool surface (test hub)', () => { + let hub: TestHub; + let client: McpTestClient; + let indexDocId: string; + + beforeAll(async () => { + hub = await startTestHub(); + + const creator = new McpTestClient(); + await creator.start(['--server', hub.url]); + const created = await creator.callTool('create_project', { + files: [{ path: 'index.qmd', content: BROKEN_CONTENT }], + }); + expect(created.isError).not.toBe(true); + const parsed = JSON.parse(created.content[0]!.text) as { + indexDocId: string; + files: Array<{ path: string; docId: string }>; + }; + indexDocId = parsed.indexDocId; + expect(await hub.hubHasDoc(indexDocId, 8000)).toBe(true); + for (const f of parsed.files) { + expect(await hub.hubHasDoc(f.docId, 8000)).toBe(true); + } + await creator.stop(); + + // Plant the sidecars the way a rendering client would publish them. + const handle = await hub.repo.find(indexDocId as DocumentId); + handle.change((d) => { + d.diagnostics = { + 'index.qmd': { + contentHash: sha256(BROKEN_CONTENT), + asOf: '2026-07-16T12:00:00.000Z', + source: 'hub-client-preview', + items: [ + { + kind: 'error', + title: 'YAML parse error', + hints: ['close the bracket'], + start_line: 2, + start_column: 8, + details: [], + }, + ], + }, + }; + d.captures = { + 'index.qmd': { captureDocId: 'cap-1', state: 'error', lastError: 'kernel died' }, + }; + }); + + client = new McpTestClient(); + await client.start(['--server', hub.url]); + }, 60000); + + afterAll(async () => { + await client?.stop(); + await hub.stop(); + }); + + it('reports fresh preview diagnostics and the execution error', async () => { + const result = await client.callTool('get_errors', { project: indexDocId }); + expect(result.isError).not.toBe(true); + + const report = JSON.parse(result.content[0]!.text) as { + files: Array<{ + path: string; + preview?: { stale: boolean; source: string; errors: Array<{ title: string; start_line?: number }>; warnings: unknown[] }; + execution?: { state: string; lastError?: string }; + }>; + note?: string; + }; + expect(report.note).toBeUndefined(); + expect(report.files).toHaveLength(1); + + const entry = report.files[0]!; + expect(entry.path).toBe('index.qmd'); + expect(entry.preview!.stale).toBe(false); + expect(entry.preview!.source).toBe('hub-client-preview'); + expect(entry.preview!.errors).toHaveLength(1); + expect(entry.preview!.errors[0]!.title).toBe('YAML parse error'); + expect(entry.preview!.errors[0]!.start_line).toBe(2); + expect(entry.execution).toEqual({ state: 'error', lastError: 'kernel died' }); + }, 60000); + + it('flips to stale after the agent edits the file', async () => { + const written = await client.callTool('write_file', { + project: indexDocId, + path: 'index.qmd', + content: '---\ntitle: fixed\n---\n', + }); + expect(written.isError).not.toBe(true); + + const result = await client.callTool('get_errors', { + project: indexDocId, + path: 'index.qmd', + }); + expect(result.isError).not.toBe(true); + const report = JSON.parse(result.content[0]!.text) as { + files: Array<{ path: string; preview?: { stale: boolean } }>; + }; + // The published diagnostics still describe the broken content; + // the agent must treat them as saying nothing about its edit. + expect(report.files[0]!.preview!.stale).toBe(true); + }, 60000); +}); From 23ec217a7fb95d6403f61658b2a8b09e86ff9635 Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 18:05:50 -0400 Subject: [PATCH 2/9] =?UTF-8?q?hub-mcp:=20local=20WASM=20renderer=20?= =?UTF-8?q?=E2=80=94=20QuartoHub's=20pipeline=20in=20the=20MCP=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderDiagnostics(files, path) fills the WASM VFS with the project the MCP already holds and calls render_page_in_project — the same wasm-quarto-hub-client module the browser preview runs — returning the structured errors/warnings plus sibling pass-1 failures and the sha256 of exactly the text it rendered. The module lives in a prebundled host (scripts/build-wasm-host.mjs: esbuild with the /src/wasm-js-bridge/* aliases and dart-sass bundled in, since html theme compilation needs it and the embedded bundle has no node_modules), loaded lazily on first use. Tests run the real WASM, no mocks — including a pin that `title: "broken` front matter is a warning, not an error, exactly as the preview reports it. Co-Authored-By: Claude Fable 5 --- ts-packages/quarto-hub-mcp/package.json | 2 +- .../scripts/build-wasm-host.mjs | 48 +++++ .../scripts/wasm-host-entry.mjs | 25 +++ .../quarto-hub-mcp/src/local-render.test.ts | 118 ++++++++++++ .../quarto-hub-mcp/src/local-render.ts | 168 ++++++++++++++++++ ts-packages/quarto-hub-mcp/vitest.config.ts | 11 ++ 6 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs create mode 100644 ts-packages/quarto-hub-mcp/scripts/wasm-host-entry.mjs create mode 100644 ts-packages/quarto-hub-mcp/src/local-render.test.ts create mode 100644 ts-packages/quarto-hub-mcp/src/local-render.ts diff --git a/ts-packages/quarto-hub-mcp/package.json b/ts-packages/quarto-hub-mcp/package.json index 056d17bab..3a9a3d2d0 100644 --- a/ts-packages/quarto-hub-mcp/package.json +++ b/ts-packages/quarto-hub-mcp/package.json @@ -24,7 +24,7 @@ "dist" ], "scripts": { - "build": "tsc && node -e \"import('node:fs').then(fs => fs.chmodSync('dist/index.js', 0o755))\"", + "build": "tsc && node scripts/build-wasm-host.mjs && node -e \"import('node:fs').then(fs => fs.chmodSync('dist/index.js', 0o755))\"", "bundle": "node scripts/bundle.mjs", "typecheck": "tsc --noEmit", "clean": "rm -rf dist dist-bundle", diff --git a/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs new file mode 100644 index 000000000..8c6fb2134 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs @@ -0,0 +1,48 @@ +// Prebundle the WASM host for plain-Node consumers (dist/ and the +// esbuild dist-bundle). Produces: +// dist/wasm-host.mjs — bundled wasm-bindgen JS + bridges +// dist/wasm_quarto_hub_client_bg.wasm — the WASM binary, loaded by the host +// +// The wasm-bindgen JS imports its bridge modules by the Vite-root +// paths hub-client serves them from; the alias plugin maps those to +// the ts-packages/wasm-js-bridge sources. +import * as esbuild from 'esbuild'; +import { copyFile, mkdir } from 'node:fs/promises'; +import * as path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const pkgDir = path.dirname(path.dirname(fileURLToPath(import.meta.url))); +const repoRoot = path.resolve(pkgDir, '../..'); +const wasmPkg = path.join(repoRoot, 'hub-client/wasm-quarto-hub-client'); +const bridgeDir = path.join(repoRoot, 'ts-packages/wasm-js-bridge/src'); + +const bridgeAlias = { + name: 'wasm-bridge-alias', + setup(build) { + build.onResolve({ filter: /^\/src\/wasm-js-bridge\// }, (args) => ({ + path: path.join(bridgeDir, path.basename(args.path)), + })); + build.onResolve({ filter: /^wasm-quarto-hub-client$/ }, () => ({ + path: path.join(wasmPkg, 'wasm_quarto_hub_client.js'), + })); + }, +}; + +await mkdir(path.join(pkgDir, 'dist'), { recursive: true }); +await esbuild.build({ + entryPoints: [path.join(pkgDir, 'scripts/wasm-host-entry.mjs')], + bundle: true, + platform: 'node', + format: 'esm', + outfile: path.join(pkgDir, 'dist/wasm-host.mjs'), + plugins: [bridgeAlias], + // dart-sass is pure JS and the html render's theme compilation needs + // it, so it rides inside the host bundle (the embedded dist-bundle + // has no node_modules to resolve it from at runtime). + logLevel: 'warning', +}); +await copyFile( + path.join(wasmPkg, 'wasm_quarto_hub_client_bg.wasm'), + path.join(pkgDir, 'dist/wasm_quarto_hub_client_bg.wasm'), +); +console.log('wasm-host bundled into dist/'); diff --git a/ts-packages/quarto-hub-mcp/scripts/wasm-host-entry.mjs b/ts-packages/quarto-hub-mcp/scripts/wasm-host-entry.mjs new file mode 100644 index 000000000..e95332e48 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/scripts/wasm-host-entry.mjs @@ -0,0 +1,25 @@ +// Entry point for the prebundled WASM host (dist/wasm-host.mjs). +// +// `build-wasm-host.mjs` bundles this with esbuild, aliasing the +// wasm-bindgen JS's Vite-root-absolute bridge imports +// (/src/wasm-js-bridge/*) to the ts-packages/wasm-js-bridge sources, +// so the same wasm-quarto-hub-client module the browser preview runs +// loads in plain Node. `sass` stays external — diagnostics never +// compile stylesheets, and the bridge only imports it lazily. +import { readFile } from 'node:fs/promises'; +import init from 'wasm-quarto-hub-client'; + +export * from 'wasm-quarto-hub-client'; + +let ready; + +/** + * Initialize the WASM module from the binary shipped next to this + * file. Idempotent; callers await it before any render/vfs call. + */ +export function ensureInit() { + ready ??= readFile(new URL('./wasm_quarto_hub_client_bg.wasm', import.meta.url)).then( + (bytes) => init({ module_or_path: bytes }), + ); + return ready; +} diff --git a/ts-packages/quarto-hub-mcp/src/local-render.test.ts b/ts-packages/quarto-hub-mcp/src/local-render.test.ts new file mode 100644 index 000000000..7d787f1ae --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/local-render.test.ts @@ -0,0 +1,118 @@ +/** + * Tests for the local WASM renderer backing `get_errors` (v2). + * + * These run the REAL wasm-quarto-hub-client module — the same artifact + * the browser preview loads — in Node, via the vitest aliases in + * vitest.config.ts. No mocks: the point of this layer is that the MCP + * generates diagnostics exactly the way QuartoHub does. + */ + +import { createHash } from 'node:crypto'; +import { describe, it, expect } from 'vitest'; +import type { FilePayload } from '@quarto/quarto-sync-client'; +import { renderDiagnostics } from './local-render.js'; + +const BROKEN_YAML = '---\ntitle: "broken\n---\n\n# Hello\n'; +const BROKEN_STRONG = '---\ntitle: ok\n---\n\nHello **unclosed strong\n'; +const CLEAN = '---\ntitle: ok\n---\n\nAll fine here.\n'; +const QUARTO_YML = 'project:\n type: default\n'; + +function project(files: Record): Map { + const m = new Map(); + for (const [path, content] of Object.entries(files)) { + m.set( + path, + typeof content === 'string' + ? { type: 'text', text: content } + : { type: 'binary', data: content, mimeType: 'image/png' }, + ); + } + return m; +} + +describe('renderDiagnostics — real WASM', () => { + it('reports a structured error with line/column for an unclosed strong emphasis', async () => { + const result = await renderDiagnostics( + project({ '_quarto.yml': QUARTO_YML, 'index.qmd': BROKEN_STRONG }), + 'index.qmd', + ); + + expect(result.errors.length).toBeGreaterThan(0); + const err = result.errors[0]!; + expect(err.title).toBe('Unclosed Strong Star Emphasis'); + expect(err.start_line).toBe(5); + expect(typeof err.start_column).toBe('number'); + }, 60000); + + it('reports the unclosed front-matter quote the way QuartoHub does (a warning)', async () => { + // Pinned against the real pipeline: the qmd YAML parser recovers + // from `title: "broken` and emits a warning, not an error — the + // agent must see exactly what the browser preview reports. + const result = await renderDiagnostics( + project({ '_quarto.yml': QUARTO_YML, 'index.qmd': BROKEN_YAML }), + 'index.qmd', + ); + expect(result.errors).toEqual([]); + expect(result.warnings.length).toBeGreaterThan(0); + }, 60000); + + it('returns no errors for a clean document', async () => { + const result = await renderDiagnostics( + project({ '_quarto.yml': QUARTO_YML, 'index.qmd': CLEAN }), + 'index.qmd', + ); + expect(result.errors).toEqual([]); + }, 60000); + + it('attributes a sibling pass-1 failure to the sibling path', async () => { + const result = await renderDiagnostics( + project({ + '_quarto.yml': QUARTO_YML, + 'index.qmd': CLEAN, + 'about.qmd': BROKEN_STRONG, + }), + 'index.qmd', + ); + // The active page renders clean; the broken sibling surfaces as a + // pass-1 failure keyed by its own (VFS-prefix-stripped) path. + expect(result.errors).toEqual([]); + const sibling = result.pass1Failures.find((f) => f.path === 'about.qmd'); + expect(sibling).toBeDefined(); + expect(sibling!.errors.length).toBeGreaterThan(0); + }, 60000); + + it('tolerates binary files in the project', async () => { + const result = await renderDiagnostics( + project({ + '_quarto.yml': QUARTO_YML, + 'index.qmd': CLEAN, + 'logo.png': new Uint8Array([137, 80, 78, 71]), + }), + 'index.qmd', + ); + expect(result.errors).toEqual([]); + }, 60000); + + it('reports the sha256 of exactly the content it rendered', async () => { + const result = await renderDiagnostics( + project({ '_quarto.yml': QUARTO_YML, 'index.qmd': CLEAN }), + 'index.qmd', + ); + const expected = `sha256:${createHash('sha256').update(CLEAN, 'utf8').digest('hex')}`; + expect(result.checkedContentSha256).toBe(expected); + }, 60000); + + it('serializes concurrent renders (VFS is per-instance global state)', async () => { + const a = renderDiagnostics( + project({ '_quarto.yml': QUARTO_YML, 'index.qmd': BROKEN_STRONG }), + 'index.qmd', + ); + const b = renderDiagnostics( + project({ '_quarto.yml': QUARTO_YML, 'index.qmd': CLEAN }), + 'index.qmd', + ); + const [ra, rb] = await Promise.all([a, b]); + expect(ra.errors.length).toBeGreaterThan(0); + expect(rb.errors).toEqual([]); + }, 60000); +}); diff --git a/ts-packages/quarto-hub-mcp/src/local-render.ts b/ts-packages/quarto-hub-mcp/src/local-render.ts new file mode 100644 index 000000000..e577dab44 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/local-render.ts @@ -0,0 +1,168 @@ +/** + * Local WASM renderer backing `get_errors` (v2). + * + * Renders the project files the MCP already holds using the SAME + * wasm-quarto-hub-client module the browser preview runs, and returns + * the diagnostics of exactly what was rendered. No CRDT choreography: + * validity is a function of content, per the v2 plan + * (claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md). + * + * The WASM lives in a prebundled host module (dist/wasm-host.mjs, built + * by scripts/build-wasm-host.mjs) loaded lazily on first use — server + * startup stays instant and projects that never call get_errors never + * pay the ~38 MB init. `QUARTO_HUB_MCP_WASM_HOST` overrides the host + * location (used by vitest, whose import.meta.url points at src/). + */ + +import { createHash } from 'node:crypto'; +import type { FilePayload } from '@quarto/quarto-sync-client'; + +/** Structured diagnostic as produced by the WASM render pipeline. */ +export interface RenderedDiagnostic { + kind: 'error' | 'warning' | 'info' | 'note'; + title: string; + code?: string; + problem?: string; + hints: string[]; + start_line?: number; + start_column?: number; + end_line?: number; + end_column?: number; + details: unknown[]; +} + +export interface SiblingFailure { + /** Project-relative path of the failing sibling (VFS prefix stripped). */ + path: string; + errors: RenderedDiagnostic[]; +} + +export interface LocalRenderResult { + /** `sha256:` of the text that was rendered for `path`. */ + checkedContentSha256: string; + errors: RenderedDiagnostic[]; + warnings: RenderedDiagnostic[]; + /** Pass-1 failures in OTHER project files, keyed by their own path. */ + pass1Failures: SiblingFailure[]; +} + +interface WasmHost { + ensureInit(): Promise; + vfs_clear(): string; + vfs_add_file(path: string, content: string): string; + vfs_add_binary_file(path: string, content: Uint8Array): string; + render_page_in_project(path: string): Promise; +} + +interface WasmRenderResponse { + success: boolean; + error?: string; + diagnostics?: RenderedDiagnostic[]; + warnings?: RenderedDiagnostic[]; + pass1_failures?: Array<{ + source_file: string; + error: string; + diagnostics: RenderedDiagnostic[]; + }>; +} + +let hostPromise: Promise | null = null; + +function loadHost(): Promise { + hostPromise ??= (async () => { + const spec = + process.env['QUARTO_HUB_MCP_WASM_HOST'] ?? new URL('./wasm-host.mjs', import.meta.url).href; + const host = (await import(spec)) as WasmHost; + await host.ensureInit(); + return host; + })(); + return hostPromise; +} + +/** Strip the `/project/` VFS prefix (and any leading slash) from a WASM-reported path. */ +function normalizeProjectPath(p: string): string { + const noVfs = p.startsWith('/project/') ? p.slice('/project/'.length) : p; + return noVfs.startsWith('/') ? noVfs.slice(1) : noVfs; +} + +/** The WASM VFS is instance-global state — renders must not interleave. */ +let renderChain: Promise = Promise.resolve(); + +/** + * Render `path` against a VFS filled with `files` and return the + * structured diagnostics the render produced. Throws only on host + * failures; render errors come back as diagnostics. + */ +export function renderDiagnostics( + files: Map, + path: string, +): Promise { + const run = renderChain.then(async (): Promise => { + const host = await loadHost(); + + const target = files.get(path); + if (!target || target.type !== 'text') { + throw new Error(`Not a text file in this project: ${path}`); + } + + host.vfs_clear(); + for (const [p, payload] of files) { + if (payload.type === 'text') { + host.vfs_add_file(`/project/${p}`, payload.text); + } else { + host.vfs_add_binary_file(`/project/${p}`, payload.data); + } + } + + const response = JSON.parse(await host.render_page_in_project(path)) as WasmRenderResponse; + + const errors: RenderedDiagnostic[] = []; + const warnings: RenderedDiagnostic[] = []; + for (const d of response.diagnostics ?? []) { + (d.kind === 'error' ? errors : warnings).push(d); + } + for (const d of response.warnings ?? []) { + warnings.push(d); + } + + const pass1Failures: SiblingFailure[] = []; + for (const failure of response.pass1_failures ?? []) { + const sibling = normalizeProjectPath(failure.source_file); + if (sibling === path) continue; // active-page failures are in `errors` + pass1Failures.push({ + path: sibling, + errors: + failure.diagnostics.length > 0 + ? failure.diagnostics + : [{ kind: 'error', title: failure.error, hints: [], details: [] }], + }); + } + + // A failed render whose error names the ACTIVE page but produced no + // structured diagnostics still needs to surface (defensive). + if (!response.success && errors.length === 0 && response.error !== undefined) { + const named = normalizeProjectPath( + /Pass 1 failed for (\S+?):/.exec(response.error)?.[1] ?? path, + ); + if (named === path) { + errors.push({ + kind: 'error', + // eslint-disable-next-line no-control-regex + title: response.error.replace(/\[[0-9;]*m/g, '').slice(0, 300), + hints: [], + details: [], + }); + } + } + + return { + checkedContentSha256: `sha256:${createHash('sha256').update(target.text, 'utf8').digest('hex')}`, + errors, + warnings, + pass1Failures, + }; + }); + // Keep the chain alive whether or not this render succeeded. + renderChain = run.catch(() => undefined); + return run; +} diff --git a/ts-packages/quarto-hub-mcp/vitest.config.ts b/ts-packages/quarto-hub-mcp/vitest.config.ts index 697ae2925..4efea0da9 100644 --- a/ts-packages/quarto-hub-mcp/vitest.config.ts +++ b/ts-packages/quarto-hub-mcp/vitest.config.ts @@ -1,7 +1,18 @@ +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; import { defineConfig } from 'vitest/config'; export default defineConfig({ test: { exclude: ['dist/**', 'node_modules/**'], + env: { + // local-render loads the prebundled WASM host next to itself at + // runtime (dist/), but under vitest import.meta.url points into + // src/ — steer it at the build artifact. `npm run build` must + // have run (the live tests already require dist/index.js). + QUARTO_HUB_MCP_WASM_HOST: pathToFileURL( + path.resolve(__dirname, 'dist/wasm-host.mjs'), + ).href, + }, }, }); From 5493577b525bec300284cdfd0a76b2773f044a73 Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 18:09:35 -0400 Subject: [PATCH 3/9] =?UTF-8?q?hub-mcp:=20get=5Ferrors=20validates=20local?= =?UTF-8?q?ly=20=E2=80=94=20render=20on=20demand,=20report=20what=20was=20?= =?UTF-8?q?checked?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tool renders the requested document (or every .qmd, capped at 25) through the local WASM pipeline and reports structured errors/warnings plus the sha256 of exactly the content it checked; sibling pass-1 failures surface under their own paths, and execution errors still come from the captures sidecar (now mirrored into ProjectState via onCapturesChange). No staleness concept: after an edit, calling the tool again validates the new content immediately. Live integration test drives the real server binary + real WASM over stdio against the in-process test hub: broken doc reported at line 5, patch_file fix, immediate clean re-check. Co-Authored-By: Claude Fable 5 --- .../quarto-hub-mcp/src/connection-manager.ts | 25 +- .../src/get-errors-handler.test.ts | 282 +++++++----------- .../src/get-errors-live.test.ts | 130 +++----- .../quarto-hub-mcp/src/hub-mcp.test.ts | 2 + ts-packages/quarto-hub-mcp/src/tools.ts | 121 ++++++++ 5 files changed, 292 insertions(+), 268 deletions(-) diff --git a/ts-packages/quarto-hub-mcp/src/connection-manager.ts b/ts-packages/quarto-hub-mcp/src/connection-manager.ts index 4d353fc90..2ae172f16 100644 --- a/ts-packages/quarto-hub-mcp/src/connection-manager.ts +++ b/ts-packages/quarto-hub-mcp/src/connection-manager.ts @@ -28,6 +28,7 @@ import { createHash } from 'node:crypto'; import { createSyncClient, type AuthRejectionEvidence, + type CaptureRef, type DisconnectOptions, type SyncClient, type SyncClientCallbacks, @@ -114,11 +115,23 @@ interface ChangeWaiter { fire: (payload: FilePayload | null) => void; } +/** + * Latest index-doc sidecar snapshots, mirrored from the sync client's + * `onCapturesChange` callback. Read by the `get_errors` tool for + * execution errors. A mutable holder (rather than fields on + * {@link ProjectState}) because the callbacks are wired before the + * state object exists and the initial fire happens during `connect`. + */ +interface SidecarState { + captures: Record; +} + interface ProjectState { client: SyncClient; files: Map; /** Pending long-poll waiters, keyed implicitly by their `path` field. */ waiters: Set; + sidecars: SidecarState; } /** @@ -266,6 +279,7 @@ export class ConnectionManager { const files = new Map(); const waiters = new Set(); + const sidecars: SidecarState = { captures: {} }; const callbacks: SyncClientCallbacks = { onFileAdded(path: string, file: FilePayload) { files.set(path, file); @@ -285,6 +299,9 @@ export class ConnectionManager { files.delete(path); fireWaiters(waiters, path, null); }, + onCapturesChange(captures) { + sidecars.captures = captures; + }, onError(err: Error) { console.error( `[hub-mcp] Sync error for project ${indexDocId}:`, @@ -307,7 +324,7 @@ export class ConnectionManager { peerTimeoutMs: PEER_TIMEOUT_MS, }); - const state: ProjectState = { client, files, waiters }; + const state: ProjectState = { client, files, waiters, sidecars }; this.projects.set(indexDocId, state); return state; } @@ -370,6 +387,7 @@ export class ConnectionManager { const tempFiles = new Map(); const waiters = new Set(); + const sidecars: SidecarState = { captures: {} }; const callbacks: SyncClientCallbacks = { onFileAdded(path: string, file: FilePayload) { tempFiles.set(path, file); @@ -389,6 +407,9 @@ export class ConnectionManager { tempFiles.delete(path); fireWaiters(waiters, path, null); }, + onCapturesChange(captures) { + sidecars.captures = captures; + }, }; const client = this.syncClientFactory(callbacks); @@ -406,7 +427,7 @@ export class ConnectionManager { peerTimeoutMs: PEER_TIMEOUT_MS, }); - const state: ProjectState = { client, files: tempFiles, waiters }; + const state: ProjectState = { client, files: tempFiles, waiters, sidecars }; this.projects.set(result.indexDocId, state); return { indexDocId: result.indexDocId, files: result.files }; } diff --git a/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts b/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts index 26dfa3da6..a332b3eb0 100644 --- a/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts +++ b/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts @@ -1,42 +1,50 @@ /** - * Handler-level tests for the `get_errors` tool (handleGetErrors). + * Handler-level tests for the `get_errors` tool (v2: local validation). * * Same harness pattern as wait-for-change-handler.test.ts: the REAL - * `registerTools` dispatch runs against a fake ConnectionManager whose - * `connect` returns a fabricated project state. Coverage: - * - * - the no-publisher note when the diagnostics sidecar is empty - * - `stale` false when the stored contentHash matches the current text - * - `stale` true on hash mismatch, missing file, and binary payloads - * - errors/warnings split by diagnostic kind - * - execution errors surfaced from the captures sidecar (state 'error' - * with lastError; 'running' surfaced; 'idle' suppressed) - * - the optional `path` filter + * `registerTools` dispatch runs against a fake ConnectionManager. The + * local renderer is mocked at its module seam — its own behavior is + * covered by local-render.test.ts against the real WASM. */ -import { createHash } from 'node:crypto'; -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; import { CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js'; import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; -import type { FilePayload, CaptureRef, FileDiagnostics } from '@quarto/quarto-sync-client'; +import type { FilePayload, CaptureRef } from '@quarto/quarto-sync-client'; +import type { LocalRenderResult } from './local-render.js'; + +const renderDiagnostics = vi.hoisted(() => vi.fn()); +vi.mock('./local-render.js', () => ({ renderDiagnostics })); + import { registerTools } from './tools.js'; import type { ConnectionManager } from './connection-manager.js'; -function sha256(text: string): string { - return `sha256:${createHash('sha256').update(text, 'utf8').digest('hex')}`; +const ERROR_ITEM = { + kind: 'error' as const, + title: 'Unclosed Strong Star Emphasis', + hints: [], + start_line: 5, + start_column: 24, + details: [], +}; +const WARNING_ITEM = { kind: 'warning' as const, title: 'unknown option', hints: [], details: [] }; + +function cleanResult(overrides: Partial = {}): LocalRenderResult { + return { + checkedContentSha256: 'sha256:abc', + errors: [], + warnings: [], + pass1Failures: [], + ...overrides, + }; } interface FakeStateInit { files?: Record; captures?: Record; - diagnostics?: Record; } -/** - * Register the real tool handlers against a fake manager whose `connect` - * resolves to a state fabricated from `init`. - */ function harness(init: FakeStateInit): { call: (args: Record) => Promise; } { @@ -44,10 +52,7 @@ function harness(init: FakeStateInit): { client: {} as never, files: new Map(Object.entries(init.files ?? {})), waiters: new Set(), - sidecars: { - captures: init.captures ?? {}, - diagnostics: init.diagnostics ?? {}, - }, + sidecars: { captures: init.captures ?? {} }, }; const manager = { async connect(_project: string) { @@ -69,10 +74,9 @@ function harness(init: FakeStateInit): { registerTools(server, manager, false); if (!callToolHandler) throw new Error('CallTool handler was not registered'); - const call = (args: Record) => - callToolHandler!({ params: { name: 'get_errors', arguments: args } }, {}); - - return { call }; + return { + call: (args) => callToolHandler!({ params: { name: 'get_errors', arguments: args } }, {}), + }; } function parse(result: CallToolResult): Record { @@ -81,184 +85,100 @@ function parse(result: CallToolResult): Record { return JSON.parse(block.text) as Record; } -const ERROR_ITEM = { - kind: 'error' as const, - title: 'YAML parse error', - hints: ['close the bracket'], - start_line: 2, - start_column: 8, - details: [], +type FileEntry = { + path: string; + checkedContentSha256?: string; + errors?: unknown[]; + warnings?: unknown[]; + note?: string; + execution?: Record; }; -const WARNING_ITEM = { - kind: 'warning' as const, - title: 'unknown option', - hints: [], - details: [], -}; - -function fileDiagnostics( - content: string, - items: FileDiagnostics['items'], -): FileDiagnostics { - return { - contentHash: sha256(content), - asOf: '2026-07-16T12:00:00.000Z', - source: 'hub-client-preview', - items, - }; -} +beforeEach(() => { + renderDiagnostics.mockReset(); + renderDiagnostics.mockResolvedValue(cleanResult()); +}); -describe('handleGetErrors — no publisher yet', () => { - it('returns a note explaining diagnostics appear when a preview is open', async () => { - const h = harness({ files: { 'index.qmd': { type: 'text', text: '# hi' } } }); - const out = parse(await h.call({ project: 'idx' })); - expect(out.note).toContain('preview'); - expect(out.files).toEqual([]); - }); +describe('handleGetErrors — local validation', () => { + it('renders the requested path and reports its diagnostics + content hash', async () => { + renderDiagnostics.mockResolvedValue( + cleanResult({ errors: [ERROR_ITEM], warnings: [WARNING_ITEM], checkedContentSha256: 'sha256:def' }), + ); + const h = harness({ files: { 'index.qmd': { type: 'text', text: 'x' } } }); - it('still reports execution errors when only the captures sidecar exists', async () => { - const h = harness({ - files: { 'index.qmd': { type: 'text', text: '# hi' } }, - captures: { - 'index.qmd': { captureDocId: 'cap-1', state: 'error', lastError: 'kernel died' }, - }, - }); - const out = parse(await h.call({ project: 'idx' })); - expect(out.note).toContain('preview'); - const files = out.files as Array>; + const out = parse(await h.call({ project: 'idx', path: 'index.qmd' })); + const files = out.files as FileEntry[]; expect(files).toHaveLength(1); expect(files[0].path).toBe('index.qmd'); - expect(files[0].execution).toEqual({ state: 'error', lastError: 'kernel died' }); - }); -}); - -describe('handleGetErrors — staleness', () => { - it('reports stale: false when the stored hash matches the current file text', async () => { - const content = '---\ntitle: [broken\n---\n'; - const h = harness({ - files: { 'index.qmd': { type: 'text', text: content } }, - diagnostics: { 'index.qmd': fileDiagnostics(content, [ERROR_ITEM]) }, - }); - const out = parse(await h.call({ project: 'idx' })); - const files = out.files as Array>; - expect(files).toHaveLength(1); - const preview = files[0].preview as Record; - expect(preview.stale).toBe(false); - expect(preview.asOf).toBe('2026-07-16T12:00:00.000Z'); - expect(preview.source).toBe('hub-client-preview'); - }); - - it('reports stale: true when the file has changed since the render', async () => { - const h = harness({ - files: { 'index.qmd': { type: 'text', text: 'edited since the render' } }, - diagnostics: { 'index.qmd': fileDiagnostics('what was rendered', [ERROR_ITEM]) }, - }); - const out = parse(await h.call({ project: 'idx' })); - const files = out.files as Array>; - expect((files[0].preview as Record).stale).toBe(true); + expect(files[0].checkedContentSha256).toBe('sha256:def'); + expect(files[0].errors).toEqual([ERROR_ITEM]); + expect(files[0].warnings).toEqual([WARNING_ITEM]); + expect(renderDiagnostics).toHaveBeenCalledTimes(1); }); - it('reports stale: true when the file is missing from the project', async () => { + it('renders every .qmd when no path is given, sorted', async () => { const h = harness({ - diagnostics: { 'gone.qmd': fileDiagnostics('old content', [ERROR_ITEM]) }, + files: { + 'b.qmd': { type: 'text', text: 'b' }, + 'a.qmd': { type: 'text', text: 'a' }, + '_quarto.yml': { type: 'text', text: 'project:\n' }, + 'img.png': { type: 'binary', data: new Uint8Array([1]), mimeType: 'image/png' }, + }, }); - const out = parse(await h.call({ project: 'idx' })); - const files = out.files as Array>; - expect((files[0].preview as Record).stale).toBe(true); - }); - it('reports stale: true when the payload is binary', async () => { - const h = harness({ - files: { 'img.png': { type: 'binary', data: new Uint8Array([1]), mimeType: 'image/png' } }, - diagnostics: { 'img.png': fileDiagnostics('old text', [ERROR_ITEM]) }, - }); const out = parse(await h.call({ project: 'idx' })); - const files = out.files as Array>; - expect((files[0].preview as Record).stale).toBe(true); - }); -}); - -describe('handleGetErrors — content shape', () => { - it('splits items into errors and warnings by kind', async () => { - const content = 'x'; - const h = harness({ - files: { 'a.qmd': { type: 'text', text: content } }, - diagnostics: { 'a.qmd': fileDiagnostics(content, [ERROR_ITEM, WARNING_ITEM]) }, - }); - const out = parse(await h.call({ project: 'idx' })); - const preview = (out.files as Array>)[0] - .preview as Record; - expect(preview.errors).toEqual([ERROR_ITEM]); - expect(preview.warnings).toEqual([WARNING_ITEM]); + const files = out.files as FileEntry[]; + expect(files.map((f) => f.path)).toEqual(['a.qmd', 'b.qmd']); + const rendered = renderDiagnostics.mock.calls.map((c) => c[1]); + expect(rendered).toEqual(['a.qmd', 'b.qmd']); }); - it('an empty items array reports a clean render (no errors, no warnings)', async () => { - const content = '# fine'; - const h = harness({ - files: { 'a.qmd': { type: 'text', text: content } }, - diagnostics: { 'a.qmd': fileDiagnostics(content, []) }, - }); - const out = parse(await h.call({ project: 'idx' })); - const preview = (out.files as Array>)[0] - .preview as Record; - expect(preview.errors).toEqual([]); - expect(preview.warnings).toEqual([]); - expect(preview.stale).toBe(false); - expect(out.note).toBeUndefined(); + it('folds a sibling pass-1 failure into the sibling entry', async () => { + renderDiagnostics.mockImplementation(async (_files, path: string) => + path === 'index.qmd' + ? cleanResult({ pass1Failures: [{ path: 'about.qmd', errors: [ERROR_ITEM] }] }) + : cleanResult(), + ); + const h = harness({ files: { 'index.qmd': { type: 'text', text: 'x' } } }); + + const out = parse(await h.call({ project: 'idx', path: 'index.qmd' })); + const files = out.files as FileEntry[]; + const sibling = files.find((f) => f.path === 'about.qmd'); + expect(sibling).toBeDefined(); + expect(sibling!.errors).toEqual([ERROR_ITEM]); }); - it('surfaces a running capture but suppresses idle ones', async () => { - const content = 'x'; + it('surfaces capture execution errors and running state, suppresses idle', async () => { const h = harness({ - files: { - 'running.qmd': { type: 'text', text: content }, - 'idle.qmd': { type: 'text', text: content }, - }, + files: { 'a.qmd': { type: 'text', text: 'x' } }, captures: { - 'running.qmd': { captureDocId: 'cap-r', state: 'running' }, - 'idle.qmd': { captureDocId: 'cap-i', state: 'idle' }, - }, - diagnostics: { - 'running.qmd': fileDiagnostics(content, []), - 'idle.qmd': fileDiagnostics(content, []), + 'a.qmd': { captureDocId: 'c1', state: 'error', lastError: 'kernel died' }, + 'b.qmd': { captureDocId: 'c2', state: 'running' }, + 'c.qmd': { captureDocId: 'c3', state: 'idle' }, }, }); + const out = parse(await h.call({ project: 'idx' })); - const files = out.files as Array>; - const running = files.find((f) => f.path === 'running.qmd')!; - const idle = files.find((f) => f.path === 'idle.qmd')!; - expect(running.execution).toEqual({ state: 'running' }); - expect(idle.execution).toBeUndefined(); + const files = out.files as FileEntry[]; + expect(files.find((f) => f.path === 'a.qmd')!.execution).toEqual({ + state: 'error', + lastError: 'kernel died', + }); + expect(files.find((f) => f.path === 'b.qmd')!.execution).toEqual({ state: 'running' }); + expect(files.find((f) => f.path === 'c.qmd')).toBeUndefined(); }); - it('filters to a single path when `path` is given', async () => { - const content = 'x'; + it('errors clearly when the requested path is not a text file', async () => { const h = harness({ - files: { - 'a.qmd': { type: 'text', text: content }, - 'b.qmd': { type: 'text', text: content }, - }, - diagnostics: { - 'a.qmd': fileDiagnostics(content, [ERROR_ITEM]), - 'b.qmd': fileDiagnostics(content, [WARNING_ITEM]), - }, + files: { 'img.png': { type: 'binary', data: new Uint8Array([1]), mimeType: 'image/png' } }, }); - const out = parse(await h.call({ project: 'idx', path: 'b.qmd' })); - const files = out.files as Array>; - expect(files).toHaveLength(1); - expect(files[0].path).toBe('b.qmd'); + const res = await h.call({ project: 'idx', path: 'img.png' }); + expect(res.isError).toBe(true); }); - it('lists paths sorted and unions diagnostics + capture paths', async () => { - const content = 'x'; - const h = harness({ - files: { 'b.qmd': { type: 'text', text: content } }, - captures: { 'a.qmd': { captureDocId: 'cap', state: 'error', lastError: 'boom' } }, - diagnostics: { 'b.qmd': fileDiagnostics(content, [ERROR_ITEM]) }, - }); - const out = parse(await h.call({ project: 'idx' })); - const files = out.files as Array>; - expect(files.map((f) => f.path)).toEqual(['a.qmd', 'b.qmd']); + it('errors clearly when the requested path is missing', async () => { + const h = harness({ files: {} }); + const res = await h.call({ project: 'idx', path: 'nope.qmd' }); + expect(res.isError).toBe(true); }); }); diff --git a/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts index 96d9e53bd..8640b4263 100644 --- a/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts +++ b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts @@ -1,120 +1,78 @@ /** - * MCP-level integration test for `get_errors` against the in-process - * test hub: the diagnostics sidecar is planted by mutating the project - * index through the hub's own repo handle (standing in for the - * hub-client preview publisher), then a fresh MCP server instance - * reads it back through the real tool surface — including the - * staleness flip after the agent edits the file via `write_file`. + * MCP-level integration test for `get_errors` (v2: local validation) + * against the in-process test hub: the real server binary + * (dist/index.js, which loads the real WASM host) over stdio. * - * Same harness as dangling-entries.test.ts: drives the real server - * binary (dist/index.js) over stdio; no external network. + * Pins the whole agent loop with zero cross-peer choreography: + * create a broken project → get_errors reports the diagnostic for + * exactly that content → patch_file fixes it → get_errors immediately + * reports clean, no waiting on any other peer. */ -import { createHash } from 'node:crypto'; import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import type { DocumentId } from '@automerge/automerge-repo'; -import type { IndexDocument } from '@quarto/quarto-automerge-schema'; import { McpTestClient } from './mcp-test-client.js'; import { startTestHub, type TestHub } from './test-hub.js'; -const BROKEN_CONTENT = '---\ntitle: [broken\n---\n'; +const BROKEN = '---\ntitle: ok\n---\n\nHello **unclosed strong\n'; +const FIXED = '---\ntitle: ok\n---\n\nHello **closed strong**\n'; -function sha256(text: string): string { - return `sha256:${createHash('sha256').update(text, 'utf8').digest('hex')}`; -} - -describe('get_errors at the MCP tool surface (test hub)', () => { +describe('get_errors at the MCP tool surface (test hub, local WASM)', () => { let hub: TestHub; let client: McpTestClient; let indexDocId: string; beforeAll(async () => { hub = await startTestHub(); + client = new McpTestClient(); + await client.start(['--server', hub.url]); - const creator = new McpTestClient(); - await creator.start(['--server', hub.url]); - const created = await creator.callTool('create_project', { - files: [{ path: 'index.qmd', content: BROKEN_CONTENT }], + const created = await client.callTool('create_project', { + files: [ + { path: 'index.qmd', content: BROKEN }, + { path: '_quarto.yml', content: 'project:\n type: default\n' }, + ], }); expect(created.isError).not.toBe(true); - const parsed = JSON.parse(created.content[0]!.text) as { - indexDocId: string; - files: Array<{ path: string; docId: string }>; - }; - indexDocId = parsed.indexDocId; - expect(await hub.hubHasDoc(indexDocId, 8000)).toBe(true); - for (const f of parsed.files) { - expect(await hub.hubHasDoc(f.docId, 8000)).toBe(true); - } - await creator.stop(); - - // Plant the sidecars the way a rendering client would publish them. - const handle = await hub.repo.find(indexDocId as DocumentId); - handle.change((d) => { - d.diagnostics = { - 'index.qmd': { - contentHash: sha256(BROKEN_CONTENT), - asOf: '2026-07-16T12:00:00.000Z', - source: 'hub-client-preview', - items: [ - { - kind: 'error', - title: 'YAML parse error', - hints: ['close the bracket'], - start_line: 2, - start_column: 8, - details: [], - }, - ], - }, - }; - d.captures = { - 'index.qmd': { captureDocId: 'cap-1', state: 'error', lastError: 'kernel died' }, - }; - }); - - client = new McpTestClient(); - await client.start(['--server', hub.url]); - }, 60000); + indexDocId = (JSON.parse(created.content[0]!.text) as { indexDocId: string }).indexDocId; + expect(await hub.hubHasDoc(indexDocId as DocumentId, 8000)).toBe(true); + }, 120000); afterAll(async () => { await client?.stop(); await hub.stop(); }); - it('reports fresh preview diagnostics and the execution error', async () => { - const result = await client.callTool('get_errors', { project: indexDocId }); + it('reports the render diagnostic for the broken document', async () => { + const result = await client.callTool('get_errors', { + project: indexDocId, + path: 'index.qmd', + }); expect(result.isError).not.toBe(true); const report = JSON.parse(result.content[0]!.text) as { files: Array<{ path: string; - preview?: { stale: boolean; source: string; errors: Array<{ title: string; start_line?: number }>; warnings: unknown[] }; - execution?: { state: string; lastError?: string }; + checkedContentSha256?: string; + errors: Array<{ title: string; start_line?: number }>; }>; - note?: string; }; - expect(report.note).toBeUndefined(); - expect(report.files).toHaveLength(1); - - const entry = report.files[0]!; - expect(entry.path).toBe('index.qmd'); - expect(entry.preview!.stale).toBe(false); - expect(entry.preview!.source).toBe('hub-client-preview'); - expect(entry.preview!.errors).toHaveLength(1); - expect(entry.preview!.errors[0]!.title).toBe('YAML parse error'); - expect(entry.preview!.errors[0]!.start_line).toBe(2); - expect(entry.execution).toEqual({ state: 'error', lastError: 'kernel died' }); - }, 60000); + const entry = report.files.find((f) => f.path === 'index.qmd')!; + expect(entry.errors).toHaveLength(1); + expect(entry.errors[0]!.title).toBe('Unclosed Strong Star Emphasis'); + expect(entry.errors[0]!.start_line).toBe(5); + expect(entry.checkedContentSha256).toMatch(/^sha256:[0-9a-f]{64}$/); + }, 120000); - it('flips to stale after the agent edits the file', async () => { - const written = await client.callTool('write_file', { + it('reports clean immediately after the agent fixes the file', async () => { + const patched = await client.callTool('patch_file', { project: indexDocId, path: 'index.qmd', - content: '---\ntitle: fixed\n---\n', + old_string: 'Hello **unclosed strong', + new_string: 'Hello **closed strong**', }); - expect(written.isError).not.toBe(true); + expect(patched.isError).not.toBe(true); const result = await client.callTool('get_errors', { project: indexDocId, @@ -122,10 +80,12 @@ describe('get_errors at the MCP tool surface (test hub)', () => { }); expect(result.isError).not.toBe(true); const report = JSON.parse(result.content[0]!.text) as { - files: Array<{ path: string; preview?: { stale: boolean } }>; + files: Array<{ path: string; errors: unknown[]; warnings: unknown[] }>; }; - // The published diagnostics still describe the broken content; - // the agent must treat them as saying nothing about its edit. - expect(report.files[0]!.preview!.stale).toBe(true); - }, 60000); + const entry = report.files.find((f) => f.path === 'index.qmd')!; + expect(entry.errors).toEqual([]); + // Sanity: the fixed content really is what we think it is. + const read = await client.callTool('read_file', { project: indexDocId, path: 'index.qmd' }); + expect(read.content[0]!.text).toBe(FIXED); + }, 120000); }); diff --git a/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts b/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts index 4371c7d29..ea9be3d0a 100644 --- a/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts +++ b/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts @@ -41,6 +41,7 @@ describe('MCP protocol', () => { 'create_file', 'create_project', 'delete_file', + 'get_errors', 'list_files', 'patch_file', 'read_file', @@ -113,6 +114,7 @@ describe('MCP protocol (read-only mode)', () => { const names = tools.map(t => t.name).sort(); expect(names).toEqual([ 'connect_project', + 'get_errors', 'list_files', 'read_file', 'wait_for_change', diff --git a/ts-packages/quarto-hub-mcp/src/tools.ts b/ts-packages/quarto-hub-mcp/src/tools.ts index 954fec0a9..6b70d5f77 100644 --- a/ts-packages/quarto-hub-mcp/src/tools.ts +++ b/ts-packages/quarto-hub-mcp/src/tools.ts @@ -16,6 +16,7 @@ import { import type { Tool, CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { fileUnavailableMessage, type SyncClient } from '@quarto/quarto-sync-client'; import { ConnectionManager } from './connection-manager.js'; +import { renderDiagnostics, type RenderedDiagnostic } from './local-render.js'; import { AUTH_TOOL_DEFINITIONS, AuthToolsState, @@ -123,6 +124,29 @@ function getReadTools(): Tool[] { }, annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: false }, }, + { + name: 'get_errors', + description: + 'Check a Quarto Hub project for errors by rendering it with the same pipeline ' + + 'the browser preview uses, locally and on demand. Reports structured render ' + + 'errors and warnings (with line/column and hints) for exactly the file content ' + + 'the tool read (`checkedContentSha256` names it), plus engine execution errors ' + + 'recorded by executors. After you edit a file, just call get_errors again — it ' + + 'validates the new content immediately; there is nothing to wait for. Pass ' + + '`path` to check one document; omit it to check every .qmd in the project.', + inputSchema: { + type: 'object', + properties: { + project: { type: 'string', description: PROJECT_PARAM_DESC }, + path: { + type: 'string', + description: 'Optional: check only this file path', + }, + }, + required: ['project'], + }, + annotations: { readOnlyHint: true, destructiveHint: false, idempotentHint: true }, + }, ]; } @@ -325,6 +349,8 @@ async function handleTool( return handleReadFile(args, manager); case 'wait_for_change': return handleWaitForChange(args, manager); + case 'get_errors': + return handleGetErrors(args, manager); case 'write_file': return handleWriteFile(args, manager); case 'patch_file': @@ -413,6 +439,101 @@ async function handleWaitForChange(args: ToolArgs, manager: ConnectionManager): ); } +/** One file's entry in the `get_errors` report. */ +interface FileErrorsEntry { + path: string; + /** `sha256:` of the text this entry's render checked. */ + checkedContentSha256?: string; + errors?: RenderedDiagnostic[]; + warnings?: RenderedDiagnostic[]; + /** Present on entries derived from a sibling's pass-1 failure. */ + note?: string; + execution?: { + state: string; + lastError?: string; + }; +} + +/** Cap on how many documents a no-path call renders (each is a full render). */ +const MAX_CHECKED_DOCUMENTS = 25; + +async function handleGetErrors(args: ToolArgs, manager: ConnectionManager): Promise { + const project = args.project as string; + const pathFilter = typeof args.path === 'string' && args.path !== '' ? args.path : undefined; + const state = await manager.connect(project); + + let targets: string[]; + let capped = false; + if (pathFilter) { + const payload = state.files.get(pathFilter); + if (!payload) { + const ghost = findUnavailable(state.client, pathFilter); + if (ghost) { + return unavailableFileError(pathFilter, ghost.docId); + } + return error(`Error: File not found: ${pathFilter}`); + } + if (payload.type !== 'text') { + return error(`Error: ${pathFilter} is a binary file; only text documents can be checked.`); + } + targets = [pathFilter]; + } else { + targets = [...state.files.keys()] + .filter((p) => p.endsWith('.qmd') && state.files.get(p)!.type === 'text') + .sort(); + if (targets.length > MAX_CHECKED_DOCUMENTS) { + targets = targets.slice(0, MAX_CHECKED_DOCUMENTS); + capped = true; + } + } + + const entries = new Map(); + const entryFor = (p: string): FileErrorsEntry => { + let e = entries.get(p); + if (!e) { + e = { path: p }; + entries.set(p, e); + } + return e; + }; + + for (const path of targets) { + const result = await renderDiagnostics(state.files, path); + const entry = entryFor(path); + entry.checkedContentSha256 = result.checkedContentSha256; + entry.errors = result.errors; + entry.warnings = result.warnings; + // Sibling pass-1 failures surface under the failing file's own path + // (only when that file wasn't/won't be rendered directly). + for (const sibling of result.pass1Failures) { + if (targets.includes(sibling.path)) continue; + const se = entryFor(sibling.path); + if (!se.errors?.length) { + se.errors = sibling.errors; + se.note = `pass-1 failure observed while rendering ${path}`; + } + } + } + + // Execution errors come from the captures sidecar — they happen on + // executors elsewhere and cannot be recomputed locally. + for (const [p, cap] of Object.entries(state.sidecars.captures)) { + if (cap.state === 'error' || cap.state === 'running') { + entryFor(p).execution = { + state: cap.state, + ...(cap.lastError !== undefined ? { lastError: cap.lastError } : {}), + }; + } + } + + const files = [...entries.values()].sort((a, b) => (a.path < b.path ? -1 : 1)); + const report: { project: string; files: FileErrorsEntry[]; note?: string } = { project, files }; + if (capped) { + report.note = `Checked the first ${MAX_CHECKED_DOCUMENTS} .qmd documents; pass a path to check a specific other file.`; + } + return text(JSON.stringify(report, null, 2)); +} + async function handleWriteFile(args: ToolArgs, manager: ConnectionManager): Promise { const project = args.project as string; const path = args.path as string; From 4661109376e71d20b3f41c749e1385256072a087 Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 18:13:06 -0400 Subject: [PATCH 4/9] hub-mcp: ship the WASM render host in dist-bundle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bundle.mjs now builds wasm-host.mjs (bridges + dart-sass inlined) and copies wasm_quarto_hub_client_bg.wasm alongside index.mjs, so the embedded `q2 mcp` and the npx channel can run get_errors' local validation with no node_modules. bundle.test.ts pins the artifacts. Note: the q2 binary now embeds a second copy of this WASM (the preview SPA has its own) — dedupe tracked as follow-up in the v2 plan. Co-Authored-By: Claude Fable 5 --- .../scripts/build-wasm-host.mjs | 46 +++++++++++-------- ts-packages/quarto-hub-mcp/scripts/bundle.mjs | 6 +++ ts-packages/quarto-hub-mcp/src/bundle.test.ts | 6 +++ 3 files changed, 40 insertions(+), 18 deletions(-) diff --git a/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs index 8c6fb2134..6720644a4 100644 --- a/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs +++ b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs @@ -28,21 +28,31 @@ const bridgeAlias = { }, }; -await mkdir(path.join(pkgDir, 'dist'), { recursive: true }); -await esbuild.build({ - entryPoints: [path.join(pkgDir, 'scripts/wasm-host-entry.mjs')], - bundle: true, - platform: 'node', - format: 'esm', - outfile: path.join(pkgDir, 'dist/wasm-host.mjs'), - plugins: [bridgeAlias], - // dart-sass is pure JS and the html render's theme compilation needs - // it, so it rides inside the host bundle (the embedded dist-bundle - // has no node_modules to resolve it from at runtime). - logLevel: 'warning', -}); -await copyFile( - path.join(wasmPkg, 'wasm_quarto_hub_client_bg.wasm'), - path.join(pkgDir, 'dist/wasm_quarto_hub_client_bg.wasm'), -); -console.log('wasm-host bundled into dist/'); +/** + * Build the host bundle + WASM binary into `outDir`. Called with + * dist/ by the package build and with dist-bundle/ by bundle.mjs. + */ +export async function buildWasmHost(outDir) { + await mkdir(outDir, { recursive: true }); + await esbuild.build({ + entryPoints: [path.join(pkgDir, 'scripts/wasm-host-entry.mjs')], + bundle: true, + platform: 'node', + format: 'esm', + outfile: path.join(outDir, 'wasm-host.mjs'), + plugins: [bridgeAlias], + // dart-sass is pure JS and the html render's theme compilation + // needs it, so it rides inside the host bundle (the embedded + // dist-bundle has no node_modules to resolve it from at runtime). + logLevel: 'warning', + }); + await copyFile( + path.join(wasmPkg, 'wasm_quarto_hub_client_bg.wasm'), + path.join(outDir, 'wasm_quarto_hub_client_bg.wasm'), + ); + console.log(`wasm-host bundled into ${path.relative(pkgDir, outDir)}/`); +} + +if (import.meta.url === (await import('node:url')).pathToFileURL(process.argv[1] ?? '').href) { + await buildWasmHost(path.join(pkgDir, 'dist')); +} diff --git a/ts-packages/quarto-hub-mcp/scripts/bundle.mjs b/ts-packages/quarto-hub-mcp/scripts/bundle.mjs index 9a565c70b..76ef3a8c2 100644 --- a/ts-packages/quarto-hub-mcp/scripts/bundle.mjs +++ b/ts-packages/quarto-hub-mcp/scripts/bundle.mjs @@ -43,6 +43,7 @@ import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { parsePlatformList, stageKeyring } from './stage-keyring.mjs'; +import { buildWasmHost } from './build-wasm-host.mjs'; const here = dirname(fileURLToPath(import.meta.url)); const pkgRoot = join(here, '..'); @@ -125,6 +126,11 @@ await esbuild.build({ outfile: join(outDir, 'auth-stream.mjs'), }); +// The WASM render host (`get_errors` local validation): index.mjs +// dynamic-imports ./wasm-host.mjs next to itself at first use, which +// loads ./wasm_quarto_hub_client_bg.wasm next to *itself*. +await buildWasmHost(outDir); + // --- ship the keyring addon as a mini node_modules --------------------- // The staged platform packages must match the **release target's** // users, not the build host: release jobs request explicit platforms diff --git a/ts-packages/quarto-hub-mcp/src/bundle.test.ts b/ts-packages/quarto-hub-mcp/src/bundle.test.ts index 12e199baf..41a16d898 100644 --- a/ts-packages/quarto-hub-mcp/src/bundle.test.ts +++ b/ts-packages/quarto-hub-mcp/src/bundle.test.ts @@ -48,6 +48,12 @@ describe('bundle smoke', () => { it('ships the expected artifacts', () => { const bundleDir = path.join(tmpDir, 'bundle'); expect(fs.existsSync(path.join(bundleDir, 'index.mjs'))).toBe(true); + // get_errors local validation: the WASM host + binary must ride in + // the bundle (index.mjs dynamic-imports ./wasm-host.mjs at first use). + expect(fs.existsSync(path.join(bundleDir, 'wasm-host.mjs'))).toBe(true); + const wasm = path.join(bundleDir, 'wasm_quarto_hub_client_bg.wasm'); + expect(fs.existsSync(wasm)).toBe(true); + expect(fs.statSync(wasm).size).toBeGreaterThan(10_000_000); const info = JSON.parse( fs.readFileSync(path.join(bundleDir, 'build-info.json'), 'utf8'), ) as { gitCommit: string; nodeTarget: string; keyringPackages: string[] }; From 8a82574db94c2f33ddb81a07c4610634748ed0aa Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 18:32:22 -0400 Subject: [PATCH 5/9] hub-mcp: strip ANSI rendered snippets from get_errors output; record e2e MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents get structured fields (code, problem, line/col, sub-details) — the ariadne escape-code blob is token noise. Plan doc records the end-to-end run: broken doc via q2 mcp against a real hub reported Q-2-13 at 5:24 with the checked content hash; patch_file then an immediate clean re-check. Co-Authored-By: Claude Fable 5 --- .../plans/2026-07-28-hub-mcp-get-errors-v2.md | 35 +++++++++++++++++-- .../quarto-hub-mcp/src/local-render.test.ts | 3 ++ .../quarto-hub-mcp/src/local-render.ts | 17 +++++++-- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md index 289aef2aa..a96ab8889 100644 --- a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md +++ b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md @@ -94,9 +94,38 @@ Bundling (two consumers): - [ ] `bundle.test.ts` covers the wasm asset presence ### Phase 4 — verification -- [ ] Package suites green; `cargo xtask verify` legs; e2e against local-prod - hub (create broken project via MCP → get_errors → patch_file → - get_errors clean), recorded here per the end-to-end policy +- [x] Package suites green; e2e recorded below. v2 contains ZERO Rust + changes and does not touch hub-client or the schema/sync packages + (all verified green at their upstream state), so the Rust verify + legs are unaffected; CI covers them on the PR. + +## End-to-end verification record (2026-07-28) + +Throwaway Rust hub (`target/debug/hub --data-dir --port 3105 +--allow-insecure-auth`); real MCP server (`dist/index.js`, which loads +the real WASM host) driven over stdio in a single session: + +1. `create_project` with `index.qmd` containing + `Hello **unclosed strong` → indexDocId `2APRALdSKxe8RbrcF3JckcFnbDQL`. +2. `get_errors { project }` → inspected output: + `errors: [ { kind: "error", title: "Unclosed Strong Star Emphasis", + code: "Q-2-13", problem: "I reached the end of the block before + finding a closing '**' …", start_line: 5, start_column: 24, details: + [ { kind: "info", content: "This is the opening '**' mark.", … } ] } ]` + plus `checkedContentSha256: sha256:4305d4…` naming the exact text + rendered. (The ANSI `rendered` snippet observed in this first run is + stripped from tool output as of the follow-up commit — structured + fields only.) +3. `patch_file` closing the emphasis → `get_errors { project, path }` + immediately returned `errors: [], warnings: []` with the new + `checkedContentSha256: sha256:7aef66…`. No polling, no other peer. + +Also verified via the committed integration test +`src/get-errors-live.test.ts` (real server binary + in-process test +hub + real WASM: same loop), and `bundle.test.ts` pins that +dist-bundle ships `wasm-host.mjs` + the `.wasm`; the embedded `q2 mcp` +bundle rebuilt and `--launcher-info` confirmed 15 bundle files at the +branch commit. ## Carried over from v1 (independent of architecture) - [x] `scripts/local-prod-server.mjs`: WS proxy no longer crashes on client diff --git a/ts-packages/quarto-hub-mcp/src/local-render.test.ts b/ts-packages/quarto-hub-mcp/src/local-render.test.ts index 7d787f1ae..045c9ad70 100644 --- a/ts-packages/quarto-hub-mcp/src/local-render.test.ts +++ b/ts-packages/quarto-hub-mcp/src/local-render.test.ts @@ -42,6 +42,9 @@ describe('renderDiagnostics — real WASM', () => { expect(err.title).toBe('Unclosed Strong Star Emphasis'); expect(err.start_line).toBe(5); expect(typeof err.start_column).toBe('number'); + // The ANSI `rendered` snippet is stripped: agents get structured + // fields + the file content; escape codes are token noise. + expect(Object.keys(err)).not.toContain('rendered'); }, 60000); it('reports the unclosed front-matter quote the way QuartoHub does (a warning)', async () => { diff --git a/ts-packages/quarto-hub-mcp/src/local-render.ts b/ts-packages/quarto-hub-mcp/src/local-render.ts index e577dab44..8a1e1af72 100644 --- a/ts-packages/quarto-hub-mcp/src/local-render.ts +++ b/ts-packages/quarto-hub-mcp/src/local-render.ts @@ -116,13 +116,24 @@ export function renderDiagnostics( const response = JSON.parse(await host.render_page_in_project(path)) as WasmRenderResponse; + // Strip fields agents don't need: the ANSI `rendered` snippet is + // escape-code noise (they have line/col + the file content), and + // `$schema` is wire ceremony. + const clean = (d: RenderedDiagnostic): RenderedDiagnostic => { + const { rendered: _r, $schema: _s, ...rest } = d as RenderedDiagnostic & { + rendered?: string; + $schema?: string; + }; + return rest; + }; + const errors: RenderedDiagnostic[] = []; const warnings: RenderedDiagnostic[] = []; for (const d of response.diagnostics ?? []) { - (d.kind === 'error' ? errors : warnings).push(d); + (d.kind === 'error' ? errors : warnings).push(clean(d)); } for (const d of response.warnings ?? []) { - warnings.push(d); + warnings.push(clean(d)); } const pass1Failures: SiblingFailure[] = []; @@ -133,7 +144,7 @@ export function renderDiagnostics( path: sibling, errors: failure.diagnostics.length > 0 - ? failure.diagnostics + ? failure.diagnostics.map(clean) : [{ kind: 'error', title: failure.error, hints: [], details: [] }], }); } From 6223ada0b12fa06d91b4301260a9d59f5a27650c Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 18:49:13 -0400 Subject: [PATCH 6/9] hub-mcp: ponytail review fixes for local-render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shrink the speculative failed-render fallback to a plain correctness guard (a failed render must never read as clean) — which also fixes its ANSI-strip regex, previously missing the ESC byte. Tidy the CLI-main check in build-wasm-host.mjs to reuse the existing node:url import. Co-Authored-By: Claude Fable 5 --- ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs index 6720644a4..37fc3dde3 100644 --- a/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs +++ b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs @@ -9,7 +9,7 @@ import * as esbuild from 'esbuild'; import { copyFile, mkdir } from 'node:fs/promises'; import * as path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; const pkgDir = path.dirname(path.dirname(fileURLToPath(import.meta.url))); const repoRoot = path.resolve(pkgDir, '../..'); @@ -53,6 +53,6 @@ export async function buildWasmHost(outDir) { console.log(`wasm-host bundled into ${path.relative(pkgDir, outDir)}/`); } -if (import.meta.url === (await import('node:url')).pathToFileURL(process.argv[1] ?? '').href) { +if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { await buildWasmHost(path.join(pkgDir, 'dist')); } From 9ceb814c982f0c8c09c27c02d0465c51324dc986 Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 28 Jul 2026 19:01:45 -0400 Subject: [PATCH 7/9] =?UTF-8?q?hub-mcp:=20fix-errors=20MCP=20prompt=20?= =?UTF-8?q?=E2=80=94=20one-command=20entry=20into=20the=20fix=20loop?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clients surface it as a slash command (Claude Code: /mcp__quarto-hub__fix-errors [path]). The prompt expands to loop instructions — get_errors, minimal patch_file fixes at the reported line/column, re-check until clean, report — and the LLM does the fixing; the server stays a set of primitives. Protocol round-trip test pins the prompts capability declaration. Co-Authored-By: Claude Fable 5 --- .../quarto-hub-mcp/src/hub-mcp.test.ts | 9 +++ ts-packages/quarto-hub-mcp/src/index.ts | 3 + .../quarto-hub-mcp/src/mcp-test-client.ts | 27 +++++++ .../quarto-hub-mcp/src/prompts.test.ts | 80 +++++++++++++++++++ ts-packages/quarto-hub-mcp/src/prompts.ts | 78 ++++++++++++++++++ 5 files changed, 197 insertions(+) create mode 100644 ts-packages/quarto-hub-mcp/src/prompts.test.ts create mode 100644 ts-packages/quarto-hub-mcp/src/prompts.ts diff --git a/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts b/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts index ea9be3d0a..e94cb6bec 100644 --- a/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts +++ b/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts @@ -81,6 +81,15 @@ describe('MCP protocol', () => { }); }); + it('lists the fix-errors prompt and expands it over the protocol', async () => { + const prompts = await client.listPrompts(); + expect(prompts.map((p) => p.name)).toContain('fix-errors'); + + const res = await client.getPrompt('fix-errors', { project: 'automerge:xyz' }); + expect(res.messages[0]!.content.text).toContain('automerge:xyz'); + expect(res.messages[0]!.content.text).toContain('get_errors'); + }); + // A share URL whose server= names a hub other than this server's configured // one (--server wss://dummy.example.com) must error *before* connecting, so no // network is needed here. (bd-m4slev7a) diff --git a/ts-packages/quarto-hub-mcp/src/index.ts b/ts-packages/quarto-hub-mcp/src/index.ts index 1689f8c11..abc920ff6 100644 --- a/ts-packages/quarto-hub-mcp/src/index.ts +++ b/ts-packages/quarto-hub-mcp/src/index.ts @@ -33,6 +33,7 @@ import { setSyncLogger } from '@quarto/quarto-sync-client'; import { ConnectionManager } from './connection-manager.js'; import { registerTools } from './tools.js'; +import { registerPrompts } from './prompts.js'; import { AuthToolsState } from './auth/auth-tools.js'; import { CredentialStore } from './auth/credential-store.js'; import { @@ -235,6 +236,7 @@ async function main(): Promise { { capabilities: { tools: {}, + prompts: {}, }, instructions: 'Tools operate on a project identified by its automerge index document ID. ' + @@ -265,6 +267,7 @@ async function main(): Promise { : undefined; registerTools(server, manager, readOnly, authToolsState); + registerPrompts(server); const transport = new StdioServerTransport(); await server.connect(transport); diff --git a/ts-packages/quarto-hub-mcp/src/mcp-test-client.ts b/ts-packages/quarto-hub-mcp/src/mcp-test-client.ts index 056febbb4..6e477361a 100644 --- a/ts-packages/quarto-hub-mcp/src/mcp-test-client.ts +++ b/ts-packages/quarto-hub-mcp/src/mcp-test-client.ts @@ -227,6 +227,33 @@ export class McpTestClient { return result.tools; } + /** + * List all available prompts. + */ + async listPrompts(): Promise> { + const response = await this.sendRequest('prompts/list'); + if (response.error) { + throw new Error(`MCP error: ${response.error.message}`); + } + return (response.result as { prompts: Array<{ name: string }> }).prompts; + } + + /** + * Get a prompt with arguments filled in. + */ + async getPrompt( + name: string, + args: Record, + ): Promise<{ messages: Array<{ role: string; content: { type: string; text: string } }> }> { + const response = await this.sendRequest('prompts/get', { name, arguments: args }); + if (response.error) { + throw new Error(`MCP error: ${response.error.message}`); + } + return response.result as { + messages: Array<{ role: string; content: { type: string; text: string } }>; + }; + } + // ---- Internal ---- private parseResponses(): void { diff --git a/ts-packages/quarto-hub-mcp/src/prompts.test.ts b/ts-packages/quarto-hub-mcp/src/prompts.test.ts new file mode 100644 index 000000000..acb9f45f8 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/prompts.test.ts @@ -0,0 +1,80 @@ +/** + * Tests for the `fix-errors` MCP prompt — the one-command entry into + * the agent fix loop. The prompt only instructs; the LLM does the + * fixing with the existing tools (get_errors, read_file, patch_file). + */ + +import { describe, it, expect } from 'vitest'; +import { + ListPromptsRequestSchema, + GetPromptRequestSchema, +} from '@modelcontextprotocol/sdk/types.js'; +import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import { registerPrompts } from './prompts.js'; + +type Handler = (req: { params?: Record }) => Promise>; + +function harness(): { list: Handler; get: Handler } { + let list: Handler | undefined; + let get: Handler | undefined; + const server = { + setRequestHandler(schema: unknown, cb: unknown) { + if (schema === ListPromptsRequestSchema) list = cb as Handler; + if (schema === GetPromptRequestSchema) get = cb as Handler; + }, + } as unknown as Server; + registerPrompts(server); + if (!list || !get) throw new Error('prompt handlers were not registered'); + return { list, get }; +} + +describe('fix-errors prompt', () => { + it('is listed with a required project argument and optional path', async () => { + const { list } = harness(); + const res = (await list({})) as { + prompts: Array<{ name: string; arguments?: Array<{ name: string; required?: boolean }> }>; + }; + const p = res.prompts.find((x) => x.name === 'fix-errors'); + expect(p).toBeDefined(); + expect(p!.arguments).toEqual([ + expect.objectContaining({ name: 'project', required: true }), + expect.objectContaining({ name: 'path', required: false }), + ]); + }); + + it('expands to loop instructions naming the project and the tools', async () => { + const { get } = harness(); + const res = (await get({ + params: { name: 'fix-errors', arguments: { project: 'automerge:abc123' } }, + })) as { messages: Array<{ role: string; content: { type: string; text: string } }> }; + + expect(res.messages).toHaveLength(1); + expect(res.messages[0]!.role).toBe('user'); + const text = res.messages[0]!.content.text; + expect(text).toContain('automerge:abc123'); + expect(text).toContain('get_errors'); + expect(text).toContain('patch_file'); + expect(text).toMatch(/until/i); + expect(text).toMatch(/minimal/i); + }); + + it('scopes the instructions to a single file when path is given', async () => { + const { get } = harness(); + const res = (await get({ + params: { name: 'fix-errors', arguments: { project: 'abc', path: 'chapter2.qmd' } }, + })) as { messages: Array<{ content: { text: string } }> }; + expect(res.messages[0]!.content.text).toContain('chapter2.qmd'); + }); + + it('rejects an unknown prompt name', async () => { + const { get } = harness(); + await expect(get({ params: { name: 'nope' } })).rejects.toThrow(/unknown prompt/i); + }); + + it('rejects a missing project argument', async () => { + const { get } = harness(); + await expect(get({ params: { name: 'fix-errors', arguments: {} } })).rejects.toThrow( + /project/i, + ); + }); +}); diff --git a/ts-packages/quarto-hub-mcp/src/prompts.ts b/ts-packages/quarto-hub-mcp/src/prompts.ts new file mode 100644 index 000000000..3a3414c4a --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/prompts.ts @@ -0,0 +1,78 @@ +/** + * MCP prompts — named prompt templates clients surface as slash + * commands (Claude Code shows this one as /mcp__quarto-hub__fix-errors). + * + * A prompt only instructs; the LLM does the fixing with the existing + * tools. This is deliberately NOT a `fix_errors` tool: fixing requires + * judgment (read the file, pick the minimal edit), which is the + * calling agent's job — tools stay primitives. + */ + +import { + ListPromptsRequestSchema, + GetPromptRequestSchema, +} from '@modelcontextprotocol/sdk/types.js'; +import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; + +const FIX_ERRORS = { + name: 'fix-errors', + description: + 'Find and fix the render errors in a Quarto Hub project: checks with ' + + 'get_errors, applies minimal fixes with patch_file, and re-checks until clean.', + arguments: [ + { + name: 'project', + description: "The project's automerge index document ID, or a quarto-hub.com share URL", + required: true, + }, + { + name: 'path', + description: 'Optional: fix only this file', + required: false, + }, + ], +}; + +function fixErrorsText(project: string, path?: string): string { + const scope = path ? ` with path "${path}"` : ''; + const target = path ? `the file ${path}` : 'every affected file'; + return [ + `Fix the render errors in Quarto Hub project ${project}.`, + '', + `1. Call get_errors with project "${project}"${scope} to see the current errors and warnings.`, + `2. For ${target}: read_file it, then apply the smallest fix for each error with patch_file. ` + + 'Diagnostics carry line/column, error codes, and hints — fix the reported problem and ' + + "preserve the author's content and intent; never rewrite beyond the minimal change.", + '3. Call get_errors again and repeat until `errors` is empty for every file you touched.', + '4. Leave warnings alone unless they are trivially part of the same fix.', + '5. Report each fix: file, line, what was wrong, and what you changed.', + ].join('\n'); +} + +/** Register the prompt handlers on the MCP server. */ +export function registerPrompts(server: Server): void { + server.setRequestHandler(ListPromptsRequestSchema, async () => ({ + prompts: [FIX_ERRORS], + })); + + server.setRequestHandler(GetPromptRequestSchema, async (request) => { + const { name, arguments: args } = request.params; + if (name !== FIX_ERRORS.name) { + throw new Error(`Unknown prompt: ${name}`); + } + const project = args?.['project']; + if (typeof project !== 'string' || project === '') { + throw new Error("The 'project' argument is required"); + } + const path = typeof args?.['path'] === 'string' && args['path'] !== '' ? args['path'] : undefined; + return { + description: FIX_ERRORS.description, + messages: [ + { + role: 'user' as const, + content: { type: 'text' as const, text: fixErrorsText(project, path) }, + }, + ], + }; + }); +} From 7b17531bc755b5aabadf518f93d7789e8a92ea5f Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Mon, 3 Aug 2026 13:40:24 -0400 Subject: [PATCH 8/9] hub-mcp: write tools render-check the .qmd content they just wrote write_file/patch_file/create_file on a .qmd now stage the new text over the project file map, render it locally, and append the outcome to the tool response (clean / error list / check-unavailable note). Error visibility becomes part of completing an update instead of a separate call the agent must remember; the fix-errors prompt now leans on the in-response check with one confirming get_errors at the end. Co-Authored-By: Claude Fable 5 --- .../plans/2026-07-28-hub-mcp-get-errors-v2.md | 20 ++ .../src/get-errors-live.test.ts | 2 + ts-packages/quarto-hub-mcp/src/prompts.ts | 4 +- ts-packages/quarto-hub-mcp/src/tools.ts | 59 ++++- .../src/write-render-check.test.ts | 202 ++++++++++++++++++ 5 files changed, 278 insertions(+), 9 deletions(-) create mode 100644 ts-packages/quarto-hub-mcp/src/write-render-check.test.ts diff --git a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md index a96ab8889..0de40fe90 100644 --- a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md +++ b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md @@ -99,6 +99,26 @@ Bundling (two consumers): (all verified green at their upstream state), so the Rust verify legs are unaffected; CI covers them on the PR. +## Follow-up: writes render-check their own content (2026-08-03) + +User request after the first production fix loop: error checking should be +part of completing a set of updates, not a separate call the agent must +remember. Since validity = f(content) and the renderer is in-process, the +write tools now do it themselves: + +- `write_file` / `patch_file` / `create_file` on a `.qmd` stage the new + text over the current file map, render it, and append the result to the + tool response: `Render check: clean.` (with warning count when nonzero), + or the structured error list when the new content is broken. +- Non-`.qmd` writes are unchanged; a check that cannot run degrades to + `Render check unavailable (…); call get_errors to verify` and never + fails the write (`renderCheckSuffix` in `src/tools.ts`). +- The `fix-errors` prompt now points the loop at the in-response check, + with one final `get_errors` to confirm. +- Tests: `src/write-render-check.test.ts` (7, handler-level, renderer + mocked at the module seam, fail-first verified); `get-errors-live.test.ts` + extended to pin `Render check: clean` in the real-binary patch response. + ## End-to-end verification record (2026-07-28) Throwaway Rust hub (`target/debug/hub --data-dir --port 3105 diff --git a/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts index 8640b4263..51120eaf8 100644 --- a/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts +++ b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts @@ -73,6 +73,8 @@ describe('get_errors at the MCP tool surface (test hub, local WASM)', () => { new_string: 'Hello **closed strong**', }); expect(patched.isError).not.toBe(true); + // The write itself carries a render check of the new content. + expect(patched.content[0]!.text).toMatch(/Render check: clean/); const result = await client.callTool('get_errors', { project: indexDocId, diff --git a/ts-packages/quarto-hub-mcp/src/prompts.ts b/ts-packages/quarto-hub-mcp/src/prompts.ts index 3a3414c4a..bb04baed4 100644 --- a/ts-packages/quarto-hub-mcp/src/prompts.ts +++ b/ts-packages/quarto-hub-mcp/src/prompts.ts @@ -43,7 +43,9 @@ function fixErrorsText(project: string, path?: string): string { `2. For ${target}: read_file it, then apply the smallest fix for each error with patch_file. ` + 'Diagnostics carry line/column, error codes, and hints — fix the reported problem and ' + "preserve the author's content and intent; never rewrite beyond the minimal change.", - '3. Call get_errors again and repeat until `errors` is empty for every file you touched.', + '3. Each patch_file/write_file response includes a render check of the new content — ' + + 'repeat the fix step until it reports clean, then call get_errors once at the end to ' + + 'confirm `errors` is empty for every file you touched.', '4. Leave warnings alone unless they are trivially part of the same fix.', '5. Report each fix: file, line, what was wrong, and what you changed.', ].join('\n'); diff --git a/ts-packages/quarto-hub-mcp/src/tools.ts b/ts-packages/quarto-hub-mcp/src/tools.ts index 6b70d5f77..330539474 100644 --- a/ts-packages/quarto-hub-mcp/src/tools.ts +++ b/ts-packages/quarto-hub-mcp/src/tools.ts @@ -14,7 +14,7 @@ import { ListToolsRequestSchema, } from '@modelcontextprotocol/sdk/types.js'; import type { Tool, CallToolResult } from '@modelcontextprotocol/sdk/types.js'; -import { fileUnavailableMessage, type SyncClient } from '@quarto/quarto-sync-client'; +import { fileUnavailableMessage, type FilePayload, type SyncClient } from '@quarto/quarto-sync-client'; import { ConnectionManager } from './connection-manager.js'; import { renderDiagnostics, type RenderedDiagnostic } from './local-render.js'; import { @@ -154,7 +154,10 @@ function getWriteTools(): Tool[] { return [ { name: 'write_file', - description: 'Replace the entire content of a text file in a Quarto Hub project. Creates the file if it does not exist.', + description: + 'Replace the entire content of a text file in a Quarto Hub project. Creates the file ' + + 'if it does not exist. Writes to .qmd documents automatically render-check the new ' + + 'content and report any errors in the response — fix them before moving on.', inputSchema: { type: 'object', properties: { @@ -168,7 +171,11 @@ function getWriteTools(): Tool[] { }, { name: 'patch_file', - description: 'Apply a targeted edit to a text file by replacing a specific string. More context-efficient than write_file for small changes to large files.', + description: + 'Apply a targeted edit to a text file by replacing a specific string. More ' + + 'context-efficient than write_file for small changes to large files. Edits to .qmd ' + + 'documents automatically render-check the new content and report any errors in the ' + + 'response — fix them before moving on.', inputSchema: { type: 'object', properties: { @@ -183,7 +190,9 @@ function getWriteTools(): Tool[] { }, { name: 'create_file', - description: 'Create a new text file in a Quarto Hub project.', + description: + 'Create a new text file in a Quarto Hub project. New .qmd documents are automatically ' + + 'render-checked; any errors in the initial content are reported in the response.', inputSchema: { type: 'object', properties: { @@ -534,6 +543,40 @@ async function handleGetErrors(args: ToolArgs, manager: ConnectionManager): Prom return text(JSON.stringify(report, null, 2)); } +/** + * Render-check the content a write tool just committed and return a + * suffix for the tool response. Validity is a function of content, so + * the check stages the new text over the current file map rather than + * waiting for the CRDT callback to land. Never fails the write: a + * check that cannot run degrades to a pointer at get_errors. + */ +async function renderCheckSuffix( + files: Map, + path: string, + newText: string, +): Promise { + if (!path.endsWith('.qmd')) return ''; + try { + const staged = new Map(files); + staged.set(path, { type: 'text', text: newText }); + const result = await renderDiagnostics(staged, path); + if (result.errors.length > 0) { + const n = result.errors.length; + return ( + `\nRender check: ${n} error${n === 1 ? '' : 's'} in ${path}:\n` + + JSON.stringify(result.errors, null, 2) + ); + } + const w = result.warnings.length; + return w > 0 + ? `\nRender check: clean (${w} warning${w === 1 ? '' : 's'}; call get_errors to see them).` + : '\nRender check: clean.'; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + return `\nRender check unavailable (${msg}); call get_errors to verify.`; + } +} + async function handleWriteFile(args: ToolArgs, manager: ConnectionManager): Promise { const project = args.project as string; const path = args.path as string; @@ -550,14 +593,14 @@ async function handleWriteFile(args: ToolArgs, manager: ConnectionManager): Prom return unavailableFileError(path, ghost.docId); } await state.client.createFile(path, content); - return text(`Created ${path}`); + return text(`Created ${path}` + (await renderCheckSuffix(state.files, path, content))); } if (existing.type === 'binary') { return error(`Error: ${path} is a binary file. Cannot write text content to it.`); } state.client.updateFileContent(path, content); - return text(`Updated ${path}`); + return text(`Updated ${path}` + (await renderCheckSuffix(state.files, path, content))); } async function handlePatchFile(args: ToolArgs, manager: ConnectionManager): Promise { @@ -596,7 +639,7 @@ async function handlePatchFile(args: ToolArgs, manager: ConnectionManager): Prom currentContent.slice(index + oldString.length); state.client.updateFileContent(path, newContent); - return text(`Patched ${path}`); + return text(`Patched ${path}` + (await renderCheckSuffix(state.files, path, newContent))); } async function handleCreateFile(args: ToolArgs, manager: ConnectionManager): Promise { @@ -615,7 +658,7 @@ async function handleCreateFile(args: ToolArgs, manager: ConnectionManager): Pro } await state.client.createFile(path, content); - return text(`Created ${path}`); + return text(`Created ${path}` + (await renderCheckSuffix(state.files, path, content))); } async function handleDeleteFile(args: ToolArgs, manager: ConnectionManager): Promise { diff --git a/ts-packages/quarto-hub-mcp/src/write-render-check.test.ts b/ts-packages/quarto-hub-mcp/src/write-render-check.test.ts new file mode 100644 index 000000000..24e9addef --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/write-render-check.test.ts @@ -0,0 +1,202 @@ +/** + * Write tools auto-check the content they just wrote: after write_file / + * patch_file / create_file touches a .qmd, the response carries a render + * check of the NEW content so the agent sees immediately whether the + * edit broke (or fixed) the document — no separate get_errors call + * needed to close a batch of updates. + * + * Same harness pattern as get-errors-handler.test.ts: real + * `registerTools` dispatch, fake ConnectionManager, renderer mocked at + * the module seam (its behavior is covered by local-render.test.ts). + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { CallToolRequestSchema } from '@modelcontextprotocol/sdk/types.js'; +import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; +import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; +import type { FilePayload } from '@quarto/quarto-sync-client'; +import type { LocalRenderResult } from './local-render.js'; + +const renderDiagnostics = vi.hoisted(() => vi.fn()); +vi.mock('./local-render.js', () => ({ renderDiagnostics })); + +import { registerTools } from './tools.js'; +import type { ConnectionManager } from './connection-manager.js'; + +const ERROR_ITEM = { + kind: 'error' as const, + title: 'Unclosed Strong Star Emphasis', + hints: [], + start_line: 16, + start_column: 50, + details: [], +}; +const WARNING_ITEM = { kind: 'warning' as const, title: 'raw HTML', hints: [], details: [] }; + +function cleanResult(overrides: Partial = {}): LocalRenderResult { + return { + checkedContentSha256: 'sha256:abc', + errors: [], + warnings: [], + pass1Failures: [], + ...overrides, + }; +} + +function harness(files: Record): { + call: (name: string, args: Record) => Promise; + files: Map; +} { + const fileMap = new Map(Object.entries(files)); + const client = { + getUnavailableFiles: () => [], + updateFileContent: (path: string, content: string) => { + fileMap.set(path, { type: 'text', text: content }); + }, + createFile: async (path: string, content: string) => { + fileMap.set(path, { type: 'text', text: content }); + }, + }; + const state = { + client: client as never, + files: fileMap, + waiters: new Set(), + sidecars: { captures: {} }, + }; + const manager = { + async connect(_project: string) { + return state; + }, + } as unknown as ConnectionManager; + + let callToolHandler: + | ((req: { params: { name: string; arguments?: Record } }, extra: unknown) => Promise) + | undefined; + const server = { + setRequestHandler(schema: unknown, cb: unknown) { + if (schema === CallToolRequestSchema) { + callToolHandler = cb as typeof callToolHandler; + } + }, + } as unknown as Server; + + registerTools(server, manager, false); + if (!callToolHandler) throw new Error('CallTool handler was not registered'); + + return { + call: (name, args) => callToolHandler!({ params: { name, arguments: args } }, {}), + files: fileMap, + }; +} + +function textOf(result: CallToolResult): string { + const block = result.content[0]; + if (block.type !== 'text') throw new Error('expected a text result block'); + return block.text; +} + +beforeEach(() => { + renderDiagnostics.mockReset(); + renderDiagnostics.mockResolvedValue(cleanResult()); +}); + +describe('write tools render-check the new .qmd content', () => { + it('patch_file reports a clean render check for the patched content', async () => { + const h = harness({ 'a.qmd': { type: 'text', text: 'Hello **world**\n' } }); + + const res = await h.call('patch_file', { + project: 'idx', + path: 'a.qmd', + old_string: 'world', + new_string: 'there', + }); + + const out = textOf(res); + expect(out).toContain('Patched a.qmd'); + expect(out).toMatch(/render check: clean/i); + // The check ran against the NEW content, not the pre-edit content. + expect(renderDiagnostics).toHaveBeenCalledTimes(1); + const [checkedFiles, checkedPath] = renderDiagnostics.mock.calls[0] as [ + Map, + string, + ]; + expect(checkedPath).toBe('a.qmd'); + const payload = checkedFiles.get('a.qmd'); + expect(payload?.type === 'text' && payload.text).toBe('Hello **there**\n'); + }); + + it('patch_file reports the errors the new content renders with', async () => { + renderDiagnostics.mockResolvedValue(cleanResult({ errors: [ERROR_ITEM] })); + const h = harness({ 'a.qmd': { type: 'text', text: 'fine\n' } }); + + const res = await h.call('patch_file', { + project: 'idx', + path: 'a.qmd', + old_string: 'fine', + new_string: '**broken', + }); + + const out = textOf(res); + expect(out).toContain('Patched a.qmd'); + expect(out).toMatch(/render check: 1 error/i); + expect(out).toContain('Unclosed Strong Star Emphasis'); + expect(res.isError).not.toBe(true); // the write itself succeeded + }); + + it('write_file (update) render-checks the replacement content', async () => { + const h = harness({ 'a.qmd': { type: 'text', text: 'old\n' } }); + + const res = await h.call('write_file', { project: 'idx', path: 'a.qmd', content: 'new\n' }); + + expect(textOf(res)).toContain('Updated a.qmd'); + expect(textOf(res)).toMatch(/render check: clean/i); + const [checkedFiles] = renderDiagnostics.mock.calls[0] as [Map, string]; + const payload = checkedFiles.get('a.qmd'); + expect(payload?.type === 'text' && payload.text).toBe('new\n'); + }); + + it('write_file (create) and create_file render-check the initial content', async () => { + const h = harness({}); + const created = await h.call('write_file', { project: 'idx', path: 'new.qmd', content: 'x\n' }); + expect(textOf(created)).toContain('Created new.qmd'); + expect(textOf(created)).toMatch(/render check: clean/i); + + const h2 = harness({}); + const created2 = await h2.call('create_file', { project: 'idx', path: 'n2.qmd', content: 'y\n' }); + expect(textOf(created2)).toContain('Created n2.qmd'); + expect(textOf(created2)).toMatch(/render check: clean/i); + }); + + it('mentions warning count on a clean check but does not dump warnings', async () => { + renderDiagnostics.mockResolvedValue(cleanResult({ warnings: [WARNING_ITEM, WARNING_ITEM] })); + const h = harness({ 'a.qmd': { type: 'text', text: 'x\n' } }); + + const res = await h.call('write_file', { project: 'idx', path: 'a.qmd', content: 'y\n' }); + + const out = textOf(res); + expect(out).toMatch(/render check: clean \(2 warnings/i); + expect(out).not.toContain('raw HTML'); + }); + + it('does not render-check non-qmd writes', async () => { + const h = harness({ '_quarto.yml': { type: 'text', text: 'project:\n' } }); + + const res = await h.call('write_file', { project: 'idx', path: '_quarto.yml', content: 'x\n' }); + + expect(textOf(res)).toBe('Updated _quarto.yml'); + expect(renderDiagnostics).not.toHaveBeenCalled(); + }); + + it('a failed render check never fails the write', async () => { + renderDiagnostics.mockRejectedValue(new Error('wasm exploded')); + const h = harness({ 'a.qmd': { type: 'text', text: 'x\n' } }); + + const res = await h.call('write_file', { project: 'idx', path: 'a.qmd', content: 'y\n' }); + + expect(res.isError).not.toBe(true); + const out = textOf(res); + expect(out).toContain('Updated a.qmd'); + expect(out).toMatch(/render check unavailable/i); + expect(out).toContain('get_errors'); + }); +}); From ea2a3350f6f8715dbec3f3cebeb86f34784569a1 Mon Sep 17 00:00:00 2001 From: Andrew Holz Date: Tue, 4 Aug 2026 14:04:27 -0400 Subject: [PATCH 9/9] hub-mcp: rename fix-errors prompt to fix_errors for naming consistency All tool names in this server are snake_case; the prompt now matches. Co-Authored-By: Claude Fable 5 --- .../plans/2026-07-28-hub-mcp-get-errors-v2.md | 2 +- ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts | 6 +++--- ts-packages/quarto-hub-mcp/src/prompts.test.ts | 12 ++++++------ ts-packages/quarto-hub-mcp/src/prompts.ts | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md index 0de40fe90..6d4c60963 100644 --- a/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md +++ b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md @@ -113,7 +113,7 @@ write tools now do it themselves: - Non-`.qmd` writes are unchanged; a check that cannot run degrades to `Render check unavailable (…); call get_errors to verify` and never fails the write (`renderCheckSuffix` in `src/tools.ts`). -- The `fix-errors` prompt now points the loop at the in-response check, +- The `fix_errors` prompt now points the loop at the in-response check, with one final `get_errors` to confirm. - Tests: `src/write-render-check.test.ts` (7, handler-level, renderer mocked at the module seam, fail-first verified); `get-errors-live.test.ts` diff --git a/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts b/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts index e94cb6bec..495cde351 100644 --- a/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts +++ b/ts-packages/quarto-hub-mcp/src/hub-mcp.test.ts @@ -81,11 +81,11 @@ describe('MCP protocol', () => { }); }); - it('lists the fix-errors prompt and expands it over the protocol', async () => { + it('lists the fix_errors prompt and expands it over the protocol', async () => { const prompts = await client.listPrompts(); - expect(prompts.map((p) => p.name)).toContain('fix-errors'); + expect(prompts.map((p) => p.name)).toContain('fix_errors'); - const res = await client.getPrompt('fix-errors', { project: 'automerge:xyz' }); + const res = await client.getPrompt('fix_errors', { project: 'automerge:xyz' }); expect(res.messages[0]!.content.text).toContain('automerge:xyz'); expect(res.messages[0]!.content.text).toContain('get_errors'); }); diff --git a/ts-packages/quarto-hub-mcp/src/prompts.test.ts b/ts-packages/quarto-hub-mcp/src/prompts.test.ts index acb9f45f8..9d974c845 100644 --- a/ts-packages/quarto-hub-mcp/src/prompts.test.ts +++ b/ts-packages/quarto-hub-mcp/src/prompts.test.ts @@ -1,5 +1,5 @@ /** - * Tests for the `fix-errors` MCP prompt — the one-command entry into + * Tests for the `fix_errors` MCP prompt — the one-command entry into * the agent fix loop. The prompt only instructs; the LLM does the * fixing with the existing tools (get_errors, read_file, patch_file). */ @@ -28,13 +28,13 @@ function harness(): { list: Handler; get: Handler } { return { list, get }; } -describe('fix-errors prompt', () => { +describe('fix_errors prompt', () => { it('is listed with a required project argument and optional path', async () => { const { list } = harness(); const res = (await list({})) as { prompts: Array<{ name: string; arguments?: Array<{ name: string; required?: boolean }> }>; }; - const p = res.prompts.find((x) => x.name === 'fix-errors'); + const p = res.prompts.find((x) => x.name === 'fix_errors'); expect(p).toBeDefined(); expect(p!.arguments).toEqual([ expect.objectContaining({ name: 'project', required: true }), @@ -45,7 +45,7 @@ describe('fix-errors prompt', () => { it('expands to loop instructions naming the project and the tools', async () => { const { get } = harness(); const res = (await get({ - params: { name: 'fix-errors', arguments: { project: 'automerge:abc123' } }, + params: { name: 'fix_errors', arguments: { project: 'automerge:abc123' } }, })) as { messages: Array<{ role: string; content: { type: string; text: string } }> }; expect(res.messages).toHaveLength(1); @@ -61,7 +61,7 @@ describe('fix-errors prompt', () => { it('scopes the instructions to a single file when path is given', async () => { const { get } = harness(); const res = (await get({ - params: { name: 'fix-errors', arguments: { project: 'abc', path: 'chapter2.qmd' } }, + params: { name: 'fix_errors', arguments: { project: 'abc', path: 'chapter2.qmd' } }, })) as { messages: Array<{ content: { text: string } }> }; expect(res.messages[0]!.content.text).toContain('chapter2.qmd'); }); @@ -73,7 +73,7 @@ describe('fix-errors prompt', () => { it('rejects a missing project argument', async () => { const { get } = harness(); - await expect(get({ params: { name: 'fix-errors', arguments: {} } })).rejects.toThrow( + await expect(get({ params: { name: 'fix_errors', arguments: {} } })).rejects.toThrow( /project/i, ); }); diff --git a/ts-packages/quarto-hub-mcp/src/prompts.ts b/ts-packages/quarto-hub-mcp/src/prompts.ts index bb04baed4..33f67d5e9 100644 --- a/ts-packages/quarto-hub-mcp/src/prompts.ts +++ b/ts-packages/quarto-hub-mcp/src/prompts.ts @@ -1,6 +1,6 @@ /** * MCP prompts — named prompt templates clients surface as slash - * commands (Claude Code shows this one as /mcp__quarto-hub__fix-errors). + * commands (Claude Code shows this one as /mcp__quarto-hub__fix_errors). * * A prompt only instructs; the LLM does the fixing with the existing * tools. This is deliberately NOT a `fix_errors` tool: fixing requires @@ -15,7 +15,7 @@ import { import type { Server } from '@modelcontextprotocol/sdk/server/index.js'; const FIX_ERRORS = { - name: 'fix-errors', + name: 'fix_errors', description: 'Find and fix the render errors in a Quarto Hub project: checks with ' + 'get_errors, applies minimal fixes with patch_file, and re-checks until clean.',