mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-21 21:15:10 +00:00
First batch of pre-Phase-2 cleanup work. 5 GitHub issues closed in
one cohesive commit covering streaming-path correctness, IR
validator hardening, and CI path-trigger hygiene.
Changes (4 files, +302 / -5):
**Code fixes**
1. **#9 — Streaming empty-then-clean-exit headers** (server.mjs)
Pre-D35: when a provider's streaming spawn finished cleanly with
zero chunks (e.g. spec-degenerate stop with no content), the
response went out the SSE_DONE / res.end path without ever calling
writeHead. Result: client saw stream open + close with no headers,
no status code path applied. Now: zero-chunk branch guards
`!res.headersSent` and emits Content-Type + Cache-Control +
Connection + X-Accel-Buffering + all 5 X-OLP-* headers via
olpHeaders (provider attempted, cache miss) before writing the
terminator. Zero-chunk path correctly does NOT cache (cache write
remains gated on irChunk.type === 'stop').
2. **#10 — Streaming post-first-chunk error truncation marker**
(server.mjs, two sibling sites)
Pre-D35: if a provider yielded an error AFTER first content chunk
was emitted, the SSE stream was abandoned with raw socket close.
Client SDKs that wait for finish_reason hung. Now:
- Catch-block firstChunkEmitted=true path: emit synthetic
`{type:'stop', finish_reason:'length'}` via irChunkToOpenAISSE,
write SSE_DONE, end. Per ADR 0004 § Fallback safety: post-first-
chunk truncation surfaces as `length` finish, not a hang.
- Sibling fix in error-chunk path (provider yields `type:'error'`
chunk AFTER first content chunk): same recovery (marker + DONE +
end). Scope-creep acknowledged but identical semantic; clean to
fix together. Comments cross-reference D26 F19 and D35 #10.
3. **#11 — validateIRRequest irVersion strict check** (lib/ir/types.mjs)
ADR 0003 IR contract pins irVersion to '1.0'. Validator pre-D35
accepted ANY value (including no value, undefined, '2.0',
numeric 1.0). Now: `obj.irVersion !== undefined && obj.irVersion
!== '1.0'` → rejection. Strict string match. `undefined` still
accepted (pre-D35 IRs without the field remain valid — back-compat
with sites that haven't yet been migrated to emit it). Error
message uses JSON.stringify for safe rendering.
4. **#12 — alignment.yml scripts/** trigger removal**
(.github/workflows/alignment.yml)
Pre-D35 push.paths and pull_request.paths listed scripts/**. The
scripts/ directory does not currently exist (per AGENTS.md note:
scripts/migrate-from-ocp.mjs is planned for Phase 7). A path
filter referencing a non-existent directory has no effect on
trigger evaluation BUT misleads readers about the workflow's
intent. Removed from both push.paths and pull_request.paths. When
scripts/ lands in Phase 7, the trigger should be re-added at the
same time (see release_kit_overlay.bootstrap_quirk_policy).
**Verification — #4 (uniform X-OLP-Latency-Ms across error paths)**
#4 was found to already be correct via D32. Re-audit of all 7
in-handler sendError sites in handleChatCompletions confirmed all
attach a 5-header set via olpHeaders or olpErrorHeaders:
- 360-361 (415 wrong Content-Type) → olpErrorHeaders
- 368-369 (400 bad JSON) → olpErrorHeaders
- 378-379 (400 BadRequestError IR translation) → olpErrorHeaders
- 402-407 (503 no chain) → olpErrorHeaders
- 617-618 (503 provider disappeared) → olpErrorHeaders
- 760-761 (502 streaming pre-first-chunk error) → olpHeaders
- 778-779 (500 fallback engine error) → olpErrorHeaders
The 404 (line 922) and outer 500 (line 926) are router-level paths
without startMs context and correctly lack OLP headers. D35 adds
the #4-audit regression test pinning the 5-header invariant on the
503 no-provider response so future drift is caught immediately.
**Tests** (test-features.mjs): 416 → 424 (+8):
- #4-audit ×1 (5-header invariant on 503 no-provider sendError)
- #9 ×1 (zero-chunk streaming → 200 + Content-Type=text/event-stream
+ 5 X-OLP-* headers + [DONE])
- #10 ×1 (catch-throw after first chunk → marker + length finish + DONE)
- #10b ×1 (provider error chunk after first chunk → same recovery)
- #11a ×1 (irVersion undefined accepted)
- #11b ×1 (irVersion '1.0' accepted)
- #11c ×1 (irVersion '2.0' rejected)
- #11d ×1 (irVersion numeric 1.0 rejected)
Pre-commit fold-in (per evidence-first checkpoint #4):
- **D35 reviewer flagged JSDoc/validator drift on irVersion**
(Suggestion #1, non-blocking). The @property typedef at
lib/ir/types.mjs:43 said `{string} irVersion - always IR_VERSION`
but the validator at lines 185-186 accepts `undefined`. Future
reader who scans the @property alone sees contradiction without
the rationale comment 140 lines below. Folded: typedef marked
`[irVersion]` (optional) and description updated to "optional;
when present must equal IR_VERSION ('1.0'). Pre-D35 IRs lack
this field and remain valid."
Two other non-blocking reviewer suggestions not folded (out of
scope for D35; tracked as future polish):
- Distinct event names for the two streaming_error_after_first_chunk
log sites (provider-emitted string vs JS exception message).
- Phase 7 TODO: re-add scripts/** trigger to alignment.yml when
scripts/migrate-from-ocp.mjs lands.
Authority:
- ADR 0004 § Fallback safety — post-first-chunk truncation surfaces
as `length` finish (#10 + #10b)
- ADR 0003 § IR contract — irVersion pinned to '1.0' (#11)
- AGENTS.md § Implementation status — scripts/ planned for Phase 7
(#12)
- CLAUDE.md release_kit_overlay phase_rolling_mode — D35 lands
under "Unreleased" against Phase 2; no version bump
- CC 开发铁律 v1.6 § 10.x — independent fresh-context reviewer
required for code change
Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus,
independent of drafter): APPROVE_WITH_MINOR. Critical depth checks:
- Verified all 7 sendError sites in handleChatCompletions attach
5-header set (cited line numbers reconciled with current state)
- Verified writeHead block guarded by !res.headersSent; correctly
placed AFTER optional truncation marker, BEFORE SSE_DONE
- Verified irVersion validator strict-equality semantics across all
4 cases (undefined / '1.0' / '2.0' / numeric 1.0)
- Verified scripts/** removed from both push and pull_request paths
- Verified hygiene: 0 hits for personal markers, home paths, OAuth
tokens, internal IPs
- 424/424 tests pass independently in reviewer's run
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
210 lines
7.7 KiB
JavaScript
210 lines
7.7 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] - optional; when present must equal IR_VERSION ('1.0'). Pre-D35 IRs lack this field and remain valid.
|
|
* @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: irVersion — must be '1.0' if present; undefined accepted for pre-existing IRs
|
|
// Per ADR 0003 § Required fields: analogous to contractVersion='1.0' in base.mjs.
|
|
// Decision: undefined accepted because openai-to-ir.mjs sets irVersion on construction;
|
|
// pre-existing IRs without it still validate. Strict '1.0' rejection only when explicitly
|
|
// set wrong.
|
|
if (obj.irVersion !== undefined && obj.irVersion !== '1.0') {
|
|
errors.push(`irVersion must be '1.0' (got: ${JSON.stringify(obj.irVersion)})`);
|
|
}
|
|
|
|
// 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 };
|
|
}
|