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..6d4c60963 --- /dev/null +++ b/claude-notes/plans/2026-07-28-hub-mcp-get-errors-v2.md @@ -0,0 +1,167 @@ +# 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 +- [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. + +## 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 +--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 + 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/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..37fc3dde3 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/scripts/build-wasm-host.mjs @@ -0,0 +1,58 @@ +// 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, pathToFileURL } 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'), + })); + }, +}; + +/** + * 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 === 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/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/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[] }; 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 new file mode 100644 index 000000000..a332b3eb0 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/get-errors-handler.test.ts @@ -0,0 +1,184 @@ +/** + * 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. The + * local renderer is mocked at its module seam — its own behavior is + * covered by local-render.test.ts against the real WASM. + */ + +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 } 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: 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; +} + +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 ?? {} }, + }; + 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: (args) => callToolHandler!({ params: { name: 'get_errors', arguments: args } }, {}), + }; +} + +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; +} + +type FileEntry = { + path: string; + checkedContentSha256?: string; + errors?: unknown[]; + warnings?: unknown[]; + note?: string; + execution?: Record; +}; + +beforeEach(() => { + renderDiagnostics.mockReset(); + renderDiagnostics.mockResolvedValue(cleanResult()); +}); + +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' } } }); + + 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].checkedContentSha256).toBe('sha256:def'); + expect(files[0].errors).toEqual([ERROR_ITEM]); + expect(files[0].warnings).toEqual([WARNING_ITEM]); + expect(renderDiagnostics).toHaveBeenCalledTimes(1); + }); + + it('renders every .qmd when no path is given, sorted', async () => { + const h = harness({ + 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 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('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 capture execution errors and running state, suppresses idle', async () => { + const h = harness({ + files: { 'a.qmd': { type: 'text', text: 'x' } }, + captures: { + '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 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('errors clearly when the requested path is not a text file', async () => { + const h = harness({ + files: { 'img.png': { type: 'binary', data: new Uint8Array([1]), mimeType: 'image/png' } }, + }); + const res = await h.call({ project: 'idx', path: 'img.png' }); + expect(res.isError).toBe(true); + }); + + 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 new file mode 100644 index 000000000..51120eaf8 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/get-errors-live.test.ts @@ -0,0 +1,93 @@ +/** + * 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. + * + * 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 { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { DocumentId } from '@automerge/automerge-repo'; + +import { McpTestClient } from './mcp-test-client.js'; +import { startTestHub, type TestHub } from './test-hub.js'; + +const BROKEN = '---\ntitle: ok\n---\n\nHello **unclosed strong\n'; +const FIXED = '---\ntitle: ok\n---\n\nHello **closed strong**\n'; + +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 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); + 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 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; + checkedContentSha256?: string; + errors: Array<{ title: string; start_line?: number }>; + }>; + }; + 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('reports clean immediately after the agent fixes the file', async () => { + const patched = await client.callTool('patch_file', { + project: indexDocId, + path: 'index.qmd', + old_string: 'Hello **unclosed strong', + 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, + path: 'index.qmd', + }); + expect(result.isError).not.toBe(true); + const report = JSON.parse(result.content[0]!.text) as { + files: Array<{ path: string; errors: unknown[]; warnings: unknown[] }>; + }; + 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..495cde351 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', @@ -80,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) @@ -113,6 +123,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/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/local-render.test.ts b/ts-packages/quarto-hub-mcp/src/local-render.test.ts new file mode 100644 index 000000000..045c9ad70 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/local-render.test.ts @@ -0,0 +1,121 @@ +/** + * 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'); + // 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 () => { + // 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..8a1e1af72 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/local-render.ts @@ -0,0 +1,179 @@ +/** + * 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; + + // 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(clean(d)); + } + for (const d of response.warnings ?? []) { + warnings.push(clean(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.map(clean) + : [{ 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/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..9d974c845 --- /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..33f67d5e9 --- /dev/null +++ b/ts-packages/quarto-hub-mcp/src/prompts.ts @@ -0,0 +1,80 @@ +/** + * 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. 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'); +} + +/** 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) }, + }, + ], + }; + }); +} diff --git a/ts-packages/quarto-hub-mcp/src/tools.ts b/ts-packages/quarto-hub-mcp/src/tools.ts index 954fec0a9..330539474 100644 --- a/ts-packages/quarto-hub-mcp/src/tools.ts +++ b/ts-packages/quarto-hub-mcp/src/tools.ts @@ -14,8 +14,9 @@ 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 { 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 }, + }, ]; } @@ -130,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: { @@ -144,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: { @@ -159,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: { @@ -325,6 +358,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 +448,135 @@ 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)); +} + +/** + * 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; @@ -429,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 { @@ -475,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 { @@ -494,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'); + }); +}); 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, + }, }, });