diff --git a/src/cache/hash.ts b/src/cache/hash.ts index 7f87385..57e50fc 100644 --- a/src/cache/hash.ts +++ b/src/cache/hash.ts @@ -20,6 +20,51 @@ export function canonicalJson(value: unknown): string { }); } +/** SHA-256 hex digest of a string. */ +async function sha256Hex(input: string): Promise { + const data = new TextEncoder().encode(input); + const buf = await crypto.subtle.digest("SHA-256", data); + return Array.from(new Uint8Array(buf)) + .map((b) => b.toString(16).padStart(2, "0")) + .join(""); +} + +/** + * Aggregate fingerprint of a service's whole schema surface. + * + * Order-independent: hashes the sorted list of per-tool hashes, so the same set + * of tools always yields the same fingerprint regardless of listing order. A + * change to ANY tool's surface (including a new inputSchema field like + * `create_if_missing`) changes the fingerprint. This is the fallback signal for + * servers that do not publish an authoritative contract hash; for servers that + * do (e.g. Open Brain's `get_contract.schema_hash`), prefer that value. + */ +export async function fingerprintSchemas( + tools: { hash: string }[], +): Promise { + const joined = tools + .map((t) => t.hash) + .sort() + .join("|"); + return sha256Hex(joined); +} + +/** + * Placeholder some call sites substitute for a missing description before they + * reach the hasher. Normalized to "" so the fingerprint is identical whether a + * writer passed `undefined`, "", or this placeholder -- otherwise the daemon + * (which hashes `description ?? ""`) and the client warm path (which hashes the + * already-defaulted "(no description)") would produce divergent fingerprints + * for the same tool, clearing the cache on every read. See #58. + */ +const MISSING_DESCRIPTION = "(no description)"; + +/** Canonical, writer-independent description used for hashing. */ +function normalizeDescription(description?: string): string { + if (!description || description === MISSING_DESCRIPTION) return ""; + return description; +} + /** * Compute SHA-256 hash of a tool's schema surface. * The "surface" is: name + description + inputSchema + annotations. @@ -33,7 +78,7 @@ export async function hashToolSchema(tool: { }): Promise { const surface = canonicalJson({ name: tool.name, - description: tool.description ?? "", + description: normalizeDescription(tool.description), inputSchema: tool.inputSchema, annotations: tool.annotations ?? null, }); diff --git a/src/cache/index.ts b/src/cache/index.ts index 1990774..91d26c5 100644 --- a/src/cache/index.ts +++ b/src/cache/index.ts @@ -10,7 +10,7 @@ export type { DriftResult, } from "./types.ts"; -export { canonicalJson, hashToolSchema } from "./hash.ts"; +export { canonicalJson, hashToolSchema, fingerprintSchemas } from "./hash.ts"; export { getCacheDir, @@ -19,6 +19,8 @@ export { readCacheRaw, writeCache, clearCache, + clearServiceCacheKeys, + readCacheFingerprint, listCachedServices, isCacheExpired, resolveTtlMs, diff --git a/src/cache/storage.ts b/src/cache/storage.ts index 723ab42..c4968ff 100644 --- a/src/cache/storage.ts +++ b/src/cache/storage.ts @@ -7,6 +7,7 @@ import { mkdir, unlink, readdir, rename } from "node:fs/promises"; import { dirname, join } from "node:path"; import { createLogger } from "../logger/index.ts"; import type { CacheEntry, CacheMetadata, CachedToolSchema } from "./types.ts"; +import { fingerprintSchemas } from "./hash.ts"; const log = createLogger("cache"); @@ -126,6 +127,13 @@ export async function writeCache( service: string, tools: CachedToolSchema[], ttlMs: number = DEFAULT_TTL_MS, + /** + * Authoritative contract fingerprint from a server that publishes one + * (e.g. Open Brain's `get_contract.schema_hash`). When omitted, the + * fingerprint is derived from the tool surface hashes so staleness can still + * be detected for servers without a contract. + */ + contractFingerprint?: string, ): Promise { const filePath = getCacheFilePath(service); const dir = dirname(filePath); @@ -133,12 +141,16 @@ export async function writeCache( // Ensure cache directory exists await mkdir(dir, { recursive: true }); + const schemaFingerprint = + contractFingerprint ?? (await fingerprintSchemas(tools)); + const entry: CacheEntry = { metadata: { service, cachedAt: new Date().toISOString(), ttlMs, toolCount: tools.length, + schemaFingerprint, }, tools, }; @@ -196,6 +208,81 @@ export async function clearCache(service?: string): Promise { return cleared; } +/** + * Clear every cache key belonging to a service: the bare `.json` + * entry AND all per-credential entries `credential:.json`. + * + * A contract bump must invalidate all of these together -- otherwise the base + * key looks refreshed while a user's credential-scoped read still serves the + * old schema (the #58 failure). Credential keys encode the service name as the + * first element of the base64url-encoded `[service, userId]` tuple, so we decode + * each and match. Returns the number of files removed. + */ +export async function clearServiceCacheKeys(service: string): Promise { + validateServiceName(service); + const cacheDir = getCacheDir(); + let cleared = 0; + + let entries: string[]; + try { + entries = await readdir(cacheDir); + } catch { + return 0; + } + + for (const entry of entries) { + if (!entry.endsWith(".json")) continue; + const key = entry.replace(/\.json$/, ""); + + let matches = key === service; + if (!matches && key.startsWith("credential:")) { + const encoded = key.slice("credential:".length); + try { + const decoded = JSON.parse( + Buffer.from(encoded, "base64url").toString("utf8"), + ); + matches = Array.isArray(decoded) && decoded[0] === service; + } catch { + // Not a decodable credential key -- leave it alone. + matches = false; + } + } + + if (matches) { + // Count only files actually removed. A swallowed unlink that still + // incremented the count would report success while a stale credential + // entry survives -- silently re-opening the #58 staleness it exists to fix. + try { + await unlink(join(cacheDir, entry)); + cleared++; + } catch (err) { + log.warn("cache_unlink_failed", { + service, + entry, + error: err instanceof Error ? err.message : String(err), + }); + } + } + } + + if (cleared > 0) { + log.info("cache_keys_cleared", { service, count: cleared }); + } + return cleared; +} + +/** + * Read just the schema fingerprint for a cached service key, ignoring TTL. + * Returns undefined when there is no cache entry or it predates fingerprinting. + * Used to stamp daemon responses for #58 client-side cache coherence. + */ +export async function readCacheFingerprint( + service: string, +): Promise { + const entry = await readCacheRaw(service); + return entry?.metadata.schemaFingerprint; +} + /** * List all cached services. * Returns service names that have cache files. diff --git a/src/cache/types.ts b/src/cache/types.ts index b791591..aa3b7a6 100644 --- a/src/cache/types.ts +++ b/src/cache/types.ts @@ -23,6 +23,14 @@ export interface CacheMetadata { ttlMs: number; /** Number of tools cached */ toolCount: number; + /** + * Deterministic fingerprint of the full schema surface (every tool's + * name + description + inputSchema + annotations) at write time. Lets the + * cache detect a contract change independent of the TTL: when a live fetch + * produces a different fingerprint, the cache is stale and must be refetched. + * Optional for backward compatibility with entries written before this field. + */ + schemaFingerprint?: string; } /** A complete cache entry -- metadata + tool schemas */ diff --git a/src/cli/commands/cache.ts b/src/cli/commands/cache.ts index 0a85004..012c8e1 100644 --- a/src/cli/commands/cache.ts +++ b/src/cli/commands/cache.ts @@ -41,6 +41,11 @@ export const handleCache: CommandHandler = async (args: string[]) => { " status Show cache status for all services", " diff Compare cached vs live schemas for a service", " warm [service] Fetch and cache schemas (all or specific service)", + "", + "OPTIONS:", + " --force (warm) clear the existing cache entry before refetching.", + " Use this after an upstream contract/schema bump -- the", + " cache otherwise serves the old schema until its TTL.", ].join("\n"), ); process.exitCode = subcommand @@ -132,7 +137,9 @@ async function handleCacheDiff(args: string[]): Promise { } async function handleCacheWarm(args: string[]): Promise { - const targetService = args[0]; + const force = args.includes("--force"); + const positional = args.filter((a) => !a.startsWith("--")); + const targetService = positional[0]; const config = await loadConfig(); const serviceNames = targetService ? [targetService] @@ -153,6 +160,11 @@ async function handleCacheWarm(args: string[]): Promise { const service = config.services[serviceName]!; console.log(` warming ${serviceName}...`); try { + // --force: drop the stale entry first so a refetch failure can't leave the + // old schema in place. The recommended recovery after a contract bump. + if (force) { + await clearCache(serviceName); + } const result = await Promise.race([ (async () => { const { cachedSchemas } = await discoverServiceSchemas( diff --git a/src/daemon/drift-hook.ts b/src/daemon/drift-hook.ts index 84aef15..58e8f9d 100644 --- a/src/daemon/drift-hook.ts +++ b/src/daemon/drift-hook.ts @@ -5,7 +5,7 @@ * ADV-06: Triggers auto-regeneration of skill files when drift is detected. */ import type { McpConnection } from "../connection/types.ts"; -import { readCacheRaw, writeCache, detectDrift, resolveTtlMs, mapToolsToCachedSchemas } from "../cache/index.ts"; +import { readCacheRaw, writeCache, detectDrift, resolveTtlMs, mapToolsToCachedSchemas, clearServiceCacheKeys } from "../cache/index.ts"; import type { AccessPolicy } from "../access/types.ts"; import { listAllTools } from "../schema/introspect.ts"; import { autoRegenerateSkills } from "../generation/auto-regen.ts"; @@ -54,6 +54,20 @@ export async function checkDriftOnConnect( changeCount: drift.changes.length, }); + // #58: a contract change must invalidate EVERY cache key for this + // service -- the bare entry AND all per-credential keys -- not just the + // base entry rewritten below. Otherwise a user's credential-scoped read + // keeps serving the old schema after the bump. Clear them all here; the + // base entry is repopulated immediately below, and credential entries + // refill on their next read. + const clearedKeys = await clearServiceCacheKeys(serviceName); + if (clearedKeys > 0) { + log.info("drift_cache_invalidated", { + service: serviceName, + keysCleared: clearedKeys, + }); + } + // ADV-06: Auto-regenerate skill files on drift const toolSummaries = liveTools.map((t) => ({ name: t.name, diff --git a/src/daemon/server.ts b/src/daemon/server.ts index 938b411..39adc46 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -22,6 +22,7 @@ import { resolveToolNameCached, } from "../schema/cached.ts"; import { getToolSchema, listToolsForService } from "../schema/introspect.ts"; +import { readCacheFingerprint } from "../cache/index.ts"; import { auditToolCall, sanitizeParams } from "../logger/audit.ts"; import { checkToolAccess, extractPolicy } from "../access/index.ts"; import { ConnectionError } from "../connection/errors.ts"; @@ -424,7 +425,18 @@ export function createDaemonServer(opts: DaemonServerOptions) { const tools = body.fresh ? await listToolsForService(conn.client) : await listToolsCached(conn.client, listPoolKey); - return Response.json({ success: true, result: tools }); + // #58: stamp the BARE-service fingerprint (not the credential pool + // key). The drift-hook maintains the bare entry as the canonical, + // credential-independent per-service signal, and the client also keys + // its cache by bare service -- so both sides compare the same key. + // Comparing a credential-scoped fingerprint against the client's bare + // key never converges and would clear the cache on every call. + const listFingerprint = await readCacheFingerprint(body.service); + return Response.json({ + success: true, + result: tools, + schemaFingerprint: listFingerprint, + }); } catch (err) { return handleEndpointError(err, pool); } finally { @@ -463,7 +475,9 @@ export function createDaemonServer(opts: DaemonServerOptions) { 404, ); } - return Response.json({ success: true, result }); + // #58: stamp the BARE-service fingerprint (see /list-tools). + const schemaFingerprint = await readCacheFingerprint(body.service); + return Response.json({ success: true, result, schemaFingerprint }); } catch (err) { return handleEndpointError(err, pool); } finally { diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 55321f1..52d502e 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -34,6 +34,15 @@ export interface DaemonSchemaRequest { export interface DaemonCallResponse { success: true; result: unknown; + /** + * #58 cache coherence: the daemon's current schema fingerprint for the + * service this response targeted. The client compares it against its own + * cached fingerprint and drops its local cache on mismatch -- so a contract + * bump that the daemon has already absorbed invalidates the client cache on + * the next call, with no extra round-trip. Per-service: a response only + * carries the fingerprint for the single service it was about. + */ + schemaFingerprint?: string; } /** Error daemon response envelope with typed error code */ diff --git a/src/process/client.ts b/src/process/client.ts index 85d0a98..4c0846b 100644 --- a/src/process/client.ts +++ b/src/process/client.ts @@ -11,6 +11,7 @@ import { getDaemonPaths, getRemoteConfig } from "../daemon/paths.ts"; import { ConnectionError } from "../connection/errors.ts"; import { getDaemonStatus, cleanStaleDaemon } from "./liveness.ts"; import { getRemoteServiceAvailability } from "./remote-discovery.ts"; +import { readCacheFingerprint, clearServiceCacheKeys } from "../cache/index.ts"; import type { ServiceSource, ServicesConfig } from "../config/index.ts"; import type { DaemonPaths } from "../daemon/types.ts"; import type { @@ -517,13 +518,39 @@ export async function callViaDaemon( return fetchDaemon("/call", request); } +/** + * #58 client-side cache coherence: if the daemon stamped a schemaFingerprint + * that differs from the client's locally-cached fingerprint for this service, + * the client's cache is stale (the daemon already absorbed an upstream contract + * bump). Drop the client's local entries so the next read refetches. Best-effort + * and per-service -- a response only carries the fingerprint for its own service. + */ +export async function reconcileClientCache( + service: string, + response: DaemonResponse, +): Promise { + if (!response.success || !response.schemaFingerprint) return; + try { + const localFingerprint = await readCacheFingerprint(service); + // Only invalidate when we HAVE a local fingerprint that disagrees. A missing + // local fingerprint is a cold/absent cache -- nothing to invalidate. + if (localFingerprint && localFingerprint !== response.schemaFingerprint) { + await clearServiceCacheKeys(service); + } + } catch { + // Coherence is an optimization; never fail a read because of it. + } +} + /** * Send a list-tools request to the daemon. */ export async function listToolsViaDaemon( request: DaemonListToolsRequest, ): Promise { - return fetchDaemon("/list-tools", request); + const response = await fetchDaemon("/list-tools", request); + await reconcileClientCache(request.service, response); + return response; } /** @@ -532,7 +559,9 @@ export async function listToolsViaDaemon( export async function getSchemaViaDaemon( request: DaemonSchemaRequest, ): Promise { - return fetchDaemon("/schema", request); + const response = await fetchDaemon("/schema", request); + await reconcileClientCache(request.service, response); + return response; } /** diff --git a/tests/cache/hash.test.ts b/tests/cache/hash.test.ts index 5515f0f..9bcb294 100644 --- a/tests/cache/hash.test.ts +++ b/tests/cache/hash.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { canonicalJson, hashToolSchema } from "../../src/cache/hash.ts"; +import { + canonicalJson, + hashToolSchema, + fingerprintSchemas, +} from "../../src/cache/hash.ts"; // -- canonicalJson -- @@ -142,3 +146,41 @@ describe("hashToolSchema", () => { expect(hash1).not.toBe(hash2); }); }); + +// -- fingerprintSchemas (#58 staleness signal) -- + +describe("fingerprintSchemas", () => { + test("returns a 64-character hex string", async () => { + const fp = await fingerprintSchemas([{ hash: "aaa" }, { hash: "bbb" }]); + expect(fp).toMatch(/^[0-9a-f]{64}$/); + }); + + test("is order-independent", async () => { + const fp1 = await fingerprintSchemas([{ hash: "aaa" }, { hash: "bbb" }]); + const fp2 = await fingerprintSchemas([{ hash: "bbb" }, { hash: "aaa" }]); + expect(fp1).toBe(fp2); + }); + + test("changes when any tool hash changes (e.g. a new inputSchema field)", async () => { + const before = await fingerprintSchemas([ + { hash: "tool_a_v1" }, + { hash: "tool_b" }, + ]); + // tool_a gains create_if_missing -> its surface hash changes + const after = await fingerprintSchemas([ + { hash: "tool_a_v2" }, + { hash: "tool_b" }, + ]); + expect(before).not.toBe(after); + }); + + test("changes when a tool is added or removed", async () => { + const two = await fingerprintSchemas([{ hash: "a" }, { hash: "b" }]); + const three = await fingerprintSchemas([ + { hash: "a" }, + { hash: "b" }, + { hash: "c" }, + ]); + expect(two).not.toBe(three); + }); +}); diff --git a/tests/cache/storage.test.ts b/tests/cache/storage.test.ts index 8619890..d5e0e3c 100644 --- a/tests/cache/storage.test.ts +++ b/tests/cache/storage.test.ts @@ -7,6 +7,7 @@ import { readCacheRaw, writeCache, clearCache, + clearServiceCacheKeys, listCachedServices, isCacheExpired, getCacheDir, @@ -201,6 +202,54 @@ describe("clearCache", () => { }); }); +// -- clearServiceCacheKeys (#58: invalidate base + all credential keys) -- + +describe("clearServiceCacheKeys", () => { + // Mirror the production key format: credential:. + function credentialKey(service: string, userId: string): string { + const encoded = Buffer.from(JSON.stringify([service, userId])).toString( + "base64url", + ); + return `credential:${encoded}`; + } + + test("clears the base entry and every credential entry for the service", async () => { + await writeCache("open-brain", [makeTool("base")]); + await writeCache(credentialKey("open-brain", "user:rico"), [ + makeTool("rico"), + ]); + await writeCache(credentialKey("open-brain", "user:skippy"), [ + makeTool("skippy"), + ]); + // An unrelated service and its credential key must survive. + await writeCache("n8n", [makeTool("n8n_base")]); + await writeCache(credentialKey("n8n", "user:rico"), [makeTool("n8n_rico")]); + + const cleared = await clearServiceCacheKeys("open-brain"); + expect(cleared).toBe(3); + + expect(await readCacheRaw("open-brain")).toBeNull(); + expect( + await readCacheRaw(credentialKey("open-brain", "user:rico")), + ).toBeNull(); + expect( + await readCacheRaw(credentialKey("open-brain", "user:skippy")), + ).toBeNull(); + + // Unrelated service untouched. + expect(await readCacheRaw("n8n")).not.toBeNull(); + expect( + await readCacheRaw(credentialKey("n8n", "user:rico")), + ).not.toBeNull(); + }); + + test("returns 0 when no keys match", async () => { + await writeCache("n8n", [makeTool("t1")]); + const cleared = await clearServiceCacheKeys("open-brain"); + expect(cleared).toBe(0); + }); +}); + // -- listCachedServices -- describe("listCachedServices", () => { diff --git a/tests/cli/cache-warm-force.test.ts b/tests/cli/cache-warm-force.test.ts new file mode 100644 index 0000000..22b1b9b --- /dev/null +++ b/tests/cli/cache-warm-force.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { join } from "node:path"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { handleCache } from "../../src/cli/commands/cache.ts"; +import { writeCache, getCacheFilePath } from "../../src/cache/storage.ts"; + +// Verifies the `cache warm --force` argument handling: the flag is recognized, +// it does not interfere with positional service extraction, and (for a service +// that fails to connect) the stale entry is cleared before the refetch attempt +// -- the documented recovery after an upstream contract bump. + +let testCacheDir: string; +let testConfigPath: string; +let origCacheDir: string | undefined; +let origConfig: string | undefined; +let origNoDaemon: string | undefined; + +beforeEach(async () => { + testCacheDir = await mkdtemp(join(tmpdir(), "mcp2cli-warm-force-cache-")); + const cfgDir = await mkdtemp(join(tmpdir(), "mcp2cli-warm-force-cfg-")); + testConfigPath = join(cfgDir, "services.json"); + + origCacheDir = process.env.MCP2CLI_CACHE_DIR; + origConfig = process.env.MCP2CLI_CONFIG; + origNoDaemon = process.env.MCP2CLI_NO_DAEMON; + process.env.MCP2CLI_CACHE_DIR = testCacheDir; + process.env.MCP2CLI_CONFIG = testConfigPath; + // Use the direct path so the unreachable-service refetch fails fast with a + // connection error rather than a 10s daemon-start timeout. This test is about + // --force's clear-before-refetch behavior, not daemon routing. + process.env.MCP2CLI_NO_DAEMON = "1"; + + // A service that will fail to connect quickly (unreachable URL), so warm + // exercises the clear-then-fetch path without a real daemon. + await writeFile( + testConfigPath, + JSON.stringify({ + services: { + "dead-svc": { + backend: "http", + url: "http://127.0.0.1:1/mcp", + }, + }, + }), + ); +}); + +afterEach(async () => { + if (origCacheDir !== undefined) process.env.MCP2CLI_CACHE_DIR = origCacheDir; + else delete process.env.MCP2CLI_CACHE_DIR; + if (origConfig !== undefined) process.env.MCP2CLI_CONFIG = origConfig; + else delete process.env.MCP2CLI_CONFIG; + if (origNoDaemon !== undefined) process.env.MCP2CLI_NO_DAEMON = origNoDaemon; + else delete process.env.MCP2CLI_NO_DAEMON; + await rm(testCacheDir, { recursive: true, force: true }); +}); + +describe("cache warm --force", () => { + test("clears the existing stale entry before refetching", async () => { + // Seed a stale cache entry. + await writeCache( + "dead-svc", + [ + { + name: "old_tool", + description: "stale", + inputSchema: {}, + hash: "deadbeef", + }, + ], + 60_000, + ); + const cachePath = getCacheFilePath("dead-svc"); + expect(await Bun.file(cachePath).exists()).toBe(true); + + // warm --force: the refetch will fail (unreachable), but --force must have + // already cleared the stale entry so the old schema can't be served. + await handleCache(["warm", "dead-svc", "--force"]); + + expect(await Bun.file(cachePath).exists()).toBe(false); + }, 40_000); + + test("without --force a failed refetch leaves the stale entry intact", async () => { + await writeCache( + "dead-svc", + [ + { + name: "old_tool", + description: "stale", + inputSchema: {}, + hash: "deadbeef", + }, + ], + 60_000, + ); + const cachePath = getCacheFilePath("dead-svc"); + + await handleCache(["warm", "dead-svc"]); + + // No --force: the failed connect must not have touched the existing entry. + expect(await Bun.file(cachePath).exists()).toBe(true); + }, 40_000); + + test("--force flag is not treated as the service name", async () => { + // `warm --force` (no positional service) should warm all configured + // services, not error with `Unknown service: "--force"`. + await handleCache(["warm", "--force"]); + // dead-svc fails to connect, but the run completes without a validation + // error about an unknown "--force" service. + expect(process.exitCode).not.toBe(1); // EXIT_CODES.VALIDATION + }, 40_000); +}); diff --git a/tests/daemon/list-tools-fingerprint.test.ts b/tests/daemon/list-tools-fingerprint.test.ts new file mode 100644 index 0000000..e21e876 --- /dev/null +++ b/tests/daemon/list-tools-fingerprint.test.ts @@ -0,0 +1,150 @@ +/** + * #58 / PR #59 regression guard: the daemon must stamp the BARE-service schema + * fingerprint onto /list-tools and /schema responses -- NOT the per-credential + * pool-key fingerprint. The client compares against its bare-key fingerprint, so + * stamping the credential key never converges and clears the client cache on + * every call. This test wires a per-user credential so the daemon's pool key is + * genuinely a `credential:` key, distinct from the bare key -- it FAILS if the + * stamp is reverted to the pool key. + */ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createDaemonServer } from "../../src/daemon/server.ts"; +import { ConnectionPool } from "../../src/daemon/pool.ts"; +import { IdleTimer } from "../../src/daemon/idle.ts"; +import { MetricsCollector } from "../../src/daemon/metrics.ts"; +import { writeCache } from "../../src/cache/index.ts"; +import type { CredentialManager } from "../../src/credentials/index.ts"; +import type { + AuthProvider, + AuthContext, +} from "../../src/daemon/auth-provider.ts"; +import type { ServicesConfig } from "../../src/config/index.ts"; + +let tempDir: string; +let cacheDir: string; +let origCacheDir: string | undefined; +let pool: ConnectionPool; +let server: ReturnType; + +const BARE_FP = "a".repeat(64); +const CREDENTIAL_FP = "b".repeat(64); +const USER_ID = "rico"; + +function makeConfig(): ServicesConfig { + return { + services: { + "open-brain": { + backend: "http", + url: "http://ob.example/mcp", + headers: {}, + }, + }, + }; +} + +class FakePool { + async getConnection() { + return { + client: { + async listTools() { + return { + tools: [ + { name: "ob_search", description: "Search", inputSchema: {} }, + ], + }; + }, + }, + }; + } + async closeAll() {} +} + +// Auth provider that resolves any request to a fixed user identity. +const fakeAuth: AuthProvider = { + enabled: true, + authenticate(): AuthContext | null { + return { userId: USER_ID, role: "admin" } as AuthContext; + }, +}; + +// Credential manager that returns a per-USER credential for open-brain, so +// resolveCredentialPool produces a `credential:` pool key (not the bare name). +const fakeCm = { + resolveWithSource(userId: string, serviceName: string) { + if (userId === USER_ID && serviceName === "open-brain") { + return { + credential: { headers: { Authorization: "Bearer x" } }, + source: "user" as const, + identity: userId, + }; + } + return null; + }, +} as unknown as CredentialManager; + +function cacheTool(name: string, hash: string) { + return { name, description: `d-${name}`, inputSchema: {}, hash }; +} + +describe("#58 daemon stamps the bare-service fingerprint", () => { + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "mcp2cli-fp-")); + cacheDir = await mkdtemp(join(tmpdir(), "mcp2cli-fp-cache-")); + origCacheDir = process.env.MCP2CLI_CACHE_DIR; + process.env.MCP2CLI_CACHE_DIR = cacheDir; + + pool = new FakePool() as unknown as ConnectionPool; + server = createDaemonServer({ + listenConfig: { mode: "unix", socketPath: join(tempDir, "daemon.sock") }, + pool, + config: makeConfig(), + idleTimer: new IdleTimer(60_000, () => {}), + onShutdown: () => {}, + authProvider: fakeAuth, + credentialManager: fakeCm, + metrics: new MetricsCollector(), + }); + }); + + afterEach(async () => { + server.stop(true); + await pool.closeAll(); + if (origCacheDir !== undefined) process.env.MCP2CLI_CACHE_DIR = origCacheDir; + else delete process.env.MCP2CLI_CACHE_DIR; + await rm(tempDir, { recursive: true, force: true }); + await rm(cacheDir, { recursive: true, force: true }); + }); + + test("/list-tools stamps the bare fingerprint even when the pool key is a credential key", async () => { + // Bare entry and the credential entry carry DIFFERENT fingerprints. + await writeCache("open-brain", [cacheTool("a", "h1")], 60_000, BARE_FP); + const credKey = `credential:${Buffer.from( + JSON.stringify(["open-brain", `user:${USER_ID}`]), + ).toString("base64url")}`; + await writeCache(credKey, [cacheTool("b", "h2")], 60_000, CREDENTIAL_FP); + + const res = await server.fetch( + new Request("http://localhost/list-tools", { + method: "POST", + headers: { + "Content-Type": "application/json", + Authorization: "Bearer any", + }, + body: JSON.stringify({ service: "open-brain" }), + }), + ); + const body = (await res.json()) as { + success: boolean; + schemaFingerprint?: string; + }; + + expect(body.success).toBe(true); + // Must be the BARE fingerprint. Reverting the stamp to the pool key yields + // CREDENTIAL_FP here and fails the test -- catching a B1 regression. + expect(body.schemaFingerprint).toBe(BARE_FP); + expect(body.schemaFingerprint).not.toBe(CREDENTIAL_FP); + }); +}); diff --git a/tests/process/client-cache-coherence.test.ts b/tests/process/client-cache-coherence.test.ts new file mode 100644 index 0000000..94ff0d4 --- /dev/null +++ b/tests/process/client-cache-coherence.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, test, beforeEach, afterEach } from "bun:test"; +import { join } from "node:path"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { + writeCache, + readCacheRaw, + readCacheFingerprint, + fingerprintSchemas, + mapToolsToCachedSchemas, +} from "../../src/cache/index.ts"; +import { reconcileClientCache } from "../../src/process/client.ts"; +import type { DaemonResponse } from "../../src/daemon/types.ts"; + +// #58: when a daemon response carries a schemaFingerprint that differs from the +// client's locally-cached fingerprint, the client must drop its stale cache so +// the next read refetches. These tests exercise reconcileClientCache directly, +// independent of daemon routing/transport. + +let testCacheDir: string; +let origCacheDir: string | undefined; + +const FRESH_FINGERPRINT = "f".repeat(64); + +function tool(name: string, hash: string) { + return { name, description: `d-${name}`, inputSchema: {}, hash }; +} + +function okResponse(schemaFingerprint?: string): DaemonResponse { + return { success: true, result: [], schemaFingerprint }; +} + +beforeEach(async () => { + testCacheDir = await mkdtemp(join(tmpdir(), "mcp2cli-coherence-")); + origCacheDir = process.env.MCP2CLI_CACHE_DIR; + process.env.MCP2CLI_CACHE_DIR = testCacheDir; +}); + +afterEach(async () => { + if (origCacheDir !== undefined) process.env.MCP2CLI_CACHE_DIR = origCacheDir; + else delete process.env.MCP2CLI_CACHE_DIR; + await rm(testCacheDir, { recursive: true, force: true }); +}); + +describe("reconcileClientCache (#58 piggyback)", () => { + test("drops the stale local cache when the daemon fingerprint differs", async () => { + await writeCache("open-brain", [tool("old", "h1")], 60_000, "stale-fp"); + await reconcileClientCache("open-brain", okResponse(FRESH_FINGERPRINT)); + expect(await readCacheRaw("open-brain")).toBeNull(); + }); + + test("keeps the local cache when the fingerprint matches", async () => { + await writeCache("open-brain", [tool("t", "h1")], 60_000, FRESH_FINGERPRINT); + await reconcileClientCache("open-brain", okResponse(FRESH_FINGERPRINT)); + expect(await readCacheRaw("open-brain")).not.toBeNull(); + }); + + test("does nothing when the response carries no fingerprint", async () => { + await writeCache("open-brain", [tool("t", "h1")], 60_000, "local-fp"); + await reconcileClientCache("open-brain", okResponse(undefined)); + expect(await readCacheRaw("open-brain")).not.toBeNull(); + }); + + test("does nothing on a cold cache (no local fingerprint to compare)", async () => { + // No cache entry written -> nothing to invalidate, must not throw. + await reconcileClientCache("open-brain", okResponse(FRESH_FINGERPRINT)); + expect(await readCacheRaw("open-brain")).toBeNull(); + }); + + test("ignores error responses", async () => { + await writeCache("open-brain", [tool("t", "h1")], 60_000, "local-fp"); + const errResponse: DaemonResponse = { + success: false, + error: { code: "CONNECTION_ERROR", message: "down" }, + }; + await reconcileClientCache("open-brain", errResponse); + expect(await readCacheRaw("open-brain")).not.toBeNull(); + }); +}); + +// Real round-trip: the daemon (raw upstream tools, missing descriptions -> +// hashToolSchema's "" default) and the client warm path (descriptions defaulted +// to "(no description)") must derive the SAME fingerprint for the same tools. +// This is the regression guard for the swarm-found blockers (B1: key alignment, +// B2: description-default divergence) -- both manifested as "fingerprints never +// match -> reconcile clears on every call." +describe("daemon/client fingerprint convergence (#58 blocker regression)", () => { + test("identical upstream tools -> identical fingerprint despite description defaulting + reconcile keeps cache", async () => { + // Daemon side: raw MCP tools, one with NO description (undefined). + const rawTools = [ + { name: "ob_search", description: "Search", inputSchema: {} }, + { name: "ob_no_desc", inputSchema: {} }, // missing description + ]; + const daemonSchemas = await mapToolsToCachedSchemas(rawTools); + // Daemon writes the BARE service key (what the drift-hook maintains and what + // the daemon now stamps), letting writeCache derive the fingerprint. + await writeCache("open-brain", daemonSchemas, 60_000); + const daemonFingerprint = await readCacheFingerprint("open-brain"); + if (!daemonFingerprint) throw new Error("daemon fingerprint not written"); + + // Client warm path: the same tools, but descriptions already defaulted to + // "(no description)" (as getToolSchemaCached does) before hashing. + const clientSchemas = await mapToolsToCachedSchemas([ + { name: "ob_search", description: "Search", inputSchema: {} }, + { name: "ob_no_desc", description: "(no description)", inputSchema: {} }, + ]); + const clientFingerprint = (await fingerprintSchemas(clientSchemas)); + + // The fix: both derive the SAME fingerprint for the same tools. + expect(clientFingerprint).toBe(daemonFingerprint); + + // And therefore reconcile against the daemon's stamp must NOT clear. + await writeCache("open-brain", clientSchemas, 60_000); + await reconcileClientCache("open-brain", okResponse(daemonFingerprint)); + expect(await readCacheRaw("open-brain")).not.toBeNull(); + }); +});