diff --git a/server.mjs b/server.mjs index 2025862..cb0c4ad 100644 --- a/server.mjs +++ b/server.mjs @@ -2804,6 +2804,20 @@ async function handleChatCompletions(req, res) { const messages = parsed.messages || parsed.input || [{ role: "user", content: parsed.prompt || "" }]; const model = parsed.model || modelsConfig.aliases.sonnet; + // Cache keys must hash the RESOLVED model, never the string the client happened to use. + // `model` is whatever was sent — a canonical id, an alias ("opus"), or a legacyAlias + // ("claude-opus-4"). MODEL_MAP carries all three, and models.json is read once at boot, so + // repointing an alias only takes effect on restart — while the SQLite response_cache outlives + // it. Hashing the raw string would therefore keep serving the OLD model's answers under that + // alias until TTL expiry, silently defeating the repoint (the #176 hazard, for aliases). + // Resolving first also means "opus" and "claude-opus-5" correctly share one slot: identical + // spawn, identical answer. Only the cache KEY is resolved — `model` is still echoed back to + // the client verbatim, so the wire response is unchanged. + // hasOwn, not a bare lookup: MODEL_MAP is a plain object, so `MODEL_MAP["constructor"]` + // would return an inherited FUNCTION. Unreachable today (the VALID_MODELS gate below 400s + // first, and it is built from Object.keys so it holds only own keys), but a bare lookup + // would hand cacheHash a function the day anyone widens that gate or moves this binding. + const cacheModel = Object.hasOwn(MODEL_MAP, model) ? MODEL_MAP[model] : model; const stream = parsed.stream; // Validate model against known models @@ -2900,7 +2914,7 @@ async function handleChatCompletions(req, res) { const promptCharsS = messages.reduce((a, m) => a + contentToText(m.content).length, 0); let structuredHash = null; if (CACHE_TTL > 0 && !conversationId && !hasCacheControl(messages)) { - structuredHash = cacheHash(model, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured }); + structuredHash = cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH }); try { const cached = getCachedResponse(structuredHash, CACHE_TTL); if (cached) { @@ -2919,7 +2933,7 @@ async function handleChatCompletions(req, res) { // one-off structured request (not stateful sessions / client-side prompt caching), independent of // whether OCP response caching is enabled; when caching IS on, the same key gates cache read/write. const dedupKey = (!conversationId && !hasCacheControl(messages)) - ? cacheHash(model, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured }) + ? cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH }) : null; const runStructured = async () => { const c = await runStructuredCompletion(upstreamCall, model, messages, conversationId, req._authKeyName, res, structured); @@ -2968,7 +2982,7 @@ async function handleChatCompletions(req, res) { } else { // D1: include keyId in hash to isolate per-key cache pools (v2 format). // configEpoch (#176): any boot-config change that shapes answers invalidates the cache. - const hash = cacheHash(model, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, configEpoch: CONFIG_EPOCH }); + const hash = cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, configEpoch: CONFIG_EPOCH }); req._cacheHash = hash; // store for later write-back try { const cached = getCachedResponse(hash, CACHE_TTL); diff --git a/test-features.mjs b/test-features.mjs index 5896ee0..f73813d 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -1104,6 +1104,106 @@ test("integration: toggling OCP_LOCAL_TOOLS invalidates the standard response ca } finally { _ltRm(dir, { recursive: true, force: true }); } }); +// ── Cache keys hash the RESOLVED model, not the alias string (#194) ────────── +// models.json is read once at boot, so repointing an alias only takes effect on restart — +// while the SQLite response_cache outlives it. Hashing the raw string would keep serving the +// OLD model's answers under that alias until TTL expiry. Rather than mutate models.json +// mid-suite, these assert the equivalent observable: an alias and its canonical target must +// land on the SAME cache slot, which is true only if the key is resolved before hashing. +// Mutation: change `cacheModel` back to `model` at the three cacheHash call sites in +// server.mjs and both tests go red (2 spawns instead of 1). + +// Fake that emits schema-valid JSON, so the structured path caches a VALIDATED result +// (the stock LT_FAKE returns "OK", which fails validation → refusal → never cached). +const LT_FAKE_JSON = `#!/bin/sh +if [ -n "$SP_COUNTER" ]; then c=$(cat "$SP_COUNTER" 2>/dev/null || echo 0); echo $((c+1)) > "$SP_COUNTER"; fi +printf '%s\\n' '{"type":"assistant","message":{"content":[{"type":"text","text":"{\\"ok\\":true}"}]}}' +printf '%s\\n' '{"type":"result"}' +exit 0 +`; +function ltFakeJson(dir) { const p = join(dir, "claude-json"); _ltWrite(p, LT_FAKE_JSON); _ltChmod(p, 0o755); return p; } +const LT_SCHEMA = { type: "object", properties: { ok: { type: "boolean" } }, required: ["ok"], additionalProperties: false }; + +console.log("\nCache key resolves the model alias (#194):"); + +test("integration: an alias and its canonical target share ONE cache slot (normal path)", async () => { + if (!LT_POSIX) return; + const dir = ltMkdir(); const fake = ltFake(dir); const counter = join(dir, "spawns.txt"); + const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39360", CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir); + try { + assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); + _ltWrite(counter, "0"); + const msgs = [{ role: "user", content: "alias-resolution-probe" }]; + await ltPost(39360, { model: "sonnet", messages: msgs }); // miss → spawn + await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 3000); + await ltPost(39360, { model: "claude-sonnet-5", messages: msgs }); // same resolved model → HIT + await new Promise(r => setTimeout(r, 600)); + assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1, + "the canonical id must hit the slot the alias populated — a 2nd spawn means the key still hashes the raw alias"); + } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); } +}); + +test("integration: an alias and its canonical target share ONE cache slot (STRUCTURED path)", async () => { + if (!LT_POSIX) return; + const dir = ltMkdir(); const fake = ltFakeJson(dir); const counter = join(dir, "spawns.txt"); + const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39361", CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir); + try { + assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); + _ltWrite(counter, "0"); + const rf = { type: "json_schema", json_schema: { name: "probe", schema: LT_SCHEMA } }; + const msgs = [{ role: "user", content: "structured-alias-probe" }]; + await ltPost(39361, { model: "sonnet", messages: msgs, response_format: rf }); + await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 4000); + await ltPost(39361, { model: "claude-sonnet-5", messages: msgs, response_format: rf }); + await new Promise(r => setTimeout(r, 600)); + assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1, + "structured cache key must resolve the alias too — this is the path the epoch-only fix missed"); + } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); } +}); + +// MODEL_MAP is models[] + aliases + legacyAliases, so resolving covers legacyAliases for free. +// The three tests above all use `sonnet` (a plain alias); this pins the legacyAlias leg explicitly +// rather than leaving it covered only by construction. +test("integration: a legacyAlias shares ONE cache slot with its canonical target", async () => { + if (!LT_POSIX) return; + const dir = ltMkdir(); const fake = ltFake(dir); const counter = join(dir, "spawns.txt"); + const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: "39364", CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter }, dir); + try { + assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); + _ltWrite(counter, "0"); + const msgs = [{ role: "user", content: "legacy-alias-probe" }]; + await ltPost(39364, { model: "claude-haiku-4-5", messages: msgs }); // legacyAlias + await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 3000); + await ltPost(39364, { model: "claude-haiku-4-5-20251001", messages: msgs }); // canonical + await new Promise(r => setTimeout(r, 600)); + assert.equal(Number(_ltRead(counter, "utf8")) || 0, 1, + "legacyAliases live in MODEL_MAP too — resolving must collapse them onto the canonical slot"); + } finally { child.kill("SIGKILL"); _ltRm(dir, { recursive: true, force: true }); } +}); + +test("integration: a config change invalidates the STRUCTURED cache too (closes the #177 gap)", async () => { + if (!LT_POSIX) return; + const dir = ltMkdir(); const fake = ltFakeJson(dir); const counter = join(dir, "spawns.txt"); + const rf = { type: "json_schema", json_schema: { name: "probe", schema: LT_SCHEMA } }; + const req = { model: "sonnet", messages: [{ role: "user", content: "structured-epoch-probe" }], response_format: rf }; + const bootOnce = async (env, port) => { + const { child, buf } = ltBoot({ CLAUDE_BIN: fake, CLAUDE_PROXY_PORT: String(port), CLAUDE_CACHE_TTL: "60000", SP_COUNTER: counter, ...env }, dir); + try { + assert.ok(await ltWait(() => buf.out.includes("listening on")), `did not start: ${buf.err.slice(0, 200)}`); + _ltWrite(counter, "0"); + await ltPost(port, req); + await ltWait(() => (Number(_ltRead(counter, "utf8")) || 0) >= 1, 4000); + return Number(_ltRead(counter, "utf8")) || 0; + } finally { child.kill("SIGKILL"); } + }; + try { + const off = await bootOnce({}, 39362); // caches under epoch(negative wrapper) + const on = await bootOnce({ OCP_LOCAL_TOOLS: "1" }, 39363); // same DB, epoch differs → must re-spawn + assert.equal(off, 1, "first structured request (cache empty) must spawn claude"); + assert.equal(on, 1, "structured cache must honor CONFIG_EPOCH — before #194 it omitted the epoch entirely and served the stale answer"); + } finally { _ltRm(dir, { recursive: true, force: true }); } +}); + // ── Upgrade Tests ── import { runUpgrade, postFlightOk } from "./scripts/upgrade.mjs";