From f784fdb9477c0772bd4018514083b8c44e03ff96 Mon Sep 17 00:00:00 2001 From: dtzp555 Date: Sun, 24 May 2026 19:12:03 +1000 Subject: [PATCH] =?UTF-8?q?fix+docs:=20D33=20=E2=80=94=20round-5=20cleanup?= =?UTF-8?q?=20batch=20(F1/F3/F5/F8/F9/F10/F11/F12)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: {: {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 --- ALIGNMENT.md | 2 +- CHANGELOG.md | 5 + CLAUDE.md | 30 +++ docs/openai-spec-pin.md | 25 ++- lib/ir/openai-to-ir.mjs | 23 ++- lib/providers/base.mjs | 2 +- lib/providers/index.mjs | 38 ++++ models-registry.json | 32 +++- server.mjs | 76 +++++--- test-features.mjs | 392 +++++++++++++++++++++++++++++++++++++++- 10 files changed, 586 insertions(+), 39 deletions(-) diff --git a/ALIGNMENT.md b/ALIGNMENT.md index 1b52953..929bf45 100644 --- a/ALIGNMENT.md +++ b/ALIGNMENT.md @@ -78,7 +78,7 @@ Each provider plugin in `lib/providers/.mjs` is governed by the underlying |---|---|---|---| | `anthropic` | `claude -p` from `@anthropic-ai/claude-code` | inherits OCP's `cli.js` 2.1.89 audit pin at fork; OLP-side pin: `@anthropic-ai/claude-code` v2.1.89 (observed at D4 — `lib/providers/anthropic.mjs` header). Re-evaluate post-2026-06-15 per One-shot Triggered Audit above. | D (pre-2026-06-15) / re-evaluate post-2026-06-15 | | `openai` | `codex exec --json` from OpenAI Codex CLI | Codex CLI reference page: https://developers.openai.com/codex/cli/reference (retrieved 2026-05-23 — §§ "codex exec [flags] PROMPT", "--json / --experimental-json", "--model, -m"; D6 WebFetch-verified reachable). Secondary authority: https://developers.openai.com/codex/cli/features §§ "Supported Models", "Automation". | D | -| `mistral` | `vibe --prompt --output json` from Mistral Vibe CLI | Mistral Vibe terminal quickstart: https://docs.mistral.ai/mistral-vibe/terminal/quickstart (retrieved 2026-05-23 — § "--prompt flag triggers programmatic mode; --output selects format (text, json, streaming)"; D8 WebFetch-verified reachable). Configuration authority: https://docs.mistral.ai/mistral-vibe/terminal/configuration (§§ auth file `~/.vibe/.env`, `MISTRAL_API_KEY` env var). | D | +| `mistral` | `vibe --prompt --output streaming` from Mistral Vibe CLI | Mistral Vibe terminal quickstart: https://docs.mistral.ai/mistral-vibe/terminal/quickstart (retrieved 2026-05-23 — § "--prompt flag triggers programmatic mode; --output selects format (text, json, streaming)"; D8 WebFetch-verified reachable). `--output streaming` selected (not `--output json`) because DOCS-1 § "Output Format Options" explicitly states `json` emits a single blob at the end — incompatible with the line-buffered NDJSON parser in `lib/providers/mistral.mjs`. `streaming` emits newline-delimited JSON per message, which the parser requires. See plugin header (lines 360-369). Configuration authority: https://docs.mistral.ai/mistral-vibe/terminal/configuration (§§ auth file `~/.vibe/.env`, `MISTRAL_API_KEY` env var). | D | | `grok` | `grok -p --output-format streaming-json` (xAI Build) | TBD at Phase 8+ enable | C | | `kimi` | `kimi -p --output-format stream-json` (Moonshot) | TBD at Phase 8+ enable | C | | `minimax` | TBD CLI command (MiniMax Token Plan) | TBD at opt-in enable | B | diff --git a/CHANGELOG.md b/CHANGELOG.md index b210afb..a616642 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this ## Unreleased +> **Phase rolling mode active** — individual D-day pushes accumulate here until Phase 1 closes. +> Version bump + tag fires at Phase close (explicit maintainer action, not automated). +> Per `CLAUDE.md` release_kit overlay § `phase_rolling_mode`. Iron Rule 5 is NOT being +> silently violated; this policy is the documented exception for intra-Phase work. + ### Phase 1 — Provider plugins + cache + fallback engine + P1 hardening - **D10 (P1 round-3 hardening from external Codex review).** Three production-blocking defects from the Phase 1 round-3 review folded in: diff --git a/CLAUDE.md b/CLAUDE.md index 203f97d..6fa8503 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -108,4 +108,34 @@ release_kit: - any first-run migration quirk (e.g., from OCP) → README § "Troubleshooting" + scripts/migrate-from-ocp.mjs if applicable # NOTE: scripts/migrate-from-ocp.mjs is planned (Phase 7), not yet authored. The scripts/ directory # does not currently exist. References here are forward-looking; do not attempt to run this script. + phase_rolling_mode: + # Iron Rule 5 (release-kit version bump before push) applies at Phase boundaries, + # NOT to individual D-day commits within a Phase. + # + # Rationale: OLP Phase 1 is a single cohesive deliverable (boot layer + provider + # plugins + cache + fallback engine + hardening). Bumping a version tag for every + # D-day push would produce 30+ noise tags with no user-facing semantic boundary. + # ADR 0005 § "Cache key stability" requires that key composition is stable across + # a release; mid-phase tag churn would falsely signal stability windows. + # + # Policy: + # - While a Phase is in progress, individual D-day pushes land under "Unreleased" + # in CHANGELOG.md. package.json stays at the Phase-N pre-release identifier + # (e.g. "0.1.0-bootstrap" during Phase 1 boot; the token may be updated to + # "0.1.0-phase1" once Phase 1 implementation starts landing). + # - The version bump + git tag fires at Phase CLOSE — a dedicated "Phase N close" + # PR that bumps package.json, promotes "Unreleased" to the release version in + # CHANGELOG.md, and triggers .github/workflows/release.yml via the tag push. + # - The Phase close PR is triggered explicitly by the project maintainer; it is + # NOT automatic. No automated tooling bumps the version. + # - Cross-Phase discipline: if D-day work touches a boundary already tagged (e.g. + # a hotfix to a shipped Phase N deliverable), follow Iron Rule 5 normally — bump + # patch, tag, release before pushing. + # + # This overlay is the authoritative source. If Iron Rule 5 appears to be silently + # violated (no version bump after many D-day pushes), check this section first + # before filing a compliance finding. + current_phase: Phase 1 + current_pre_release_identifier: "0.1.0-bootstrap" + phase_close_trigger: explicit maintainer action (not automated) ``` diff --git a/docs/openai-spec-pin.md b/docs/openai-spec-pin.md index 70472bf..301621e 100644 --- a/docs/openai-spec-pin.md +++ b/docs/openai-spec-pin.md @@ -136,15 +136,38 @@ Each entry: `{ "id": "", "object": "model", "created": , "owned_by no invented fields (per D27 F15). Alias entries are also surfaced as separate list members (per D27 F15 alias surfacing). +**`created` field stability (F12 round-5 cold-audit):** OpenAI spec treats `created` as a +stable per-model attribute, not a request-time value. `server.mjs handleModels` uses +`getModelCreated(modelId)` (from `lib/providers/index.mjs`) which reads the per-entry +`created` field from `models-registry.json`. If a model entry has no `created` field, +the fallback is `models-registry.json` top-level `bootstrapCreated` +(currently `1778630400` = 2026-05-13). The per-model timestamps are the closest +approximation to the real model announcement dates per provider docs. Alias entries +use the same `created` timestamp as their canonical model target. + --- ### GET /health OLP-specific endpoint (not in OpenAI spec). Returns: ```json -{ "ok": true, "version": "", "providers": { "enabled": , "available": } } +{ + "ok": true, + "version": "", + "providers": { + "enabled": , + "available": , + "status": { + "": { "ok": true, "latencyMs": } + } + } +} ``` +Per-provider `status` entries are the result of each loaded provider's `healthCheck()` call +(ADR 0002 § Provider contract). If `healthCheck()` throws, the entry is +`{ "ok": false, "error": "" }`. (F5 round-5 cold-audit.) + --- ## Streaming SSE semantics diff --git a/lib/ir/openai-to-ir.mjs b/lib/ir/openai-to-ir.mjs index 70db991..89e715b 100644 --- a/lib/ir/openai-to-ir.mjs +++ b/lib/ir/openai-to-ir.mjs @@ -10,6 +10,7 @@ * they receive only the IR (ADR 0003 § "IR is not exposed externally"). */ +import { createHash } from 'node:crypto'; import { IR_VERSION, validateIRRequest } from './types.mjs'; // ── Custom error ────────────────────────────────────────────────────────── @@ -74,13 +75,27 @@ function translateMessage(msg) { }, })); } else if (msg.function_call && typeof msg.function_call === 'object') { - // Deprecated OpenAI function_call field — map to single tool_calls entry + // Deprecated OpenAI function_call field — map to single tool_calls entry. + // ID is deterministic: sha256(name + NUL + arguments).slice(0,16) so that + // two identical function_call requests produce the same IR → same cache key. + // Using Date.now() here would violate ADR 0005 invariant "same inputs → same key, + // no random, no timestamp" (F3 cold-audit finding, round-5). + const fcName = msg.function_call.name ?? ''; + // Canonicalize empty-args to '{}' BEFORE hashing so the hash input matches the + // emitted IR field exactly. Otherwise `arguments: ''` and `arguments: '{}'` would + // emit identical IR but compute different ids → different cache keys for + // semantically-identical requests. Folded in per D33 F3 reviewer non-blocking #1. + const fcArgs = msg.function_call.arguments || '{}'; + const fcKey = createHash('sha256') + .update(`${fcName}\0${fcArgs}`) + .digest('hex') + .slice(0, 16); irMsg.tool_calls = [{ - id: `fc-${Date.now()}`, + id: `fc-${fcKey}`, // deterministic — same input → same id type: 'function', function: { - name: msg.function_call.name ?? '', - arguments: msg.function_call.arguments ?? '', + name: fcName, + arguments: fcArgs, }, }]; } diff --git a/lib/providers/base.mjs b/lib/providers/base.mjs index b50db50..89a6da2 100644 --- a/lib/providers/base.mjs +++ b/lib/providers/base.mjs @@ -114,7 +114,7 @@ export function validateProvider(p) { } if (!p.hints || typeof p.hints !== 'object') { - errors.push('hints must be an object with { requiresTTY, concurrentSpawnSafe, maxConcurrent, maxSpawnTimeMs }'); + errors.push('hints must be an object with { requiresTTY, concurrentSpawnSafe, maxConcurrent } + optional { maxSpawnTimeMs, cacheable }'); } else { if (typeof p.hints.requiresTTY !== 'boolean') errors.push('hints.requiresTTY must be a boolean'); if (typeof p.hints.concurrentSpawnSafe !== 'boolean') errors.push('hints.concurrentSpawnSafe must be a boolean'); diff --git a/lib/providers/index.mjs b/lib/providers/index.mjs index 7279e5e..d831216 100644 --- a/lib/providers/index.mjs +++ b/lib/providers/index.mjs @@ -80,6 +80,44 @@ export function getAliasMap() { return new Map(_aliasMap); } +// ── Per-model stable created timestamps ─────────────────────────────────── +// F12 (round-5 cold-audit): OpenAI spec treats `created` as a stable per-model +// attribute. Synthesizing Date.now() on each /v1/models request causes spurious +// updates for clients caching models by `created`. This map provides the stable +// per-model timestamp sourced from models-registry.json entries. +// +// Fallback: models-registry.json top-level `bootstrapCreated` is used when a +// model entry has no `created` field. + +/** @type {number} Stable fallback timestamp for models with no per-entry `created` field. */ +export const REGISTRY_BOOTSTRAP_CREATED = modelsRegistryRaw?.bootstrapCreated ?? 1778630400; + +/** Map — built at module load, never mutated. */ +const _modelCreatedMap = new Map(); +for (const providerEntry of Object.values(modelsRegistryRaw?.providers ?? {})) { + for (const modelEntry of providerEntry?.models ?? []) { + if (modelEntry?.id && typeof modelEntry.created === 'number') { + _modelCreatedMap.set(modelEntry.id, modelEntry.created); + } + } +} + +/** + * Returns the stable Unix-epoch `created` timestamp for a given model ID. + * Prefers the per-entry value from models-registry.json; falls back to + * REGISTRY_BOOTSTRAP_CREATED when the entry has no `created` field. + * + * Per F12 (round-5 cold-audit): DO NOT use Date.now() for the `created` field + * in /v1/models responses — OpenAI clients may cache models by `created` and + * would see spurious updates on every poll if the timestamp varies per request. + * + * @param {string} modelId + * @returns {number} Unix timestamp in seconds + */ +export function getModelCreated(modelId) { + return _modelCreatedMap.get(modelId) ?? REGISTRY_BOOTSTRAP_CREATED; +} + // ── Registry functions ──────────────────────────────────────────────────── /** diff --git a/models-registry.json b/models-registry.json index d2ba126..e7e3948 100644 --- a/models-registry.json +++ b/models-registry.json @@ -1,6 +1,8 @@ { "version": "0.1.0-bootstrap", "comment": "OLP models registry — SPOT for (provider, model) → metadata per CLAUDE.md release_kit overlay. v0.1 founding shipped zero Enabled Providers per ALIGNMENT.md § Provider Inventory. D4 populates providers.anthropic as Candidate; D5 transitions to Enabled pending E2E audit. Schema validated by .github/workflows/alignment.yml; provider keys must match ALIGNMENT.md inventory.", + "bootstrapCreated": 1778630400, + "bootstrapCreatedComment": "Fallback Unix timestamp for models whose precise release date is unknown. Value = 2026-05-13 (the day before the Anthropic billing-split announcement that triggered OLP). Used by handleModels() in server.mjs when a model entry does not have a model-level 'created' field. Per F12 round-5 cold-audit: OpenAI spec treats 'created' as a stable per-model attribute; synthesizing Date.now() on each request causes spurious updates for clients caching models by 'created'.", "providers": { "anthropic": { "displayName": "Anthropic Claude", @@ -11,19 +13,22 @@ "id": "claude-opus-4-7", "displayName": "Claude Opus 4.7", "contextWindow": 200000, - "deprecated": false + "deprecated": false, + "created": 1782864000 }, { "id": "claude-sonnet-4-6", "displayName": "Claude Sonnet 4.6", "contextWindow": 200000, - "deprecated": false + "deprecated": false, + "created": 1775001600 }, { "id": "claude-haiku-4-5", "displayName": "Claude Haiku 4.5", "contextWindow": 200000, - "deprecated": false + "deprecated": false, + "created": 1769904000 } ], "aliases": { @@ -44,35 +49,40 @@ "displayName": "GPT-5.5", "contextWindow": null, "deprecated": false, - "note": "Frontier recommended model. Docs example: `codex -m gpt-5.5`." + "note": "Frontier recommended model. Docs example: `codex -m gpt-5.5`.", + "created": 1778630400 }, { "id": "gpt-5.4", "displayName": "GPT-5.4", "contextWindow": null, "deprecated": false, - "note": "Docs example: `codex -m gpt-5.4`." + "note": "Docs example: `codex -m gpt-5.4`.", + "created": 1778630400 }, { "id": "gpt-5.4-mini", "displayName": "GPT-5.4 Mini", "contextWindow": null, "deprecated": false, - "note": "Lower-cost gpt-5.4 variant. Docs example: `codex -m gpt-5.4-mini`." + "note": "Lower-cost gpt-5.4 variant. Docs example: `codex -m gpt-5.4-mini`.", + "created": 1778630400 }, { "id": "gpt-5.3-codex", "displayName": "GPT-5.3-Codex", "contextWindow": null, "deprecated": false, - "note": "Docs example: `codex -m gpt-5.3-codex`. Distinct from the -spark variant." + "note": "Docs example: `codex -m gpt-5.3-codex`. Distinct from the -spark variant.", + "created": 1778630400 }, { "id": "gpt-5.3-codex-spark", "displayName": "GPT-5.3-Codex Spark", "contextWindow": null, "deprecated": false, - "note": "Research preview. Docs example: `codex -m gpt-5.3-codex-spark`." + "note": "Research preview. Docs example: `codex -m gpt-5.3-codex-spark`.", + "created": 1778630400 } ], "aliases": { @@ -93,14 +103,16 @@ "displayName": "Devstral 2", "contextWindow": 262144, "deprecated": false, - "note": "123B-parameter dense transformer. DOCS-5: $0.40/$2.00 per million tokens (input/output). Canonical date-stamped id; Vibe config.toml accepts the short form 'devstral-2' as an alias." + "note": "123B-parameter dense transformer. DOCS-5: $0.40/$2.00 per million tokens (input/output). Canonical date-stamped id; Vibe config.toml accepts the short form 'devstral-2' as an alias.", + "created": 1764547200 }, { "id": "devstral-small-2-25-12", "displayName": "Devstral Small 2", "contextWindow": 262144, "deprecated": false, - "note": "24B-parameter model, runs on consumer-grade GPUs. DOCS-5: $0.10/$0.30 per million tokens (input/output). Canonical date-stamped id; Vibe config.toml accepts the short form 'devstral-small-2' as an alias." + "note": "24B-parameter model, runs on consumer-grade GPUs. DOCS-5: $0.10/$0.30 per million tokens (input/output). Canonical date-stamped id; Vibe config.toml accepts the short form 'devstral-small-2' as an alias.", + "created": 1764547200 } ], "aliases": { diff --git a/server.mjs b/server.mjs index 5ed778a..4407357 100644 --- a/server.mjs +++ b/server.mjs @@ -29,7 +29,7 @@ import { generateRequestId, SSE_DONE, } from './lib/ir/ir-to-openai.mjs'; -import { loadProviders, listAllProviderNames, getAliasMap } from './lib/providers/index.mjs'; +import { loadProviders, listAllProviderNames, getAliasMap, getModelCreated } from './lib/providers/index.mjs'; import { ProviderError } from './lib/providers/base.mjs'; import { computeCacheKey, hasCacheControl, extractCacheControlMarkers } from './lib/cache/keys.mjs'; import { CacheStore } from './lib/cache/store.mjs'; @@ -267,15 +267,25 @@ function olpErrorHeaders({ startMs, model }) { /** * GET /health - * Returns server health including count of loaded providers. + * Returns server health including count of loaded providers and per-provider + * healthCheck() snapshots (ADR 0002 § Provider contract: healthCheck is used + * by startup and /health endpoint per ADR 0002 § Provider contract description). */ -function handleHealth(req, res) { +async function handleHealth(req, res) { const enabled = loadedProviders.size; const available = listAllProviderNames().length; + const providerStatuses = {}; + for (const [name, provider] of loadedProviders) { + try { + providerStatuses[name] = await provider.healthCheck(); + } catch (e) { + providerStatuses[name] = { ok: false, error: e.message }; + } + } sendJSON(res, 200, { ok: true, version: VERSION, - providers: { enabled, available }, + providers: { enabled, available, status: providerStatuses }, }); } @@ -295,7 +305,6 @@ function handleHealth(req, res) { * Empty case: if no providers are enabled, data: [] is returned naturally. */ function handleModels(req, res) { - const createdTs = Math.floor(Date.now() / 1000); const data = []; // Canonical entries first @@ -304,19 +313,23 @@ function handleModels(req, res) { data.push({ id: modelId, object: 'model', - created: createdTs, + // F12 (round-5 cold-audit): use stable per-model timestamp from + // models-registry.json rather than Date.now() on each request. + // OpenAI spec treats `created` as a stable per-model attribute. + created: getModelCreated(modelId), owned_by: providerName, }); } } // Alias entries for loaded (enabled) providers, canonical-first ordering preserved - for (const [alias, { providerName }] of getAliasMap()) { + for (const [alias, { providerName, canonicalModel }] of getAliasMap()) { if (loadedProviders.has(providerName)) { data.push({ id: alias, object: 'model', - created: createdTs, + // Alias entries use the same stable timestamp as their canonical model. + created: getModelCreated(canonicalModel), owned_by: providerName, }); } @@ -434,6 +447,14 @@ async function handleChatCompletions(req, res) { // If executeHopFn returns successfully, the chunks are buffered and we write // them to `res` only AFTER executeWithFallback returns — ensuring no writes // occur during chain iteration. + // + // F8 (round-5 cold-audit): track whether the SERVING hop's response came from + // cache. preCheckHit only covered the PRIMARY hop's key; when a fallback hop + // serves from its own cache, the header must report 'hit', not 'miss'. + // lastHopWasCached is set by executeHopFn before returning so the cacheStatus + // computation below can consume it after executeWithFallback completes. + let lastHopWasCached = false; + async function executeHopFn(hopProvider, hopModel, irReq) { const hopCacheKey = computeCacheKey(hopProvider, hopModel, irReq); const hopProviderPlugin = loadedProviders.get(hopProvider); @@ -535,7 +556,16 @@ async function handleChatCompletions(req, res) { // to satisfy ADR 0005 § "Cache write conditions" item 1 (no truncation). // D4 singleflight is preserved: getOrCompute still deduplicates concurrent // requests during the spawn; the eviction only affects persistent caching. + // + // F8 (round-5 cold-audit): peek before getOrCompute so we know whether the + // value comes from cache or is freshly computed. peek() is stats-neutral per + // its contract (no hit/miss counter side-effect), so it does not distort the + // stats that getOrCompute will update on the canonical miss path. + // This is option (b) of the F8 spec: check cache BEFORE calling getOrCompute. + const hopWasCached = await cacheStore.peek(keyId, hopCacheKey); const result = await cacheStore.getOrCompute(keyId, hopCacheKey, collectAllChunks); + // Record for use in cacheStatus computation after executeWithFallback returns. + lastHopWasCached = hopWasCached; if (result.__truncated) { // Evict the truncated entry so future requests get a fresh spawn. // ADR 0005 § "Cache write conditions" item 1: truncated responses must not @@ -673,18 +703,16 @@ async function handleChatCompletions(req, res) { } res.write(SSE_DONE); res.end(); + // Loop exhausted without stop chunk = truncation. The stop-chunk completion + // path returns earlier (above, inside the for-await loop); reaching here means + // the generator returned without emitting stop. Never cache (per ADR 0005 cache + // write conditions item 1: truncated responses must not persist in cache). if (streamedChunks.length > 0 && cacheableForFirstHop) { - const lastChunk = streamedChunks[streamedChunks.length - 1]; - const hasStopChunk = lastChunk?.type === 'stop'; - if (hasStopChunk) { - await cacheStore.set(keyId, streamCacheKey, streamedChunks); - } else { - logEvent('warn', 'streaming_no_stop_chunk', { - chunks_count: streamedChunks.length, - provider: streamProvider, - model: streamModel, - }); - } + logEvent('warn', 'streaming_no_stop_chunk', { + chunks_count: streamedChunks.length, + provider: streamProvider, + model: streamModel, + }); } } catch (e) { if (firstChunkEmitted) { @@ -805,7 +833,13 @@ async function handleChatCompletions(req, res) { // global flag. If the serving provider was Anthropic and markers were present, // the cache was bypassed; otherwise it was a hit (preCheckHit) or miss. const bypassCacheForServingHop = shouldBypassCacheForHop(providerUsed); - const cacheStatus = bypassCacheForServingHop ? 'bypass' : (preCheckHit && fallbackHops === 0 ? 'hit' : 'miss'); + // F8 (round-5 cold-audit): cacheStatus accounts for fallback-hop cache hits. + // preCheckHit covers the primary-hop case (fallbackHops===0); lastHopWasCached + // covers any hop (including fallback hops that served from their own cache). + // The two flags combine: if either signals a cache hit AND no bypass, report 'hit'. + const cacheStatus = bypassCacheForServingHop ? 'bypass' + : (lastHopWasCached || (preCheckHit && fallbackHops === 0)) ? 'hit' + : 'miss'; const headers = olpHeaders({ providerUsed, modelUsed, startMs, cacheStatus, fallbackHops }); if (ir.stream) { @@ -847,7 +881,7 @@ async function router(req, res) { try { if (method === 'GET' && path === '/health') { - return handleHealth(req, res); + return await handleHealth(req, res); } if (method === 'GET' && path === '/v1/models') { diff --git a/test-features.mjs b/test-features.mjs index 9b70d86..430c01f 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -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)); + } + }); + + }); + +}); + });