fix+docs: D32 — round-4 cleanup batch (F2/F3/F4/F5/F7/F8/F9)

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

Round-4 cold-audit cleanup batch. 7 items grouped per IDR cleanup-batch
convention (D19/D20/D25/D26/D30/D31 precedent). 2 P2 + 3 ADR amendments
+ 2 small docs/code cleanups. F10 (P3 semantic edge case) filed
separately as GitHub issue #8.

Changes (8 files, +200 / -38):

**P2 fixes**

1. **F3 — README missing 6 provider auth env vars** (README.md):
   D30 fixed the `OLP_*` table but missed the auth-bearing env vars
   actually read by plugin code:
   - `CLAUDE_CODE_OAUTH_TOKEN` (anthropic.mjs:84) — highest-precedence
     override; bypasses keychain + .credentials.json file lookup
   - `OPENAI_CODEX_AUTH_PATH` (codex.mjs:163) — overrides full auth
     file path; when set, no other path tried
   - `CODEX_HOME` (codex.mjs:176) — overrides base dir; default auth
     path becomes `$CODEX_HOME/auth.json`
   - `MISTRAL_API_KEY` (mistral.mjs:260) — directly supplies API key;
     highest precedence per Mistral DOCS-2
   - `MISTRAL_VIBE_AUTH_PATH` (mistral.mjs:265) — overrides full .env
     path; evaluated only when MISTRAL_API_KEY absent
   - `VIBE_HOME` (mistral.mjs:277) — overrides Vibe base dir; default
     auth path becomes `$VIBE_HOME/.env`
   Real onboarding-blocker fix: new users couldn't run OLP without
   knowing about these env vars; README now documents them in a
   "Per-provider auth env vars" subsection.

2. **F8 — Early-return error paths missing X-OLP-* headers** (server.mjs):
   ADR 0004 § Observability + README claim "every response carries the
   5 X-OLP-* headers" but 503 (no-enabled-providers), 415 (wrong
   Content-Type), and early 400s (bad JSON, IR validation) emitted only
   Content-Type + Content-Length + X-OLP-Latency-Ms (D18 only wired the
   chain-exhausted path). Operators debugging these paths got less info
   than the ADR promised.
   New helper `olpErrorHeaders({ startMs, model })` emits the canonical
   "no provider attempted" defaults:
     X-OLP-Provider-Used: 'none'
     X-OLP-Model-Used: model ?? 'unknown'
     X-OLP-Fallback-Hops: '0'
     X-OLP-Cache: 'bypass'
     X-OLP-Latency-Ms: <delta>
   6 sendError call sites updated: 415 / 400-bad-JSON / 400-IR-parse
   (model: undefined → 'unknown') + 503-no-providers / 503-provider-
   disappeared / 500-engine-programming (model: ir.model). The 502
   streaming-error-before-first-chunk path correctly stays on
   `olpHeaders(...)` since a provider WAS attempted.

**ADR amendments**

3. **F2 — ADR 0003 model-mapping example correction** (docs/adr/0003,
   Amendment 2): the § Required fields example claimed `claude-sonnet-4-6`
   → `claude-sonnet-4-6-20260301` mapping happens in the provider plugin.
   This was wrong: per D17 SPOT decision (commit cb86807), `irRequest.model`
   is passed verbatim to the CLI; each provider CLI accepts its own
   aliases natively. Both inline correction (in § Decision) AND new
   Amendment 2 (in § Amendments) added.

4. **F4 — OUTPUT_PARSE_ERROR dead code removal** (base.mjs, engine.mjs):
   `PROVIDER_ERROR_CODES.OUTPUT_PARSE_ERROR` and
   `HARD_TRIGGER_CODES.OUTPUT_PARSE_ERROR = true` were registered but
   ZERO plugins emit OUTPUT_PARSE_ERROR. ADR 0004 § Trigger taxonomy
   enumerates 4 hard-trigger categories (5xx, quota 4xx, exit-code,
   spawn-timeout) — OUTPUT_PARSE_ERROR was a 5th never authorized by
   ADR. Removed from both enums with removal-reason comments for
   auditability. Re-add via ADR 0004 amendment if a future plugin
   surfaces parse failures (cheaper than authoring an amendment for
   dead code now).

5. **F5 — ADR 0002 Amendment 4 ratifying contractVersion** (docs/adr/0002):
   `lib/providers/base.mjs:76-81` enforces `p.contractVersion === '1.0'`
   and all 3 plugins declare it, but ADR 0002 § Provider contract field
   list never named it. Same governance-violation class as D11's
   `maxSpawnTimeMs` retroactive sync (Amendment 1). Amendment 4 ratifies
   contractVersion as a required v1.0 contract field; § Provider contract
   field list updated to 10 fields (was 9).

**Small cleanups**

6. **F7 — README API Endpoints table 📋 markers** (README.md): added
   Status column with  Shipped (3 rows: /v1/chat/completions, /v1/models,
   /health) and 📋 Planned (3 rows: /cache/stats Phase 5, /v0/management/
   quota Phase 6, /dashboard Phase 6). Matches D20's Implementation
   Status table convention.

7. **F9 — Codex parser inline assumption labels** (codex.mjs): added 5
   inline `// A4:` comments on each defensive branch in `codexChunkToIR`.
   Pre-D32 these branches had only the top-of-function block comment;
   ALIGNMENT.md Speculative-Candidate condition 5 promises future
   implementers can grep assumption labels to find every speculative
   branch — D32 honors that promise for codex.mjs (mistral.mjs already
   had this pattern from D8).

**Tests** (test-features.mjs):
- 17g retitled + assertion expanded to full 5-header set (was partial)
- 17h new: 415 wrong Content-Type → 5 X-OLP-* headers with 'unknown' model
- 17i new: 503 no-enabled-providers → 5 X-OLP-* headers with ir.model
- OUTPUT_PARSE_ERROR test removed (replaced with comment for audit trail)

Test count: 400 → 401 (-1 OUTPUT_PARSE_ERROR + 2 new F8 + 1 retitled
existing = net +1).

**F10 filed as issue, NOT in D32**:
https://github.com/dtzp555-max/olp/issues/8 — "Soft-skip + chain-
exhausted: X-OLP-Provider-Used semantics ambiguity". Semantic edge
case requiring soft trigger reactivation (deferred to v1.x per D22
Amendment 2); not a v0.1 user-affecting issue.

Pre-commit fold-in (per evidence-first checkpoint #4):
- **D32 reviewer caught a doc-accuracy bug**: README's
  CLAUDE_CODE_OAUTH_TOKEN row described the no-env fallback chain as
  "Searches keychain first, then `~/.claude/.credentials.json`" — but
  the actual code in anthropic.mjs:82-112 checks file FIRST, then
  keychain (darwin-only). Order was reversed. Folded in: corrected
  to "Searches `~/.claude/.credentials.json` first, then macOS
  keychain (darwin only)". Same class of doc-accuracy mistake as the
  D25 → D31 / D27 → D31 pattern: D-batch reviewer catches docs that
  contradict source.

Reviewer (Iron Rule v1.6 § 10.x Mode A, fresh-context opus, independent
of drafter): APPROVE_WITH_MINOR. Independently verified:
- All 6 F3 env vars: precedence chains traced through plugin source
  (caught the anthropic reversed-order — folded in)
- F8 olpErrorHeaders helper correctly distinguishes "no provider"
  defaults from olpHeaders' "provider attempted" defaults
- F8: all 6 sendError call sites use the right model arg (undefined
  pre-IR-parse → 'unknown'; ir.model post-IR-parse)
- F8: 502 streaming-error stays on olpHeaders (provider was attempted)
- F4: OUTPUT_PARSE_ERROR removed from both enums, zero plugin emit
  references remain, test cleanup is auditable
- F5: ADR 0002 Amendment 4 matches Amendment 1's retroactive-sync
  structure; contract field list now 10 items
- F2: D17 commit cb86807 verified to match the SPOT decision claim
- F7/F9: docs/code cleanups verified
- F10 issue #8 verified OPEN with correct title
- 401/401 tests pass

3 non-blocking suggestions (anthropic precedence column folded; F2
inline mistral `--model` caveat could be added; F4 wording asymmetry
between base.mjs and engine.mjs comments) — minor polish, not folded.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-05-24 17:28:42 +10:00
co-authored by Claude Opus 4.7
parent d6347e33f2
commit 30de965e8e
8 changed files with 200 additions and 38 deletions
+34 -9
View File
@@ -96,14 +96,14 @@ Trigger types, fallback safety, idempotency rules, and the full example config l
_placeholder — full table lands as each endpoint lands._ _placeholder — full table lands as each endpoint lands._
| Endpoint | Method | Phase | Description | | Endpoint | Method | Phase | Status | Description |
|---|---|---|---| |---|---|---|---|---|
| `/v1/chat/completions` | POST | 1 | OpenAI-compatible Chat Completions entry. Internally normalized to IR, dispatched to a provider plugin, response shape converted back. | | `/v1/chat/completions` | POST | 1 | ✅ Shipped | OpenAI-compatible Chat Completions entry. Internally normalized to IR, dispatched to a provider plugin, response shape converted back. |
| `/v1/models` | GET | 1 | Lists models from `models-registry.json`. | | `/v1/models` | GET | 1 | ✅ Shipped | Lists models from `models-registry.json`. |
| `/health` | GET | 1 | Per-provider health snapshot (owner-only). | | `/health` | GET | 1 | ✅ Shipped | Per-provider health snapshot (owner-only). |
| `/cache/stats` | GET | 5 | Cache hit rate, by-provider breakdown. | | `/cache/stats` | GET | 5 | 📋 Planned | Cache hit rate, by-provider breakdown. |
| `/v0/management/quota` | GET | 6 | Per-provider quota / credit pool status (best-effort). | | `/v0/management/quota` | GET | 6 | 📋 Planned | Per-provider quota / credit pool status (best-effort). |
| `/dashboard` | GET | 6 | Owner-only dashboard (localhost-bound by default). | | `/dashboard` | GET | 6 | 📋 Planned | Owner-only dashboard (localhost-bound by default). |
--- ---
@@ -118,11 +118,36 @@ _placeholder — full table lands per-phase as variables are introduced._
| `OLP_CODEX_BIN` | `codex` (from PATH) | Override path to the `codex` binary (OpenAI provider). | | `OLP_CODEX_BIN` | `codex` (from PATH) | Override path to the `codex` binary (OpenAI provider). |
| `OLP_VIBE_BIN` | `vibe` (from PATH) | Override path to the `vibe` binary (Mistral provider). | | `OLP_VIBE_BIN` | `vibe` (from PATH) | Override path to the `vibe` binary (Mistral provider). |
### Per-provider auth env vars
These variables configure credential discovery for each provider plugin. Setting the correct one for your provider is usually required for OLP to make successful requests.
**Anthropic (`claude -p`)**
| Variable | Default behavior | Description |
|---|---|---|
| `CLAUDE_CODE_OAUTH_TOKEN` | Searches `~/.claude/.credentials.json` first, then macOS keychain (darwin only) | Directly supplies the OAuth access token. Highest-precedence override; bypasses all credential-discovery logic. Useful in CI/headless environments where the keychain is unavailable. |
**OpenAI Codex (`codex exec --json`)**
| Variable | Default behavior | Description |
|---|---|---|
| `OPENAI_CODEX_AUTH_PATH` | `~/.codex/auth.json` (or `$CODEX_HOME/auth.json`) | Overrides the full path to the Codex auth artifact. When set, no other path is tried; missing or malformed file returns null (no auth). Useful for CI test fixtures. |
| `CODEX_HOME` | `~/.codex` | Overrides the Codex home directory. `$CODEX_HOME/auth.json` becomes the default auth path. Ignored when `OPENAI_CODEX_AUTH_PATH` is set explicitly. |
**Mistral Vibe (`vibe --prompt`)**
| Variable | Default behavior | Description |
|---|---|---|
| `MISTRAL_API_KEY` | Reads `$VIBE_HOME/.env` (default `~/.vibe/.env`) | Directly supplies the Mistral API key. Highest-precedence override per DOCS-2; bypasses `.env` file lookup. |
| `MISTRAL_VIBE_AUTH_PATH` | `~/.vibe/.env` (or `$VIBE_HOME/.env`) | Overrides the full path to the Vibe `.env` auth file. Evaluated only when `MISTRAL_API_KEY` is not set. When set, no other path is tried; missing or malformed file returns null (no auth). |
| `VIBE_HOME` | `~/.vibe` | Overrides the Vibe home directory. `$VIBE_HOME/.env` becomes the default auth path. Ignored when `MISTRAL_API_KEY` or `MISTRAL_VIBE_AUTH_PATH` is set explicitly. |
> **📋 Planned (Phase 2) — not yet read by the codebase:** > **📋 Planned (Phase 2) — not yet read by the codebase:**
> - `OLP_HOME` (`~/.olp`) — Config, providers, keys, cache, logs root. Currently hardcoded to `~/.olp/config.json` in `loadFallbackConfigSync`; the env override path is a Phase 2 config-layer deliverable. > - `OLP_HOME` (`~/.olp`) — Config, providers, keys, cache, logs root. Currently hardcoded to `~/.olp/config.json` in `loadFallbackConfigSync`; the env override path is a Phase 2 config-layer deliverable.
> - `OLP_LOG_LEVEL` (`info`) — Log level filter (`error`/`warn`/`info`/`debug`). `logEvent` currently writes unconditionally; level filtering is a Phase 2 observability deliverable. > - `OLP_LOG_LEVEL` (`info`) — Log level filter (`error`/`warn`/`info`/`debug`). `logEvent` currently writes unconditionally; level filtering is a Phase 2 observability deliverable.
Further variables (per-provider auth path overrides, cache size limits, fallback-engine knobs) land with the relevant phase. See also the [Implementation status](#implementation-status-as-of-2026-05-24) table. See also the [Implementation status](#implementation-status-as-of-2026-05-24) table.
--- ---
+9
View File
@@ -7,6 +7,14 @@
## Amendments ## Amendments
### Amendment 4 — 2026-05-24: Ratify `contractVersion` as a required Provider contract field (D32 F5)
- **Finding:** Round-4 cold-audit F5 (P3 governance omission) — `lib/providers/base.mjs` `validateProvider` enforces `p.contractVersion === '1.0'` and all three shipped plugins declare it, but the Provider contract field list in § Decision (lines ~63-74) does not include `contractVersion`. It was mentioned only in § Mitigations as a forward-looking note ("The contract is versioned. v1.0 is the subset in this ADR; future additions … require ADR amendment plus a contract-version bump. Old provider plugins continue to declare `contractVersion: '1.0'`…"), not as a required field. This is the same class of documentationimplementation gap as Amendment 1 (`maxSpawnTimeMs` retroactive sync).
- **Change:** Add `contractVersion: '1.0'` as the 10th required field in the Provider contract field list in § Decision. The `hints` block moves from 9th to 10th entry to keep the list logically ordered (name → displayName → models → auth → spawn → estimateCost → quotaStatus → healthCheck → contractVersion → hints). `validateProvider` in `base.mjs` already enforces it; this amendment is a retroactive documentation sync, not a code change.
- **Semantics:** `contractVersion` is a required string field set to `'1.0'` for all v0.1 plugins. Its purpose is to allow the loader to detect plugins authored against an older or newer contract version when OLP's contract evolves. A future contract revision (v1.1 or v2.0) will increment this value and update this ADR accordingly.
- **Authority:** `lib/providers/base.mjs` `validateProvider` function — live enforcement as of v0.1 bootstrap. All three shipped plugins (`anthropic.mjs`, `codex.mjs`, `mistral.mjs`) already export `contractVersion: '1.0'`.
- **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Round-4 Cold Audit caught it as F5). Parallel to Amendment 1 (D11 `maxSpawnTimeMs` retroactive sync).
### Amendment 3 — 2026-05-24: Add `cacheable` to Provider contract hints (D23) ### Amendment 3 — 2026-05-24: Add `cacheable` to Provider contract hints (D23)
- **Finding:** Cold-audit round-2 Finding 3 (P2 contract/cache-condition drift) — ADR 0005 § "Cache write conditions" item 3 references `hints.cacheable` but the field was never named in this ADR's Provider contract hints list. `validateProvider` did not enforce it; no plugin declared it; no consumer read it. The field existed only in prose. - **Finding:** Cold-audit round-2 Finding 3 (P2 contract/cache-condition drift) — ADR 0005 § "Cache write conditions" item 3 references `hints.cacheable` but the field was never named in this ADR's Provider contract hints list. `validateProvider` did not enforce it; no plugin declared it; no consumer read it. The field existed only in prose.
@@ -71,6 +79,7 @@ Every provider plugin exports an object conforming to:
- `estimateCost: (request) => { inputTokens, outputTokensEstimate, currency, usd }` — best-effort, may return null - `estimateCost: (request) => { inputTokens, outputTokensEstimate, currency, usd }` — best-effort, may return null
- `quotaStatus: async (authContext) => { available, percentUsed, resetsAt, pool }` — best-effort, null if unretrievable - `quotaStatus: async (authContext) => { available, percentUsed, resetsAt, pool }` — best-effort, null if unretrievable
- `healthCheck: async () => { ok, latencyMs, error? }` — startup and `/health` endpoint use this - `healthCheck: async () => { ok, latencyMs, error? }` — startup and `/health` endpoint use this
- `contractVersion: '1.0'` — required string identifying the Provider contract version the plugin was authored against. Set to `'1.0'` for all v0.1 plugins. `validateProvider` in `base.mjs` enforces this field. Future contract revisions will increment the value; the loader uses it to detect stale plugins. See Amendment 4. (D32 F5)
- `hints: { requiresTTY, concurrentSpawnSafe, maxConcurrent, maxSpawnTimeMs, cacheable }` — fingerprint, concurrency, timeout, and cache hints: - `hints: { requiresTTY, concurrentSpawnSafe, maxConcurrent, maxSpawnTimeMs, cacheable }` — fingerprint, concurrency, timeout, and cache hints:
- `requiresTTY` — boolean; whether the provider CLI requires a TTY to produce non-interactive output (e.g., some CLIs suppress JSON output unless forced with a flag or a TTY is present). - `requiresTTY` — boolean; whether the provider CLI requires a TTY to produce non-interactive output (e.g., some CLIs suppress JSON output unless forced with a flag or a TTY is present).
- `concurrentSpawnSafe` — boolean; whether the provider CLI is safe to spawn concurrently under the same auth context without rate-limit or session collisions. - `concurrentSpawnSafe` — boolean; whether the provider CLI is safe to spawn concurrently under the same auth context without rate-limit or session collisions.
+9 -1
View File
@@ -7,6 +7,14 @@
## Amendments ## Amendments
### Amendment 2 — 2026-05-24: Correct model-mapping example; document verbatim-pass-through design (D32 F2)
- **Finding:** Round-4 cold-audit F2 (P3 ADR example vs implementation drift) — § Decision "Required fields" item `model` reads: "The provider plugin maps this to the provider-native model identifier (e.g., `claude-sonnet-4-6``claude-sonnet-4-6-20260301` for Anthropic)." This is WRONG per the D17 SPOT decision (commit `cb86807`): OLP does NOT perform a model-alias mapping inside the provider plugin. `irRequest.model` is passed verbatim to the provider CLI (`claude -p --model <model>`, `codex exec --model <model>`, etc.); each provider's CLI resolves its own aliases natively per its documented behaviour.
- **Decision:** Update the `model` field description in § Decision to reflect the actual verbatim-pass-through design. The erroneous example (`claude-sonnet-4-6``claude-sonnet-4-6-20260301`) implied OLP maintained an explicit alias map, which it does not. See also inline update in § Decision below.
- **Authority:** D17 SPOT decision (commit `cb86807`) — "provider plugin passes `irRequest.model` verbatim; CLI alias resolution is the provider's responsibility." ADR 0003 § Decision is updated in-place to match; the original v1.0 text is preserved as a struck annotation.
- **Forward note:** v1.x may add an explicit OLP-side mapping step if any provider's CLI drops alias support or if a unified OLP-owned alias layer is needed across providers. That change requires a further ADR 0003 amendment.
- **Procedural mechanism:** CC 开发铁律 v1.6 § 10.x (Round-4 Cold Audit caught it as F2).
### Amendment 1 — 2026-05-24: Remove __irRoundTripTest() claim; document substitute test strategy (D31 F5) ### Amendment 1 — 2026-05-24: Remove __irRoundTripTest() claim; document substitute test strategy (D31 F5)
- **Finding:** Round-3 cold-audit F5 (P2 ADR-claim vs implementation drift) — § Mitigations names a `__irRoundTripTest()` export on every provider plugin as the structural counter-measure against silent lossy translation. ZERO plugins export this function; the export-name was an early-design aspiration that does not fit the actual plugin shape (spawn-wrappers with asymmetric request→CLI-args+stdin and response-stream→IR, not symmetric `irToNative`+`nativeToIR` functions that could be tested in isolation via a round-trip fixture). - **Finding:** Round-3 cold-audit F5 (P2 ADR-claim vs implementation drift) — § Mitigations names a `__irRoundTripTest()` export on every provider plugin as the structural counter-measure against silent lossy translation. ZERO plugins export this function; the export-name was an early-design aspiration that does not fit the actual plugin shape (spawn-wrappers with asymmetric request→CLI-args+stdin and response-stream→IR, not symmetric `irToNative`+`nativeToIR` functions that could be tested in isolation via a round-trip fixture).
@@ -48,7 +56,7 @@ OLP defines an Intermediate Representation (IR) as the canonical internal shape
**Required fields:** **Required fields:**
- `messages[]` — each with `role`, `content`, optional `name`, optional `tool_calls`, optional `tool_call_id`. Roles supported: `system`, `user`, `assistant`, `tool`. - `messages[]` — each with `role`, `content`, optional `name`, optional `tool_calls`, optional `tool_call_id`. Roles supported: `system`, `user`, `assistant`, `tool`.
- `model` — the user-requested model string. The provider plugin maps this to the provider-native model identifier (e.g., `claude-sonnet-4-6``claude-sonnet-4-6-20260301` for Anthropic). - `model` — the user-requested model string. v0.1: passed verbatim to the provider CLI (`claude -p --model <model>`, `codex exec --model <model>`, etc.); each provider's CLI accepts its own aliases natively per its docs (D17 SPOT decision, commit `cb86807`). *(The original v1.0 text incorrectly stated the plugin maps this to a provider-native identifier — corrected by Amendment 2.)* v1.x may add an explicit OLP-side mapping step if any provider's CLI drops alias support.
- `stream` — boolean. SSE expected on the entry side iff true. - `stream` — boolean. SSE expected on the entry side iff true.
**Optional fields:** **Optional fields:**
+2 -2
View File
@@ -32,7 +32,8 @@ import { computeIRRequestHash } from '../cache/keys.mjs';
* - SPAWN_FAILED → hard trigger (provider CLI failed) * - SPAWN_FAILED → hard trigger (provider CLI failed)
* - CLI_NOT_FOUND → hard trigger (binary missing) * - CLI_NOT_FOUND → hard trigger (binary missing)
* - AUTH_MISSING → NOT a hard trigger (user-config failure; user must fix) * - AUTH_MISSING → NOT a hard trigger (user-config failure; user must fix)
* - OUTPUT_PARSE_ERROR → hard trigger * - OUTPUT_PARSE_ERROR → removed (D32 F4): no plugin emits it; dead code.
* Re-add via ADR 0004 amendment if needed.
* *
* @type {Record<string, boolean>} * @type {Record<string, boolean>}
*/ */
@@ -42,7 +43,6 @@ const HARD_TRIGGER_CODES = {
SPAWN_FAILED: true, SPAWN_FAILED: true,
CLI_NOT_FOUND: true, CLI_NOT_FOUND: true,
AUTH_MISSING: false, // user config problem — never fall over (ADR 0004 § Decision) AUTH_MISSING: false, // user config problem — never fall over (ADR 0004 § Decision)
OUTPUT_PARSE_ERROR: true,
SPAWN_TIMEOUT: true, // ADR 0004 § Trigger taxonomy bullet 4: spawn timeout is a hard trigger SPAWN_TIMEOUT: true, // ADR 0004 § Trigger taxonomy bullet 4: spawn timeout is a hard trigger
}; };
+2 -1
View File
@@ -145,7 +145,8 @@ export const PROVIDER_ERROR_CODES = /** @type {const} */ ([
'RATE_LIMITED', 'RATE_LIMITED',
'CLI_NOT_FOUND', 'CLI_NOT_FOUND',
'SPAWN_FAILED', 'SPAWN_FAILED',
'OUTPUT_PARSE_ERROR', // OUTPUT_PARSE_ERROR removed (D32 F4): no plugin emits it; dead code.
// Re-add via ADR 0004 amendment if a future plugin surfaces parse failures.
'SPAWN_TIMEOUT', // ADR 0004 § Trigger taxonomy bullet 4: spawn timeout is a hard trigger 'SPAWN_TIMEOUT', // ADR 0004 § Trigger taxonomy bullet 4: spawn timeout is a hard trigger
]); ]);
+5
View File
@@ -292,27 +292,32 @@ export function codexChunkToIR(rawNDJSONLine) {
if (!event || typeof event !== 'object') return null; if (!event || typeof event !== 'object') return null;
// Error event: type === 'error' or error field present // Error event: type === 'error' or error field present
// A4: defensive — error shape unconfirmed; D7 will pin actual field names
if (event.type === 'error' || (event.error && typeof event.error === 'string')) { if (event.type === 'error' || (event.error && typeof event.error === 'string')) {
const errMsg = event.error ?? event.message ?? 'codex error'; const errMsg = event.error ?? event.message ?? 'codex error';
return { type: 'error', error: errMsg }; return { type: 'error', error: errMsg };
} }
// Stop event: type === 'stop' or done === true // Stop event: type === 'stop' or done === true
// A4: defensive — stop shape unconfirmed; D7 will pin whether 'done' field exists
if (event.type === 'stop' || event.done === true) { if (event.type === 'stop' || event.done === true) {
return { type: 'stop', finish_reason: 'stop' }; return { type: 'stop', finish_reason: 'stop' };
} }
// Delta/content event: has a content string field // Delta/content event: has a content string field
// A4: defensive — primary delta shape assumed from Codex CLI § --json docs
if (typeof event.content === 'string') { if (typeof event.content === 'string') {
return { type: 'delta', content: event.content }; return { type: 'delta', content: event.content };
} }
// delta field (alternative shape): { type: 'delta', delta: '...' } // delta field (alternative shape): { type: 'delta', delta: '...' }
// A4: defensive — alternative shape not confirmed; D7 will probe real stdout
if (event.type === 'delta' && typeof event.delta === 'string') { if (event.type === 'delta' && typeof event.delta === 'string') {
return { type: 'delta', content: event.delta }; return { type: 'delta', content: event.delta };
} }
// Output_text event shape (possible alternative): { type: 'output_text', text: '...' } // Output_text event shape (possible alternative): { type: 'output_text', text: '...' }
// A4: defensive — text field shape not confirmed; D7 will probe real stdout
if (typeof event.text === 'string') { if (typeof event.text === 'string') {
return { type: 'delta', content: event.text }; return { type: 'delta', content: event.text };
} }
+35 -6
View File
@@ -238,6 +238,31 @@ function olpHeaders({ providerUsed, modelUsed, startMs, cacheStatus = 'miss', fa
}; };
} }
/**
* Returns the standard 5-header OLP diagnostic set for pre-chain error paths
* (no provider was attempted yet). Used by sendError call sites that occur
* before chain construction (415 wrong Content-Type, 400 bad JSON, 400 IR
* validation failure, 503 no-enabled-providers).
*
* Per F8 (D32 round-4 cold-audit): ADR 0004 § Observability requires the full
* 5-header set on every response; early error paths must emit canonical
* "no provider attempted" defaults.
*
* @param {object} opts
* @param {number} opts.startMs — request start timestamp (Date.now())
* @param {string|null|undefined} [opts.model] — ir.model if IR was parsed; undefined/null otherwise
* @returns {Record<string,string>}
*/
function olpErrorHeaders({ startMs, model }) {
return {
'X-OLP-Provider-Used': 'none',
'X-OLP-Model-Used': model ?? 'unknown',
'X-OLP-Fallback-Hops': '0',
'X-OLP-Cache': 'bypass',
'X-OLP-Latency-Ms': String(Date.now() - startMs),
};
}
// ── Route handlers ──────────────────────────────────────────────────────── // ── Route handlers ────────────────────────────────────────────────────────
/** /**
@@ -320,7 +345,7 @@ async function handleChatCompletions(req, res) {
const ct = req.headers['content-type'] ?? ''; const ct = req.headers['content-type'] ?? '';
if (!ct.includes('application/json')) { 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) }); olpErrorHeaders({ startMs }));
} }
let body; let body;
@@ -328,7 +353,7 @@ async function handleChatCompletions(req, res) {
body = await readJSON(req); body = await readJSON(req);
} catch (e) { } 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) }); olpErrorHeaders({ startMs }));
} }
// Translate OpenAI → IR (ADR 0003) // Translate OpenAI → IR (ADR 0003)
@@ -338,7 +363,7 @@ async function handleChatCompletions(req, res) {
} catch (e) { } catch (e) {
if (e instanceof BadRequestError) { 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) }); olpErrorHeaders({ startMs }));
} }
throw e; throw e;
} }
@@ -365,6 +390,7 @@ async function handleChatCompletions(req, res) {
res, 503, res, 503,
`No enabled providers for model ${ir.model}. See README § Supported Providers.`, `No enabled providers for model ${ir.model}. See README § Supported Providers.`,
'no_enabled_provider', 'no_enabled_provider',
olpErrorHeaders({ startMs, model: ir.model }),
); );
} }
@@ -558,7 +584,8 @@ async function handleChatCompletions(req, res) {
if (!streamPlugin) { if (!streamPlugin) {
// Provider disappeared between chain build and here (edge case). // Provider disappeared between chain build and here (edge case).
return sendError(res, 503, `Provider ${streamProvider} is not enabled`, 'no_enabled_provider'); return sendError(res, 503, `Provider ${streamProvider} is not enabled`, 'no_enabled_provider',
olpErrorHeaders({ startMs, model: ir.model }));
} }
const streamHeaders = olpHeaders({ const streamHeaders = olpHeaders({
@@ -676,7 +703,8 @@ async function handleChatCompletions(req, res) {
error: e.message, error: e.message,
}); });
if (!res.headersSent) { if (!res.headersSent) {
sendError(res, 502, e.message ?? 'Provider error', 'provider_error'); sendError(res, 502, e.message ?? 'Provider error', 'provider_error',
olpHeaders({ providerUsed: streamProvider, modelUsed: streamModel, startMs, cacheStatus: 'miss', fallbackHops: 0 }));
} else { } else {
res.end(); res.end();
} }
@@ -693,7 +721,8 @@ async function handleChatCompletions(req, res) {
} catch (e) { } catch (e) {
// executeWithFallback throws only on programming errors (empty chain). // executeWithFallback throws only on programming errors (empty chain).
logEvent('error', 'fallback_engine_error', { error: e.message }); logEvent('error', 'fallback_engine_error', { error: e.message });
return sendError(res, 500, 'Internal server error', 'internal_error'); return sendError(res, 500, 'Internal server error', 'internal_error',
olpErrorHeaders({ startMs, model: ir.model }));
} }
const { const {
+104 -19
View File
@@ -3891,10 +3891,8 @@ describe('Fallback engine — trigger taxonomy (D9)', () => {
assert.equal(evaluateHardTriggers(err), false); assert.equal(evaluateHardTriggers(err), false);
}); });
it('evaluateHardTriggers: ProviderError OUTPUT_PARSE_ERROR → fires', () => { // D32 F4: OUTPUT_PARSE_ERROR removed from PROVIDER_ERROR_CODES and
const err = new ProviderError('Parse error', 'OUTPUT_PARSE_ERROR'); // HARD_TRIGGER_CODES — no plugin emits it; it was dead code. Test removed.
assert.equal(evaluateHardTriggers(err), true);
});
it('evaluateHardTriggers: generic Error with no statusCode → does NOT fire', () => { it('evaluateHardTriggers: generic Error with no statusCode → does NOT fire', () => {
const err = new Error('Something went wrong'); const err = new Error('Something went wrong');
@@ -6142,16 +6140,16 @@ describe('/v1/models population + X-OLP-* error headers (Suite 17)', () => {
} }
}); });
// ── 17g: pre-routing 400 (invalid JSON) carries X-OLP-Latency-Ms ───────── // ── 17g: pre-routing 400 (invalid JSON) carries full 5 X-OLP-* headers ──────
// Updated by D32 F8: olpErrorHeaders() now emits all 5 headers with
// "no provider attempted" defaults (provider='none', model='unknown',
// hops=0, cache='bypass') on pre-chain error paths. Test updated to assert
// the full set rather than only X-OLP-Latency-Ms.
it('17g: pre-routing 400 (invalid JSON body) carries X-OLP-Latency-Ms', async () => { it('17g: pre-routing 400 (invalid JSON body) carries all 5 X-OLP-* headers with no-provider defaults', async () => {
// Pre-routing errors inside handleChatCompletions (invalid JSON body, wrong // Pre-chain errors inside handleChatCompletions now emit the full 5-header set
// Content-Type, IR parse failure) have access to startMs and emit // via olpErrorHeaders() with canonical "no provider attempted" defaults per
// X-OLP-Latency-Ms to allow latency instrumentation even when there is no // ADR 0004 § Observability (D32 F8).
// provider context. The other 4 X-OLP-* headers are omitted (no provider was
// attempted). This is consistent with ADR 0004 § Observability which requires
// the full set for chain-exhausted paths; partial emission on pre-route errors
// is the minimum viable observability for those paths.
setProviders17({}); setProviders17({});
const s = createServer17(); const s = createServer17();
await new Promise((resolve, reject) => { await new Promise((resolve, reject) => {
@@ -6177,15 +6175,102 @@ describe('/v1/models population + X-OLP-* error headers (Suite 17)', () => {
req.end(); req.end();
}); });
assert.equal(result.status, 400, `Expected 400 for invalid JSON body`); assert.equal(result.status, 400, `Expected 400 for invalid JSON body`);
// X-OLP-Latency-Ms must be present (D18 pre-routing observability) // All 5 X-OLP-* headers must be present (D32 F8 olpErrorHeaders)
assert.ok(result.headers['x-olp-latency-ms'] !== undefined, assert.ok(result.headers['x-olp-latency-ms'] !== undefined,
'Pre-routing 400 error must carry X-OLP-Latency-Ms for latency instrumentation'); 'Pre-routing 400 must carry X-OLP-Latency-Ms');
const latencyMs = parseInt(result.headers['x-olp-latency-ms'], 10); const latencyMs = parseInt(result.headers['x-olp-latency-ms'], 10);
assert.ok(!isNaN(latencyMs) && latencyMs >= 0, assert.ok(!isNaN(latencyMs) && latencyMs >= 0,
`X-OLP-Latency-Ms must be a non-negative integer, got: ${result.headers['x-olp-latency-ms']}`); `X-OLP-Latency-Ms must be non-negative integer, got: ${result.headers['x-olp-latency-ms']}`);
// The other 4 headers are NOT present (no provider context) assert.equal(result.headers['x-olp-provider-used'], 'none',
assert.ok(result.headers['x-olp-provider-used'] === undefined, 'Pre-routing 400 must carry X-OLP-Provider-Used: none (no provider attempted)');
'Pre-routing 400 must NOT carry X-OLP-Provider-Used (no provider context)'); assert.equal(result.headers['x-olp-model-used'], 'unknown',
'Pre-routing 400 must carry X-OLP-Model-Used: unknown (IR not parsed yet)');
assert.equal(result.headers['x-olp-fallback-hops'], '0',
'Pre-routing 400 must carry X-OLP-Fallback-Hops: 0');
assert.equal(result.headers['x-olp-cache'], 'bypass',
'Pre-routing 400 must carry X-OLP-Cache: bypass');
} finally {
resetProviders17();
await new Promise(r => s.close(r));
}
});
// ── 17h: 415 wrong Content-Type carries all 5 X-OLP-* headers ─────────────
// D32 F8: olpErrorHeaders on 415 pre-chain path.
it('17h: 415 wrong Content-Type carries all 5 X-OLP-* headers with no-provider defaults', async () => {
setProviders17({});
const s = createServer17();
await new Promise((resolve, reject) => {
s.listen(0, '127.0.0.1', resolve);
s.once('error', reject);
});
const p = s.address().port;
try {
const result = await new Promise((resolve, reject) => {
const req = httpRequest({
hostname: '127.0.0.1',
port: p,
method: 'POST',
path: '/v1/chat/completions',
headers: { 'Content-Type': 'text/plain', 'Content-Length': '2' },
}, res => {
let data = '';
res.on('data', c => { data += c; });
res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body: data }));
});
req.on('error', reject);
req.write('{}');
req.end();
});
assert.equal(result.status, 415, `Expected 415 for wrong Content-Type, got ${result.status}`);
assert.equal(result.headers['x-olp-provider-used'], 'none',
'415 must carry X-OLP-Provider-Used: none');
assert.equal(result.headers['x-olp-model-used'], 'unknown',
'415 must carry X-OLP-Model-Used: unknown');
assert.equal(result.headers['x-olp-fallback-hops'], '0',
'415 must carry X-OLP-Fallback-Hops: 0');
assert.equal(result.headers['x-olp-cache'], 'bypass',
'415 must carry X-OLP-Cache: bypass');
assert.ok(result.headers['x-olp-latency-ms'] !== undefined,
'415 must carry X-OLP-Latency-Ms');
} finally {
resetProviders17();
await new Promise(r => s.close(r));
}
});
// ── 17i: 503 no-enabled-providers carries all 5 X-OLP-* headers ───────────
// D32 F8: olpErrorHeaders on 503 no-chain path. X-OLP-Model-Used reflects
// the requested model (IR was parsed successfully before chain lookup).
it('17i: 503 no-enabled-providers carries all 5 X-OLP-* headers with model from IR', async () => {
setProviders17({});
const s = createServer17();
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: 'POST',
path: '/v1/chat/completions',
body: { model: 'claude-sonnet-4-6', messages: [{ role: 'user', content: 'hi' }] },
});
assert.equal(r.status, 503, `Expected 503 for no enabled providers, got ${r.status}`);
assert.equal(r.headers['x-olp-provider-used'], 'none',
'503 must carry X-OLP-Provider-Used: none');
// Model is known (IR was parsed) so model string is propagated
assert.equal(r.headers['x-olp-model-used'], 'claude-sonnet-4-6',
'503 must carry X-OLP-Model-Used reflecting the requested model');
assert.equal(r.headers['x-olp-fallback-hops'], '0',
'503 must carry X-OLP-Fallback-Hops: 0');
assert.equal(r.headers['x-olp-cache'], 'bypass',
'503 must carry X-OLP-Cache: bypass');
assert.ok(r.headers['x-olp-latency-ms'] !== undefined,
'503 must carry X-OLP-Latency-Ms');
} finally { } finally {
resetProviders17(); resetProviders17();
await new Promise(r => s.close(r)); await new Promise(r => s.close(r));