mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-19 09:45:07 +00:00
cold-audit catch from 2026-05-24 (round 3)
3 round-3 P3 items batched per IDR. Mixed surfaces but all single-severity
and conceptually independent.
Changes (5 files, +324 / -17):
1. lib/ir/types.mjs — F8 validator extension (+28):
- `response_format`: must be object with string `.type` (undefined/omitted
accepted; null, non-object, missing-type all rejected). Forward-compat:
accepts any string for type so future OpenAI additions (json_schema,
etc.) flow through without schema bump.
- `tool_choice`: string form 'auto'/'none'/'required' OR object form
`{type:'function', function:{name:string}}`. All other shapes rejected.
Pre-D27 the validator silently accepted any value; the Anthropic plugin's
`if (irRequest.response_format?.type === 'json_object')` would no-op on
a malformed string payload.
2. docs/adr/0005-cache-cross-provider.md — F10 Amendment 4 (+8):
- Documents the IR-vs-body detection ambiguity in § D2: the ADR text
reads as if detection happens on the IR, but `openai-to-ir.mjs` strips
`cache_control` from messages per ADR 0003's whitelist policy, so
IR-side detection is structurally always empty at v1.0
- Documents the actual v1.0 detection mechanism (server.mjs side-channels
into the raw body)
- Documents the cache key `cache_control` slot's always-null status as
forward-compat (when a future ADR 0003 amendment adds cache_control
to IR, the slot will start carrying meaningful data without schema bump)
- Explicit "no code change" — F10 is docs-only
3. lib/providers/index.mjs — F15 alias map export (+11):
- `getAliasMap()` returns `new Map(_aliasMap)` — defensive copy preventing
caller mutation of the module-private alias map
- JSDoc documents use case + defensive-copy intent
4. server.mjs — F15 /v1/models alias surfacing (+25/-7):
- `handleModels` now emits TWO loops: canonical entries first (preserves
existing client expectations), then alias entries via `getAliasMap()`
- Each alias entry has the same 4 OpenAI-spec fields (id/object/created/
owned_by) — no invented fields per ALIGNMENT Rule 2(b)
- Disabled-provider alias non-leak: `loadedProviders.has(providerName)`
gate skips aliases whose target provider is not currently enabled
- createdTs reused (same per-request timestamp across all entries)
- JSDoc updated to document new ordering + F15 origin
5. test-features.mjs — +269 / +18 new tests:
- F8: 13 tests covering response_format (object/string/non-object/missing-
type) + tool_choice (string variants/object variants/wrong type/no name)
- F15: 5 tests covering canonical-first ordering, alias presence/count,
disabled-provider non-leak (with anthropic-only enabled, mistral and
openai aliases must NOT appear), Rule 2(b) shape conformance
Tests: 358 → 376 (+18). All pass on Node 20.
Reviewer notes (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE. Critical checks verified:
- F8: all 9+ tool_choice rejection axes traced through code (including
partial-function-object edge cases). The code handles them correctly even
where tests don't exercise (non-blocking gap).
- F10: amendment substantively correct. One wording-precision note: the
amendment says "body only" but the actual code is an OR-disjunction
(hasCacheControl(ir) || extractCacheControlMarkers(body.messages)).
Functionally identical at v1.0 because IR strips markers, so first
disjunct is always false. Forward-compat by construction.
- F15: disabled-provider non-leak verified by manual trace with
anthropic-only enabled — mistral/openai aliases correctly filtered out
by `loadedProviders.has(providerName)` gate. Test 17e explicitly
asserts this.
- F15+D17 round-trip: client GET /v1/models → sees alias entry → POSTs
with alias → getProviderForModel resolves via same _aliasMap → cache
key uses canonical → response works. Both surfaces read the same Map
(SPOT).
- F15+D23 cacheable interaction: cacheable opt-out doesn't suppress
alias surfacing in /v1/models — cacheable is about cache-write behavior
while discovery should still surface enabled providers. Intentional.
Authority:
- ADR 0003 § Optional fields (response_format + tool_choice IR shape)
- OpenAI Chat Completions API spec (the field semantics)
https://platform.openai.com/docs/api-reference/chat/create
- ADR 0005 § D2 + Amendment 4 (F10's own amendment landing here)
- ADR 0002 § Loading model + D17 getProviderForModel SPOT (F15's alias
origin)
- ALIGNMENT.md Rule 2(b) — no invented OpenAI fields
- CC 开发铁律 v1.6 § 10.x — Round-3 Cold Audit caught all 3
Follow-up items (reviewer's non-blocking suggestions, NOT in this PR):
- F8 micro-tests for null/boolean/partial-function inputs (code handles;
test gap only)
- F10 wording precision on OR-disjunction
- Nested-describe wrapper quirk in test-features.mjs (pre-existing
structural issue; D26/D27 describes are children of Suite 16 wrapper).
Cleanup in a future hygiene pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
201 lines
7.1 KiB
JavaScript
201 lines
7.1 KiB
JavaScript
/**
|
|
* lib/ir/types.mjs — IR v1.0 type definitions and validators
|
|
*
|
|
* Authority: ADR 0003 § "Required fields" and "Optional fields"
|
|
* The IR is OLP-internal; there is no external IR endpoint.
|
|
* Per ADR 0003, the IR encodes the common subset that every provider
|
|
* plugin can consume, with lossy edges documented per-provider.
|
|
*/
|
|
|
|
// ── Constants ──────────────────────────────────────────────────────────────
|
|
|
|
export const IR_VERSION = '1.0';
|
|
|
|
/** Per ADR 0003 § Required fields. role='function' is OpenAI-deprecated; openai-to-ir.mjs maps it to 'tool'. */
|
|
export const VALID_ROLES = ['system', 'user', 'assistant', 'tool'];
|
|
|
|
// ── JSDoc typedefs ────────────────────────────────────────────────────────
|
|
|
|
/**
|
|
* @typedef {Object} IRToolCall
|
|
* @property {string} id
|
|
* @property {'function'} type
|
|
* @property {{ name: string, arguments: string }} function
|
|
*/
|
|
|
|
/**
|
|
* @typedef {Object} IRMessage
|
|
* @property {'system'|'user'|'assistant'|'tool'} role
|
|
* @property {string|Array<{type:string,[key:string]:any}>} content
|
|
* @property {string} [name]
|
|
* @property {IRToolCall[]} [tool_calls]
|
|
* @property {string} [tool_call_id]
|
|
*/
|
|
|
|
/**
|
|
* @typedef {Object} IRToolDefinition
|
|
* @property {'function'} type
|
|
* @property {{ name: string, description?: string, parameters?: object }} function
|
|
*/
|
|
|
|
/**
|
|
* @typedef {Object} IRRequest
|
|
* @property {string} irVersion - always IR_VERSION
|
|
* @property {IRMessage[]} messages
|
|
* @property {string} model
|
|
* @property {boolean} stream
|
|
* @property {number} [max_tokens]
|
|
* @property {number} [temperature]
|
|
* @property {number} [top_p]
|
|
* @property {string|string[]} [stop]
|
|
* @property {IRToolDefinition[]} [tools]
|
|
* @property {string|object} [tool_choice]
|
|
* @property {object} [response_format]
|
|
*/
|
|
|
|
/**
|
|
* @typedef {Object} IRResponseChunk
|
|
* @property {'delta'|'stop'|'error'} type
|
|
* @property {string} [content] - present when type==='delta'
|
|
* @property {string} [role] - present on first delta
|
|
* @property {IRToolCall[]} [tool_calls] - present when provider emits tool-use in stream
|
|
* @property {string} [finish_reason] - present when type==='stop'
|
|
* @property {string} [error] - present when type==='error'
|
|
* @property {Object} [usage]
|
|
* @property {number} [usage.prompt_tokens]
|
|
* @property {number} [usage.completion_tokens]
|
|
* @property {number} [usage.total_tokens]
|
|
*/
|
|
|
|
// ── Validators ────────────────────────────────────────────────────────────
|
|
|
|
/**
|
|
* @param {*} obj
|
|
* @returns {{ valid: boolean, errors: string[] }}
|
|
*/
|
|
export function validateIRMessage(obj) {
|
|
const errors = [];
|
|
if (obj === null || typeof obj !== 'object') {
|
|
errors.push('message must be an object');
|
|
return { valid: false, errors };
|
|
}
|
|
if (!VALID_ROLES.includes(obj.role)) {
|
|
errors.push(`role must be one of ${VALID_ROLES.join('|')}, got ${JSON.stringify(obj.role)}`);
|
|
}
|
|
if (obj.content === undefined || obj.content === null) {
|
|
errors.push('content is required');
|
|
} else if (typeof obj.content !== 'string' && !Array.isArray(obj.content)) {
|
|
errors.push('content must be a string or array of content parts');
|
|
}
|
|
if (obj.name !== undefined && typeof obj.name !== 'string') {
|
|
errors.push('name must be a string when present');
|
|
}
|
|
if (obj.tool_call_id !== undefined && typeof obj.tool_call_id !== 'string') {
|
|
errors.push('tool_call_id must be a string when present');
|
|
}
|
|
if (obj.tool_calls !== undefined) {
|
|
if (!Array.isArray(obj.tool_calls)) {
|
|
errors.push('tool_calls must be an array when present');
|
|
}
|
|
}
|
|
return { valid: errors.length === 0, errors };
|
|
}
|
|
|
|
/**
|
|
* @param {*} obj
|
|
* @returns {{ valid: boolean, errors: string[] }}
|
|
*/
|
|
export function validateIRRequest(obj) {
|
|
const errors = [];
|
|
if (obj === null || typeof obj !== 'object') {
|
|
errors.push('IR request must be an object');
|
|
return { valid: false, errors };
|
|
}
|
|
|
|
// Required: messages
|
|
if (!Array.isArray(obj.messages)) {
|
|
errors.push('messages must be an array');
|
|
} else {
|
|
obj.messages.forEach((m, i) => {
|
|
const r = validateIRMessage(m);
|
|
if (!r.valid) {
|
|
r.errors.forEach(e => errors.push(`messages[${i}]: ${e}`));
|
|
}
|
|
});
|
|
if (obj.messages.length === 0) {
|
|
errors.push('messages must not be empty');
|
|
}
|
|
}
|
|
|
|
// Required: model
|
|
if (typeof obj.model !== 'string' || obj.model.trim() === '') {
|
|
errors.push('model must be a non-empty string');
|
|
}
|
|
|
|
// Required: stream
|
|
if (typeof obj.stream !== 'boolean') {
|
|
errors.push('stream must be a boolean');
|
|
}
|
|
|
|
// Optional: max_tokens
|
|
if (obj.max_tokens !== undefined && (!Number.isInteger(obj.max_tokens) || obj.max_tokens <= 0)) {
|
|
errors.push('max_tokens must be a positive integer when present');
|
|
}
|
|
|
|
// Optional: temperature [0,2]
|
|
if (obj.temperature !== undefined) {
|
|
if (typeof obj.temperature !== 'number' || obj.temperature < 0 || obj.temperature > 2) {
|
|
errors.push('temperature must be a number in [0,2] when present');
|
|
}
|
|
}
|
|
|
|
// Optional: top_p [0,1]
|
|
if (obj.top_p !== undefined) {
|
|
if (typeof obj.top_p !== 'number' || obj.top_p < 0 || obj.top_p > 1) {
|
|
errors.push('top_p must be a number in [0,1] when present');
|
|
}
|
|
}
|
|
|
|
// Optional: stop
|
|
if (obj.stop !== undefined) {
|
|
if (typeof obj.stop !== 'string' && !Array.isArray(obj.stop)) {
|
|
errors.push('stop must be a string or array of strings when present');
|
|
}
|
|
}
|
|
|
|
// Optional: tools
|
|
if (obj.tools !== undefined && !Array.isArray(obj.tools)) {
|
|
errors.push('tools must be an array when present');
|
|
}
|
|
|
|
// Optional: response_format — object with .type (string)
|
|
if (obj.response_format !== undefined) {
|
|
if (typeof obj.response_format !== 'object' || obj.response_format === null) {
|
|
errors.push('response_format must be an object');
|
|
} else if (typeof obj.response_format.type !== 'string') {
|
|
errors.push('response_format.type must be a string');
|
|
}
|
|
}
|
|
|
|
// Optional: tool_choice — 'auto' | 'none' | 'required' | {type:'function', function:{name}}
|
|
if (obj.tool_choice !== undefined) {
|
|
if (typeof obj.tool_choice === 'string') {
|
|
if (!['auto', 'none', 'required'].includes(obj.tool_choice)) {
|
|
errors.push("tool_choice string must be 'auto' | 'none' | 'required'");
|
|
}
|
|
} else if (typeof obj.tool_choice === 'object' && obj.tool_choice !== null) {
|
|
if (obj.tool_choice.type !== 'function') {
|
|
errors.push("tool_choice.type must be 'function'");
|
|
} else if (typeof obj.tool_choice.function !== 'object' || obj.tool_choice.function === null) {
|
|
errors.push('tool_choice.function must be an object');
|
|
} else if (typeof obj.tool_choice.function.name !== 'string') {
|
|
errors.push('tool_choice.function.name must be a string');
|
|
}
|
|
} else {
|
|
errors.push('tool_choice must be a string or an object');
|
|
}
|
|
}
|
|
|
|
return { valid: errors.length === 0, errors };
|
|
}
|