diff --git a/capability/loadtesting.capability-index.json b/capability/loadtesting.capability-index.json index 75b68345..3f8d1b94 100644 --- a/capability/loadtesting.capability-index.json +++ b/capability/loadtesting.capability-index.json @@ -68,6 +68,9 @@ } ], "intent": "List the caller's load-testing projects, most recently active first — use this to find a projectId or answer 'what projects do I have?'", + "cache": { + "ttlSec": 300 + }, "guidance": [ "projectId from here is required by listLoadTests and createLoadTest.", "Page with cursor + limit; nextCursor is opaque." @@ -156,6 +159,9 @@ } ], "intent": "List the load tests in a project — use this to find a testId or browse tests, optionally filtered by type, framework, tag or recent activity.", + "cache": { + "ttlSec": 300 + }, "guidance": [ "Resolve projectId via listLoadTestProjects first.", "The numeric testId returned here is what getLoadTest and startLoadTestRun need — not a runId.", @@ -754,7 +760,8 @@ "Read-only despite being a POST — nothing is started.", "fitsInQuota / remainingAfterEstimate compare the estimate against current quota.", "For the same check at start time, call startLoadTestRun with dryRun:true.", - "Always use this for a cost or VU-hour estimate of a hypothetical run — it models ramp-up. Do not compute VU-hours by hand from VUs × duration." + "Always use this for a cost or VU-hour estimate of a hypothetical run — it models ramp-up. Do not compute VU-hours by hand from VUs × duration.", + "A protocol (plu) run is billed at a full load-generator pod's capacity (e.g. 1000 VUs for k6), not the VUs it actually uses, and carries a minimum billing floor (5 minutes) — so a small or short plu run can estimate far higher than VUs × duration would suggest. That is expected: report the returned estimatedVuHours as-is rather than second-guessing it as an error." ], "returns": [ "estimatedVuHours", @@ -941,7 +948,9 @@ "guidance": [ "Provide exactly one of projectId or projectName; projectName creates the project if absent.", "Script upload is two-phase: send pendingScriptUpload to get a presigned uploadUrl + s3Key, PUT the file, then call create again with the scriptRef source and s3Key returned in the response's nextStep.", - "Pass idempotencyKey so a retried create does not duplicate the test." + "Pass idempotencyKey so a retried create does not duplicate the test.", + "testType and framework are required and are never inferred: testType is plu (protocol / API load), blu (real-browser load) or hybrid (both); framework is the load tool (k6, jmeter, gatling, locust). If the user has not stated them, ask — do not guess a default.", + "There is no clone capability. To duplicate a test, getLoadTest the source and copy its full config into this create — vuRamp/vus, durationSec, loadGeneratorLocations and slaThresholds included; nothing is inherited from the source, so anything you omit is dropped." ], "returns": [ "testId", @@ -1230,6 +1239,9 @@ } ], "intent": "Discover which metrics, @ aliases and groupBy options a test supports — call this before requesting a report, trend or comparison so metric names are valid.", + "cache": { + "ttlSec": 900 + }, "guidance": [ "Depends only on the test's type; cache the result client-side.", "Use the returned names/aliases in the metrics parameter of report, trends and compare." @@ -1310,7 +1322,8 @@ "guidance": [ "Partial update: send only the fields to change.", "Pass ifVersion for optimistic concurrency; a stale value returns 409 VERSION_CONFLICT.", - "Script replacement uses the same two-phase pendingScriptUpload flow as create." + "Script replacement uses the same two-phase pendingScriptUpload flow as create.", + "config is a partial update, but each field it carries REPLACES that field wholesale — it does not merge. tags overwrites the entire tag set; it does not append. To add a tag to a test (or the same tag across several tests), getLoadTest each one first and send the union under config.tags." ], "returns": [ "testId", @@ -1368,6 +1381,9 @@ } ], "intent": "Get the full configuration of a single load test — use this to inspect a test's settings, script reference, SLA thresholds and children.", + "cache": { + "ttlSec": 300 + }, "guidance": [ "testId is numeric (from listLoadTests).", "Use include to expand config / thresholds / tags / children.", diff --git a/src/tools/capability-registry/register.ts b/src/tools/capability-registry/register.ts index 084384f3..d747c7d8 100644 --- a/src/tools/capability-registry/register.ts +++ b/src/tools/capability-registry/register.ts @@ -27,7 +27,7 @@ import { ResponseSelection, resolveResponses, } from "./index-loader.js"; -import { invoke } from "./resolve.js"; +import { invoke, InvokeResult } from "./resolve.js"; import { searchCapabilities } from "./search.js"; import { Mode } from "./types.js"; @@ -117,6 +117,23 @@ function ok(payload: unknown): CallToolResult { return { content: [{ type: "text", text: JSON.stringify(payload) }] }; } +/** + * A deterministic string for a cache key: object keys sorted at every level, so the same + * arguments in a different order produce the same key. Not a general serialiser — it handles + * exactly what grouped arguments are (nested objects, arrays, primitives). + */ +function stableStringify(value: unknown): string { + if (Array.isArray(value)) return `[${value.map(stableStringify).join(",")}]`; + if (value && typeof value === "object") { + const obj = value as Record; + return `{${Object.keys(obj) + .sort() + .map((key) => `${JSON.stringify(key)}:${stableStringify(obj[key])}`) + .join(",")}}`; + } + return JSON.stringify(value ?? null); +} + function failed(message: string): CallToolResult { return { content: [ @@ -177,6 +194,37 @@ export function addCapabilityRegistryTools( const transport = deps.transport || fetchTransport(); const tools: Record = {}; + // A response cache for reads a capability explicitly marks cacheable, so a task that asks + // for the same stable thing twice pays for one request, not two. It exists because the + // discover→invoke surface has no other memory: nothing else stops an agent re-fetching a + // test's configuration three times in one task. + // + // Scoped to the CALLING CREDENTIAL, not merely to this closure. The remote host may reuse + // one registration across sessions — the same reason `credentialsFor` is read per call and + // never captured — so a key that left the username out could serve one account's read to + // another. Only `mode:"read"` capabilities that declare `cache.ttlSec` are stored, and only + // a complete 2xx answer; a successful write to a product drops that product's entries for + // the writer. Bounded so a long-lived registration cannot grow without limit. + const READ_CACHE_MAX = 256; + const readCache = new Map< + string, + { expiresAt: number; result: InvokeResult } + >(); + const cacheKeyFor = ( + username: string, + product: string, + method: string, + path: string, + args: GroupedArguments, + ) => + `${username}\n${product}\n${method.toUpperCase()} ${path}\n${stableStringify(args)}`; + const dropProductReads = (username: string, product: string) => { + const prefix = `${username}\n${product}\n`; + for (const key of readCache.keys()) { + if (key.startsWith(prefix)) readCache.delete(key); + } + }; + /** Instrumentation in the house style, and never fatal to the call it wraps. */ const track = (name: string) => { try { @@ -464,14 +512,58 @@ export function addCapabilityRegistryTools( } } + const credentials = deps.credentialsFor(); + const cacheable = + capability.mode === "read" && + typeof capability.cache?.ttlSec === "number" && + capability.cache.ttlSec > 0 && + !!credentials?.username; + const cacheKey = cacheable + ? cacheKeyFor( + credentials.username, + product, + input.method, + input.path, + args, + ) + : null; + + if (cacheKey) { + const hit = readCache.get(cacheKey); + if (hit && hit.expiresAt > Date.now()) { + // A repeat of a read already answered this task: the same body, flagged so the + // caller can see it did not need to ask again. The product is not touched. + return ok({ ...hit.result, cached: true }); + } + if (hit) readCache.delete(cacheKey); + } + const result = await invoke( capability, args, await deps.baseUrlFor(product), - deps.credentialsFor(), + credentials, transport, registry.index.products[product]?.auth, ); + + if (capability.mode === "write" && result.ok && credentials?.username) { + // A successful write can change what a later read returns; drop this credential's + // cached reads for the product so the next read re-fetches. + dropProductReads(credentials.username, product); + } + + if (cacheKey && result.ok && result.completed) { + if (readCache.size >= READ_CACHE_MAX) { + const oldest = readCache.keys().next().value; + if (oldest !== undefined) readCache.delete(oldest); + } + readCache.set(cacheKey, { + expiresAt: Date.now() + capability.cache!.ttlSec * 1000, + result, + }); + } + return ok(result); } catch (error) { if (error instanceof InvocationError) return failed(error.message); diff --git a/src/tools/capability-registry/types.ts b/src/tools/capability-registry/types.ts index 9db3fdb1..b87b6724 100644 --- a/src/tools/capability-registry/types.ts +++ b/src/tools/capability-registry/types.ts @@ -101,6 +101,18 @@ export interface Capability { paginated?: boolean; /** The largest page the operation declares. */ max_page_size?: number; + /** + * Opt-in response cache for a read whose answer is stable within a task. + * + * ABSENT MEANS NEVER CACHED — the default, and the only safe one for a volatile read (a + * run's live status, the active-run list, a run report). A capability declares this only + * when repeating it inside one task must return the same thing: a test's configuration or + * its metrics manifest, a project or test listing. Honoured solely for `mode: "read"`; + * ignored on a write. The stored answer is scoped to the calling credential and dropped + * when a write to the same product succeeds — the caching itself lives in `register.ts`, + * not here. + */ + cache?: { ttlSec: number }; /** * Declared responses by status code, values possibly `{$response: "Name"}` references. * diff --git a/tests/tools/capabilityRegistryCache.test.ts b/tests/tools/capabilityRegistryCache.test.ts new file mode 100644 index 00000000..7a3a86cf --- /dev/null +++ b/tests/tools/capabilityRegistryCache.test.ts @@ -0,0 +1,216 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; + +import { addCapabilityRegistryTools } from "../../src/tools/capability-registry/register.js"; +import { CapabilityRegistry } from "../../src/tools/capability-registry/index-loader.js"; +import { RegistryIndex } from "../../src/tools/capability-registry/types.js"; +import type { + HttpResponse, + Transport, +} from "../../src/tools/capability-registry/egress.js"; + +// A product with one cacheable read (declares `cache`), one read that does not, and one +// write — the three shapes the cache has to tell apart. +const INDEX: RegistryIndex = { + schema_version: 1, + build_id: "cache-test", + products: { + lt: { + summary: "Load testing, test double.", + capabilities: [ + { + method: "GET", + path: "/config/{id}", + mode: "read", + entity: "test", + path_params: [{ name: "id", type: "integer", required: true }], + query: [ + { name: "a", type: "integer" }, + { name: "b", type: "integer" }, + ], + cache: { ttlSec: 300 }, + }, + { + method: "GET", + path: "/status/{id}", + mode: "read", + entity: "run", + path_params: [{ name: "id", type: "integer", required: true }], + }, + { + method: "PUT", + path: "/config/{id}", + mode: "write", + entity: "test", + path_params: [{ name: "id", type: "integer", required: true }], + body: [{ name: "name", type: "string" }], + }, + ], + entities: { test: {}, run: {} }, + }, + }, +}; + +interface Harness { + invoke: (input: Record) => Promise>; + gets: () => number; + writes: () => number; + setUser: (username: string) => void; +} + +function build( + respond: (n: number) => HttpResponse = () => ({ + status: 200, + body: { ok: true }, + }), +): Harness { + const server = new McpServer({ name: "cache-test", version: "0" }); + let calls = 0; + let gets = 0; + let writes = 0; + let username = "userA"; + const transport: Transport = async (method) => { + calls += 1; + if (method === "GET") gets += 1; + else writes += 1; + return respond(calls); + }; + const tools = addCapabilityRegistryTools(server, { + registry: new CapabilityRegistry(INDEX), + baseUrlFor: async () => "https://lt.example", + credentialsFor: () => ({ username, accessKey: "k" }), + transport, + }); + return { + invoke: async (input) => { + const result = await ( + tools.invokeEndpoint as unknown as { + handler: ( + i: unknown, + e: unknown, + ) => Promise<{ content: { text: string }[] }>; + } + ).handler(input, {}); + return JSON.parse(result.content[0].text); + }, + gets: () => gets, + writes: () => writes, + setUser: (u) => { + username = u; + }, + }; +} + +const readConfig = (id: number) => ({ + method: "GET", + path: "/config/{id}", + path_params: { id }, +}); + +describe("capability registry read cache", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("serves a repeat of a cacheable read from the cache, flagged, without a second request", async () => { + const h = build(); + const first = await h.invoke(readConfig(1)); + const second = await h.invoke(readConfig(1)); + + expect(h.gets()).toBe(1); + expect(first.cached).toBeUndefined(); + expect(second.cached).toBe(true); + // The body the caller sees is the product's, unchanged. + expect(second.http_response).toEqual(first.http_response); + }); + + it("keys on the arguments, so a different id is a separate request", async () => { + const h = build(); + await h.invoke(readConfig(1)); + await h.invoke(readConfig(2)); + expect(h.gets()).toBe(2); + }); + + it("ignores argument order when keying", async () => { + const h = build(); + await h.invoke({ + method: "GET", + path: "/config/{id}", + path_params: { id: 1 }, + query: { a: 1, b: 2 }, + }); + await h.invoke({ + method: "GET", + path: "/config/{id}", + query: { b: 2, a: 1 }, + path_params: { id: 1 }, + }); + expect(h.gets()).toBe(1); + }); + + it("does not cache a read the capability has not marked cacheable", async () => { + const h = build(); + await h.invoke({ + method: "GET", + path: "/status/{id}", + path_params: { id: 1 }, + }); + await h.invoke({ + method: "GET", + path: "/status/{id}", + path_params: { id: 1 }, + }); + expect(h.gets()).toBe(2); + }); + + it("drops the product's cached reads after a successful write", async () => { + const h = build(); + await h.invoke(readConfig(1)); // miss -> stored + await h.invoke(readConfig(1)); // hit + expect(h.gets()).toBe(1); + + await h.invoke({ + method: "PUT", + path: "/config/{id}", + path_params: { id: 1 }, + body: { name: "x" }, + user_permission: "granted", + change_summary: "rename", + }); + expect(h.writes()).toBe(1); + + await h.invoke(readConfig(1)); // cache was invalidated -> re-fetch + expect(h.gets()).toBe(2); + }); + + it("scopes the cache to the calling credential", async () => { + const h = build(); + h.setUser("userA"); + await h.invoke(readConfig(1)); // stored under userA + h.setUser("userB"); + const other = await h.invoke(readConfig(1)); // must not see userA's entry + expect(h.gets()).toBe(2); + expect(other.cached).toBeUndefined(); + h.setUser("userA"); + await h.invoke(readConfig(1)); // userA still hits the cache + expect(h.gets()).toBe(2); + }); + + it("does not cache a non-2xx read", async () => { + const h = build((n) => ({ status: 500, body: { error: `boom ${n}` } })); + await h.invoke(readConfig(1)); + await h.invoke(readConfig(1)); + expect(h.gets()).toBe(2); + }); + + it("re-fetches once the entry is past its ttl", async () => { + vi.useFakeTimers(); + const h = build(); + await h.invoke(readConfig(1)); // stored, ttl 300s + await h.invoke(readConfig(1)); // hit + expect(h.gets()).toBe(1); + vi.advanceTimersByTime(301_000); + await h.invoke(readConfig(1)); // expired -> re-fetch + expect(h.gets()).toBe(2); + }); +});