Compare commits

..

6 Commits

Author SHA1 Message Date
Michal
22d8e13390 fix(smoke): match the _resultId format the pipeline actually emits
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m23s
CI/CD / lint (pull_request) Successful in 2m19s
CI/CD / test (pull_request) Successful in 1m41s
CI/CD / smoke (pull_request) Failing after 2m6s
CI/CD / build (pull_request) Successful in 2m22s
CI/CD / publish (pull_request) Has been skipped
The drill-down checks matched /_resultId:\s*(\S+)/ — the legacy paginator's
JSON spelling. The content pipeline writes _resultId="pm-abc", so the regex
found nothing: "Has _resultId for drill-down" reported a false failure on
every run, and the drill-down tests that used the same regex to extract an id
skipped themselves as "not large enough for section-split". Both were
invisible in a green suite.

One shared extractResultId() in the smoke harness now accepts either
spelling and returns the bare id, so the ad-hoc character strip on the
captured group goes away too. proxy-pipeline's check block reports 21/21
instead of 20 with a phantom failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQr5Z9gYrqBQXTGBBuemZ2
2026-09-05 18:03:56 +01:00
b5d0234d0b Merge pull request #129: drill-down required params
Some checks failed
CI/CD / typecheck (push) Successful in 1m22s
CI/CD / lint (push) Successful in 2m13s
CI/CD / test (push) Has started running
CI/CD / smoke (push) Has been cancelled
CI/CD / build (push) Has been cancelled
CI/CD / publish (push) Has been cancelled
2026-09-05 16:59:46 +00:00
Michal
0e12637271 fix(mcplocal): stop upstream required params from blocking a drill-down
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m27s
CI/CD / lint (pull_request) Successful in 2m22s
CI/CD / test (pull_request) Successful in 1m31s
CI/CD / smoke (pull_request) Failing after 2m41s
CI/CD / build (pull_request) Successful in 2m21s
CI/CD / publish (pull_request) Has been skipped
A paginated tool result tells the model to call the same tool again with
only _resultId/_section. The advertised schema still carried the upstream's
own `required` list, so a client that validates arguments rejected exactly
that call — "Received tool input did not match expected schema" — and the
model concluded pagination was broken and gave up
(websearch/fetch_content requires `url`; verified live on llm-model-tester).

`required` is ANDed with everything else in a schema, so no property
declaration could rescue it. It now becomes an alternation: either the
upstream's requirements (a fresh call) or `_resultId` (a re-read, which is
answered from cache and never reaches the upstream). Validators that
understand anyOf enforce that; naive ones that only read top-level
`required` now find none and accept both. Schemas that already carry a
combinator get the alternation appended to `allOf`, and a schema with no
required params is left untouched.

Covered by unit tests on a required-bearing upstream schema plus a smoke
assertion that no advertised tool keeps a drill-down-blocking `required`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JQr5Z9gYrqBQXTGBBuemZ2
2026-09-05 17:58:57 +01:00
33d7007af5 Merge pull request #128: learn sampling-param rejection
Some checks failed
CI/CD / typecheck (push) Successful in 1m20s
CI/CD / lint (push) Successful in 2m32s
CI/CD / test (push) Successful in 1m26s
CI/CD / smoke (push) Failing after 2m3s
CI/CD / build (push) Successful in 4m34s
CI/CD / publish (push) Has been skipped
2026-08-25 23:12:08 +00:00
bc7eb5a0ad fix(mcplocal): learn which models reject sampling params, don't hardcode them
Some checks failed
CI/CD / typecheck (pull_request) Successful in 1m22s
CI/CD / lint (pull_request) Successful in 2m34s
CI/CD / test (pull_request) Successful in 1m28s
CI/CD / build (pull_request) Successful in 2m27s
CI/CD / smoke (pull_request) Failing after 3m6s
CI/CD / publish (pull_request) Has been skipped
Auto-following to the newest Opus moved the failure rather than removing it.
The deploy's own smoke output showed it: 404 "model not found" became

  HTTP 400: `temperature` is deprecated for this model.

Anthropic removed temperature/top_p/top_k on the current generation (Opus 5,
Sonnet 5, Opus 4.7/4.8, Fable 5), and the adapter sends temperature: 0
unconditionally. So the gate's prompt-selection was still degrading on every
call -- just with a different status code.

A list of which models accept sampling would rot exactly the way the pinned
model ids did, which is the whole thing this branch is trying to stop. So the
provider learns it instead: the first 400 naming a sampling parameter drops it
and retries, and remembers the model so every later call omits it up front.
One wasted call, once, rather than a hardcoded table to maintain.

The match is deliberately narrow -- a 400 must actually name temperature/top_p/
top_k. An unrelated 400 (missing max_tokens, bad schema) propagates untouched;
there is a test for that, because a broad match here would silently swallow
real request errors and retry them pointlessly.

Verified live: claude-opus-latest -> claude-opus-5, first attempt 400 on
temperature, retry succeeds past it. The retry then hit HTTP 429 -- the
personal OAuth token's rate limit, which is the pre-existing credential-tiering
issue, not this path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2
2026-08-26 00:11:50 +01:00
cd20d8b980 Merge pull request #127: bounded, legible MCP failures
Some checks failed
CI/CD / typecheck (push) Successful in 1m24s
CI/CD / lint (push) Successful in 2m32s
CI/CD / test (push) Successful in 1m32s
CI/CD / smoke (push) Has started running
CI/CD / publish (push) Has been cancelled
CI/CD / build (push) Has been cancelled
2026-08-25 23:03:50 +00:00
8 changed files with 285 additions and 27 deletions

View File

@@ -30,6 +30,15 @@ function familyOf(model: string): string | null {
return null;
}
/**
* A 400 naming a sampling parameter, e.g.
* "`temperature` is deprecated for this model."
*/
function isSamplingRejection(err: unknown): boolean {
const msg = err instanceof Error ? err.message : String(err);
return msg.includes('HTTP 400') && /`?(temperature|top_p|top_k)`?/.test(msg);
}
/** Resolutions are cached this long before the models endpoint is consulted again. */
const MODEL_CACHE_TTL_MS = Number(process.env['MCPCTL_ANTHROPIC_MODEL_TTL_MS']) || 12 * 60 * 60 * 1000;
@@ -40,6 +49,17 @@ export class AnthropicProvider implements LlmProvider {
readonly name = 'anthropic';
/** Shared across instances: the model list is account-wide, not per-provider. */
private static readonly modelCache = new Map<string, { id: string; expiresAt: number }>();
/**
* Models that rejected a sampling parameter, learned at runtime.
*
* Anthropic removed `temperature`/`top_p`/`top_k` on the current generation
* (Opus 5, Sonnet 5, Opus 4.7/4.8, Fable 5) — sending one is a hard 400. A
* hardcoded list of which models accept it would rot exactly the way the
* pinned model ids did, so learn it from the API instead: the first call
* retries without the parameter and remembers, and every later call omits it
* up front.
*/
private static readonly rejectsSampling = new Set<string>();
private apiKey: string;
private defaultModel: string;
@@ -66,7 +86,9 @@ export class AnthropicProvider implements LlmProvider {
if (systemMessages.length > 0) {
body.system = systemMessages.map((m) => m.content).join('\n');
}
if (options.temperature !== undefined) body.temperature = options.temperature;
if (options.temperature !== undefined && !AnthropicProvider.rejectsSampling.has(model)) {
body.temperature = options.temperature;
}
if (options.tools && options.tools.length > 0) {
body.tools = options.tools.map((t) => ({
@@ -76,8 +98,19 @@ export class AnthropicProvider implements LlmProvider {
}));
}
const response = await this.request(body, options.signal);
return parseAnthropicResponse(response);
try {
return parseAnthropicResponse(await this.request(body, options.signal));
} catch (err) {
// Learn-and-retry rather than fail: a model that has dropped sampling
// support should cost one wasted call, once, not every call forever.
if (body.temperature !== undefined && isSamplingRejection(err)) {
AnthropicProvider.rejectsSampling.add(model);
delete body.temperature;
process.stderr.write(`[anthropic] ${model} rejects sampling params — retrying without\n`);
return parseAnthropicResponse(await this.request(body, options.signal));
}
throw err;
}
}
/**

View File

@@ -137,9 +137,24 @@ function drillDownInstruction(toolName: string, resultId: string, sections: Sect
/**
* Declare the drill-down params on a tool's advertised input schema.
*
* Only `properties` is extended: `additionalProperties: false` stays as the
* `properties` is extended: `additionalProperties: false` stays as the
* upstream set it, because a property listed in `properties` is allowed by
* that keyword. Keeping it false preserves the upstream's typo protection.
*
* `required` also has to give way, or the contract is still unsatisfiable. A
* drill-down call carries only _resultId/_section — it never reaches the
* upstream, so the upstream's own required params (websearch/fetch_content
* requires `url`) are meaningless for it. Left in place, a client that
* validates arguments against the schema rejects the very call the stub just
* instructed the model to make ("Received tool input did not match expected
* schema"), and the model concludes pagination is broken.
*
* So the flat `required` becomes an alternation: either the upstream's
* requirements (a fresh call) or `_resultId` (a re-read of a cached result).
* A validator that understands `anyOf` enforces exactly that; a naive one that
* only looks at top-level `required` now finds none and accepts both. The
* upstream still rejects a fresh call that omits its required params, and the
* requirements stay visible to the model inside the alternation.
*/
function withDrillDownParams(tool: ToolDefinition): ToolDefinition {
if (GATE_TOOLS.has(tool.name)) return tool;
@@ -171,7 +186,42 @@ function withDrillDownParams(tool: ToolDefinition): ToolDefinition {
+ 'previous large result. Requires _resultId.',
};
return { ...tool, inputSchema: { ...s, properties: props } };
return { ...tool, inputSchema: withDrillDownRequired({ ...s, properties: props }) };
}
/**
* Rewrite a flat `required` into "upstream's requirements OR a drill-down".
*
* Schemas that already carry a top-level combinator keep it: the alternation
* is appended to `allOf` instead of colliding with the existing `anyOf`/
* `oneOf`. A schema with no `required` needs nothing — every call already
* validates.
*/
function withDrillDownRequired(s: Record<string, unknown>): Record<string, unknown> {
const required = s['required'];
const upstreamRequired = Array.isArray(required)
? required.filter((r): r is string => typeof r === 'string')
: [];
if (upstreamRequired.length === 0) return s;
const alternation = {
anyOf: [
{ required: upstreamRequired },
// _resultId alone is a real call too: it re-shows the table of contents.
{ required: ['_resultId'] },
],
};
const next = { ...s };
delete next['required'];
if ('anyOf' in next || 'oneOf' in next || 'allOf' in next) {
const existing = Array.isArray(next['allOf']) ? next['allOf'] : [];
next['allOf'] = [...existing, alternation];
return next;
}
return { ...next, ...alternation };
}
/** Build a single-text-part tool result. */

View File

@@ -81,3 +81,68 @@ describe('Anthropic model resolution', () => {
await expect(providerWith(MODELS).listModels()).resolves.toContain('claude-opus-5');
});
});
describe('sampling-parameter rejection', () => {
afterEach(() => {
(AnthropicProvider as unknown as { rejectsSampling: Set<string> }).rejectsSampling.clear();
});
/** Rejects `temperature` exactly as the current Anthropic models do. */
function samplingStrictProvider(): { provider: AnthropicProvider; bodies: Array<Record<string, unknown>> } {
const provider = new AnthropicProvider({ apiKey: 'sk-ant-api-test' });
const bodies: Array<Record<string, unknown>> = [];
(provider as unknown as { request: (b: unknown) => Promise<unknown> }).request = async (b) => {
const body = b as Record<string, unknown>;
bodies.push({ ...body });
if (body.temperature !== undefined) {
throw new Error(
'Anthropic HTTP 400: {"type":"error","error":{"type":"invalid_request_error",'
+ '"message":"`temperature` is deprecated for this model."}}',
);
}
return { content: [{ type: 'text', text: 'ok' }], stop_reason: 'end_turn' };
};
return { provider, bodies };
}
it('retries without temperature when the model rejects it', async () => {
vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
const { provider, bodies } = samplingStrictProvider();
const result = await provider.complete({
model: 'claude-opus-5',
messages: [{ role: 'user', content: 'hi' }],
temperature: 0,
});
expect(result.content).toBe('ok');
expect(bodies).toHaveLength(2);
expect(bodies[0]!.temperature).toBe(0);
expect(bodies[1]!.temperature).toBeUndefined();
});
it('remembers, so the wasted call happens once and not forever', async () => {
vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
const { provider, bodies } = samplingStrictProvider();
const req = { model: 'claude-opus-5', messages: [{ role: 'user' as const, content: 'hi' }], temperature: 0 };
await provider.complete(req);
await provider.complete(req);
await provider.complete(req);
// 2 for the first call (reject + retry), then 1 each — not 2 each.
expect(bodies).toHaveLength(4);
});
it('does not swallow unrelated 400s', async () => {
const provider = new AnthropicProvider({ apiKey: 'sk-ant-api-test' });
(provider as unknown as { request: () => Promise<unknown> }).request = async () => {
throw new Error('Anthropic HTTP 400: {"error":{"message":"max_tokens is required"}}');
};
await expect(provider.complete({
model: 'claude-opus-5',
messages: [{ role: 'user', content: 'hi' }],
temperature: 0,
})).rejects.toThrow(/max_tokens/);
});
});

View File

@@ -33,6 +33,20 @@ const STRICT_SCHEMA = {
$schema: 'http://json-schema.org/draft-07/schema#',
};
/**
* Mirrors websearch/fetch_content: a required param that a drill-down call
* cannot supply, since the drill-down never reaches the upstream.
*/
const REQUIRED_SCHEMA = {
type: 'object',
properties: {
url: { type: 'string' },
site: { type: 'string' },
},
required: ['url'],
additionalProperties: false,
};
const BIG_PAYLOAD = 'x'.repeat(20_000);
/**
@@ -74,7 +88,11 @@ interface Upstream {
calls: Array<Record<string, unknown>>;
}
function mockUpstream(name: string, payloads: Record<string, string> = PAYLOADS): Upstream {
function mockUpstream(
name: string,
payloads: Record<string, string> = PAYLOADS,
schema: unknown = STRICT_SCHEMA,
): Upstream {
const calls: Array<Record<string, unknown>> = [];
const conn = {
name,
@@ -90,7 +108,7 @@ function mockUpstream(name: string, payloads: Record<string, string> = PAYLOADS)
tools: Object.keys(payloads).map((n) => ({
name: n,
description: `Retrieve ${n}`,
inputSchema: STRICT_SCHEMA,
inputSchema: schema,
})),
},
};
@@ -120,7 +138,7 @@ function mockMcpdClient(): McpdClient {
} as unknown as McpdClient;
}
function setup(opts: { gated?: boolean; payloads?: Record<string, string> } = {}) {
function setup(opts: { gated?: boolean; payloads?: Record<string, string>; schema?: unknown } = {}) {
const router = new McpRouter();
router.setPromptConfig(mockMcpdClient(), 'test-project');
router.setPlugin(createDefaultPlugin({ gated: opts.gated ?? false, providerRegistry: null }));
@@ -129,7 +147,7 @@ function setup(opts: { gated?: boolean; payloads?: Record<string, string> } = {}
{ complete: async () => '', available: () => false } as unknown as LLMProviderAdapter,
new MemoryCache(),
);
const upstream = mockUpstream('unifi-network', opts.payloads ?? PAYLOADS);
const upstream = mockUpstream('unifi-network', opts.payloads ?? PAYLOADS, opts.schema ?? STRICT_SCHEMA);
router.addUpstream(upstream.conn);
return { router, upstream };
}
@@ -187,6 +205,63 @@ describe('content-pipeline drill-down contract', () => {
expect(schema['additionalProperties']).toBe(false);
});
it('does not leave the upstream\'s required params blocking a drill-down call', async () => {
// websearch/fetch_content requires `url`. A drill-down carries only
// _resultId/_section, so a client validating against a flat
// `required: ["url"]` rejected the exact call the stub asked for.
const { router } = setup({ schema: REQUIRED_SCHEMA });
const tool = (await listTools(router)).find((t) => t.name.endsWith('get_devices'));
const schema = tool!.inputSchema as Record<string, unknown>;
expect(schema['required']).toBeUndefined();
expect(schema['anyOf']).toEqual([
{ required: ['url'] },
{ required: ['_resultId'] },
]);
// The upstream's requirement is preserved, not dropped: a fresh call still
// has to carry `url`.
expect((schema['properties'] as Record<string, unknown>)['url']).toMatchObject({ type: 'string' });
});
it('serves a drill-down that omits the upstream\'s required params', async () => {
const { router, upstream } = setup({ schema: REQUIRED_SCHEMA });
await listTools(router);
const stub = textOf(await callTool(router, { url: 'https://example.com' }, 3));
const resultId = /_resultId="([^"]+)"/.exec(stub)?.[1];
const callsAfterFirst = upstream.calls.length;
const page = textOf(await callTool(router, { _resultId: resultId!, _section: 'page-1' }, 4));
expect(page).toContain('x'.repeat(100));
expect(upstream.calls.length).toBe(callsAfterFirst);
});
it('leaves a schema without required params alone', async () => {
const { router } = setup();
const tool = (await listTools(router)).find((t) => t.name.endsWith('get_devices'));
const schema = tool!.inputSchema as Record<string, unknown>;
expect(schema['anyOf']).toBeUndefined();
expect(schema['allOf']).toBeUndefined();
});
it('composes with a schema that already carries a combinator', async () => {
const { router } = setup({
schema: { ...REQUIRED_SCHEMA, oneOf: [{ required: ['site'] }] },
});
const tool = (await listTools(router)).find((t) => t.name.endsWith('get_devices'));
const schema = tool!.inputSchema as Record<string, unknown>;
// The upstream's own combinator survives; ours is appended, not merged
// into it.
expect(schema['oneOf']).toEqual([{ required: ['site'] }]);
expect(schema['required']).toBeUndefined();
expect(schema['allOf']).toEqual([
{ anyOf: [{ required: ['url'] }, { required: ['_resultId'] }] },
]);
});
it('leaves gate tools alone (they never reach the pipeline)', async () => {
const { router } = setup({ gated: true });
const tools = await listTools(router);

View File

@@ -261,6 +261,21 @@ export async function isMcplocalRunning(): Promise<boolean> {
}
}
/**
* Pull the drill-down id out of a paginated result or TOC.
*
* The instruction is written `_resultId="pm-abc"`, while the legacy paginator
* emits it as JSON (`"_resultId": "pm-abc"`). A regex matching only the JSON
* form silently found nothing in the current format, so drill-down checks
* either reported a false failure or skipped themselves as "not large enough
* for section-split" — both invisible in a green run. Accept either spelling
* and return the bare id.
*/
export function extractResultId(text: string): string | null {
const match = /_resultId"?\s*[:=]\s*"?(pm-[a-zA-Z0-9]+)/.exec(text);
return match?.[1] ?? null;
}
/**
* Run an mcpctl CLI command and return stdout.
*/

View File

@@ -7,7 +7,7 @@
* Requires: mcplocal running on localhost:3200, mcpd at https://mcpctl.ad.itaz.eu
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { SmokeMcpSession, isMcplocalRunning } from './mcp-client.js';
import { SmokeMcpSession, isMcplocalRunning, extractResultId } from './mcp-client.js';
const PROJECT_NAME = 'smoke-data';
@@ -90,9 +90,8 @@ describe('Smoke: Prompt section drill-down', () => {
console.log(` TOC preview: ${text.slice(0, 200)}...`);
// Extract _resultId
const match = /_resultId:\s*(pm-[a-z0-9]+)/.exec(text);
if (match) {
const resultId = match[1];
const resultId = extractResultId(text);
if (resultId !== null) {
// Extract first section id from TOC
const sectionMatch = /\[([^\]]+)\]/.exec(text);
if (sectionMatch) {

View File

@@ -17,7 +17,7 @@
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { writeFile, mkdir, rm } from 'node:fs/promises';
import { join, resolve } from 'node:path';
import { SmokeMcpSession, isMcplocalRunning, mcpctl } from './mcp-client.js';
import { SmokeMcpSession, isMcplocalRunning, mcpctl, extractResultId } from './mcp-client.js';
import { ChatReporter } from './reporter.js';
const PROJECT_NAME = 'smoke-data';
@@ -286,8 +286,7 @@ describe('Smoke: ProxyModel pipeline', () => {
chat.check('Response is manageable size', text.length, (v) => v < 20_000);
if (text.includes('_resultId')) {
const match = text.match(/_resultId:\s*(\S+)/);
chat.check('_resultId is present', !!match, (v) => v === true);
chat.check('_resultId is present', extractResultId(text) !== null, (v) => v === true);
} else {
chat.info('Content small enough — no pagination needed');
}
@@ -307,14 +306,12 @@ describe('Smoke: ProxyModel pipeline', () => {
});
const text = result.content[0]?.text ?? '';
const match = text.match(/_resultId:\s*(\S+)/);
if (!match) {
const resultId = extractResultId(text);
if (resultId === null) {
chat.info('Content not large enough for pagination — skip drill-down');
return;
}
const resultId = match[1]!.replace(/[^a-zA-Z0-9-]/g, '');
const sectionResult = await chat.callTool('smoke-aws-docs_read_documentation', {
url: 'https://docs.aws.amazon.com/AmazonS3/latest/userguide/Welcome.html',
_resultId: resultId,
@@ -401,8 +398,7 @@ describe('Smoke: ProxyModel pipeline', () => {
chat.check('Response is manageable size', text.length, (v) => v < 20_000);
if (text.includes('_resultId')) {
const match = text.match(/_resultId:\s*(\S+)/);
chat.check('Has _resultId for drill-down', !!match, (v) => v === true);
chat.check('Has _resultId for drill-down', extractResultId(text) !== null, (v) => v === true);
}
}, 60_000);
@@ -420,14 +416,12 @@ describe('Smoke: ProxyModel pipeline', () => {
});
const text = result.content[0]?.text ?? '';
const match = text.match(/_resultId:\s*(\S+)/);
if (!match) {
const resultId = extractResultId(text);
if (resultId === null) {
chat.info('Content not large enough for section-split');
return;
}
const resultId = match[1]!.replace(/[^a-zA-Z0-9-]/g, '');
const sectionResult = await chat.callTool('smoke-aws-docs_read_documentation', {
url: 'https://docs.aws.amazon.com/AmazonS3/latest/userguide/Welcome.html',
_resultId: resultId,

View File

@@ -20,9 +20,15 @@ const TOKEN = process.env['SMOKE_MCPTOKEN'];
interface Tool {
name: string;
inputSchema?: { type?: string; properties?: Record<string, unknown>; additionalProperties?: unknown };
inputSchema?: {
type?: string;
properties?: Record<string, unknown>;
additionalProperties?: unknown;
required?: unknown;
};
}
/** Tools served by the gate plugin — intercepted before the pipeline, so exempt. */
const GATE_TOOLS = new Set(['begin_session', 'read_prompts', 'propose_prompt', 'propose_skill']);
@@ -97,6 +103,27 @@ describe('Smoke: tool drill-down contract', () => {
console.log(` ${strict.length} strict-schema tools keep additionalProperties: false`);
});
it('required params never block a drill-down call', async () => {
if (!available) return;
// A drill-down carries only _resultId/_section. If the advertised schema
// still demands the upstream's own required params (websearch's `url`), a
// validating client rejects the call the stub just asked for — "Received
// tool input did not match expected schema".
// A top-level `required` is ANDed with everything else in the schema, so
// any entry there is unsatisfiable for a drill-down. The upstream's
// requirements live in an `anyOf` alternation instead.
const blocked = tools
.filter((t) => {
if (GATE_TOOLS.has(t.name)) return false;
const required = t.inputSchema?.required;
return Array.isArray(required) && required.length > 0;
})
.map((t) => t.name);
expect(blocked).toEqual([]);
});
it('gate tools do not advertise drill-down params', async () => {
if (!available) return;