fix+docs: D33 — round-5 cleanup batch (F1/F3/F5/F8/F9/F10/F11/F12)

cold-audit catch from 2026-05-24 (round 5)

Round-5 cold-audit cleanup batch. 8 items + 1 release-discipline
reconciliation. Largest batch by line count (582+/39-) but every item
is small-and-focused. 3 P2 items (F1/F3/F5 of which F3 + F1 are real
correctness/observability fixes; F5 backfills /health to spec).

Changes (10 files, +583/-39):

**P2 fixes**

1. **F1 — ALIGNMENT.md mistral authority pin self-contradicted plugin**
   (ALIGNMENT.md): row cited `vibe --prompt --output json` but mistral.mjs
   uses `--output streaming` (the plugin header at lines 360-369 even
   justifies WHY: `--output json` emits single blob, breaks NDJSON
   line-buffered parser). Constitution self-contradicting itself —
   missed across 4 prior rounds. Pin updated to `--output streaming`
   with DOCS-1 reference.

2. **F3 — Deterministic function_call synth ID**
   (lib/ir/openai-to-ir.mjs): deprecated `function_call` translation
   produced `id: \`fc-${Date.now()}\`` → ID flows into normalized
   tool_calls → cache key SHA-256. Two identical requests separated
   by ≥1ms → different cache keys → cache always misses for
   `function_call` request shape. Violates ADR 0005 invariant
   "same inputs → same key, no random, no timestamp."

   Fixed: id is now `fc-<16-hex>` from SHA-256 of `${name}\0${arguments}`.
   NUL separator prevents the (name='ab',args='c') vs (name='a',args='bc')
   collision. 2^64 collision resistance is more than sufficient for
   tool_call ID disambiguation (per-request semantic key, not crypto
   primitive).

**P3 fixes**

3. **F5 — /health invokes per-plugin healthCheck()** (server.mjs +
   docs/openai-spec-pin.md): ADR 0002 says "healthCheck — startup AND
   /health endpoint use this." Pre-D33 /health returned only
   {enabled, available} counts. Now async, iterates loadedProviders,
   awaits each plugin's healthCheck() in try/catch. Returns
   `providers: {enabled, available, status: {<name>: {ok, latencyMs?, error?}}}`.

4. **F8 — X-OLP-Cache reports fallback-hop cache hits** (server.mjs):
   pre-D33 cacheStatus computed from `preCheckHit && fallbackHops === 0`
   — only counted primary-hop cache hits. When fallback fires + the
   fallback hop's getOrCompute returns from cache, header reported
   `miss` despite no spawn happening.

   Fixed: peek BEFORE getOrCompute inside executeHopFn, set
   `lastHopWasCached` closure variable on every hop (last-write-wins
   = serving hop's state). cacheStatus combines
   `lastHopWasCached || (preCheckHit && fallbackHops === 0)`.

   F8 chose option (b) peek-then-getOrCompute over option (a)
   getOrCompute API change because option (a) would break ~15 test
   callsites for marginal benefit. Accepted race window same as
   existing preCheckHit pattern.

5. **F9 — validateProvider hints error message updated** (lib/providers/
   base.mjs): pre-D33 message listed cacheable as missing and
   maxSpawnTimeMs as required. Now: `'hints must be an object with
   { requiresTTY, concurrentSpawnSafe, maxConcurrent } + optional
   { maxSpawnTimeMs, cacheable }'`.

6. **F10 — Dead cache-write branch removed** (server.mjs): the
   `if (hasStopChunk)` check in the streaming stop-less exhaustion
   branch was unreachable (the stop-chunk completion path returns
   earlier inside the for-await loop). Removed the dead code + added
   a comment documenting the invariant.

**Governance/policy**

7. **F11 — Phase rolling mode policy formalized** (CLAUDE.md +
   CHANGELOG.md): 22+ D-day commits accumulated under "Unreleased"
   without per-D version bumps — Iron Rule 5 (release-kit bump-before-
   push) appeared to be silently violated. Reality: per-D bumps would
   produce 30+ noise tags during Phase 1. F11 formalizes the policy:
   intra-Phase D-day commits accumulate under Unreleased; bump+tag
   fires explicitly at Phase close (maintainer-triggered, not
   automated). CLAUDE.md release_kit overlay gains `phase_rolling_mode`
   block documenting the exception with self-pointer ("if Rule 5
   appears silently violated, check this section first"). CHANGELOG
   "Unreleased" gets a notice at top.

   **No version bump, no git tag in D33** — policy formalization only.

8. **F12 — /v1/models created is stable per-model timestamp**
   (models-registry.json + lib/providers/index.mjs + server.mjs +
   docs/openai-spec-pin.md): pre-D33 used Math.floor(Date.now()/1000)
   per request — violates OpenAI spec which treats `created` as
   per-model attribute. Clients caching models by created would see
   spurious updates on every poll.

   Fixed: models-registry.json gains `bootstrapCreated: 1778630400`
   top-level constant + per-model `created` fields where known
   (anthropic claude-{opus,sonnet,haiku} with estimated release dates;
   devstral models from "25-12" suffix; codex models pinned to
   bootstrap pending verified release dates). handleModels uses
   `getModelCreated(modelId)` helper from lib/providers/index.mjs.
   Aliases share canonical's timestamp.

**Tests** (test-features.mjs): 401 → 414 (+13):
- F3 ×3 (same input → same id → same cache key; different name → different)
- F5 ×4 (empty/single/multi/throwing-plugin /health shapes)
- F8 ×1 (2-hop primary-fail + secondary-cache-hit → X-OLP-Cache: hit)
- F12 ×5 (stability/fallback/alias-equals-canonical)

Pre-commit fold-in (per evidence-first checkpoint #4):
- **D33 reviewer flagged F3 empty-args asymmetry** (Concern #1): hash
  input used `?? ''` (empty stays) but emitted IR field used
  `|| '{}'` (empty becomes '{}'). Consequence: `arguments: ''` and
  `arguments: '{}'` emit identical IR but compute different ids →
  different cache keys for semantically-identical requests. The exact
  cache-stability bug F3 was supposed to fix.

  Folded in: canonicalize empty-args to '{}' BEFORE hashing. Hash
  input now matches IR emission exactly. Same line change resolves
  the asymmetry.

Authority:
- ALIGNMENT.md self-amendment (F1 pin correction)
- ADR 0005 invariant "same inputs → same key, no random, no timestamp"
  (F3 restoration)
- ADR 0002 § Provider contract "/health uses healthCheck" (F5)
- ADR 0004 § Observability headers (F8 X-OLP-Cache correctness)
- ADR 0005 § Cache write conditions item 1 (F10 truncation-not-cached
  invariant explicit)
- Iron Rule 5 (F11 release-kit reconciliation)
- OpenAI /v1/models spec — `created` per-model stable (F12)
- CC 开发铁律 v1.6 § 10.x — Round-5 Cold Audit caught all 8

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified:
- F1 plugin cross-reference (mistral.mjs:360-369) accurately documents
  the rationale
- F3 collision resistance + NUL separator + restored cache invariant
- F5 all 4 cases (empty/single/multi/throwing) work
- F8 closure semantics across multi-hop chains (verified hop-fail +
  fallback-hit case)
- F10 dead code removal preserves the stop-chunk completion path
- F11 phase_rolling_mode policy honest about what happened and what
  the going-forward rule is
- F12 stability across consecutive /v1/models calls; alias-canonical
  parity
- 414/414 tests pass

3 remaining non-blocking suggestions (F3-vs-modern-tool_calls path
canonicalization symmetry; F12 codex models explicit-vs-fallback
writeup mismatch; F8 servingHopWasCached naming) tracked as future
polish; not folded.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-05-24 19:12:03 +10:00
co-authored by Claude Opus 4.7
parent 30de965e8e
commit f784fdb947
10 changed files with 586 additions and 39 deletions
+391 -1
View File
@@ -28,7 +28,7 @@ import {
SSE_DONE,
} from './lib/ir/ir-to-openai.mjs';
import { validateProvider, ProviderError, withTimeout } from './lib/providers/base.mjs';
import { loadProviders, getProviderForModel, getProviderByName, listAllProviderNames, getAliasMap } from './lib/providers/index.mjs';
import { loadProviders, getProviderForModel, getProviderByName, listAllProviderNames, getAliasMap, getModelCreated, REGISTRY_BOOTSTRAP_CREATED } from './lib/providers/index.mjs';
import anthropic, {
irToAnthropic,
anthropicChunkToIR,
@@ -5833,6 +5833,13 @@ import {
createOlpServer as createServer27,
__setProvidersEnabled as setProviders27,
__resetProvidersEnabled as resetProviders27,
createOlpServer as createServer33,
__setProvidersEnabled as setProviders33,
__resetProvidersEnabled as resetProviders33,
__setFallbackConfig as setFallbackConfig33,
__resetFallbackConfig as resetFallbackConfig33,
__clearCache as clearCache33,
loadedProviders as loadedProviders33,
} from './server.mjs';
describe('/v1/models population + X-OLP-* error headers (Suite 17)', () => {
@@ -7287,4 +7294,387 @@ describe('D27 F15 — /v1/models alias surfacing', () => {
});
// ── Suite D33: round-5 cold-audit cleanup batch ───────────────────────────────
//
// F3: function_call deprecated field → deterministic IR id (no Date.now())
// F5: /health returns per-provider healthCheck() snapshots
// F8_fallback: fallback-hop cache-hit → X-OLP-Cache: hit (not miss)
// F12: /v1/models `created` is stable per-model timestamp
//
// Authority:
// F3 — ADR 0005 § "same inputs → same key, no random, no timestamp"
// F5 — ADR 0002 § Provider contract (healthCheck)
// F8_fallback — ADR 0005 § D1 cache; ADR 0004 § Observability headers
// F12 — OpenAI /v1/models spec (https://platform.openai.com/docs/api-reference/models);
// models-registry.json bootstrapCreated fallback
describe('D33 round-5 cold-audit cleanup', () => {
// ── F3: deterministic function_call ID ────────────────────────────────────
describe('F3 — function_call deprecated field → deterministic cache key', () => {
it('F3a: two identical function_call requests produce the same IR id', () => {
const req = {
model: 'test-model',
messages: [
{
role: 'assistant',
function_call: { name: 'get_weather', arguments: '{"city":"Paris"}' },
},
],
};
const ir1 = openAIToIR(req);
const ir2 = openAIToIR(req);
const id1 = ir1.messages[0].tool_calls?.[0]?.id;
const id2 = ir2.messages[0].tool_calls?.[0]?.id;
assert.ok(id1, 'function_call must produce a tool_calls entry with an id');
assert.equal(id1, id2,
`Two identical function_call requests must produce the same id; got ${id1} vs ${id2}`);
assert.ok(id1.startsWith('fc-'), 'id must start with "fc-"');
});
it('F3b: two identical function_call requests produce the same cache key', () => {
const req = {
model: 'test-model',
messages: [
{
role: 'assistant',
function_call: { name: 'my_tool', arguments: '{}' },
},
],
};
const ir1 = openAIToIR(req);
const ir2 = openAIToIR(req);
const key1 = computeCacheKey('anthropic', 'claude-sonnet-4-6', ir1);
const key2 = computeCacheKey('anthropic', 'claude-sonnet-4-6', ir2);
assert.equal(key1, key2,
'Identical function_call requests must produce identical cache keys');
});
it('F3c: different function_call names produce different cache keys', () => {
const req1 = openAIToIR({
model: 'test-model',
messages: [{ role: 'assistant', function_call: { name: 'tool_a', arguments: '{}' } }],
});
const req2 = openAIToIR({
model: 'test-model',
messages: [{ role: 'assistant', function_call: { name: 'tool_b', arguments: '{}' } }],
});
const key1 = computeCacheKey('anthropic', 'claude-sonnet-4-6', req1);
const key2 = computeCacheKey('anthropic', 'claude-sonnet-4-6', req2);
assert.notEqual(key1, key2,
'Different function_call names must produce different cache keys');
});
});
// ── F5: /health per-provider snapshot ─────────────────────────────────────
describe('F5 — /health returns per-provider healthCheck() snapshots', () => {
it('F5a: /health with no providers enabled returns providers.status = {}', async () => {
setProviders33({});
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const r = await fetch({ port: p, method: 'GET', path: '/health' });
assert.equal(r.status, 200);
const body = JSON.parse(r.body);
assert.ok(body.ok, '/health must return ok:true');
assert.ok('status' in body.providers,
'/health providers must include a "status" key');
assert.deepEqual(body.providers.status, {},
'status must be empty when no providers are loaded');
} finally {
resetProviders33();
await new Promise(r => s.close(r));
}
});
it('F5b: /health with anthropic enabled includes providers.status.anthropic', async () => {
setProviders33({ anthropic: true });
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const r = await fetch({ port: p, method: 'GET', path: '/health' });
assert.equal(r.status, 200);
const body = JSON.parse(r.body);
assert.ok('anthropic' in body.providers.status,
'providers.status must include anthropic when it is enabled');
const ahc = body.providers.status.anthropic;
assert.ok(typeof ahc === 'object' && ahc !== null,
'providers.status.anthropic must be an object');
assert.ok('ok' in ahc,
'providers.status.anthropic must have an "ok" field');
} finally {
resetProviders33();
await new Promise(r => s.close(r));
}
});
it('F5c: /health includes status for each loaded provider', async () => {
setProviders33({ anthropic: true, mistral: true });
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const r = await fetch({ port: p, method: 'GET', path: '/health' });
assert.equal(r.status, 200);
const body = JSON.parse(r.body);
assert.equal(body.providers.enabled, 2, 'enabled must be 2');
assert.ok('anthropic' in body.providers.status, 'anthropic status must appear');
assert.ok('mistral' in body.providers.status, 'mistral status must appear');
assert.ok(!('openai' in body.providers.status),
'openai must NOT appear in status when not loaded');
} finally {
resetProviders33();
await new Promise(r => s.close(r));
}
});
it('F5d: /health status entry has ok:false + error on healthCheck() throw', async () => {
setProviders33({ anthropic: true });
const savedProvider = loadedProviders33.get('anthropic');
// Inject a provider whose healthCheck always throws
const throwingProvider = {
...savedProvider,
healthCheck: async () => { throw new Error('probe-failed'); },
};
loadedProviders33.set('anthropic', throwingProvider);
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const r = await fetch({ port: p, method: 'GET', path: '/health' });
assert.equal(r.status, 200, '/health must still return 200 even when healthCheck throws');
const body = JSON.parse(r.body);
assert.ok(body.ok, 'top-level ok must still be true');
const ahc = body.providers.status.anthropic;
assert.equal(ahc.ok, false, 'status.anthropic.ok must be false when healthCheck throws');
assert.ok(typeof ahc.error === 'string', 'status.anthropic.error must be a string');
assert.ok(ahc.error.includes('probe-failed'), 'error must include the thrown message');
} finally {
if (savedProvider !== undefined) {
loadedProviders33.set('anthropic', savedProvider);
} else {
loadedProviders33.delete('anthropic');
}
resetProviders33();
await new Promise(r => s.close(r));
}
});
});
// ── F8_fallback: fallback-hop cache-hit → X-OLP-Cache: hit ───────────────
describe('F8_fallback — fallback-hop cache-hit reports X-OLP-Cache: hit', () => {
it('F8_fallback: 2-hop chain, primary fails, secondary cache-hit → X-OLP-Cache: hit', async () => {
// Two providers: primary (alpha) always fails with QUOTA_EXHAUSTED; secondary
// (beta) succeeds. On second request, secondary serves from cache.
// The X-OLP-Cache header must be 'hit' on the second request.
//
// Uses an explicit chain config: [alpha → beta]. alpha is enabled but fails;
// beta is enabled and succeeds. Secondary's cache key is different from primary's
// (different provider name), so the second request hits beta's cache.
setProviders33({ anthropic: true, mistral: true });
clearCache33();
// Override anthropic spawn to always throw QUOTA_EXHAUSTED
const savedAnthropic = loadedProviders33.get('anthropic');
const failingPrimary = {
...savedAnthropic,
spawn: async function* () {
throw new ProviderError('quota exhausted (test)', 'QUOTA_EXHAUSTED');
},
};
loadedProviders33.set('anthropic', failingPrimary);
// Override mistral spawn to yield a valid response
const savedMistral = loadedProviders33.get('mistral');
let secondarySpawnCount = 0;
const succeedingSecondary = {
...savedMistral,
spawn: async function* () {
secondarySpawnCount++;
yield { type: 'delta', role: 'assistant', content: 'fallback-response' };
yield { type: 'stop', finish_reason: 'stop' };
},
};
loadedProviders33.set('mistral', succeedingSecondary);
// Wire a 2-hop chain: anthropic (claude-sonnet-4-6) → mistral (devstral-2-25-12)
setFallbackConfig33({
chains: {
'claude-sonnet-4-6': [
{ provider: 'anthropic', model: 'claude-sonnet-4-6' },
{ provider: 'mistral', model: 'devstral-2-25-12' },
],
},
soft_triggers: {},
providersEnabled: { anthropic: true, mistral: true },
});
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
const makeRequest = () => fetch({
port: p,
method: 'POST',
path: '/v1/chat/completions',
body: {
model: 'claude-sonnet-4-6',
messages: [{ role: 'user', content: 'f8-fallback-cache-test' }],
stream: false,
},
});
try {
// First request: primary fails → secondary spawns → result cached under secondary key
const r1 = await makeRequest();
assert.equal(r1.status, 200, `r1 must be 200; got ${r1.status}`);
assert.equal(r1.headers['x-olp-cache'], 'miss',
'r1 must be cache miss (first time secondary serves)');
assert.equal(secondarySpawnCount, 1, 'secondary must spawn once for r1');
// Second request: primary still fails → secondary serves from its own cache
const r2 = await makeRequest();
assert.equal(r2.status, 200, `r2 must be 200; got ${r2.status}`);
assert.equal(r2.headers['x-olp-cache'], 'hit',
'r2 must be cache hit — fallback hop served from secondary cache');
assert.equal(secondarySpawnCount, 1,
'secondary must NOT spawn again for r2 (served from cache)');
} finally {
if (savedAnthropic !== undefined) {
loadedProviders33.set('anthropic', savedAnthropic);
} else {
loadedProviders33.delete('anthropic');
}
if (savedMistral !== undefined) {
loadedProviders33.set('mistral', savedMistral);
} else {
loadedProviders33.delete('mistral');
}
resetFallbackConfig33();
resetProviders33();
clearCache33();
await new Promise(r => s.close(r));
}
});
});
// ── F12: /v1/models stable `created` timestamps ──────────────────────────
describe('F12 — /v1/models stable per-model created timestamp', () => {
it('F12a: getModelCreated returns a stable number for known model IDs', () => {
// These models have explicit created fields in models-registry.json
const claudeSonnet = getModelCreated('claude-sonnet-4-6');
const claudeSonnet2 = getModelCreated('claude-sonnet-4-6');
assert.equal(claudeSonnet, claudeSonnet2,
'getModelCreated must return the same value on repeated calls');
assert.ok(typeof claudeSonnet === 'number' && claudeSonnet > 0,
'getModelCreated must return a positive number');
// claude-sonnet-4-6 has created=1775001600 (2026-04-01)
assert.equal(claudeSonnet, 1775001600,
'claude-sonnet-4-6 created must match models-registry.json entry');
});
it('F12b: getModelCreated falls back to REGISTRY_BOOTSTRAP_CREATED for unknown IDs', () => {
const unknown = getModelCreated('non-existent-model-xyz');
assert.equal(unknown, REGISTRY_BOOTSTRAP_CREATED,
'unknown model must fall back to REGISTRY_BOOTSTRAP_CREATED');
});
it('F12c: REGISTRY_BOOTSTRAP_CREATED matches models-registry.json bootstrapCreated', () => {
assert.equal(REGISTRY_BOOTSTRAP_CREATED, modelsRegistry.bootstrapCreated,
'REGISTRY_BOOTSTRAP_CREATED must equal models-registry.json bootstrapCreated');
});
it('F12d: /v1/models returns stable created for canonical models (not Date.now())', async () => {
setProviders33({ anthropic: true });
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const r1 = await fetch({ port: p, method: 'GET', path: '/v1/models' });
const r2 = await fetch({ port: p, method: 'GET', path: '/v1/models' });
assert.equal(r1.status, 200);
assert.equal(r2.status, 200);
const body1 = JSON.parse(r1.body);
const body2 = JSON.parse(r2.body);
// Both responses must have same created values for all entries
const entries1 = Object.fromEntries(body1.data.map(e => [e.id, e.created]));
const entries2 = Object.fromEntries(body2.data.map(e => [e.id, e.created]));
for (const [id, ts] of Object.entries(entries1)) {
assert.equal(entries2[id], ts,
`created for '${id}' must be stable across requests; got ${ts} vs ${entries2[id]}`);
}
// claude-sonnet-4-6 must use the registry value, not Date.now()
const sonnetEntry = body1.data.find(e => e.id === 'claude-sonnet-4-6');
assert.ok(sonnetEntry, 'claude-sonnet-4-6 must appear in /v1/models');
assert.equal(sonnetEntry.created, 1775001600,
'claude-sonnet-4-6 created must be 1775001600 (2026-04-01), not Date.now()');
} finally {
resetProviders33();
await new Promise(r => s.close(r));
}
});
it('F12e: alias entries in /v1/models use the canonical model\'s created timestamp', async () => {
setProviders33({ anthropic: true });
const s = createServer33();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const r = await fetch({ port: p, method: 'GET', path: '/v1/models' });
assert.equal(r.status, 200);
const body = JSON.parse(r.body);
// 'sonnet' alias → canonical 'claude-sonnet-4-6' → created 1775001600
const sonnetAlias = body.data.find(e => e.id === 'sonnet');
const sonnetCanon = body.data.find(e => e.id === 'claude-sonnet-4-6');
assert.ok(sonnetAlias, '"sonnet" alias must appear in /v1/models');
assert.ok(sonnetCanon, 'claude-sonnet-4-6 canonical must appear in /v1/models');
assert.equal(sonnetAlias.created, sonnetCanon.created,
'alias "sonnet" must have same created as canonical "claude-sonnet-4-6"');
} finally {
resetProviders33();
await new Promise(r => s.close(r));
}
});
});
});
});