From 6edf6e0b9404f785253f3bd6ad525b72b552a7b8 Mon Sep 17 00:00:00 2001 From: dtzp555-max Date: Tue, 26 May 2026 12:50:41 +1000 Subject: [PATCH] =?UTF-8?q?fix+release(v0.4.2):=20D75=20=E2=80=94=20codex?= =?UTF-8?q?=20CLI=20v0.133.0=20schema=20+=20per-hop=20model=20override=20(?= =?UTF-8?q?#47)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch release fixing 5 bugs caught by real-machine E2E testing on PI231 + Mac mini (2026-05-26 session). Prior D-day reviewers + the post-v0.4.0 maintainer review all missed these because they reviewed against spec text and against the local OLP install's cached codex CLI shape, not against a fresh `npm install -g @openai/codex` on a remote operator host getting v0.133.0 for the first time. F1 — codex auth.json schema pin (lib/providers/codex.mjs readAuthArtifact) Real codex CLI v0.133.0 nests the access token under `tokens.access_token`, not at top-level access_token / token / accessToken. Pre-D75 readAuthArtifact returned null → OLP reported "auth artifact missing" even for fully logged-in users. Fix: prepend creds?.tokens?.access_token to the precedence chain at both override + default branches. Legacy fields preserved as fallback. Authority: codex CLI v0.133.0 on-disk auth.json shape verified empirically on PI231 2026-05-26 E2E session. F2 — codex spawn args + --skip-git-repo-check (lib/providers/codex.mjs irToCodex) codex CLI v0.133.0 trusted-directory sandbox refuses with "Not inside a trusted directory" outside git repos. OLP deploys typically outside a git repo. Fix: add '--skip-git-repo-check' to args before '--model'. Authority: codex CLI v0.133.0 reference (`codex exec --help` documents the flag). F3 — codex NDJSON event shape pin (lib/providers/codex.mjs codexChunkToIR) Real v0.133.0 stream: thread.started → turn.started → item.completed (item.type='agent_message', item.text=) → turn.completed. D6 defensive parser only recognised top-level content/delta/text + type:'stop'/done:true → every chunk silently dropped → response body had content: null. Fix: add three new recognisers (item.completed → delta; turn.completed → stop; turn.failed → error) before the legacy fallback chain. Legacy recognisers preserved for backward/forward compat. F4 — `olp status` reads body.stats.cache.size, not body.cache.entries (bin/olp.mjs cmdStatus). Server payload nests stats under stats.cache; CacheStore.stats() exposes {hits, misses, size, inflightCount} — there is no `entries` field. D74 P2-3 fixed cmdUsage + cmdCache for the same bug class but missed cmdStatus. F7 — per-hop chain `model` overrides IR model in provider.spawn() (server.mjs executeHopFn + streaming sourceFactory). Pre-D75 executeHopFn used hopModel for cache key + audit ctx but passed the original irReq (with irReq.model = user's request) to provider.spawn(). Chain config [{anthropic, claude-X}, {openai, gpt-5.5}] would always spawn BOTH plugins with --model claude-X — openai rejected the unknown model and the chain died. This broke the core OLP value prop (cross-provider fallback with provider-appropriate model substitution). Fix: build per-hop IR variant with { ...irReq, model: hopModel } and pass to spawn. Conditional skips clone when hopModel === irReq.model. Applied to BOTH buffered path AND streaming path. Authority: ADR 0004 § Chain advancement step 1 (per-hop config supplies provider AND model — contract always specified, code didn't complete it). Out of scope (deferred to Phase 5): - F5 (server bind / OLP_BIND env) — needs anonymous-key trust review - F6 (doctor client-vs-server-side limit) — needs trigger-taxonomy ADR Test count: 704 → 714 (+10 Suite 36 D75 regression tests: 36i–36r). Files touched: lib/providers/codex.mjs, bin/olp.mjs, server.mjs, test-features.mjs, package.json, CHANGELOG.md. Phase 5 process learning: every provider plugin D-day must include a real-CLI E2E on a remote operator host before merging — not on the maintainer workstation (which may have an older CLI cached from a prior install). D6/D7 codex E2E was deferred and that deferral compounded across 3 layers. F7 reinforces a separate lesson: when a function signature takes (provider, model, ir), reviewers must check that `model` is consumed everywhere downstream — not just at the call site they happened to look at. Authority: ADR 0002 (provider contract — codex plugin), ADR 0004 (fallback engine — per-hop model contract), lib/providers/codex.mjs D6 assumption A2/A3/A4 docstrings (which all said "D7 will pin" and D7 never did); codex CLI v0.133.0 on-disk schema + `codex exec --help` output verified empirically on PI231 (2026-05-26 E2E session); Iron Rule 第二律 evidence-over-should-work; CLAUDE.md release_kit.phase_rolling_mode cross-Phase discipline ("hotfix to a shipped Phase N deliverable → bump patch, tag, release before next push"). Co-authored-by: dtzp555 Co-authored-by: Claude Opus 4.7 --- CHANGELOG.md | 24 +++ bin/olp.mjs | 8 +- lib/providers/codex.mjs | 111 +++++++++++- package.json | 2 +- server.mjs | 31 +++- test-features.mjs | 378 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 543 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0958bdf..1279162 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,30 @@ All notable changes to OLP land here. Per `CLAUDE.md` release_kit overlay, this (empty — Phase 5 entries land here once Phase 5 opens) +## v0.4.2 — 2026-05-26 + +### Post-v0.4.1 hotfix batch (D75) — real-machine E2E findings + +Patch release fixing 5 bugs caught by **real-machine E2E testing on PI231 + Mac mini (2026-05-26 session)** — bugs that prior D-day reviewers AND the post-v0.4.0 maintainer review both missed because they reviewed against spec text and against the local OLP install's `~/.codex/auth.json` shape (cached from an older codex CLI version), not against real provider CLIs running on a remote operator host that did `npm install -g @openai/codex` for the first time on 2026-05-26 and got codex CLI v0.133.0. + +**Root cause of the missed-bug class.** D6 (codex plugin authoring) explicitly documented three unpinned assumptions (A3 = access-token field name, A4 = NDJSON event schema, A2-adjacent = trusted-directory sandbox). D6 noted "D7 E2E will pin." D7 then shipped without performing real-codex-CLI E2E (the E2E gating mark was carried but the actual run was deferred). Every subsequent D-day reviewer trusted the D6/D7 codex plugin code unchanged because the static review couldn't see that the v0.133.0 CLI had moved the auth-token field, the event schema, AND added a new trusted-directory sandbox flag. The D74 maintainer review focused on `/health` / `/cache/stats` / `/v0/management/dashboard-data` payload shapes — none of which exercise the codex plugin's spawn path. F7 (per-hop model override) is a different class of miss — every reviewer read `executeHopFn(provider, model, ir)` and saw `model` consumed for cache key + audit ctx, but none traced through to confirm `model` is ALSO substituted into the IR passed to `provider.spawn()`. The function signature implied per-hop semantics that the body never fully delivered. + +- **[F1] codex auth.json schema pin — codex CLI v0.133.0 nests the access token under `tokens.access_token`** (verified empirically on PI231 / Mac mini, 2026-05-26). Pre-D75 `readAuthArtifact()` read only top-level `creds.access_token` / `creds.token` / `creds.accessToken` — all undefined under v0.133.0 → returned `null` → OLP reported "auth artifact missing" via `/health` and `olp doctor` AND refused to spawn codex even when the user had fully completed `codex login`. Fix: prepend `creds?.tokens?.access_token` to the precedence chain at BOTH call sites (`OPENAI_CODEX_AUTH_PATH` override branch + default `$CODEX_HOME/auth.json` branch). Legacy top-level fields preserved as fallback for backward compat with older codex CLI versions. +- **[F2] codex spawn args — codex CLI v0.133.0 trusted-directory sandbox requires `--skip-git-repo-check`.** v0.133.0 refuses with `"Not inside a trusted directory and --skip-git-repo-check was not specified."` when spawned outside a git repo, exits non-zero with zero NDJSON output → OLP surfaces `SPAWN_FAILED` with no usable chunks → the fallback engine advances to next hop unnecessarily even when codex is configured and authenticated. OLP's typical deploy CWD (`~/olp/`) is NOT a git repo on operator hosts. Fix: add `'--skip-git-repo-check'` to the args array before `--model`. OLP is the trusted caller (operator's own server invoking the operator's own subscription via documented `codex exec` automation); the sandbox safeguards interactive shells, not pre-authorized automation. +- **[F3] codex NDJSON event shape pin — codex CLI v0.133.0 emits `item.completed` + `turn.completed` + `turn.failed`**, not the D6-assumed `content`/`delta`/`text` + `type:'stop'`/`done:true` shapes. Real v0.133.0 stream (verified empirically): `{"type":"thread.started",...}` → `{"type":"turn.started"}` → `{"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":""}}` → `{"type":"turn.completed","usage":{...}}`. Pre-D75, every chunk was silently dropped by `codexChunkToIR()` → response body had `content: null`. Fix: prepend three new recognizers (`item.completed` with `item.type === 'agent_message'` → IR delta; `turn.completed` → IR stop; `turn.failed` → IR error). Legacy D6 defensive recognizers preserved below as forward/backward compat fallbacks. +- **[F4] `olp status` reads `body.stats.cache.size` (not OCP-era `body.cache.entries`).** Same class as D74 P2-3 (which fixed `cmdUsage` + `cmdCache`); D74 missed the parallel bug in `cmdStatus`. Server payload nests cache stats as `body.stats.cache.{hits, misses, size, inflightCount}` per `server.mjs handleManagementStatus`, and `CacheStore.stats()` has no `entries` field per `lib/cache/store.mjs`. Pre-D75 output showed `entries=?`. Fix: read `c.size` for entries display; also surface `inflightCount` when present. +- **[F7] per-hop chain `model` field now overrides IR model in `provider.spawn()`.** Pre-D75 `executeHopFn(hopProvider, hopModel, irReq)` used `hopModel` for cache key + audit ctx but passed the ORIGINAL `irReq` (with `irReq.model` = user's original request) to `hopProviderPlugin.spawn(irReq, authContext)`. A chain config `[{provider:anthropic, model:claude-X}, {provider:openai, model:gpt-5.5}]` would always spawn BOTH plugins with `--model claude-X` — openai rejected the unknown model and the chain died. This broke the core OLP value prop (cross-provider fallback with provider-appropriate model substitution per hop). Fix: build a per-hop IR variant with `{...irReq, model: hopModel}` and pass that to spawn. Conditional skips clone when `hopModel === irReq.model` (common case: single-provider chains, or single-hop chains where the chain config repeats the request model). Applied to BOTH the buffered path (`executeHopFn`) AND the streaming path (`sourceFactory` for `getOrComputeStreaming`). **Authority:** ADR 0004 § Chain advancement step 1 (per-hop config supplies provider AND model — the contract was always specified, but the code didn't complete it). + +**Phase 5 process learning recorded.** Every provider plugin's D-day must include a real-CLI E2E ON A REMOTE OPERATOR HOST before merging — not on the maintainer workstation (which may have an older CLI cached from a prior install, hiding new field renames / new sandbox flags / new event shapes). The D6/D7 codex E2E was deferred and that deferral compounded across 3 layers (D6 = unpinned, D7 = pinning deferred, D8+ = trusted D6/D7 unchanged). F7 reinforces a separate lesson: when a function signature takes `(provider, model, ir)`, reviewers must check that `model` is consumed everywhere downstream, not just at the call site they happened to look at. + +**Out of D75 scope (deferred to Phase 5 explicit ADR amendments):** +- F5 (server bind 127.0.0.1 / `OLP_BIND` env) — needs `lib/keys.mjs` anonymous-key trust boundary review before binding to non-loopback by default +- F6 (`olp doctor` client-vs-server-side limit detection) — needs design ADR amendment for trigger taxonomy + +- **Test count delta:** 704 (v0.4.1) → 714 (v0.4.2). +10 D75 regression tests in Suite 36 (36i through 36r). +- **Files touched:** `lib/providers/codex.mjs` (F1+F2+F3), `bin/olp.mjs` (F4 cmdStatus), `server.mjs` (F7 buffered + streaming spawn sites), `test-features.mjs` (Suite 36 extension), `package.json` (version), `CHANGELOG.md` (this entry). +- **Authority:** ADR 0002 (provider contract — codex plugin), ADR 0004 (fallback engine — per-hop model contract), `lib/providers/codex.mjs` D6 assumption A2/A3/A4 docstrings (which all said "D7 will pin" and D7 never did); codex CLI v0.133.0 on-disk schema + `codex exec --help` output verified empirically on PI231 (2026-05-26 E2E session); Iron Rule 第二律 evidence-over-should-work; CLAUDE.md `release_kit.phase_rolling_mode` cross-Phase discipline. + ## v0.4.1 — 2026-05-26 ### Post-Phase-4 hotfix batch (D74) — maintainer-review findings diff --git a/bin/olp.mjs b/bin/olp.mjs index 64604cc..b468202 100755 --- a/bin/olp.mjs +++ b/bin/olp.mjs @@ -251,9 +251,15 @@ async function cmdStatus(flags, io) { } io.log(` total reqs: ${body.stats?.total_requests ?? 0}`); io.log(` active reqs: ${body.stats?.active_requests ?? 0}`); + // D75 F4 fix: server payload nests cache stats under stats.cache (per + // server.mjs handleManagementStatus, ~line 2092). The CacheStore.stats() + // contract returns { hits, misses, size, inflightCount } per + // lib/cache/store.mjs — there is no `entries` field. Pre-D75 cmdStatus read + // `body.stats.cache.entries` (OCP-era) which was always undefined → output + // showed "entries=?". Same pattern as D74 P2-3 applied to cmdCache/cmdUsage. if (body.stats?.cache) { const c = body.stats.cache; - io.log(` cache: hits=${c.hits ?? 0} misses=${c.misses ?? 0} entries=${c.entries ?? '?'}`); + io.log(` cache: hits=${c.hits ?? 0} misses=${c.misses ?? 0} entries=${c.size ?? 0}${typeof c.inflightCount === 'number' ? ` inflight=${c.inflightCount}` : ''}`); } if (Array.isArray(body.recent_errors) && body.recent_errors.length > 0) { io.log(` recent errors: ${body.recent_errors.length}`); diff --git a/lib/providers/codex.mjs b/lib/providers/codex.mjs index 1c2cb6c..aa0b757 100644 --- a/lib/providers/codex.mjs +++ b/lib/providers/codex.mjs @@ -157,6 +157,36 @@ function resolveCodexBin() { // D6 assumption A2: auth file is named auth.json (unconfirmed — D7 will pin). // D6 assumption A3: access token field is `access_token` or `token` (unconfirmed). // +// ── D75 (v0.4.2) F1 — codex CLI v0.133.0 schema pin ───────────────────────── +// +// Real codex CLI v0.133.0 auth.json (verified empirically on PI231 / Mac mini, +// 2026-05-26 E2E session): +// +// { +// "auth_mode": "chatgpt", +// "OPENAI_API_KEY": null | "", +// "tokens": { +// "id_token": "", +// "access_token": "", <-- THIS is the access token +// "refresh_token": "", +// "account_id": "" +// }, +// "last_refresh": "" +// } +// +// D6 assumption A3 originally tried `creds.access_token` at TOP level. Under +// codex v0.133.0 this field does not exist at the top level → readAuthArtifact() +// returned null even when the user had fully completed `codex login`. OLP then +// reported "auth artifact missing" via /health and `olp doctor`, and refused +// to spawn codex — false negative blocking the entire openai provider. +// +// Fix: prepend `creds?.tokens?.access_token` to the precedence chain. Keep all +// existing fallbacks unchanged so older codex CLI versions (pre-v0.133) and any +// future shape variants still resolve. +// +// Authority pin: codex CLI v0.133.0 source + on-disk auth.json captured during +// PI231 E2E. See D75 commit body for the verification transcript. +// // Returns { accessToken: string } or null (never throws). export function readAuthArtifact() { // 1. Explicit test override — always takes precedence. @@ -165,7 +195,12 @@ export function readAuthArtifact() { try { const raw = readFileSync(authPathOverride, 'utf8'); const creds = JSON.parse(raw); - const token = creds?.access_token ?? creds?.token ?? creds?.accessToken; + // D75 F1: codex CLI v0.133.0 nests the token under `tokens.access_token`. + // Preserve top-level fallbacks for backward / forward compat. + const token = creds?.tokens?.access_token + ?? creds?.access_token + ?? creds?.token + ?? creds?.accessToken; if (token && typeof token === 'string') return { accessToken: token }; } catch { /* fall through */ } return null; // explicit path set but file missing / malformed @@ -178,8 +213,12 @@ export function readAuthArtifact() { try { const raw = readFileSync(authPath, 'utf8'); const creds = JSON.parse(raw); - // D6 assumption A3: try common OAuth field names in precedence order. - const token = creds?.access_token ?? creds?.token ?? creds?.accessToken; + // D75 F1: codex CLI v0.133.0 nests the token under `tokens.access_token`. + // Try the nested location FIRST, then fall back to legacy top-level fields. + const token = creds?.tokens?.access_token + ?? creds?.access_token + ?? creds?.token + ?? creds?.accessToken; if (token && typeof token === 'string') return { accessToken: token }; } catch { /* file missing or malformed */ } @@ -242,9 +281,30 @@ export function irToCodex(irRequest) { // model string (e.g., gpt-5.5, gpt-5.4, gpt-5.3-codex). // PROMPT: "Initial instruction for the task. Use '-' to pipe the prompt // from stdin." + // + // ── D75 (v0.4.2) F2 — codex CLI v0.133.0 trusted-directory sandbox ───────── + // codex CLI v0.133.0 added a trusted-directory sandbox: invocations outside + // a git repo (or outside any directory explicitly trusted via + // `codex config trusted-directories`) refuse with: + // "Not inside a trusted directory and --skip-git-repo-check was not specified." + // and exit non-zero with zero NDJSON output → OLP surfaces SPAWN_FAILED with + // no usable chunks → fallback engine advances to next hop unnecessarily. + // + // The CWD that OLP spawns from is typically the server install dir (`~/olp/` + // on Pi231) which is a git repo on maintainer workstations but is NOT a git + // repo on most operator hosts. We bypass the sandbox unconditionally because + // OLP is the trusted caller (it is the operator's own server invoking its own + // configured Codex subscription via the documented `codex exec` automation + // entry point). The trusted-directory sandbox is a foot-gun safeguard for + // interactive users; OLP's spawn is non-interactive and pre-authorized. + // + // Authority: codex CLI v0.133.0 release notes / `codex exec --help` output + // documenting `--skip-git-repo-check`. Verified empirically on PI231 E2E + // 2026-05-26. const args = [ 'exec', '--json', + '--skip-git-repo-check', '--model', irRequest.model, ]; @@ -291,6 +351,51 @@ export function codexChunkToIR(rawNDJSONLine) { if (!event || typeof event !== 'object') return null; + // ── D75 (v0.4.2) F3 — codex CLI v0.133.0 event shape pin ───────────────── + // Real codex CLI v0.133.0 NDJSON event stream (verified empirically on PI231 + // / Mac mini, 2026-05-26 E2E session): + // {"type":"thread.started","thread_id":"019e..."} + // {"type":"turn.started"} + // {"type":"item.started","item":{"id":"item_0","type":"reasoning","text":""}} + // {"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":""}} + // {"type":"turn.completed","usage":{"input_tokens":..,"output_tokens":..}} + // + // The D6 defensive parser recognized `content`/`delta`/`text` fields at the + // top level and `type === 'stop'`/`done === true`. None of these match + // v0.133.0's actual shape → every chunk was silently dropped → response body + // had `content: null`. F3 adds three NEW recognizers (item.completed → + // agent_message; turn.completed → stop; turn.failed → error) BEFORE the + // legacy fallback chain. Legacy recognizers preserved for forward/backward + // compat (older codex versions; future shape variants). + + // F3-a: agent_message item completion. + // codex v0.133.0 emits assistant text as a single item.completed event whose + // item.type is 'agent_message' and item.text carries the full text. There + // are no incremental deltas — the entire response arrives in one chunk. + if (event.type === 'item.completed' + && event.item?.type === 'agent_message' + && typeof event.item?.text === 'string') { + return { type: 'delta', content: event.item.text }; + } + + // F3-b: turn completion → stop chunk. + // codex v0.133.0 emits turn.completed with a usage block when the model + // finishes. We map this to IR stop with finish_reason 'stop'. + if (event.type === 'turn.completed') { + return { type: 'stop', finish_reason: 'stop' }; + } + + // F3-c: turn failure → error chunk. + // codex v0.133.0 emits turn.failed with an embedded error object when the + // turn cannot complete. Extract a human-readable message for the IR error. + if (event.type === 'turn.failed') { + const errMsg = (typeof event.error === 'string') + ? event.error + : (event.error?.message ?? 'codex turn.failed'); + return { type: 'error', error: errMsg }; + } + + // ── Legacy/fallback recognizers (kept for backward + forward compat) ──── // 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')) { diff --git a/package.json b/package.json index 1c13ddd..2b68a0b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "olp", - "version": "0.4.1", + "version": "0.4.2", "description": "Personal multi-provider LLM proxy. Successor to OCP. One HTTP endpoint, multiple subscriptions behind it, automatic routing + fallback + caching.", "type": "module", "main": "server.mjs", diff --git a/server.mjs b/server.mjs index 7233dee..7b772a0 100644 --- a/server.mjs +++ b/server.mjs @@ -1224,13 +1224,25 @@ async function handleChatCompletions(req, res) { } const chunks = []; - // try/finally: releaseSpawn MUST fire on every exit path — success - // (return at end), spawn throw (caught and re-thrown below), or the - // D16 truncation-salvage return. The finally is the only mechanism - // that guarantees release across all three. + // D75 (v0.4.2) F7 — per-hop model override. + // The chain config (routing.chains[][].model) is the + // model name to pass to THIS hop's provider plugin — NOT the model the + // user originally requested. Pre-D75, executeHopFn used hopModel for the + // cache key + audit ctx but passed the ORIGINAL irReq (with irReq.model + // still set to the user's request) into hopProviderPlugin.spawn(). Result: + // a 2-hop chain like [{anthropic, claude-sonnet-4-6}, {openai, gpt-5.5}] + // would spawn codex with --model claude-sonnet-4-6 on hop 1 — openai rejects + // the unknown model and the chain dies. This broke the core OLP value prop + // (cross-provider fallback with provider-appropriate model substitution). + // + // Fix: build a per-hop IR variant with the hop's model substituted. Skip + // the clone when hopModel === irReq.model (single-provider chains and + // chain hops whose model matches the request). Authority: ADR 0004 § + // Chain advancement step 1 (per-hop config supplies provider AND model). + const hopIrReq = irReq.model === hopModel ? irReq : { ...irReq, model: hopModel }; try { try { - for await (const irChunk of hopProviderPlugin.spawn(irReq, authContext)) { + for await (const irChunk of hopProviderPlugin.spawn(hopIrReq, authContext)) { // D16: check error chunks BEFORE pushing — preserves the invariant that // chunks array contains only delta/stop chunks. Without this, the catch // block's `chunks.length > 0` would mistake a single error chunk for @@ -1422,9 +1434,16 @@ async function handleChatCompletions(req, res) { // releaseSpawn fires exactly once in finally — regardless of normal // exhaustion, mid-stream throw, or iterator.return() from cache-layer // sourceAbortController propagation (§9). + // + // D75 (v0.4.2) F7 — per-hop model override (streaming path). + // Mirror the buffered-path fix: pass the hop's configured model (streamModel) + // into streamPlugin.spawn(), not the user's original ir.model. Skip the + // clone when ir.model === streamModel. See executeHopFn() above for the + // full F7 rationale + authority citation. + const streamIr = ir.model === streamModel ? ir : { ...ir, model: streamModel }; return (async function* sourceWithRelease() { try { - for await (const irChunk of streamPlugin.spawn(ir, authContext)) { + for await (const irChunk of streamPlugin.spawn(streamIr, authContext)) { yield irChunk; } } finally { diff --git a/test-features.mjs b/test-features.mjs index b3dcc6a..ad7e936 100644 --- a/test-features.mjs +++ b/test-features.mjs @@ -14983,4 +14983,382 @@ describe('Suite 36 — D74 v0.4.1 hotfix regression (maintainer review findings) assert.ok(!/Phase 2 in progress/.test(serverSrc)); assert.ok(!/Phase 3 in progress/.test(serverSrc)); }); + + // ── D75 (v0.4.2) extension — real-machine E2E findings F1, F2, F3, F4, F7 ── + // Bugs caught on PI231 + Mac mini 2026-05-26 E2E session. Pre-v0.4.0 reviewers + // and the post-v0.4.0 maintainer review all missed these because they reviewed + // against spec text, not against real provider CLIs running on a remote machine. + + it('36i (F1) — codex readAuthArtifact() recognises v0.133.0 nested tokens.access_token', () => { + // Real codex CLI v0.133.0 auth.json shape: { auth_mode, OPENAI_API_KEY, + // tokens: { id_token, access_token, refresh_token, account_id }, last_refresh } + // Pre-D75 readAuthArtifact read only top-level access_token → returned null → + // OLP falsely reported "auth artifact missing" for fully-logged-in users. + const tmpDir = _mkdtempSyncS36(_joinS36(_tmpdirS36(), 'olp-d75-f1-nested-')); + const authPath = _joinS36(tmpDir, 'auth.json'); + _writeFileSyncS36(authPath, JSON.stringify({ + auth_mode: 'chatgpt', + OPENAI_API_KEY: null, + tokens: { + id_token: 'header.body.sig', + access_token: 'eyJ-real-nested-access-token-d75-36i', + refresh_token: 'refresh-opaque', + account_id: '00000000-0000-0000-0000-000000000000', + }, + last_refresh: '2026-05-26T00:00:00Z', + })); + const savedPath = process.env.OPENAI_CODEX_AUTH_PATH; + process.env.OPENAI_CODEX_AUTH_PATH = authPath; + try { + const result = codexReadAuthArtifact(); + assert.ok(result, 'must return non-null for v0.133 nested-shape auth.json'); + assert.equal(result.accessToken, 'eyJ-real-nested-access-token-d75-36i', + 'must extract token from tokens.access_token (v0.133.0 nested shape)'); + } finally { + if (savedPath !== undefined) process.env.OPENAI_CODEX_AUTH_PATH = savedPath; + else delete process.env.OPENAI_CODEX_AUTH_PATH; + _rmSyncS36(tmpDir, { recursive: true, force: true }); + } + }); + + it('36j (F1) — codex readAuthArtifact() still reads legacy top-level access_token', () => { + // Backwards compat: older codex CLI versions (pre-v0.133) wrote the token + // at top level. D75 fix preserves that fallback so upgrades don't regress. + const tmpDir = _mkdtempSyncS36(_joinS36(_tmpdirS36(), 'olp-d75-f1-legacy-')); + const authPath = _joinS36(tmpDir, 'auth.json'); + _writeFileSyncS36(authPath, JSON.stringify({ + access_token: 'legacy-top-level-token-d75-36j', + })); + const savedPath = process.env.OPENAI_CODEX_AUTH_PATH; + process.env.OPENAI_CODEX_AUTH_PATH = authPath; + try { + const result = codexReadAuthArtifact(); + assert.ok(result, 'must return non-null for legacy top-level shape'); + assert.equal(result.accessToken, 'legacy-top-level-token-d75-36j', + 'must fall back to top-level access_token when tokens.access_token absent'); + } finally { + if (savedPath !== undefined) process.env.OPENAI_CODEX_AUTH_PATH = savedPath; + else delete process.env.OPENAI_CODEX_AUTH_PATH; + _rmSyncS36(tmpDir, { recursive: true, force: true }); + } + }); + + it('36k (F1) — codex readAuthArtifact() returns null for malformed auth.json', () => { + // Malformed JSON / missing token / wrong structure → null (not throw). + const tmpDir = _mkdtempSyncS36(_joinS36(_tmpdirS36(), 'olp-d75-f1-malformed-')); + const savedPath = process.env.OPENAI_CODEX_AUTH_PATH; + try { + // Case A: unparseable JSON + const badJsonPath = _joinS36(tmpDir, 'bad.json'); + _writeFileSyncS36(badJsonPath, 'not-json-at-all{{{'); + process.env.OPENAI_CODEX_AUTH_PATH = badJsonPath; + assert.equal(codexReadAuthArtifact(), null, 'unparseable JSON → null'); + + // Case B: valid JSON but no token field anywhere + const noTokenPath = _joinS36(tmpDir, 'no-token.json'); + _writeFileSyncS36(noTokenPath, JSON.stringify({ auth_mode: 'chatgpt', tokens: {} })); + process.env.OPENAI_CODEX_AUTH_PATH = noTokenPath; + assert.equal(codexReadAuthArtifact(), null, 'no token in tokens.access_token or top-level → null'); + + // Case C: tokens field present but access_token wrong type + const wrongTypePath = _joinS36(tmpDir, 'wrong-type.json'); + _writeFileSyncS36(wrongTypePath, JSON.stringify({ + tokens: { access_token: 42 }, + })); + process.env.OPENAI_CODEX_AUTH_PATH = wrongTypePath; + assert.equal(codexReadAuthArtifact(), null, 'non-string access_token → null'); + } finally { + if (savedPath !== undefined) process.env.OPENAI_CODEX_AUTH_PATH = savedPath; + else delete process.env.OPENAI_CODEX_AUTH_PATH; + _rmSyncS36(tmpDir, { recursive: true, force: true }); + } + }); + + it('36l (F2) — irToCodex() includes --skip-git-repo-check before --model', () => { + // codex CLI v0.133.0 sandboxes non-git-repo CWDs by default with: + // "Not inside a trusted directory and --skip-git-repo-check was not specified." + // OLP servers typically deploy outside a git repo on the operator host. The + // --skip-git-repo-check flag must be in args, before --model, so the spawn + // succeeds without depending on the install-time CWD. + const { args } = irToCodex({ + irVersion: IR_VERSION, + model: 'gpt-5.5', + stream: false, + messages: [{ role: 'user', content: 'hello' }], + }); + assert.ok(args.includes('--skip-git-repo-check'), + 'irToCodex().args must include --skip-git-repo-check (F2 fix)'); + const skipIdx = args.indexOf('--skip-git-repo-check'); + const modelIdx = args.indexOf('--model'); + assert.ok(skipIdx > 0 && modelIdx > 0, 'both flags must be present'); + assert.ok(skipIdx < modelIdx, + '--skip-git-repo-check must come before --model (cleaner readability + matches codex v0.133 docs ordering)'); + // Confirm exec is still first and --json still adjacent + assert.equal(args[0], 'exec'); + assert.equal(args[1], '--json'); + assert.equal(args[2], '--skip-git-repo-check'); + assert.equal(args[3], '--model'); + assert.equal(args[4], 'gpt-5.5'); + }); + + it('36m (F3) — codexChunkToIR recognises v0.133.0 item.completed agent_message', () => { + // Real codex v0.133.0 emits the assistant text as one item.completed event: + // {"type":"item.completed","item":{"id":"item_0","type":"agent_message","text":"hello world"}} + // Pre-D75 codexChunkToIR returned null for this shape → response body had + // content: null because the chunk was silently dropped. + const result = codexChunkToIR(JSON.stringify({ + type: 'item.completed', + item: { id: 'item_0', type: 'agent_message', text: 'hello world from codex' }, + })); + assert.deepEqual(result, { type: 'delta', content: 'hello world from codex' }); + }); + + it('36n (F3) — codexChunkToIR recognises v0.133.0 turn.completed → stop', () => { + // turn.completed marks the end of a codex turn. Map to IR stop. + const result = codexChunkToIR(JSON.stringify({ + type: 'turn.completed', + usage: { input_tokens: 12, output_tokens: 7 }, + })); + assert.deepEqual(result, { type: 'stop', finish_reason: 'stop' }); + }); + + it('36o (F3) — codexChunkToIR recognises v0.133.0 turn.failed → error', () => { + // turn.failed: { type: 'turn.failed', error: { message: 'X' } } + const result1 = codexChunkToIR(JSON.stringify({ + type: 'turn.failed', + error: { message: 'rate limit exceeded' }, + })); + assert.equal(result1?.type, 'error'); + assert.equal(result1.error, 'rate limit exceeded'); + // turn.failed with string error + const result2 = codexChunkToIR(JSON.stringify({ + type: 'turn.failed', + error: 'simple string error', + })); + assert.equal(result2?.type, 'error'); + assert.equal(result2.error, 'simple string error'); + // turn.failed with no error payload → falls back to default message + const result3 = codexChunkToIR(JSON.stringify({ type: 'turn.failed' })); + assert.equal(result3?.type, 'error'); + assert.match(result3.error, /turn\.failed/); + }); + + it('36p (F4) — cmdStatus reads body.stats.cache.size (not OCP-era body.cache.entries)', () => { + // Server payload (server.mjs handleManagementStatus ~line 2092): + // { ok, version, ..., stats: { total_requests, active_requests, cache: { hits, misses, size, inflightCount } } } + // Pre-D75 cmdStatus formatter read `body.cache.entries` (OCP-era) → always + // undefined → output showed "entries=?". Pin the wire contract by grepping + // the CLI source so a future server-side rename can't silently re-break. + const cliSrc = _readFileSyncS36(_joinS36(import.meta.dirname ?? process.cwd(), 'bin/olp.mjs'), 'utf8'); + // Must contain the cmdStatus block that reads body.stats.cache (not body.cache directly) + assert.ok(/cmdStatus\b/.test(cliSrc), 'cmdStatus function must exist'); + // The status block must read body.stats?.cache (the actual server payload nesting) + assert.ok(/body\.stats\?\.cache/.test(cliSrc) || /body\.stats\.cache/.test(cliSrc), + 'cmdStatus must read body.stats.cache for status display'); + // Specifically, the "entries=" output must derive from c.size (matches CacheStore.stats() shape) + // Find the status cache line and verify it uses c.size + const statusBlock = cliSrc.match(/cmdStatus[\s\S]+?(?=async function cmd|^\}\s*$)/m); + assert.ok(statusBlock, 'cmdStatus block extractable'); + assert.ok(/c\.size/.test(statusBlock[0]), + 'cmdStatus cache line must use c.size (matches CacheStore.stats() shape)'); + // And must NOT use the OCP-era c.entries + assert.ok(!/c\.entries/.test(statusBlock[0]), + 'cmdStatus cache line must NOT read c.entries (OCP-era field that does not exist in CacheStore.stats())'); + // Also pin the CacheStore.stats() shape contract (re-affirms 36e but for status path) + const cs = new CacheStore(); + const stats = cs.stats(); + assert.ok('size' in stats, 'CacheStore.stats() exposes size'); + assert.ok(!('entries' in stats), 'CacheStore.stats() does NOT expose entries'); + }); + + it('36q (F7) — per-hop chain `model` field overrides IR model on fallback hop', async () => { + // Pre-D75: executeHopFn(provider, model, irReq) used `model` for cache key + // + audit but passed irReq UNCHANGED to provider.spawn() → the chain + // [{anthropic, claude-X}, {openai, gpt-5.5}] + // would spawn codex with --model claude-X (the user's original request) + // instead of gpt-5.5 (the chain's per-hop config). This broke cross-provider + // fallback wherever the chain assigned different models per provider. + // + // Strategy: inject a mock openai provider that captures the IR it receives. + // Force the anthropic hop to fail with SPAWN_FAILED so the chain advances + // to openai. Assert the mock saw model === hop's configured model. + + const { __setAuthConfig: setAC36q, __resetAuthConfig: resetAC36q, + __setFallbackConfig: setFC36q, __resetFallbackConfig: resetFC36q, + createOlpServer, loadedProviders } = await import('./server.mjs'); + + setAC36q({ allow_anonymous: true }); + + // Capture for mock openai provider + let capturedIR = null; + const mockOpenAI = { + name: 'openai', + displayName: 'Mock OpenAI for F7', + contractVersion: '1.0', + models: ['gpt-5.5'], + auth: { type: 'oauth', storage: 'file', path: '/tmp/fake', refresh: 'auto' }, + async * spawn(ir, _authContext) { + // Snapshot the model field as received + capturedIR = { model: ir.model }; + yield { type: 'delta', role: 'assistant', content: 'mock-openai-response' }; + yield { type: 'stop', finish_reason: 'stop' }; + }, + estimateCost: () => null, + quotaStatus: async () => null, + healthCheck: async () => ({ ok: true, latencyMs: 0 }), + hints: { requiresTTY: false, concurrentSpawnSafe: true, maxConcurrent: 4, cacheable: true }, + }; + + // Mock anthropic provider that always fails immediately with SPAWN_FAILED + const mockAnthropic = { + name: 'anthropic', + displayName: 'Mock Anthropic for F7', + contractVersion: '1.0', + models: ['claude-sonnet-4-6'], + auth: { type: 'oauth', storage: 'file', path: '/tmp/fake', refresh: 'auto' }, + async * spawn(_ir, _authContext) { + throw new ProviderError('mock anthropic always fails (F7 test)', 'SPAWN_FAILED'); + // eslint-disable-next-line no-unreachable + yield { type: 'stop' }; + }, + estimateCost: () => null, + quotaStatus: async () => null, + healthCheck: async () => ({ ok: true, latencyMs: 0 }), + hints: { requiresTTY: false, concurrentSpawnSafe: true, maxConcurrent: 4, cacheable: true }, + }; + + // Save + clear loadedProviders, install the mocks + const savedProviders = new Map(loadedProviders); + loadedProviders.clear(); + loadedProviders.set('anthropic', mockAnthropic); + loadedProviders.set('openai', mockOpenAI); + + // 2-hop chain with DIFFERENT models per hop — the bug only manifests when + // hopModel !== requested model on the hop that actually serves. + setFC36q({ + chains: { + 'claude-sonnet-4-6': [ + { provider: 'anthropic', model: 'claude-sonnet-4-6' }, + { provider: 'openai', model: 'gpt-5.5' }, + ], + }, + soft_triggers: {}, + }); + + const srv = createOlpServer(); + let port = 28400 + Math.floor(Math.random() * 100); + await new Promise((resolve, reject) => { + srv.listen(port, '127.0.0.1', resolve); + srv.once('error', e => { + if (e.code === 'EADDRINUSE') { port++; srv.listen(port, '127.0.0.1', resolve); srv.once('error', reject); } + else reject(e); + }); + }); + + try { + const r = await fetch({ + port, + method: 'POST', + path: '/v1/chat/completions', + body: { + model: 'claude-sonnet-4-6', + messages: [{ role: 'user', content: 'F7 test' }], + }, + }); + assert.equal(r.status, 200, `expected 200, got ${r.status}: ${r.body.slice(0, 300)}`); + assert.equal(r.headers['x-olp-provider-used'], 'openai', + 'openai must be the serving hop (anthropic failed)'); + // The F7 assertion: the openai hop received the chain's configured model, + // NOT the user's original request model. + assert.ok(capturedIR, 'mock openai must have been invoked'); + assert.equal(capturedIR.model, 'gpt-5.5', + `F7: openai hop must receive its chain-configured model 'gpt-5.5', got '${capturedIR.model}'`); + } finally { + await new Promise(r => srv.close(r)); + resetFC36q(); + // Restore original providers + loadedProviders.clear(); + for (const [k, v] of savedProviders) loadedProviders.set(k, v); + resetAC36q(); + } + }); + + it('36r (F7) — per-hop model override preserved on streaming single-hop path', async () => { + // The streaming path (server.mjs ~line 1390-1434) has its own spawn call site + // inside sourceFactory that previously also passed `ir` unchanged. F7 fix + // wraps it with the same { ...ir, model: streamModel } substitution. This + // test exercises a single-hop streaming request and asserts the mock saw + // the chain's configured model. + + const { __setAuthConfig: setAC36r, __resetAuthConfig: resetAC36r, + __setFallbackConfig: setFC36r, __resetFallbackConfig: resetFC36r, + createOlpServer, loadedProviders } = await import('./server.mjs'); + + setAC36r({ allow_anonymous: true }); + + let capturedModel = null; + const mockOpenAIStream = { + name: 'openai', + displayName: 'Mock Codex for F7 streaming', + contractVersion: '1.0', + models: ['gpt-5.5'], + auth: { type: 'oauth', storage: 'file', path: '/tmp/fake', refresh: 'auto' }, + async * spawn(ir, _authContext) { + capturedModel = ir.model; + yield { type: 'delta', role: 'assistant', content: 'streamed-content' }; + yield { type: 'stop', finish_reason: 'stop' }; + }, + estimateCost: () => null, + quotaStatus: async () => null, + healthCheck: async () => ({ ok: true, latencyMs: 0 }), + hints: { requiresTTY: false, concurrentSpawnSafe: true, maxConcurrent: 4, cacheable: true }, + }; + + const savedProviders = new Map(loadedProviders); + loadedProviders.clear(); + loadedProviders.set('openai', mockOpenAIStream); + + // Single-hop chain with a DIFFERENT model than the request — the bug is + // visible only when the user requests one model name and the chain remaps it. + setFC36r({ + chains: { + 'gpt-aliased': [ + { provider: 'openai', model: 'gpt-5.5' }, + ], + }, + soft_triggers: {}, + }); + + const srv = createOlpServer(); + let port = 28500 + Math.floor(Math.random() * 100); + await new Promise((resolve, reject) => { + srv.listen(port, '127.0.0.1', resolve); + srv.once('error', e => { + if (e.code === 'EADDRINUSE') { port++; srv.listen(port, '127.0.0.1', resolve); srv.once('error', reject); } + else reject(e); + }); + }); + + try { + const r = await fetch({ + port, + method: 'POST', + path: '/v1/chat/completions', + body: { + model: 'gpt-aliased', + stream: true, + messages: [{ role: 'user', content: 'F7 streaming test' }], + }, + }); + assert.equal(r.status, 200, `expected 200, got ${r.status}: ${r.body.slice(0, 200)}`); + assert.equal(capturedModel, 'gpt-5.5', + `F7 streaming: openai must receive chain-configured model 'gpt-5.5', got '${capturedModel}'`); + } finally { + await new Promise(r => srv.close(r)); + resetFC36r(); + loadedProviders.clear(); + for (const [k, v] of savedProviders) loadedProviders.set(k, v); + resetAC36r(); + } + }); });