Compare commits

...
Author SHA1 Message Date
taodengandClaude Opus 5 91f6a8b8fe fix(server): hash the RESOLVED model in cache keys; give the structured path its epoch
Supersedes the first attempt on this branch, which folded modelsConfig.aliases
into CONFIG_EPOCH. An independent reviewer falsified that approach with a live
repro: the structured-output cache key never reads CONFIG_EPOCH at all, so
adding inputs to the epoch could not possibly fix it. My PR body claimed it
covered "normal, structured and singleflight" — that claim was false.

Two defects, both now fixed:

1. Cache keys hashed the model string as the client sent it. `model` is
   whatever arrived — a canonical id, an alias ("opus"), or a legacyAlias
   ("claude-opus-4"); MODEL_MAP carries all three. models.json is read once at
   boot, so repointing an alias only takes effect on restart, while the SQLite
   response_cache outlives it — serving the OLD model's answers under that
   alias until TTL expiry. Now all three call sites hash
   `cacheModel = MODEL_MAP[model] || model`.

   Chosen over folding the alias map into CONFIG_EPOCH because it is strictly
   better on three counts: it fixes the normal, structured AND dedup keys at
   once (they all already pass `model`); it covers legacyAliases for free; and
   it invalidates PRECISELY — only entries for the repointed alias change key,
   where the epoch approach flushed the entire cache on any alias edit,
   including unrelated sonnet/haiku rows. Only the cache KEY is resolved;
   `model` is still echoed to the client verbatim, so the wire is unchanged.

2. The structured and dedup keys omitted `configEpoch` entirely, so #177 never
   actually covered the structured path: changing SYSTEM_PROMPT (the original
   #176 scenario) still served structured answers composed under the OLD
   config. Live-verified by the reviewer, independently reproduced here. Both
   now pass `configEpoch: CONFIG_EPOCH`. This has been latent for
   structured-output clients since #153 landed.

Tests: +3, all mutation-proven — the previous attempt shipped with ZERO
coverage (reverting it left the suite at 449/0), which the reviewer proved.
Built on the existing `ltBoot` child-process harness (test-features.mjs:987)
that I had wrongly claimed did not exist; a fake CLAUDE_BIN means zero quota
cost. Rather than mutate models.json mid-suite, they assert the equivalent
observable: an alias and its canonical target must land on the SAME cache slot,
which holds only if the key is resolved before hashing.

  - revert cacheModel -> model at the 3 sites  => 2 failures (both alias tests)
  - drop configEpoch from the structured hash  => 1 failure (#177-gap test)
  - restored                                   => 452 passed, 0 failed

Blast radius, previously undisclosed: on upgrade, alias-addressed cache rows
change key once and orphan, reaped by the TTL cleanup within one window — the
same shape as the v3.13.0 v1->v2 hash upgrade already documented in README.
Literal-id rows are unaffected. Default CLAUDE_CACHE_TTL is 0, so a default
deployment sees nothing.

cli.js citation: NOT APPLICABLE — no cli.js-derived wire behavior is touched.
Scope justified under ALIGNMENT.md Rule 2 as OCP-owned cache bookkeeping;
cli.js has no response cache. The endpoint is Class B.1 under ADR 0006, which
authorizes the OpenAI-compat surface this caching sits behind.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
2026-07-26 22:29:54 +10:00
taodengandClaude Opus 5 042dfa63b2 fix(server): fold the alias map into CONFIG_EPOCH so alias repoints invalidate the cache
Cache keys hash the model string exactly as the client sent it — `const model =
parsed.model || modelsConfig.aliases.sonnet` (server.mjs:2806), which then goes
straight into `cacheHash(model, ...)`. A request for "opus" is therefore cached
under the literal string "opus", not under the canonical id it resolved to.

models.json is read once at boot, so repointing an alias only takes effect on
restart — and the response cache is SQLite-backed (keys.mjs getCachedResponse
queries the `response_cache` table), so it OUTLIVES that restart. CONFIG_EPOCH
folded four config values into every cache key for exactly this hazard (#176 /
#177) but omitted the alias map. Consequence: an operator running
CLAUDE_CACHE_TTL > 0 who repoints an alias keeps being served the OLD model's
answers under that alias until TTL expiry — silently defeating the repoint.

Fix: add `modelsConfig.aliases` to the epoch digest, so any alias change is an
instant whole-cache invalidation. This is the mechanism #177 already
established for the other config inputs, applied to the one it missed; it
covers the normal, structured and singleflight paths at once because they all
key off the same epoch, and it needs no change at the three call sites.

Reproduced end-to-end on the real server, both directions (NODE_ENV=test +
OCP_DIR_OVERRIDE, isolated SQLite store, CLAUDE_CACHE_TTL=600000):

  1. boot with aliases.opus = claude-opus-4-8, request model:"opus"
     -> claude_spawned model=claude-opus-4-8, response cached
  2. stop, repoint aliases.opus -> claude-sonnet-5, restart
  3. same request again:
       on origin/main : cache_hit    (stale Opus 4.8 answer, no spawn)
       on this branch : cache MISS -> claude_spawned model=claude-sonnet-5

Default CLAUDE_CACHE_TTL is 0 (caching off), so shipped default behavior is
unaffected; this bites only operators who deliberately enabled the cache.

cli.js citation: NOT APPLICABLE — no cli.js-derived wire behavior is touched.
Scope justified under ALIGNMENT.md Rule 2 as OCP-owned cache bookkeeping:
cli.js has no response cache and no equivalent operation, and the spawn
arguments and protocol are unchanged.

Found by an independent codex review (gpt-5.6-sol, reasoning=high) of the
Opus 5 alias repoint in #192; verified first-hand before applying rather than
taken on the reviewer's word.

Tests: 449 passed, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017gbqUZ8HfBZpjjbzQ85oH8
2026-07-26 22:13:45 +10:00
2 changed files with 93 additions and 3 deletions
+13 -3
View File
@@ -2804,6 +2804,16 @@ 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.
const cacheModel = MODEL_MAP[model] || model;
const stream = parsed.stream;
// Validate model against known models
@@ -2900,7 +2910,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 +2929,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 +2978,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);
+80
View File
@@ -1104,6 +1104,86 @@ 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 }); }
});
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";