Support compact SessionStart hooks#21272
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87e22371bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| { | ||
| let mut state = self.state.lock().await; | ||
| state.set_pending_session_start_source(Some(codex_hooks::SessionStartSource::Compact)); |
There was a problem hiding this comment.
Preserve pending startup source when compacting pre-turn
This unconditionally overwrites any pending Startup/Resume/Clear source. run_turn runs pre-sampling compaction before run_pending_session_start_hooks, so a resumed/cleared long thread that compacts before its first turn will lose the original SessionStart hook and run only compact, skipping durable context intended for resume/startup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
interesting, because SessionStart is fired when the turn starts we can have the following edge case
resume long thread
pending = resume
first turn begins
pre-sampling compaction runs
pending = compact
before model request
pending is skipped <-- bug!
fire compact
we need to be able to queue up multiple pending so this happens instead
resume long thread
pending = [resume]
first turn begins
pre-sampling compaction runs
pending = [resume, compact]
before model request
fire resume
fire compact
87e2237 to
2bf5efd
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- anomalyco/opencode#25959 keep-alive newlines on POST /session/:id/message [merge-after-nits] - anomalyco/opencode#25855 wide-text paste-summary order fix via Intl.Segmenter [merge-after-nits] - openai/codex#21290 extract codex-file-watcher crate from core [merge-after-nits] - openai/codex#21272 add 'compact' SessionStartSource with FIFO queue [merge-after-nits]
Why
Compaction replaces the live conversation history, so hooks that use
SessionStartto re-inject durable model context need a way to run again after that rewrite.Related: #19905 adds dedicated compact lifecycle hooks
What
compactas a supportedSessionStartsource and matcher valueSessionStartstate from a single slot to a small FIFO queue soresume/startup/clearcan be preserved alongside a latercompactSessionStartsources before the next model request, preserving their original orderTesting
The new integration coverage verifies both the basic
compactmatcher path and the stackedresume->compactcase where both hooks contributeadditionalContextto the next model turn.