mirror of
https://github.com/dtzp555-max/olp.git
synced 2026-07-19 09:45:07 +00:00
fix+release(v0.4.2): D75 — codex CLI v0.133.0 schema + per-hop model override (#47)
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=<response>) → 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 <dtzp555@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
+25
-6
@@ -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[<requested-model>][<hop>].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 {
|
||||
|
||||
Reference in New Issue
Block a user