diff --git a/packages/mcp/eval/README.md b/packages/mcp/eval/README.md index 38cfb874..7fc6205d 100644 --- a/packages/mcp/eval/README.md +++ b/packages/mcp/eval/README.md @@ -2,9 +2,9 @@ A live, opt-in eval that exercises every registered Terminal49 MCP tool against a deployed gateway and scores each response on its **objective contract** — no LLM -required. It answers: *do the tools work, return well-shaped data, handle errors -sanely, respond quickly, and ship the `_agent_steering` guidance the server -promises?* +required. It answers: _do the tools work, return well-shaped data, handle errors +sanely, respond quickly, and omit the runtime steering metadata removed from the +public tool contract?_ ## Run it @@ -23,13 +23,13 @@ MCP_EVAL_ENDPOINT="http://localhost:4000/mcp" \ MCP_EVAL_TOKEN="" npm run eval --workspace @terminal49/mcp ``` -| Env var | Meaning | Default | -| --- | --- | --- | -| `MCP_EVAL_BEARER` | OAuth 2.1 access token → `Authorization: Bearer` | — | -| `MCP_EVAL_TOKEN` | Terminal49 API key → `Authorization: Token` | — | -| `MCP_EVAL_ENDPOINT` | Gateway `/mcp` URL | `https://mcp.terminal49.com/mcp` | -| `MCP_EVAL_ENABLE_WRITE` | Opt in to the mutating `track_container` case | unset (skipped) | -| `MCP_EVAL_ALLOW_SPARSE` | Allow detail cases to skip when the account has no data | unset (strict) | +| Env var | Meaning | Default | +| ----------------------- | ------------------------------------------------------- | -------------------------------- | +| `MCP_EVAL_BEARER` | OAuth 2.1 access token → `Authorization: Bearer` | — | +| `MCP_EVAL_TOKEN` | Terminal49 API key → `Authorization: Token` | — | +| `MCP_EVAL_ENDPOINT` | Gateway `/mcp` URL | `https://mcp.terminal49.com/mcp` | +| `MCP_EVAL_ENABLE_WRITE` | Opt in to the mutating `track_container` case | unset (skipped) | +| `MCP_EVAL_ALLOW_SPARSE` | Allow detail cases to skip when the account has no data | unset (strict) | > OAuth access tokens are short-lived (~5 min). For repeatable/CI runs, prefer a > `MCP_EVAL_TOKEN` API key — it does not expire. @@ -51,9 +51,11 @@ all pass (`contractPass`) — a single failure fails the test: - primary payload parses as JSON and carries the required keys - per-tool shape predicates (e.g. `total_lines === shipping_lines.length`, id round-trips, `timeline` is an array) -- an `_agent_steering` block is present and suggests follow-ups +- removed runtime steering fields are absent from every JSON content block: + `_agent_steering`, `presentation_guidance`, `suggested_follow_ups`, and + `suggested_tools` -**Latency** is a *soft* check: recorded in the score and the report, but a slow +**Latency** is a _soft_ check: recorded in the score and the report, but a slow response alone never fails the suite. Negative cases assert error behavior: an unknown id and a missing required @@ -88,7 +90,7 @@ Terminal49 API key** stored as the `MCP_EVAL_TOKEN` repo secret. ## Not covered here: subjective quality This suite grades the deterministic contract. It does **not** judge whether a -tool's output makes an LLM agent *answer well* — that is a separate concern best +tool's output makes an LLM agent _answer well_ — that is a separate concern best handled by an LLM-as-judge harness such as [`vitest-evals`](https://github.com/getsentry/vitest-evals), which runs an agent wired to this MCP server over realistic tasks and scores the transcript. That diff --git a/packages/mcp/eval/client.ts b/packages/mcp/eval/client.ts index f30a07c0..7021067b 100644 --- a/packages/mcp/eval/client.ts +++ b/packages/mcp/eval/client.ts @@ -27,8 +27,6 @@ export interface ParsedBlock { text: string; /** Parsed JSON payload, or undefined when the block is not JSON. */ json: unknown; - /** True when the block is an `_agent_steering` guidance block. */ - isSteering: boolean; } export interface ToolResult { @@ -42,10 +40,8 @@ export interface ToolResult { bytes: number; /** Every text content block, JSON parsed where possible. */ blocks: ParsedBlock[]; - /** First non-steering JSON block: the tool's primary payload. */ + /** First JSON block: the tool's primary payload. */ payload: unknown; - /** The `_agent_steering` block, when present. */ - steering: Record | undefined; /** JSON-RPC error message, when the transport returned one. */ errorMessage: string | undefined; /** All content blocks joined, for logging and error inspection. */ @@ -142,7 +138,6 @@ function parseBlocks(texts: string[]): ParsedBlock[] { index, text, json, - isSteering: isRecord(json) && json._agent_steering === true, }; }); } @@ -231,10 +226,7 @@ export class EvalClient { const latencyMs = Date.now() - start; const texts = extractTextBlocks(body.result); const blocks = parseBlocks(texts); - const steeringBlock = blocks.find((block) => block.isSteering); - const payloadBlock = blocks.find( - (block) => !block.isSteering && block.json !== undefined, - ); + const payloadBlock = blocks.find((block) => block.json !== undefined); const isError = (isRecord(body.result) && body.result.isError === true) || Boolean(body.error); @@ -245,10 +237,6 @@ export class EvalClient { bytes: texts.reduce((sum, text) => sum + text.length, 0), blocks, payload: payloadBlock?.json, - steering: - steeringBlock && isRecord(steeringBlock.json) - ? steeringBlock.json - : undefined, errorMessage: body.error?.message, rawText: texts.join('\n'), }; diff --git a/packages/mcp/eval/quality.test.ts b/packages/mcp/eval/quality.test.ts new file mode 100644 index 00000000..84bc594a --- /dev/null +++ b/packages/mcp/eval/quality.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from 'vite-plus/test'; +import type { ToolResult } from './client.js'; +import { scoreResult } from './quality.js'; + +function resultWithJson(json: unknown): ToolResult { + const text = JSON.stringify(json); + return { + http: 200, + isError: false, + latencyMs: 10, + bytes: text.length, + blocks: [{ index: 0, text, json }], + payload: json, + errorMessage: undefined, + rawText: text, + }; +} + +describe('scoreResult', () => { + it('accepts a response without removed steering metadata', () => { + const score = scoreResult(resultWithJson({ items: [] }), {}); + + expect(score.contractPass).toBe(true); + expect( + score.checks.find( + (check) => check.name === 'removed steering metadata is absent', + )?.pass, + ).toBe(true); + }); + + it('rejects removed steering metadata at any JSON depth', () => { + const score = scoreResult( + resultWithJson({ + nested: { + _agent_steering: true, + presentation_guidance: 'Present this result', + suggested_follow_ups: ['Check another container'], + suggested_tools: ['get_container'], + }, + }), + {}, + ); + const check = score.checks.find( + (candidate) => candidate.name === 'removed steering metadata is absent', + ); + + expect(score.contractPass).toBe(false); + expect(check?.pass).toBe(false); + expect(check?.detail).toBe( + '_agent_steering, presentation_guidance, suggested_follow_ups, suggested_tools', + ); + }); +}); diff --git a/packages/mcp/eval/quality.ts b/packages/mcp/eval/quality.ts index d186f452..2ff1822e 100644 --- a/packages/mcp/eval/quality.ts +++ b/packages/mcp/eval/quality.ts @@ -3,10 +3,9 @@ * * These scorers do NOT use an LLM. They evaluate the objective contract of a * tool response: transport success, error semantics, payload shape, required - * fields, latency budget, and the presence/usefulness of the `_agent_steering` - * guidance block that this server attaches to every tool. Subjective - * "is this a good answer" judging (LLM-as-judge over an agent transcript) is a - * separate, optional layer — see eval/README.md. + * fields, latency budget, and the absence of removed runtime steering metadata. + * Subjective "is this a good answer" judging (LLM-as-judge over an agent + * transcript) is a separate, optional layer — see eval/README.md. */ import { isRecord, type ToolResult } from './client.js'; @@ -44,11 +43,15 @@ export interface QualitySpec { predicates?: Predicate[]; /** Negative test: expect an MCP tool error instead of a payload. */ expectError?: boolean; - /** Require an `_agent_steering` block that suggests follow-ups. */ - requireSteering?: boolean; } const DEFAULT_LATENCY_BUDGET_MS = 8000; +const REMOVED_STEERING_FIELDS = new Set([ + '_agent_steering', + 'presentation_guidance', + 'suggested_follow_ups', + 'suggested_tools', +]); export function scoreResult( result: ToolResult, @@ -80,6 +83,14 @@ export function scoreResult( add('not a tool error', !result.isError, result.errorMessage); add('primary payload is JSON', result.payload !== undefined); add('non-empty response', result.bytes > 0, `${result.bytes}b`); + const removedFields = findRemovedSteeringFields( + result.blocks.map((block) => block.json), + ); + add( + 'removed steering metadata is absent', + removedFields.length === 0, + removedFields.length > 0 ? removedFields.join(', ') : undefined, + ); for (const key of spec.requiredKeys ?? []) { add( @@ -107,11 +118,6 @@ export function scoreResult( } } - if (spec.requireSteering) { - add('has _agent_steering block', result.steering !== undefined); - add('steering suggests follow-ups', steeringHasFollowUps(result.steering)); - } - // Latency is a soft signal: recorded and scored, but a slow response is not // a contract violation, so it never fails the suite on its own. add( @@ -131,16 +137,24 @@ export function scoreResult( }; } -function steeringHasFollowUps( - steering: Record | undefined, -): boolean { - if (!steering) return false; - const followUps = steering.suggested_follow_ups; - const tools = steering.suggested_tools; - return ( - (Array.isArray(followUps) && followUps.length > 0) || - (Array.isArray(tools) && tools.length > 0) - ); +function findRemovedSteeringFields(values: unknown[]): string[] { + const found = new Set(); + + function visit(value: unknown): void { + if (Array.isArray(value)) { + for (const item of value) visit(item); + return; + } + if (!isRecord(value)) return; + + for (const [key, nestedValue] of Object.entries(value)) { + if (REMOVED_STEERING_FIELDS.has(key)) found.add(key); + visit(nestedValue); + } + } + + for (const value of values) visit(value); + return [...found].sort(); } // ---- small typed helpers for writing predicates against `unknown` payloads ---- diff --git a/packages/mcp/eval/report.ts b/packages/mcp/eval/report.ts index 9d797258..f222199c 100644 --- a/packages/mcp/eval/report.ts +++ b/packages/mcp/eval/report.ts @@ -41,23 +41,18 @@ export function formatScorecard(rows: EvalRow[], meta: EvalReportMeta): string { lines.push(` endpoint: ${meta.endpoint} (auth: ${meta.scheme})`); lines.push('═'.repeat(78)); lines.push( - ` ${pad('TOOL', 32)}${pad('CASE', 16)}${pad('HTTP', 6)}${pad('ms', 7)}${pad('steer', 7)}SCORE`, + ` ${pad('TOOL', 32)}${pad('CASE', 16)}${pad('HTTP', 6)}${pad('ms', 7)}${pad('bytes', 7)}SCORE`, ); lines.push(' ' + '─'.repeat(74)); for (const row of rows) { - const steer = row.result.steering - ? 'yes' - : row.score.checks.some((c) => c.name.includes('steering')) - ? 'NO' - : '-'; const flag = !row.score.contractPass ? ' ✗' : row.score.score >= 1 ? '' : ' ~'; lines.push( - ` ${pad(row.tool, 32)}${pad(row.testCase, 16)}${pad(String(row.result.http), 6)}${pad(String(row.result.latencyMs), 7)}${pad(steer, 7)}${pct(row.score.score)} (${row.score.passed}/${row.score.total})${flag}`, + ` ${pad(row.tool, 32)}${pad(row.testCase, 16)}${pad(String(row.result.http), 6)}${pad(String(row.result.latencyMs), 7)}${pad(String(row.result.bytes), 7)}${pct(row.score.score)} (${row.score.passed}/${row.score.total})${flag}`, ); const failed = row.score.checks.filter((c) => !c.pass); for (const check of failed) { @@ -106,7 +101,6 @@ export function writeReport(rows: EvalRow[], meta: EvalReportMeta): string { isError: row.result.isError, latencyMs: row.result.latencyMs, bytes: row.result.bytes, - hasSteering: row.result.steering !== undefined, score: row.score.score, contractPass: row.score.contractPass, checks: row.score.checks, diff --git a/packages/mcp/eval/tools.eval.ts b/packages/mcp/eval/tools.eval.ts index 9ba3c975..cb3dc297 100644 --- a/packages/mcp/eval/tools.eval.ts +++ b/packages/mcp/eval/tools.eval.ts @@ -3,9 +3,9 @@ * * Exercises every registered tool against a deployed gateway and scores each * response on its objective contract (shape, required fields, error semantics, - * latency, and the `_agent_steering` guidance block). Read-only: the only - * mutating tool, `track_container`, is driven with an already-tracked number so - * it takes the idempotent search-match path and creates nothing. + * latency, and absence of removed steering metadata). Read-only: the only + * mutating tool, `track_container`, is driven with an already-tracked number + * so it takes the idempotent search-match path and creates nothing. * * Opt-in — the whole suite is skipped unless auth is configured: * MCP_EVAL_BEARER= npm run eval --workspace @terminal49/mcp @@ -164,7 +164,6 @@ if (!cfg) { { page_size: 5 }, { requiredKeys: ['items'], - requireSteering: true, predicates: [ { name: 'items is an array', test: (p) => hasArray(p, 'items') }, { @@ -185,7 +184,6 @@ if (!cfg) { { page_size: 5 }, { requiredKeys: ['items'], - requireSteering: true, predicates: [ { name: 'items is an array', test: (p) => hasArray(p, 'items') }, { @@ -209,7 +207,6 @@ if (!cfg) { { page_size: 5 }, { requiredKeys: ['items'], - requireSteering: true, predicates: [ { name: 'items is an array', test: (p) => hasArray(p, 'items') }, { @@ -235,7 +232,6 @@ if (!cfg) { {}, { requiredKeys: ['total_lines', 'shipping_lines'], - requireSteering: true, predicates: [ { name: 'shipping_lines is non-empty', @@ -278,7 +274,6 @@ if (!cfg) { { id }, { requiredKeys: ['id', 'container_number', 'status'], - requireSteering: true, predicates: [ { name: 'id round-trips', test: (p) => readString(p, 'id') === id }, ], @@ -297,7 +292,6 @@ if (!cfg) { { id, include_containers: true }, { requiredKeys: ['id', 'bill_of_lading', 'status'], - requireSteering: true, predicates: [ { name: 'id round-trips', test: (p) => readString(p, 'id') === id }, ], @@ -317,7 +311,6 @@ if (!cfg) { { id: fixtures.containerId }, { requiredKeys: ['total_events', 'timeline'], - requireSteering: true, predicates: [ { name: 'timeline is an array', @@ -338,7 +331,6 @@ if (!cfg) { 'get_container_route', { id: fixtures.containerId }, { - requireSteering: true, predicates: [ { name: 'route payload or explained not-found', @@ -356,10 +348,9 @@ if (!cfg) { ], }, ); - // Soft-error responses are still HTTP 200 with a JSON payload + steering. + // Soft-error responses are still HTTP 200 with a JSON payload. expect(result.http).toBe(200); expect(result.payload).toBeDefined(); - expect(result.steering).toBeDefined(); expect(score.contractPass).toBe(true); }); @@ -373,7 +364,6 @@ if (!cfg) { { query: number }, { requiredKeys: ['containers', 'total_results'], - requireSteering: true, predicates: [ { name: 'containers is an array', @@ -404,7 +394,6 @@ if (!cfg) { { number: fixtures.containerNumber }, { requiredKeys: ['tracking_request_created'], - requireSteering: true, predicates: [ { name: 'no tracking request created', @@ -455,7 +444,6 @@ if (!cfg) { { query: 'ZZZZ0000000ZZZZ' }, { requiredKeys: ['containers', 'total_results'], - requireSteering: true, predicates: [ { name: 'total_results is 0',