[codex] fix Codex budget native compaction trigger#87158
Conversation
4a4bb8f to
47f3374
Compare
|
Codex review: needs real behavior proof before merge. Reviewed May 29, 2026, 1:17 AM ET / 05:17 UTC. Summary PR surface: Source +5, Tests +29, Docs +10. Total +44 across 4 files. Reproducibility: yes. source inspection gives a high-confidence reproduction path: current channel preflight returns before the Codex byte-budget path, and current CLI budget compaction throws on the helper's pending native result shape. I did not execute tests because this review was read-only. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Decide the Codex budget contract first, then either wire and prove the full caller lifecycle for budget-started native compaction or keep the native-owned skip path and land the narrower caller fix in the canonical related PR. Do we have a high-confidence way to reproduce the issue? Yes, source inspection gives a high-confidence reproduction path: current channel preflight returns before the Codex byte-budget path, and current CLI budget compaction throws on the helper's pending native result shape. I did not execute tests because this review was read-only. Is this the best way to solve the issue? No. The helper change is too narrow unless the real caller paths also accept or exercise budget-started native compaction; the safer alternative is to coordinate with the open skip-handling PR if maintainers prefer Codex-owned automatic compaction. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 309fdd95dad5. Label changesLabel justifications:
Evidence reviewedPR surface: Source +5, Tests +29, Docs +10. Total +44 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Cosmic Shellbean Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
Summary
budgetpreflight trigger instead of treating it like an unrelated automatic trigger.overflowtriggers on the existing Codex-owned skip path.agents.defaults.compaction.maxActiveTranscriptBytesstarts native Codex compaction for Codex harness sessions.Duplicate check
I checked current
origin/mainplus open PRs/issues before opening this PR. Currentmainstill skippedtrigger: "budget"inextensions/codex/src/app-server/compact.tsand had a test pinning that skip incompact.test.ts. I did not find an open PR from this fork for this compaction trigger; the only openSteady-aiPR was #86716 for Discord delivery accounting. I also inspected nearby compaction/Codex PRs including #81916, #63636, #54585, #86160, #83229, #82856, #86094, and #73092; none madebudgetstart native Codex compaction.Real behavior proof
Behavior addressed: Codex harness sessions with
agents.defaults.compaction.maxActiveTranscriptBytescall compaction withtrigger: "budget". Current source skipped that trigger before sendingthread/compact/start, so the local transcript-byte guard could leave long-lived channel mirrors above the configured cleanup threshold.Real environment tested: OpenClaw source worktree rebased on
origin/mainatd88681662b, PR head47f33748d4, on WSL Ubuntu 24.04 / Node 22.22.0. The source-runtime probe used the realmaybeCompactCodexAppServerSessionentry point and real Codex session-binding file I/O, with the app-server client boundary instrumented only to capture the outgoing app-server request.Exact steps or command run after this patch: Ran a
node --import tsx -source-runtime probe from the PR worktree. The probe wrote a temporary Codex app-server binding, invokedmaybeCompactCodexAppServerSessiononce withtrigger: "budget"and once withtrigger: "overflow", then printed the request/result shape. I then ran the focused checks listed in Verification.Evidence after fix: Copied terminal output from the source-runtime probe:
Observed result after fix: Budget preflight compaction now sends
thread/compact/startand returns the existing pending native-compaction result shape. The overflow trigger sends no additional app-server request and still returns the existingnon_manual_triggerskip result.What was not tested: I did not run a live Discord or Telegram gateway replay from this source branch. The live failure was observed in an installed runtime; this PR ports the source-level behavior and locks it with focused Codex extension tests.
Risk
Low. The change is limited to the Codex app-server compaction trigger gate. It does not wait for native compaction completion, add credentials, change network targets, or replace Codex compaction with OpenClaw summarization.
Verification
OPENCLAW_VITEST_MAX_WORKERS=1 node scripts/run-vitest.mjs run --config test/vitest/vitest.extension-codex.config.ts extensions/codex/src/app-server/compact.test.ts-> 1 file / 19 tests passednode_modules/.bin/oxfmt --check extensions/codex/src/app-server/compact.ts extensions/codex/src/app-server/compact.test.ts docs/plugins/codex-harness-runtime.md docs/reference/session-management-compaction.md-> passedgit diff --check-> passedpnpm docs:list-> passednode scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/codex/src/app-server/compact.ts extensions/codex/src/app-server/compact.test.ts-> passednode scripts/check-docs-mdx.mjs docs/plugins/codex-harness-runtime.md docs/reference/session-management-compaction.md-> passed