mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
feat(server)+fix(obs): D18 — populate /v1/models + standard X-OLP-* on error paths (Findings 10 + 11)
cold-audit catch from 2026-05-23
Cold-audit Findings 10 + 11 (both P3, server + observability). F10:
/v1/models returned {data: []} unconditionally; README claimed it
lists from models-registry.json — a client calling /v1/models to
discover what OLP serves would conclude OLP has no models. F11: error
responses (chain-exhausted, pre-routing 4xx) omitted the standard
5-header set; ADR 0004 § Observability requires every response to
carry X-OLP-{Provider-Used, Model-Used, Fallback-Hops, Cache, Latency-Ms}.
Changes (2 files, +427 / -9):
1. server.mjs handleModels: populate from loaded providers × canonical
model IDs. Each entry has exactly {id, object: 'model', created,
owned_by} — no invented fields (Rule 2(b)). `created` is computed
once per request as Math.floor(Date.now()/1000). Iteration order is
insertion-order of loadedProviders Map × insertion-order of each
provider's models[]. Empty case (no providers enabled) returns
{object: 'list', data: []} via natural empty iteration. Aliases are
NOT included — per D17 SPOT, models[] is canonical-only; the
/v1/models endpoint surfaces canonical IDs only.
2. server.mjs sendError extended with optional 5th arg extraHeaders.
Non-breaking — existing call sites (10 of them) use the default {}.
Three pre-routing call sites inside handleChatCompletions now pass
X-OLP-Latency-Ms (computed at-error-time as Date.now() - startMs):
- 415 Content-Type mismatch
- 400 invalid JSON body
- 400 IR parse error
The 404 (route not found) and 500 (top-level catch) paths are
outside handleChatCompletions and have no startMs — they remain
without latency rather than synthesize a value.
3. server.mjs chain-exhausted error path: replaced minimal-header
block with olpHeaders({...}) call producing the full standard
5-tuple. X-OLP-Fallback-Exhausted is preserved as an additional
flag layered on top. The 5 header values reflect engine state per
ADR 0004 step 4 ("preserve A's identity — return FIRST hop's
provider/model and original error to user"): providerUsed =
chain[0].provider, modelUsed = chain[0].model, fallbackHops =
attempted count, cacheStatus = 'miss'.
4. test-features.mjs Suite 17 — 7 new tests:
- 17a: /v1/models with anthropic enabled → 3 entries, all
owned_by='anthropic', no aliases in response
- 17b: /v1/models with no providers → {object:'list', data:[]}
- 17c: /v1/models entries have only {id, object, created, owned_by}
(no invented fields — Rule 2(b) assertion)
- 17d: /v1/models with all 3 providers → canonical IDs present,
aliases absent (sonnet/devstral not in response)
- 17e: chain-exhausted response has all 5 standard X-OLP-* headers
present + X-OLP-Fallback-Exhausted
- 17f: 2-hop chain both fail → X-OLP-Fallback-Hops: '2' value check
- 17g: pre-routing 400 invalid JSON has X-OLP-Latency-Ms (non-negative
integer); X-OLP-Provider-Used absent (no provider context — honest)
Tests: 317 → 324 (+7). All pass on Node 20.
Pre-commit fold-in (per evidence-first checkpoint #4):
- **D18 reviewer flagged Concern #1**: original inline comment described
providerUsed as "last-attempted provider name." This was wrong —
lib/fallback/engine.mjs returns chain[0].provider on chain-exhausted
(per ADR 0004 step 4 "preserve A's identity"), which is the FIRST
hop, not the last. The "last-attempted" framing came from my own
dispatch brief and the implementer accurately reflected the brief.
Folded in: corrected the comment to honestly describe what the
engine returns. Same class of doc-vs-reality drift as D11 / D16 /
D17 — the cold-audit + diff-review combination is catching these
consistently across the P3 batch.
Authority:
- ADR 0002 § Loading model — `models: string[]` enumeration
- ADR 0004 § Observability headers — the 5-header standard set
https://github.com/dtzp555-max/olp/blob/main/docs/adr/0004-fallback-engine.md
- ADR 0004 step 4 — "preserve A's identity" on chain exhaustion
- OpenAI Chat Completions spec /v1/models response shape
https://platform.openai.com/docs/api-reference/models/list
- ALIGNMENT.md Rule 2(b) — only spec-defined fields in OpenAI responses
- CC 开发铁律 v1.6 § 10.x — Cold Audit Findings 10 + 11
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Verified all 5 X-OLP-* headers fire
on chain-exhausted path; verified Math.floor(Date.now()/1000) computed
once per request (not per-model); verified aliases excluded; verified
404 + 500 paths honestly omit latency rather than synthesize. Caught
the providerUsed-comment drift which was folded in.
Follow-up items (reviewer's non-blocking observations, NOT in this PR):
- 4 other error sites inside handleChatCompletions have startMs
available but don't emit X-OLP-Latency-Ms (503 no_enabled_provider,
503 provider-not-enabled-streaming, 502 streaming post-error, 500
fallback programming error). The "Latency-Ms-when-startMs-is-available"
rule should be uniform; file as 11b or future cleanup
- Tests 17e/17f could strengthen by asserting specific values not just
presence (e.g., x-olp-provider-used === 'anthropic'); already done
for fallback-hops in 17f
- Test 17e/17f setup duplication — extractable helper for cosmetic
cleanup
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+54
-9
@@ -190,9 +190,10 @@ function sendJSON(res, status, body, extraHeaders = {}) {
|
||||
* @param {number} status
|
||||
* @param {string} message
|
||||
* @param {string} type
|
||||
* @param {Record<string,string>} [extraHeaders] — optional extra headers (e.g. X-OLP-Latency-Ms)
|
||||
*/
|
||||
function sendError(res, status, message, type) {
|
||||
sendJSON(res, status, { error: { message, type } });
|
||||
function sendError(res, status, message, type, extraHeaders = {}) {
|
||||
sendJSON(res, status, { error: { message, type } }, extraHeaders);
|
||||
}
|
||||
|
||||
// ── OLP response headers ──────────────────────────────────────────────────
|
||||
@@ -241,11 +242,31 @@ function handleHealth(req, res) {
|
||||
|
||||
/**
|
||||
* GET /v1/models
|
||||
* Returns an empty data array at D3.
|
||||
* Will be populated from models-registry.json + loaded providers in Phase 1 Day 2.
|
||||
* Returns the list of models served by all currently loaded (enabled) providers.
|
||||
* Per ADR 0002 § "Loading model" + OpenAI spec /v1/models:
|
||||
* Each entry: { id, object: 'model', created, owned_by }
|
||||
* - id: canonical model ID from the provider's models[] array
|
||||
* - object: literal 'model' (OpenAI spec)
|
||||
* - created: Unix epoch seconds (stable per request; computed once from Date.now())
|
||||
* - owned_by: provider.name (e.g. 'anthropic', 'openai', 'mistral')
|
||||
* Only canonical IDs are emitted (no aliases — per D17 SPOT decision).
|
||||
* Order: insertion order of loadedProviders, then insertion order of each provider's models[].
|
||||
* Empty case: if no providers are enabled, data: [] is returned naturally.
|
||||
*/
|
||||
function handleModels(req, res) {
|
||||
sendJSON(res, 200, { object: 'list', data: [] });
|
||||
const createdTs = Math.floor(Date.now() / 1000);
|
||||
const data = [];
|
||||
for (const [providerName, provider] of loadedProviders) {
|
||||
for (const modelId of provider.models) {
|
||||
data.push({
|
||||
id: modelId,
|
||||
object: 'model',
|
||||
created: createdTs,
|
||||
owned_by: providerName,
|
||||
});
|
||||
}
|
||||
}
|
||||
sendJSON(res, 200, { object: 'list', data });
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -267,14 +288,16 @@ async function handleChatCompletions(req, res) {
|
||||
// Require JSON content-type
|
||||
const ct = req.headers['content-type'] ?? '';
|
||||
if (!ct.includes('application/json')) {
|
||||
return sendError(res, 415, 'Content-Type must be application/json', 'invalid_request_error');
|
||||
return sendError(res, 415, 'Content-Type must be application/json', 'invalid_request_error',
|
||||
{ 'X-OLP-Latency-Ms': String(Date.now() - startMs) });
|
||||
}
|
||||
|
||||
let body;
|
||||
try {
|
||||
body = await readJSON(req);
|
||||
} catch (e) {
|
||||
return sendError(res, e.statusCode ?? 400, e.message, 'invalid_request_error');
|
||||
return sendError(res, e.statusCode ?? 400, e.message, 'invalid_request_error',
|
||||
{ 'X-OLP-Latency-Ms': String(Date.now() - startMs) });
|
||||
}
|
||||
|
||||
// Translate OpenAI → IR (ADR 0003)
|
||||
@@ -283,7 +306,8 @@ async function handleChatCompletions(req, res) {
|
||||
ir = openAIToIR(body);
|
||||
} catch (e) {
|
||||
if (e instanceof BadRequestError) {
|
||||
return sendError(res, 400, e.message, 'invalid_request_error');
|
||||
return sendError(res, 400, e.message, 'invalid_request_error',
|
||||
{ 'X-OLP-Latency-Ms': String(Date.now() - startMs) });
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
@@ -626,7 +650,27 @@ async function handleChatCompletions(req, res) {
|
||||
}
|
||||
}
|
||||
|
||||
// Send error with exhausted header
|
||||
// Per ADR 0004 § Observability headers: all responses (including errors) carry
|
||||
// the standard 5-header set. On the exhausted/error path the engine returns
|
||||
// values per ADR 0004 step 4 ("preserve A's identity — return the FIRST hop's
|
||||
// provider/model and original error to the user"):
|
||||
// - providerUsed: chain[0].provider on chain-exhausted (the primary that
|
||||
// first failed); set to 'none' only if engine somehow returned null
|
||||
// - modelUsed: chain[0].model on chain-exhausted, or the original request
|
||||
// model if engine state is unknown
|
||||
// - cacheStatus: 'miss' — all hops were attempted (bypass is per-hop, not relevant
|
||||
// when the whole chain exhausted)
|
||||
// - fallbackHops: number of hops actually attempted before exhaustion
|
||||
// X-OLP-Fallback-Exhausted is preserved as an additional flag on top of these.
|
||||
const errorOlpHeaders = olpHeaders({
|
||||
providerUsed: providerUsed ?? 'none',
|
||||
modelUsed: modelUsed ?? ir.model,
|
||||
startMs,
|
||||
cacheStatus: 'miss',
|
||||
fallbackHops: fallbackHops ?? 0,
|
||||
});
|
||||
|
||||
// Send error with standard OLP headers + optional exhausted header
|
||||
const payload = JSON.stringify({
|
||||
error: {
|
||||
message: originalError?.message ?? 'Provider error',
|
||||
@@ -636,6 +680,7 @@ async function handleChatCompletions(req, res) {
|
||||
res.writeHead(errStatus, {
|
||||
'Content-Type': 'application/json',
|
||||
'Content-Length': Buffer.byteLength(payload),
|
||||
...errorOlpHeaders,
|
||||
...exhaustedHeader,
|
||||
});
|
||||
res.end(payload);
|
||||
|
||||
Reference in New Issue
Block a user