Skip to content

[codex] Mark Codex compaction events completed#81593

Merged
steipete merged 2 commits into
openclaw:mainfrom
Kyzcreig:codex/compaction-completed-flag
May 16, 2026
Merged

[codex] Mark Codex compaction events completed#81593
steipete merged 2 commits into
openclaw:mainfrom
Kyzcreig:codex/compaction-completed-flag

Conversation

@Kyzcreig
Copy link
Copy Markdown
Contributor

@Kyzcreig Kyzcreig commented May 14, 2026

Summary

Marks Codex app-server compaction end events as completed.

Also syncs the checked Codex app-server protocol mirror with the current upstream schema so the protocol drift check passes again.

Why

The run-level compaction bookkeeping treats a compaction end event as successful only when evt.data.completed === true. The Codex app-server event projector increments its local completed-compaction count when a contextCompaction item completes, but the emitted compaction end event did not carry the completion flag.

Adding the flag keeps the emitted event contract aligned with the runner's completion check.

Real behavior proof

Behavior addressed: Codex app-server contextCompaction item completion now emits a compaction end event with completed=true, preventing false incomplete-compaction bookkeeping and notices.

Real environment tested: Local OpenClaw checkout against ../codex protocol schema, plus focused OpenClaw test lanes. Upstream Codex checkout observed at 7fa0007ea8.

Exact steps or command run after this patch: pnpm codex-app-server:protocol:check; pnpm test extensions/codex/src/app-server/event-projector.test.ts; pnpm test src/auto-reply/reply/agent-runner-execution.test.ts -- -t compaction; pnpm oxfmt --check --threads=1 CHANGELOG.md scripts/check-codex-app-server-protocol.ts extensions/codex/src/app-server/event-projector.ts extensions/codex/src/app-server/event-projector.test.ts; git diff --check; /Users/steipete/Projects/agent-scripts/skills/codex-review/scripts/codex-review --mode branch.

Evidence after fix: protocol check reported the generated protocol matches OpenClaw bridge assumptions; event-projector test file passed 43 tests; agent-runner compaction-filtered test passed 7 tests; oxfmt and git diff checks passed; Codex review reported no accepted/actionable findings.

Observed result after fix: the projected compaction end event includes completed=true while preserving the existing contextCompaction item lifecycle and hook behavior.

What was not tested: Full release/build suite locally; PR CI is the broad proof gate on the pushed head.

Verification

  • pnpm codex-app-server:protocol:check
  • pnpm test extensions/codex/src/app-server/event-projector.test.ts
  • pnpm test src/auto-reply/reply/agent-runner-execution.test.ts -- -t compaction
  • pnpm oxfmt --check --threads=1 CHANGELOG.md scripts/check-codex-app-server-protocol.ts extensions/codex/src/app-server/event-projector.ts extensions/codex/src/app-server/event-projector.test.ts
  • git diff --check
  • /Users/steipete/Projects/agent-scripts/skills/codex-review/scripts/codex-review --mode branch

@openclaw-barnacle openclaw-barnacle Bot added extensions: codex size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 14, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 14, 2026

Codex review: needs real behavior proof before merge.

Summary
The PR adds completed: true to Codex app-server contextCompaction end events, adds a regression assertion, syncs the Codex app-server protocol mirror/check snippets, and updates the changelog.

Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main emits a Codex contextCompaction end event without completed, while the runner requires completed === true for the success path. I did not run a live Codex app-server compaction in this read-only review.

Real behavior proof
Needs real behavior proof before merge: The PR body reports focused tests and protocol checks, but not a real after-fix Codex app-server compaction run log, terminal output, recording, or linked artifact. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Contributor should add redacted real compaction-run proof, such as terminal output, logs, a recording, or a linked artifact from an actual Codex app-server compaction run; updating the PR body should trigger re-review, or a maintainer can comment @clawsweeper re-review.

Security
Cleared: The diff touches a local event payload, focused tests, generated protocol mirror/check snippets, and changelog only; no CI, dependency, secret, install, or supply-chain surface is changed.

Review details

Best possible solution:

Land this emitter-side completion flag and protocol mirror sync, or an equivalent narrow emitter fix, after real compaction-run proof and maintainer review; then close #82470 as fixed.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection gives a high-confidence reproduction path: current main emits a Codex contextCompaction end event without completed, while the runner requires completed === true for the success path. I did not run a live Codex app-server compaction in this read-only review.

Is this the best way to solve the issue?

Yes, the PR’s emitter-side completed: true flag is the narrowest maintainable fix. It preserves the runner’s strict incomplete-event semantics for explicit completed: false instead of broadening every missing completion flag into success.

What I checked:

Likely related people:

  • steipete: Recent main history touches the Codex app-server projector surface, and the supplied PR context shows the protocol drift sync commit in this PR was authored by this person. (role: recent area contributor; confidence: high; commits: 9b560b8a41f4, 672cab18fc24; files: extensions/codex/src/app-server/event-projector.ts, extensions/codex/src/app-server/event-projector.test.ts, scripts/check-codex-app-server-protocol.ts)
  • Ayaan Zaidi: The existing ClawSweeper review context ties the Codex app-server projector compaction handling to commit ddd79e5, and the current main checkout still shows the missing completion flag in that projector path. (role: introduced current projector behavior; confidence: medium; commits: ddd79e51ba41; files: extensions/codex/src/app-server/event-projector.ts, extensions/codex/src/app-server/event-projector.test.ts)
  • pashpashpash: The supplied related-item context shows this person authored the merged Codex native-compaction PR at Keep Codex compaction on native threads #82027, which is adjacent to the same Codex app-server compaction behavior. (role: adjacent recent compaction contributor; confidence: medium; commits: 1b87ba8ca57b, 83741482637d; files: extensions/codex/src/app-server/compact.ts, extensions/codex/src/app-server/run-attempt.context-engine.test.ts, src/commands/doctor/shared/codex-route-warnings.test.ts)

Remaining risk / open question:

  • The PR body still lacks real after-fix Codex app-server compaction-run output or logs, so the external-PR real behavior proof gate is not met.

Codex review notes: model gpt-5.5, reasoning high; reviewed against b9921e21b93d.

Re-review progress:

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: S triage: dirty-candidate Candidate: broad unrelated surfaces; may need splitting or cleanup. proof: supplied External PR includes structured after-fix real behavior proof. and removed size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 16, 2026
@steipete steipete force-pushed the codex/compaction-completed-flag branch from 08ac0b6 to 7310cf9 Compare May 16, 2026 13:42
@steipete steipete marked this pull request as ready for review May 16, 2026 13:42
@steipete steipete force-pushed the codex/compaction-completed-flag branch from 7310cf9 to 6ce08b4 Compare May 16, 2026 13:58
@steipete
Copy link
Copy Markdown
Contributor

Verification before merge:

Local proof on rebased head 6ce08b4:

  • pnpm codex-app-server:protocol:check
  • pnpm test extensions/codex/src/app-server/event-projector.test.ts
  • pnpm test src/auto-reply/reply/agent-runner-execution.test.ts -- -t compaction
  • pnpm oxfmt --check --threads=1 CHANGELOG.md scripts/check-codex-app-server-protocol.ts extensions/codex/src/app-server/event-projector.ts extensions/codex/src/app-server/event-projector.test.ts
  • git diff --check
  • /Users/steipete/Projects/agent-scripts/skills/codex-review/scripts/codex-review --mode branch

CI proof on pushed PR head 6ce08b4:

  • CI run 25963780576: success
  • Real behavior proof run 25963790982: success
  • Critical Quality run 25963780592: selected network-runtime-boundary success, others skipped

Known proof gaps: full release validation not run locally; PR CI/build gates are green.

@steipete steipete merged commit 5808386 into openclaw:main May 16, 2026
113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensions: codex proof: supplied External PR includes structured after-fix real behavior proof. scripts Repository scripts size: S triage: dirty-candidate Candidate: broad unrelated surfaces; may need splitting or cleanup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants