From 979d495ab67d3f7f969cc6c91a9a8b824a59b286 Mon Sep 17 00:00:00 2001 From: hshum Date: Tue, 8 Sep 2026 01:00:47 -0700 Subject: [PATCH 1/2] fix(mcp): preserve failed tool outcomes across raw responses and logs Return explicit error objects from eight raw tools and preserve successful content. Use one outcome for MCP status, analytics and audit metadata; omit success guidance on failures. Bound audit batches and contain failed writes. Local candidate:19 new scenarios pass; normal suite389 pass/1 XLSX timeout/1 skip. Selected20-case timing diagnostic passes without changing the20s deadline; full-suite cause remains open. Final build and78-call native protocol/storage outage/recovery/restart proof pass. Independent final review and release/consumer adoption remain pending. No published package or production deployment. --- .../mcp-local/src/__tests__/tools.test.ts | 166 +++++++++++++++++- packages/mcp-local/src/index.ts | 16 +- packages/mcp-local/src/security/auditLog.ts | 51 +++--- .../mcp-local/src/tools/uiCaptureTools.ts | 82 ++++----- packages/mcp-local/src/tools/uiUxDiveTools.ts | 21 +-- packages/mcp-local/src/tools/visionTools.ts | 80 ++++----- packages/mcp-local/src/tools/visualQaTools.ts | 146 +++++---------- packages/mcp-local/src/types.ts | 5 +- 8 files changed, 323 insertions(+), 244 deletions(-) diff --git a/packages/mcp-local/src/__tests__/tools.test.ts b/packages/mcp-local/src/__tests__/tools.test.ts index df2574c16..033b8ea38 100644 --- a/packages/mcp-local/src/__tests__/tools.test.ts +++ b/packages/mcp-local/src/__tests__/tools.test.ts @@ -3,7 +3,7 @@ * Covers: static, unit, integration layers. * Live E2E layer is tested via bash pipe in the flywheel step. */ -import { describe, it, expect, afterEach } from "vitest"; +import { describe, it, expect, afterEach, beforeEach, vi } from "vitest"; import os from "node:os"; import path from "node:path"; import { mkdtemp, writeFile } from "node:fs/promises"; @@ -71,6 +71,170 @@ import { getQuickRef, hybridSearch, TOOL_REGISTRY, SEARCH_MODES, ALL_REGISTRY_EN import { TOOLSET_LOADERS } from "../toolsetRegistry.js"; import type { McpTool } from "../types.js"; +// A developer must be able to distinguish failed captures from usable evidence. +describe("raw tool outcome contract", () => { + const mocks = ["playwright", "sharp", "openai", "@google/genai", "os", "../db.js"]; + let home: string; + beforeEach(async () => { + vi.resetModules(); + home = await mkdtemp(path.join(os.tmpdir(), "raw-tool-outcome-")); + vi.doMock("os", async (original) => ({ ...await original(), homedir: () => home })); + for (const key of ["GEMINI_API_KEY", "GOOGLE_AI_API_KEY", "OPENAI_API_KEY", "ANTHROPIC_API_KEY", "OPENROUTER_API_KEY"]) vi.stubEnv(key, ""); + }); + afterEach(() => { + for (const name of mocks) vi.doUnmock(name); + vi.unstubAllEnvs(); + vi.restoreAllMocks(); + vi.resetModules(); + }); + async function tools() { + return [ + ...(await import("../tools/uiCaptureTools.js")).uiCaptureTools, + ...(await import("../tools/visionTools.js")).visionTools, + ...(await import("../tools/uiUxDiveTools.js")).uiUxDiveTools, + ...(await import("../tools/visualQaTools.js")).visualQaTools, + ]; + } + async function call(name: string, args: Record = {}): Promise { + return (await tools()).find(t => t.name === name)!.handler(args); + } + function failure(result: any, message: RegExp) { + expect(Array.isArray(result)).toBe(false); + expect(result).toMatchObject({ error: true, message: expect.stringMatching(message) }); + } + function browser(goto = vi.fn().mockResolvedValue(undefined)) { + const bytes = Buffer.from("controlled screenshot bytes"); + const page = { + on: vi.fn(), goto, waitForSelector: vi.fn(), + screenshot: vi.fn(async (opts: any) => { if (opts.path) writeFileSync(opts.path, bytes); return bytes; }), + title: vi.fn().mockResolvedValue("A developer's evidence"), url: () => "https://example.invalid/", + $: vi.fn().mockResolvedValue(null), accessibility: { snapshot: vi.fn().mockResolvedValue({ role: "document" }) }, + }; + const context = { newPage: vi.fn().mockResolvedValue(page), close: vi.fn() }; + const instance = { newPage: vi.fn().mockResolvedValue(page), newContext: vi.fn().mockResolvedValue(context), close: vi.fn() }; + vi.doMock("playwright", () => ({ chromium: { launch: vi.fn().mockResolvedValue(instance) } })); + return { page, instance, bytes }; + } + + it.each([ + ["capture_ui_screenshot", "playwright"], ["capture_responsive_suite", "playwright"], + ["burst_capture", "playwright"], ["run_visual_qa_suite", "playwright"], + ["manipulate_screenshot", "sharp"], ["generate_grid_collage", "sharp"], + ])("lets a fresh-install developer identify the unavailable dependency for %s", async (name, dependency) => { + vi.doMock(dependency, () => { throw new Error("Optional dependency unavailable in this consumer"); }); + failure(await call(name), /not installed/i); + }); + + it("reports missing provider and missing browser session without producing evidence", async () => { + failure(await call("analyze_screenshot", { imageBase64: "ignored" }), /No vision provider/); + failure(await call("dive_snapshot", { sessionId: "not-started" }), /No active browser/); + }); + + it.each(["capture_ui_screenshot", "capture_responsive_suite", "burst_capture", "run_visual_qa_suite"])("closes the failed browser and retains context for %s", async name => { + const { instance } = browser(vi.fn().mockRejectedValue(new Error("Page closed during navigation"))); + const result = await call(name, { url: "https://example.invalid/", label: "failed", waitMs: 0, settleMs: 0 }); + failure(result, /Page closed during navigation/); + expect(result.url).toBe("https://example.invalid/"); + expect(instance.close).toHaveBeenCalledOnce(); + }); + + it("rejects invalid visual viewports and empty or unreadable frames before calling them a collage", async () => { + browser(); + for (const name of ["burst_capture", "run_visual_qa_suite"]) failure(await call(name, { viewport: "invalid" }), /Unknown viewport/); + failure(await call("generate_grid_collage", { framePaths: [] }), /non-empty/); + failure(await call("generate_grid_collage", { framePaths: [path.join(home, "missing.png")] }), /failed/i); + }); + + it("reports a suite's missing image decoder even when its browser is available", async () => { + browser(); + vi.doMock("sharp", () => { throw new Error("Image dependency unavailable"); }); + failure(await call("run_visual_qa_suite"), /sharp is not installed/); + }); + + it("preserves single and responsive capture bytes and content ordering", async () => { + const { bytes } = browser(); + const single = await call("capture_ui_screenshot", { url: "https://example.invalid/", waitMs: 0 }); + expect(single.map((b: any) => b.type)).toEqual(["text", "image"]); + expect(single[1]).toEqual({ type: "image", data: bytes.toString("base64"), mimeType: "image/png" }); + const multi = await call("capture_responsive_suite", { url: "https://example.invalid/", label: "three-widths", waitMs: 0 }); + expect(multi.map((b: any) => b.type)).toEqual(["text", "text", "image", "text", "image", "text", "image"]); + expect(multi.filter((b: any) => b.type === "image").map((b: any) => b.data)).toEqual(Array(3).fill(bytes.toString("base64"))); + }); + + it("rejects malformed bytes and invalid crops, then processes a valid image in the same workflow", async () => { + const sharp = (await import("sharp")).default; + const input = await sharp({ create: { width: 8, height: 8, channels: 3, background: "#224466" } }).png().toBuffer(); + failure(await call("manipulate_screenshot", { imageBase64: Buffer.from("not-an-image").toString("base64"), operation: "resize", width: 4 }), /Image manipulation failed/); + failure(await call("manipulate_screenshot", { imageBase64: input.toString("base64"), operation: "crop", x: 99, y: 0, cropWidth: 2, cropHeight: 2 }), /Image manipulation failed/); + const result = await call("manipulate_screenshot", { imageBase64: input.toString("base64"), operation: "resize", width: 4 }); + expect(result[1].type).toBe("image"); + expect(await sharp(Buffer.from(result[1].data, "base64")).metadata()).toMatchObject({ width: 4, height: 4 }); + }); + + it("retains failed-provider context and successful text containing JSON error words", async () => { + const create = vi.fn().mockRejectedValueOnce(new Error("Controlled provider failure")).mockResolvedValueOnce({ choices: [{ message: { content: '{"error":true} is literal text in this screenshot' } }] }); + vi.doMock("openai", () => ({ default: class { chat = { completions: { create } }; } })); + const failed = await call("analyze_screenshot", { imageBase64: "controlled", provider: "openai" }); + failure(failed, /Controlled provider failure/); + expect(failed.provider).toBe("openai"); + const success = await call("analyze_screenshot", { imageBase64: "controlled", provider: "openai" }); + expect(success[1]).toEqual({ type: "text", text: '{"error":true} is literal text in this screenshot' }); + expect(Array.isArray(success)).toBe(true); + }); + + it("keeps multiple provider images in order without interpreting their accompanying text as failure", async () => { + vi.doMock("@google/genai", () => ({ GoogleGenAI: class { models = { generateContent: async () => ({ candidates: [{ content: { parts: [{ text: "An error label is visible" }, { inlineData: { data: "first-image" } }, { inlineData: { data: "second-image" } }] } }] }) }; } })); + const result = await call("analyze_screenshot", { imageBase64: "controlled", provider: "gemini" }); + expect(result.map((b: any) => b.type)).toEqual(["text", "text", "image", "image"]); + expect(result.slice(2).map((b: any) => b.data)).toEqual(["first-image", "second-image"]); + }); + + it("keeps a dive usable after missing selectors, screenshot failure and accessibility failure", async () => { + const { page, bytes } = browser(); + vi.doMock("../db.js", () => ({ getDb: () => ({ prepare: () => ({ run: vi.fn() }) }), genId: () => "controlled-dive" })); + const session = await call("start_ui_dive", { appUrl: "https://example.invalid/", autoDiscover: false }); + const args = { sessionId: session.sessionId }; + failure(await call("dive_snapshot", { ...args, selector: "#missing" }), /Element not found/); + page.screenshot.mockRejectedValueOnce(new Error("Closed page during screenshot")); + failure(await call("dive_snapshot", args), /Screenshot failed/); + page.accessibility.snapshot.mockRejectedValueOnce(new Error("Accessibility unavailable")); + failure(await call("dive_snapshot", { ...args, mode: "accessibility" }), /Accessibility snapshot failed/); + const screenshot = await call("dive_snapshot", args); + expect(screenshot[1].data).toBe(bytes.toString("base64")); + const accessibility = await call("dive_snapshot", { ...args, mode: "accessibility" }); + expect(accessibility).toHaveLength(1); + expect(JSON.parse(accessibility[0].text).accessibilityTree).toEqual({ role: "document" }); + }); + + it("survives an agent burst and repeated audit storage failures, then resumes truthful logging", async () => { + let unavailable = true; + const rows: unknown[][] = []; + const batches: number[] = []; + const db = { + pragma: vi.fn(), exec: vi.fn(), + prepare: vi.fn((sql: string) => { + if (sql.includes("INSERT") && unavailable) throw new Error("Audit storage unavailable"); + return { run: (...args: unknown[]) => { if (sql.includes("INSERT")) rows.push(args); } }; + }), + transaction: (fn: (entries: unknown[]) => void) => (entries: unknown[]) => { batches.push(entries.length); fn(entries); }, + }; + vi.doMock("../db.js", () => ({ openOptionalSqliteDatabase: () => db })); + const { auditLog, flushAuditLog, _resetAuditForTesting } = await import("../security/auditLog.js"); + try { + for (let round = 0; round < 3; round++) { + for (let call = 0; call < 700; call++) expect(() => auditLog("tool_call", "manipulate_screenshot", "controlled", true, "decoder failed", { resultStatus: "error" })).not.toThrow(); + expect(flushAuditLog).not.toThrow(); + } + unavailable = false; + for (let call = 0; call < 700; call++) auditLog("tool_call", "manipulate_screenshot", "controlled", true, undefined, { resultStatus: "success" }); + flushAuditLog(); + expect(rows).toHaveLength(700); + expect(Math.max(...batches)).toBeLessThanOrEqual(256); + expect(rows.every(row => row[5] === 1 && JSON.parse(String(row[7])).resultStatus === "success")).toBe(true); + } finally { _resetAuditForTesting(); } + }); +}); + // Assemble all tools like index.ts does const domainTools: McpTool[] = [ ...verificationTools, diff --git a/packages/mcp-local/src/index.ts b/packages/mcp-local/src/index.ts index 09f3a86af..7987eb946 100644 --- a/packages/mcp-local/src/index.ts +++ b/packages/mcp-local/src/index.ts @@ -3504,14 +3504,19 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { } catch { /* instrumentation */ } } - // Tools with rawContent return ContentBlock[] directly (e.g. image captures) + // Permission to execute is separate from the handler's success or failure. + // Record raw calls too, before their content-preserving early return. + auditLog("tool_call", name, JSON.stringify(args ?? {}).substring(0, 200), true, + errorMsg ?? undefined, { resultStatus }); + + // Successful raw content is opaque: text can legitimately describe an error. if (tool.rawContent && Array.isArray(result)) { - return { content: result, isError: false }; + return { content: result, isError: resultStatus === "error" }; } // Auto-append quickRef from registry (progressive disclosure) let enrichedResult = result; - if (result && typeof result === "object" && !Array.isArray(result)) { + if (resultStatus === "success" && result && typeof result === "object" && !Array.isArray(result)) { const quickRef = getQuickRef(name); if (quickRef && !(result as any)._quickRef) { enrichedResult = { ...(result as Record), _quickRef: quickRef }; @@ -3543,12 +3548,9 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { contentBlocks.push({ type: "text" as const, text: hookHint }); } - // Audit log: successful tool call - auditLog("tool_call", name, JSON.stringify(args ?? {}).substring(0, 200), true); - return { content: contentBlocks, - isError: false, + isError: resultStatus === "error", }; } catch (err: any) { // Security errors get a clean response (not a stack trace) diff --git a/packages/mcp-local/src/security/auditLog.ts b/packages/mcp-local/src/security/auditLog.ts index 61cc67d45..5a243adae 100644 --- a/packages/mcp-local/src/security/auditLog.ts +++ b/packages/mcp-local/src/security/auditLog.ts @@ -23,6 +23,7 @@ export interface AuditEntry { } // In-memory buffer for batch writes +const MAX_BUFFER_ENTRIES = 256; let _buffer: AuditEntry[] = []; let _flushTimer: ReturnType | null = null; let _db: any = null; @@ -88,40 +89,41 @@ function getDb(): any { function flushBuffer(): void { if (_buffer.length === 0) return; + // Detach the bounded batch even if storage is unavailable or a write fails. + const batch = _buffer; + _buffer = []; + const db = getDb(); if (!db) { // No SQLite — just discard (entries were already returned from auditLog) - _buffer = []; return; } - const insert = db.prepare(` - INSERT OR IGNORE INTO audit_log (id, timestamp, category, tool_name, args_preview, allowed, reason, metadata) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) - `); - - const insertMany = db.transaction((entries: AuditEntry[]) => { - for (const e of entries) { - insert.run( - e.id, - e.timestamp, - e.category, - e.toolName, - e.argsPreview, - e.allowed ? 1 : 0, - e.reason ?? null, - e.metadata ? JSON.stringify(e.metadata) : null, - ); - } - }); - try { - insertMany(_buffer); + const insert = db.prepare(` + INSERT OR IGNORE INTO audit_log (id, timestamp, category, tool_name, args_preview, allowed, reason, metadata) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + `); + + const insertMany = db.transaction((entries: AuditEntry[]) => { + for (const e of entries) { + insert.run( + e.id, + e.timestamp, + e.category, + e.toolName, + e.argsPreview, + e.allowed ? 1 : 0, + e.reason ?? null, + e.metadata ? JSON.stringify(e.metadata) : null, + ); + } + }); + + insertMany(batch); } catch { // SQLite write failed — discard silently } - - _buffer = []; } /** @@ -151,6 +153,7 @@ export function auditLog( metadata, }; + if (_buffer.length >= MAX_BUFFER_ENTRIES) flushBuffer(); _buffer.push(entry); // Batch flush every 100ms diff --git a/packages/mcp-local/src/tools/uiCaptureTools.ts b/packages/mcp-local/src/tools/uiCaptureTools.ts index f181fe5ef..36e673539 100644 --- a/packages/mcp-local/src/tools/uiCaptureTools.ts +++ b/packages/mcp-local/src/tools/uiCaptureTools.ts @@ -8,7 +8,7 @@ import { join } from "path"; import { homedir } from "os"; import { mkdirSync, existsSync, readFileSync } from "fs"; -import type { McpTool, ContentBlock } from "../types.js"; +import type { McpTool, ContentBlock, RawToolResult } from "../types.js"; // Screenshot storage directory const CAPTURE_DIR = join(homedir(), ".nodebench", "captures"); @@ -88,22 +88,17 @@ export const uiCaptureTools: McpTool[] = [ }, required: ["url"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const pw = await getPlaywright(); if (!pw) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: - "Playwright is not installed. Install it with: npm install playwright && npx playwright install chromium", - suggestion: - "The capture_ui_screenshot tool requires Playwright for headless browser automation. " + - "Run `npm install playwright` in your project, then `npx playwright install chromium` to download the browser binary.", - }), - }, - ]; + return { + error: true, + message: + "Playwright is not installed. Install it with: npm install playwright && npx playwright install chromium", + suggestion: + "The capture_ui_screenshot tool requires Playwright for headless browser automation. " + + "Run `npm install playwright` in your project, then `npx playwright install chromium` to download the browser binary.", + }; } const viewportName = args.viewport ?? "desktop"; @@ -203,19 +198,14 @@ export const uiCaptureTools: McpTool[] = [ // ignore cleanup error } } - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Screenshot capture failed: ${err.message}`, - url: args.url, - viewport: viewportName, - suggestion: - "Ensure the URL is accessible. If capturing localhost, make sure the dev server is running.", - }), - }, - ]; + return { + error: true, + message: `Screenshot capture failed: ${err.message}`, + url: args.url, + viewport: viewportName, + suggestion: + "Ensure the URL is accessible. If capturing localhost, make sure the dev server is running.", + }; } }, }, @@ -247,19 +237,14 @@ export const uiCaptureTools: McpTool[] = [ }, required: ["url", "label"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const pw = await getPlaywright(); if (!pw) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: - "Playwright is not installed. Install it with: npm install playwright && npx playwright install chromium", - }), - }, - ]; + return { + error: true, + message: + "Playwright is not installed. Install it with: npm install playwright && npx playwright install chromium", + }; } const waitMs = args.waitMs ?? 1000; @@ -380,18 +365,13 @@ export const uiCaptureTools: McpTool[] = [ // ignore cleanup error } } - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Responsive capture failed: ${err.message}`, - url: args.url, - suggestion: - "Ensure the URL is accessible and the dev server is running.", - }), - }, - ]; + return { + error: true, + message: `Responsive capture failed: ${err.message}`, + url: args.url, + suggestion: + "Ensure the URL is accessible and the dev server is running.", + }; } }, }, diff --git a/packages/mcp-local/src/tools/uiUxDiveTools.ts b/packages/mcp-local/src/tools/uiUxDiveTools.ts index 177649a2e..d1cd0b248 100644 --- a/packages/mcp-local/src/tools/uiUxDiveTools.ts +++ b/packages/mcp-local/src/tools/uiUxDiveTools.ts @@ -16,7 +16,7 @@ */ import { getDb, genId } from "../db.js"; -import type { McpTool, ContentBlock } from "../types.js"; +import type { McpTool, ContentBlock, RawToolResult } from "../types.js"; // ── Browser Session Management (Built-in Playwright) ──────────────────── // Singleton browser/page per dive session. Auto-detects Playwright. @@ -971,7 +971,7 @@ export const uiUxDiveTools: McpTool[] = [ }, required: ["sessionId"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const { sessionId, mode, selector, label } = args as { sessionId: string; mode?: string; @@ -980,13 +980,10 @@ export const uiUxDiveTools: McpTool[] = [ }; if (!_page || _activeSessionId !== sessionId) { - return [{ - type: "text", - text: JSON.stringify({ - error: true, - message: "No active browser for this session. Start a dive with start_ui_dive first, or install Playwright: npm install playwright && npx playwright install chromium", - }), - }]; + return { + error: true, + message: "No active browser for this session. Start a dive with start_ui_dive first, or install Playwright: npm install playwright && npx playwright install chromium", + }; } const captureMode = mode ?? "screenshot"; @@ -1004,7 +1001,7 @@ export const uiUxDiveTools: McpTool[] = [ }, null, 2), }]; } catch (e: any) { - return [{ type: "text", text: JSON.stringify({ error: true, message: `Accessibility snapshot failed: ${e.message}` }) }]; + return { error: true, message: `Accessibility snapshot failed: ${e.message}` }; } } @@ -1019,7 +1016,7 @@ export const uiUxDiveTools: McpTool[] = [ if (selector) { const el = await _page.$(selector); if (!el) { - return [{ type: "text", text: JSON.stringify({ error: true, message: `Element not found: ${selector}` }) }]; + return { error: true, message: `Element not found: ${selector}` }; } screenshotBuf = await el.screenshot({ type: "png" }); } else { @@ -1062,7 +1059,7 @@ export const uiUxDiveTools: McpTool[] = [ }, ]; } catch (e: any) { - return [{ type: "text", text: JSON.stringify({ error: true, message: `Screenshot failed: ${e.message}` }) }]; + return { error: true, message: `Screenshot failed: ${e.message}` }; } }, }, diff --git a/packages/mcp-local/src/tools/visionTools.ts b/packages/mcp-local/src/tools/visionTools.ts index 55fdcb42e..ab142eb01 100644 --- a/packages/mcp-local/src/tools/visionTools.ts +++ b/packages/mcp-local/src/tools/visionTools.ts @@ -12,7 +12,7 @@ import { join } from "path"; import { homedir } from "os"; import { mkdirSync, existsSync, writeFileSync } from "fs"; -import type { McpTool, ContentBlock } from "../types.js"; +import type { McpTool, ContentBlock, RawToolResult } from "../types.js"; const CAPTURE_DIR = join(homedir(), ".nodebench", "captures"); @@ -329,7 +329,7 @@ export const visionTools: McpTool[] = [ }, required: ["imageBase64"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const providerChoice = args.provider ?? "auto"; const analysisPrompt = args.context ? `Context: ${args.context}\n\n${args.prompt ?? DEFAULT_ANALYSIS_PROMPT}` @@ -367,19 +367,14 @@ export const visionTools: McpTool[] = [ } if (!selectedProvider) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: - "No vision provider available. Call discover_vision_env to see what's needed.", - suggestion: - "Set one of: GEMINI_API_KEY (recommended), OPENAI_API_KEY, ANTHROPIC_API_KEY, or OPENROUTER_API_KEY. " + - "Also install the corresponding SDK: @google/genai, openai, or @anthropic-ai/sdk.", - }), - }, - ]; + return { + error: true, + message: + "No vision provider available. Call discover_vision_env to see what's needed.", + suggestion: + "Set one of: GEMINI_API_KEY (recommended), OPENAI_API_KEY, ANTHROPIC_API_KEY, or OPENROUTER_API_KEY. " + + "Also install the corresponding SDK: @google/genai, openai, or @anthropic-ai/sdk.", + }; } try { @@ -442,18 +437,13 @@ export const visionTools: McpTool[] = [ return content; } catch (err: any) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - provider: selectedProvider, - message: `Vision analysis failed: ${err.message}`, - suggestion: - "Check that the API key is valid and the SDK is installed. Try a different provider with provider='openai' or provider='anthropic'.", - }), - }, - ]; + return { + error: true, + provider: selectedProvider, + message: `Vision analysis failed: ${err.message}`, + suggestion: + "Check that the API key is valid and the SDK is installed. Try a different provider with provider='openai' or provider='anthropic'.", + }; } }, }, @@ -517,21 +507,16 @@ export const visionTools: McpTool[] = [ }, required: ["imageBase64", "operation"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const sharp = await getSharp(); if (!sharp) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: - "sharp is not installed. Install it with: npm install sharp", - suggestion: - "The manipulate_screenshot tool requires sharp for image processing.", - }), - }, - ]; + return { + error: true, + message: + "sharp is not installed. Install it with: npm install sharp", + suggestion: + "The manipulate_screenshot tool requires sharp for image processing.", + }; } const inputBuffer = Buffer.from(args.imageBase64, "base64"); @@ -642,16 +627,11 @@ export const visionTools: McpTool[] = [ }, ]; } catch (err: any) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - operation: args.operation, - message: `Image manipulation failed: ${err.message}`, - }), - }, - ]; + return { + error: true, + operation: args.operation, + message: `Image manipulation failed: ${err.message}`, + }; } }, }, diff --git a/packages/mcp-local/src/tools/visualQaTools.ts b/packages/mcp-local/src/tools/visualQaTools.ts index 8aadd6ff4..15269a895 100644 --- a/packages/mcp-local/src/tools/visualQaTools.ts +++ b/packages/mcp-local/src/tools/visualQaTools.ts @@ -19,7 +19,7 @@ declare var sessionStorage: { clear(): void }; import { join } from "path"; import { homedir } from "os"; import { mkdirSync, existsSync, readFileSync } from "fs"; -import type { McpTool, ContentBlock } from "../types.js"; +import type { McpTool, ContentBlock, RawToolResult } from "../types.js"; import { getDb, genId } from "../db.js"; // ═══ Constants ═══ @@ -256,19 +256,14 @@ export const visualQaTools: McpTool[] = [ }, required: ["url"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const pw = await getPlaywright(); if (!pw) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: - "Playwright is not installed. Run: npm install playwright && npx playwright install chromium", - }), - }, - ]; + return { + error: true, + message: + "Playwright is not installed. Run: npm install playwright && npx playwright install chromium", + }; } const frameCount = Math.min(Math.max(args.frameCount ?? 10, 2), 30); @@ -279,15 +274,10 @@ export const visualQaTools: McpTool[] = [ const waitUntil = args.waitUntil ?? "networkidle"; if (!viewportSize) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Unknown viewport: ${viewportName}. Use: mobile, tablet, desktop, wide`, - }), - }, - ]; + return { + error: true, + message: `Unknown viewport: ${viewportName}. Use: mobile, tablet, desktop, wide`, + }; } const burstDir = ensureBurstDir(args.label ?? "burst"); @@ -428,16 +418,11 @@ export const visualQaTools: McpTool[] = [ /* ignore cleanup */ } } - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Burst capture failed: ${err.message}`, - url: args.url, - }), - }, - ]; + return { + error: true, + message: `Burst capture failed: ${err.message}`, + url: args.url, + }; } }, }, @@ -490,31 +475,21 @@ export const visualQaTools: McpTool[] = [ }, required: ["framePaths"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const sharp = await getSharp(); if (!sharp) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: "sharp is not installed. Run: npm install sharp", - }), - }, - ]; + return { + error: true, + message: "sharp is not installed. Run: npm install sharp", + }; } const paths: string[] = args.framePaths; if (!paths || paths.length === 0) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: "framePaths is required and must be non-empty", - }), - }, - ]; + return { + error: true, + message: "framePaths is required and must be non-empty", + }; } const cols = args.columns ?? 5; @@ -631,15 +606,10 @@ export const visualQaTools: McpTool[] = [ { type: "image", data: base64, mimeType: "image/png" }, ]; } catch (err: any) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Grid collage generation failed: ${err.message}`, - }), - }, - ]; + return { + error: true, + message: `Grid collage generation failed: ${err.message}`, + }; } }, }, @@ -907,32 +877,22 @@ export const visualQaTools: McpTool[] = [ }, required: ["url"], }, - handler: async (args): Promise => { + handler: async (args): Promise => { const pw = await getPlaywright(); if (!pw) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: - "Playwright is not installed. Run: npm install playwright && npx playwright install chromium", - }), - }, - ]; + return { + error: true, + message: + "Playwright is not installed. Run: npm install playwright && npx playwright install chromium", + }; } const sharpMod = await getSharp(); if (!sharpMod) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: "sharp is not installed. Run: npm install sharp", - }), - }, - ]; + return { + error: true, + message: "sharp is not installed. Run: npm install sharp", + }; } // Step 1: Burst capture (inline — reuse logic from burst_capture) @@ -945,15 +905,10 @@ export const visualQaTools: McpTool[] = [ const waitUntil = args.waitUntil ?? "networkidle"; if (!viewportSize) { - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Unknown viewport: ${viewportName}`, - }), - }, - ]; + return { + error: true, + message: `Unknown viewport: ${viewportName}`, + }; } const burstDir = ensureBurstDir(args.label ?? "suite"); @@ -1252,16 +1207,11 @@ export const visualQaTools: McpTool[] = [ /* ignore */ } } - return [ - { - type: "text", - text: JSON.stringify({ - error: true, - message: `Visual QA suite failed: ${err.message}`, - url: args.url, - }), - }, - ]; + return { + error: true, + message: `Visual QA suite failed: ${err.message}`, + url: args.url, + }; } }, }, diff --git a/packages/mcp-local/src/types.ts b/packages/mcp-local/src/types.ts index 8d99684fb..759cd1f3a 100644 --- a/packages/mcp-local/src/types.ts +++ b/packages/mcp-local/src/types.ts @@ -2,6 +2,9 @@ export type ContentBlock = | { type: "text"; text: string } | { type: "image"; data: string; mimeType: string }; +/** Preserve successful content blocks; keep explicit failures available to dispatch. */ +export type RawToolResult = ContentBlock[] | { error: true; message: string; [key: string]: unknown }; + export type McpToolAnnotations = { /** Tool only reads data — no side effects. */ readOnlyHint?: boolean; @@ -15,7 +18,7 @@ export type McpTool = { name: string; description: string; inputSchema: Record; - /** If true, handler returns ContentBlock[] directly instead of a JSON-serializable object. */ + /** If true, handler returns RawToolResult: content blocks on success or an explicit error object. */ rawContent?: boolean; /** MCP spec security annotations for trust & safety. */ annotations?: McpToolAnnotations; From b5798a33efb6184d172dbacb40a86136d00ee5e2 Mon Sep 17 00:00:00 2001 From: hshum Date: Tue, 8 Sep 2026 01:21:24 -0700 Subject: [PATCH 2/2] fix(mcp): scope patched UUID to ExcelJS Keep ExcelJS4.4 while overriding its UUID dependency to11.1.1, the patched CommonJS-compatible release. Existing MCP runtime and other dependency versions are unchanged. Verified frozen Windows install and full audit0,390 package tests with one skip, and24 before/24 after conditional-format workbook exports with120 rows read back in each lane. The prior20s XLSX timeout remains historical with unassigned cause; no deadline change. Local candidate only: independent final review, shared checks and versioned package release/consumer proof remain pending. --- packages/mcp-local/package.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/mcp-local/package.json b/packages/mcp-local/package.json index 2812792bb..0215fc46d 100644 --- a/packages/mcp-local/package.json +++ b/packages/mcp-local/package.json @@ -125,7 +125,10 @@ "ajv": "^8.18.0", "minimatch": "^10.2.5", "tar": "^7.5.13", - "prebuild-install": "https://registry.npmjs.org/prebuild-install/-/prebuild-install-7.1.3.tgz" + "prebuild-install": "https://registry.npmjs.org/prebuild-install/-/prebuild-install-7.1.3.tgz", + "exceljs": { + "uuid": "11.1.1" + } }, "engines": { "node": ">=18.0.0"