fix(codex): retain actionable evidence from the final resume observation - #759
rudycelekli wants to merge 1 commit into
Conversation
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Codex resume adapter now handles missing-session and attention-required results from its final native probe. New native-terminal tests cover these outcomes, ready prompts, and inconclusive output. ChangesCodex resume final probe
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The final resume observation now handles the specified outcomes, with native-terminal tests covering those results and the existing fallbacks. No merge-blocking issue is identified. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The change applies established recovery behavior to the final resume check. It does not add permissions, bypass sign-in, automatically start a fresh session, or introduce a new destination for diagnostic output. No material security risk was identified in the inspected change. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What a user gets
A Codex sign-in or incompatible-client notice observed at the final resume check is retained as recoverable attention, with the same evidence as an in-loop observation. A late missing-session notice retains its explicit fresh-retry reason. These outcomes reach the existing restore status mapping rather than falling through to a shell or timeout explanation.
How you verified it
Private native tmux server with isolated HOME/socket and an owned executable receiving the actual production resume command. A filesystem barrier releases controlled provider-shaped output on the final 27th capture after the default 26 loop observations. All terminal get/capture/launch calls are real; injected no-op sleep tests the bounded classification branch, not wall-clock timeout duration. No authenticated Codex account, model call, or external provider is used.
Baseline: three failures, two controls pass (late ready prompt and unchanged generic shell fallback). After: all five native cases and 127 focused native/adapter/probe tests pass. Full local build and lint pass, and all eight unchanged fork Tests jobs passed at the signed head linked below.
Reachability: startup constructs CodexResumeAdapter and passes it to RestoreOrchestrator; restore calls .resume and already maps retry_fresh/attention_required. Existing stub scenarios do not represent native Codex resume output or its final observation. The bounded native fixture instead exercises the production adapter and terminal command at that exact boundary.
Anything you were unsure about
This is deterministic controlled process output, not live Codex validation or conversation-continuity proof. Native tmux sees the staging /bin/sh wrapper in this fixture; explicit provider evidence must take precedence, while unrelated output retains the existing shell fallback. No process-identity classifier, launch command, retry count, or helper abstraction changes.
Exact-head verification
All eight unchanged fork Tests jobs passed at
0af1efc716093b834255c08fdcb5c066adb905f8: workflow evidence. Upstream PR checks are observed separately after submission.Prepared with AI assistance under human direction. The source patch, native regression and workflow receipt were reviewed before submission.
Summary by CodeRabbit