Repository navigation
fix: detect codex compaction requests before preflight; verbatim passthrough when not intercepted (#332) - #335
Conversation
📦 Built Package ArtifactBranch: Option A — Install from npm PR tag (recommended)npm install -g billion-context@pr-335Each push to this PR publishes a new version under the Option B — Download artifact
tar xzf billion-context-pr335.tgz
npm install -g packageThis comment is automatically updated on each push. |
[bot] On it — reviewing PR #332 (branch |
[bot] Review of PR #332 complete — approved, all claims verified. Details below. Verification (all run on
|
| Check | Result |
|---|---|
npm run typecheck |
✅ clean |
npm test |
✅ 761/761 pass (matches the claim) |
npm run build |
✅ clean (tsup, dist/index.js 2.53 MB) |
| 4 new tests fail pre-fix | ✅ Verified: checked out 3b34dde^:src/server.ts and re-ran tests/codex-compact-e2e.test.ts → exactly the 4 #332 tests fail, the 5 pre-existing E2 tests still pass. Restored afterwards. |
| Merge vs master | ✅ git merge-tree against origin/master (caca703, PR #329) → no conflicts (disjoint files: branch touches src/codex-compact.ts, src/server.ts, tests/codex-compact*.test.ts; master added tests/e2e/, AGENTS.md, ci-e2e.yml, test:e2e script) |
Code review
The implementation matches the description exactly:
src/codex-compact.ts—codexCompactGatePre(session, effectiveLimit)=limit > 0 && lastInputTokens < 0.9·limit && ≥1 active block;codexCompactGate = transformOk && gatePre. Semantics identical to the old combined gate.src/server.ts:1064-1082— trigger detection on the raw parsed body insidewithSessionLock, beforerunPrepare()/preflight. Three-way dispatch as proposed: intercept+gatePre →runPrepare()→ forge served locally viaforward()'scodexForgebranch (upstream untouched,rememberPluginMessagescalled); otherwise rawbodyBufferforwarded withprepared = null(so no#300chain-marker stamp — correct). Thewhylog (BILI_CODEX_COMPACT=pass/gate preconditions not met/transform/forge failed) is accurate for each branch.src/server.ts:1665-1671(prepareResponsesCompact) —gatePrechecked beforeprocessTurn, so a non-intercepted endpoint compact returnsbase(raw body) with no state mutation.- Consistency: the dispatch and
prepareResponsesboth use the samereqConfig.modelContextLimit(the dispatch'sgatePreatsrc/server.ts:1071and the forge gate atsrc/server.ts:1531see the identical limit), so there's no window where the dispatch says "intercept" butprepareResponses' full gate disagrees except viatransformOk— which is exactly the "transform/forge failed" branch. - No regression to the E2 e2e suite: the forge log
codex compact intercepted (trigger)(src/server.ts:1539) thattests/e2e/e2e-codex.test.tsasserts on is unchanged. - The two flagged out-of-scope notes check out:
x-codex-beta-featuresstripping inbuildForwardTargetis untouched, and thetriggerFinalnudge-skip (src/server.ts:1499-1502) is now dead for the trigger form but correctly left for the feat: intercept + forge codex native compaction when ACP is healthy (#321 PR-E2) #325 change set.
Minor observations (non-blocking)
- "transform/forge failed" branch still touches a little state: when mode=intercept + gatePre passes but the kernel transform throws,
runPrepare()has already run++session.stats.requests,reconcileNativeCompactionBoundary, andstripBiliCompactionItemsbefore the failure is discovered.processTurn/session.stateis not mutated on the throw path (state is only assigned after success), so this is the same pre-existing behavior as any transform-failure request — not a regression from this PR, just noting it for completeness. - Redundant (harmless) check:
src/server.ts:1684re-checks the fullcodexCompactGateafterprocessTurnin the endpoint form, but at that pointgatePrealready passed andtransformOk === true, so it can never fail. Fine as defense-in-depth. - Test quality is good: the new tests assert byte-identical forwarding (
h.bodies[last] === rawBody), session-state immutability viaJSON.stringifysnapshot, and upstream non-contact on the forge path — with the oversized 14-msg payload that specifically reproduces the old preflight-firing condition.
Merge ordering
2026-08-28_codex-compact-intercept) is not in master yet — none of its commits (008bb95, c507e0c, c959942) are ancestors of origin/master. This branch is stacked on it, so #325 must merge first (or both land together). No conflicts either way.
LGTM — ready to merge once #325 lands.
…through when not intercepted (#332)
3b34dde to
174006e
Compare
Kernel 0.0.82 (acp-kernel#339) ships the host checkpoint-carrier fix (ranxianglei#335): plain message-ref ranges skip live carriers marked via CoreMessage.summaryOfBlockId. Billion-context does not consume the marker yet (host-side adoption is dsh's), but the pin keeps the bundled kernel current.
… array class (#2663) The range selector (buildCompressibleRanges) advertised spans the apply side structurally rejects: live checkpoint carriers (#335) survive pruning but are carved out at apply time, so a span bridging carriers over covered raws resolved to zero foldable messages and threw every turn (#2638). The two screens also judged two different array classes for integrity withdrawal. - state.ts: shared isLiveCheckpointCarrier predicate — single source of truth for selector and apply side - recommend.ts: live carriers screen as gaps so no advertised group bridges across them; new optional integritySource param lets the withdrawal screening run on the same array class the apply gate judges (default: passed array) - pipeline.ts/compress.ts: prune node stashes the pre-prune resent array in effects.originalMessages; recommend feeds it as the withdrawal source - report.ts: acp_status UNCOMPRESSED rows open a gap at live carriers so no reported range bridges across them - tests: carrier-advertise-validate.test.ts — #2638 production geometry end-to-end, carrier boundary (live/stale), withdrawal array-class contract, status-row consistency, plus both directions of the #2607 media split (archivable folds like text, unrecoverable stays pinned) Rebased after #2607 landed on master: the originally drafted blanket media carve was dropped — filterProtectedToolMessages already enforces the #1188 never-fold exemption for unrecoverable payloads, and a blanket carve would have re-pinned archivable media against #2607's fold-like-text direction. Media handling itself is untouched by this PR. kernel-change: advertise/validate disagreement lives inside the kernel (recommend.ts vs compress.ts); host side cannot express carrier-aware screening or the pruned-vs-original view split. Closes #2663
… array class (#2663) The range selector (buildCompressibleRanges) advertised spans the apply side structurally rejects: live checkpoint carriers (#335) survive pruning but are carved out at apply time, so a span bridging carriers over covered raws resolved to zero foldable messages and threw every turn (#2638). The two screens also judged two different array classes for integrity withdrawal. - state.ts: shared isLiveCheckpointCarrier predicate — single source of truth for selector and apply side - recommend.ts: live carriers screen as gaps so no advertised group bridges across them; new optional integritySource param lets the withdrawal screening run on the same array class the apply gate judges (default: passed array) - pipeline.ts/compress.ts: prune node stashes the pre-prune resent array in effects.originalMessages; recommend feeds it as the withdrawal source - compress.ts: media payloads in an applied span are carved out of the block instead of silently folded (bytes unrecoverable once folded, #1188); the existing warning now tells the truth - report.ts: acp_status UNCOMPRESSED rows open a gap at live carriers so no reported range bridges across them kernel-change: advertise/validate disagreement lives inside the kernel (recommend.ts vs compress.ts); host side cannot express carrier-aware screening or the pruned-vs-original view split. Closes #2663
… array class (#2663) The range selector (buildCompressibleRanges) advertised spans the apply side structurally rejects: live checkpoint carriers (#335) survive pruning but are carved out at apply time, so a span bridging carriers over covered raws resolved to zero foldable messages and threw every turn (#2638). The two screens also judged two different array classes for integrity withdrawal. - state.ts: shared isLiveCheckpointCarrier predicate — single source of truth for selector and apply side - recommend.ts: live carriers screen as gaps so no advertised group bridges across them; new optional integritySource param lets the withdrawal screening run on the same array class the apply gate judges (default: passed array) - pipeline.ts/compress.ts: prune node stashes the pre-prune resent array in effects.originalMessages; recommend feeds it as the withdrawal source - report.ts: acp_status UNCOMPRESSED rows open a gap at live carriers so no reported range bridges across them - tests: carrier-advertise-validate.test.ts — #2638 production geometry end-to-end, carrier boundary (live/stale), withdrawal array-class contract, status-row consistency, plus both directions of the #2607 media split (archivable folds like text, unrecoverable stays pinned) Rebased after #2607 landed on master: the originally drafted blanket media carve was dropped — filterProtectedToolMessages already enforces the #1188 never-fold exemption for unrecoverable payloads, and a blanket carve would have re-pinned archivable media against #2607's fold-like-text direction. Media handling itself is untouched by this PR. kernel-change: advertise/validate disagreement lives inside the kernel (recommend.ts vs compress.ts); host side cannot express carrier-aware screening or the pruned-vs-original view split. Closes #2663
What
Fixes #333. codex's native remote-compaction request (
compaction_triggerform) used to flow throughprepareResponses(kernel transform + payload rebuild) and the preflight overflow-compress before the compact detection/gate ran. Even when the result was a passthrough, the upstream received a mutated compaction request — diverging from codex's local history, and 400-looping on non-OpenAI backends that reject thecompaction_triggerinput item (real-device evidence in the issue: 44354 tok > 32000 window → preflight compressed + rebuilt → relay 400 ×3).How
Detection is moved to the top of the per-session pipeline, before
runPrepare()/preflight, with a three-way dispatch exactly as proposed in the issue:prepareResponsesruns (transform + existing forge logic) → forged 2-frame SSE served locally, upstream not contacted.prepared = nullpath, same as the fix: stitched-stream usage double-count after compress re-request (adapter-anthropic >0 guard) #300 hop-marker passthrough). No preflight, no rebuild, no window clamp, no session-state folding as a side effect.Changes:
src/codex-compact.ts— split the safety valve:codexCompactGatePre(session, limit)(limit > 0, steady-state < 90% of window, ≥1 active block — everything knowable before the transform) andcodexCompactGate = transformOk && gatePre(identical semantics to before).src/server.ts(pipeline) — detectisCodexClient && last input item === compaction_triggeron the raw parsed body; dispatch as above. When not intercepted, logs the reason (BILI_CODEX_COMPACT=pass/gate preconditions not met/transform/forge failed).src/server.ts(prepareResponsesCompact, endpoint form) — gate preconditions checked beforeprocessTurn, so a compact that will not be intercepted no longer mutates session state as a side effect (wire behavior unchanged: raw body passthrough).Notes / scope:
x-codex-beta-features: remote_compaction_v2header stripping inbuildForwardTargetis left as-is (pre-existing deliberate behavior for all codex requests, with its own aggregator-rejection rationale) — flagging it here in case pass-mode native compaction on a backend that requires the beta header turns out to need it preserved.triggerFinalnudge-skip insideprepareResponsesis now dead code for the trigger form (the rebuilt body is never forwarded for it) but left in place — it belongs to the feat: intercept + forge codex native compaction when ACP is healthy (#321 PR-E2) #325 change set.2026-08-28_codex-compact-intercept(PR feat: intercept + forge codex native compaction when ACP is healthy (#321 PR-E2) #325, the E2 intercept/forge feature this ordering fix completes). Merge feat: intercept + forge codex native compaction when ACP is healthy (#321 PR-E2) #325 first, or both together.Tests
4 new e2e regression tests in
tests/codex-compact-e2e.test.tsusing an oversized payload (14 msgs ≈ 15.4k tok > 10k window — the condition that made the old ordering fire preflight):processTurnskipped (no state mutation)All 4 fail on the pre-fix code (verified by stashing
src/server.tsand re-running) and pass with the fix.Pre-flight
npm run typecheck— cleannpm test— 761/761 passnpm run build— clean