[codex] Fix runtime-store session checkpointing#731
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab50d59292
ℹ️ 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".
| // Inject a checkpointer for all sessions. The keep-alive attached loop | ||
| // calls it after each interaction; runtime-backed sessions keep it | ||
| // disabled so they do not bypass runtime boundary persistence. | ||
| // calls it after each interaction to keep the SessionStore projection | ||
| // current. Runtime-backed sessions still commit runtime boundary | ||
| // authority through RuntimeStore; this checkpointer writes only the | ||
| // compatibility/session snapshot projection. |
There was a problem hiding this comment.
Defer runtime-backed projection writes until commit succeeds
When this checkpointer is installed for runtime-backed sessions, AgentRunner::checkpoint_current_session runs immediately after a successful LLM/tool loop, but before start_turn_with_machine_protocol commits the runtime terminal receipt and persists the authoritative runtime snapshot. If that later runtime commit fails, the service discards the live session and returns an error, while the newly enabled checkpointer has already written the uncommitted turn into SessionStore, so compatibility consumers can observe a transcript that the runtime boundary rejected. Please keep runtime-backed projection writes behind the runtime commit path (or otherwise roll them back on commit failure) rather than writing from the pre-commit agent checkpointer.
Useful? React with 👍 / 👎.
ab50d59 to
4dbfe99
Compare
Summary
Root Cause
0.6.17 installed StoreCheckpointer for every persistent session but set
enabled: self.runtime_store.is_none(). MobKit's UnifiedRuntime always installs a runtime_store, so consumers that provide a SessionStore through MobKit stopped receiving per-turn SessionStore saves.The first CI graph for this PR also showed the same class of under-coverage at the lane detector:
meerkat-session/*changes were not triggering SDK/minimal/feature/security lanes, which is wrong for a session persistence regression visible through MobKit/SDK consumers.Validation