Skip to content

fix(codex): retain actionable evidence from the final resume observation - #759

Open
rudycelekli wants to merge 1 commit into
mvschwarz:mainfrom
rudycelekli:fix/codex-final-resume-status-20261004
Open

rudycelekli wants to merge 1 commit into
mvschwarz:mainfrom
rudycelekli:fix/codex-final-resume-status-20261004

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

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.

  • One concern per PR; no version bump; no CHANGELOG edit
  • Tests added where testable
  • Actual checks and limits listed

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

  • Bug Fixes
    • Resume attempts now correctly recognize when a saved session is unavailable, allowing a fresh retry.
    • Late access or compatibility notices are reported as requiring attention, with recent screen output included to help explain the issue.
    • Ready prompts and unrelated output continue to follow their existing resume and fallback behavior.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f003418c-a542-4b14-84da-9481776c225c
📥 Commits

Reviewing files that changed from the base of the PR and between bd82e19 and 0af1efc.

📒 Files selected for processing (2)
  • packages/daemon/src/adapters/codex-resume.ts
  • packages/daemon/test/codex-resume-final-native.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

Codex resume final probe

Layer / File(s) Summary
Handle final native probe outcomes
packages/daemon/src/adapters/codex-resume.ts, packages/daemon/test/codex-resume-final-native.test.ts
The adapter returns retry_fresh for a missing saved session and returns attention details with the last 12 pane lines as evidence. Native-terminal tests cover late output, ready prompts, and inconclusive output.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: mvschwarz

Merge Risk: ⚪ Minimal · up to 0af1e

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 Review

Security architecture risk: ⚪ Minimal · up to 0af1e

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected change extends classification and diagnostic propagation to one additional observation of the managed resume pane. It does not introduce a new caller, credential authority, evidence format or independent cross-service exposure.

Trust Boundaries and Controls

  • observed — Raw pane evidence already crossed into restore attention results before this PR. The final branch uses that same destination, while an authentication refusal remains a non-success outcome requiring operator intervention rather than granting access or bypassing sign-in.

Resilience and Maintainability Implications

  • observed — Missing-session results use the existing stop-and-ask recovery path, which attempts to kill the launched session, supersede its row and restore prior state. Attention results intentionally retain the blocked session. Cleanup is best-effort and predates this change; neither outcome automatically launches a fresh replacement.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: preserving actionable evidence from Codex’s final resume observation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant