Repository navigation
Persist Codex title session resumes - #3500
austinywang wants to merge 2 commits into
Conversation
Codex exposes its resumable session slug in the terminal title, but the persisted workspace snapshot currently only restores hook-recorded agent sessions. This regression coverage exercises the workspace snapshot path with an empty hook index and extends the relaunch harness with a title-only Codex surface. Constraint: CI should show this commit red before the fix lands.\nRejected: Grep for title parsing code | source-text tests would not prove workspace persistence behavior.\nConfidence: high\nScope-risk: narrow\nTested: Not run locally; this is the intentionally failing regression commit.\nNot-tested: Local xcodebuild and UI/E2E per repository policy.
Workspace snapshots now treat the Codex title slug as an agent resume source when hook state is missing. The title-derived snapshot is stored in the existing workspace session JSON path, restored through the existing agent startup input path, and cleared when the resumed or live title-derived command returns to a prompt. Constraint: Agent resume must remain owned by SessionPersistence/Workspace state rather than adding a parallel store.\nConstraint: Codex CLI resume command keeps existing cmux command-builder shape: codex resume --dangerously-bypass-approvals-and-sandbox <session-id>.\nRejected: UserDefaults keyed by workspace/surface | duplicates the durable workspace snapshot owner and complicates cleanup.\nRejected: Persist every Codex-looking title forever | normal exits and stale resume failures would loop.\nConfidence: high\nScope-risk: moderate\nDirective: Keep title-derived Codex resume state tied to shell activity; promptIdle is the cleanup signal for stale or completed title sessions.\nTested: git diff --check; python3 -m py_compile tests/test_session_relaunch_resumes_agent_sessions.py; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj; python3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv\nNot-tested: Local xcodebuild/unit/UI tests per task and repository constraints; final tagged reload still required.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds Codex session persistence and restoration from window title slugs. A new ChangesCodex Title-Based Session Persistence and Restoration
Sequence DiagramsequenceDiagram
participant Panel as Panel
participant Workspace as Workspace
participant Parser as CodexSessionTitleParser
participant Snapshot as Session Snapshot
Panel->>Workspace: updatePanelShellActivityState(state: .commandRunning)
Panel->>Panel: title = "codex-019df0a1-6"
Workspace->>Parser: sessionId(from: title)
Parser-->>Workspace: "019df0a1"
Workspace->>Parser: snapshot(sessionId: "019df0a1", workingDirectory: "...")
Parser-->>Workspace: SessionRestorableAgentSnapshot(.codex, launchCommand, source: "surface-title")
Workspace->>Workspace: restoredAgentAutoResumeRunningPanelIds.insert(panelId)
Workspace->>Snapshot: Store restored agent snapshot
Note over Workspace: ... later, on shell .promptIdle ...
Panel->>Workspace: updatePanelShellActivityState(state: .promptIdle)
Workspace->>Workspace: restoredAgentAutoResumeRunningPanelIds.remove(panelId)
Workspace->>Snapshot: Clear restored agent snapshot
Note over Workspace: ... on restart ...
Workspace->>Snapshot: Restore snapshot with codex agent
Workspace->>Workspace: Launch with codex --resume 019df0a1
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly Related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (9 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Greptile SummaryThis PR closes the regression from #7f4d8c38 by persisting Codex session title slugs (extracted from OSC terminal title escape sequences) as restorable
Confidence Score: 3/5Not safe to merge without resolving the nonisolated actor isolation gap on CodexSessionTitleParser and reviewing the snapshot-path side-effects. One P1 (missing nonisolated on CodexSessionTitleParser statics in a @MainActor-by-default module) caps the score at 4; the P2 architectural concern about non-idempotent snapshot reads expanding existing debt pulls it to 3. Sources/CodexSessionTitleParser.swift (actor isolation) and the restorableAgent == nil block in Sources/Workspace.swift (~line 378). Important Files Changed
Sequence DiagramsequenceDiagram
participant T as Terminal OSC Title
participant W as Workspace
participant P as CodexSessionTitleParser
participant S as restoredAgentSnapshotsByPanelId
T->>W: updatePanelTitle(panelId, "codex-019df0a1-6")
W->>P: restorableSnapshot(fromTitle:, workingDirectory:)
P-->>W: SessionRestorableAgentSnapshot(kind:.codex, source:"surface-title")
W->>S: [panelId] = snapshot (via sessionPanelSnapshot side-effect)
Note over W: sessionSnapshot() called (e.g. on app suspend)
W->>P: isSurfaceTitleSnapshot(currentSnapshot)?
P-->>W: true → keep snapshot in output
Note over W: shellState → .commandRunning
W->>P: snapshot(restoredAgent, matchesTitle:)?
P-->>W: true → insert panelId into AutoResumeRunning
Note over W: shellState → .promptIdle (codex exited)
W->>S: removeValue(panelId) — clear surface-title snapshot
W-->>T: resume loop prevented
Reviews (1): Last reviewed commit: "Persist Codex title sessions through wor..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| enum CodexSessionTitleParser { | ||
| static let launchSource = "surface-title" | ||
|
|
||
| private static let sessionSlugRegex = try! NSRegularExpression( | ||
| pattern: #"(?i)(?:^|[^A-Z0-9_-])(codex-[0-9a-f]{8,}(?:-[A-Z0-9]+)+)(?=$|[^A-Z0-9_-])"# |
There was a problem hiding this comment.
nonisolated on pure static helpers
CodexSessionTitleParser is a pure string-parsing utility with no actor-bound state. In a module with @MainActor-by-default isolation, all static members — launchSource, sessionSlugRegex, sessionId(from:), restorableSnapshot(fromTitle:workingDirectory:), isSurfaceTitleSnapshot(_:), snapshot(_:matchesTitle:), and normalized(_:) — will be implicitly @MainActor. That unnecessarily couples calls to them through the main actor, prevents use from background contexts, and violates the actor-isolation rule that pure helpers and file-scoped constants should be explicitly nonisolated. Marking each static member nonisolated (and the enum itself nonisolated to make the annotation structural rather than per-member) is the fix.
File Used: .github/review-bot-rules/swift-actor-isolation.md (source)
| private static let sessionSlugRegex = try! NSRegularExpression( | ||
| pattern: #"(?i)(?:^|[^A-Z0-9_-])(codex-[0-9a-f]{8,}(?:-[A-Z0-9]+)+)(?=$|[^A-Z0-9_-])"# | ||
| ) |
There was a problem hiding this comment.
try! on static regex initialization
The regex is a compile-time-constant literal so this will never trap in practice, but try! on a static let is fragile — any future edit to the pattern string (typo, feature addition) silently becomes a launch crash instead of a compile or test failure. Swift's native Regex literal (#/pattern/) validates the pattern at compile time and eliminates the forced try entirely; if that spelling isn't available on the OS minimum target, a static var with a do/catch and preconditionFailure gives a clear crash site and message.
| if restorableAgent == nil { | ||
| let currentSnapshot = restoredAgentSnapshotsByPanelId[panelId] | ||
| if shellActivityState == .promptIdle, | ||
| let currentSnapshot, | ||
| CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) { | ||
| restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId) | ||
| restoredAgentAutoResumePendingPanelIds.remove(panelId) | ||
| restoredAgentAutoResumeRunningPanelIds.remove(panelId) | ||
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | ||
| } else if shellActivityState != .promptIdle, | ||
| let titleRestorableAgent = CodexSessionTitleParser.restorableSnapshot( | ||
| fromTitle: panelTitles[panelId] ?? panel.displayTitle, | ||
| workingDirectory: panelDirectories[panelId] | ||
| ) { | ||
| restoredAgentSnapshotsByPanelId[panelId] = titleRestorableAgent | ||
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | ||
| } | ||
| } |
There was a problem hiding this comment.
The new restorableAgent == nil block mutates restoredAgentSnapshotsByPanelId, restoredAgentAutoResumePendingPanelIds, restoredAgentAutoResumeRunningPanelIds, and invalidatedRestoredAgentFingerprintsByPanelId inside what is otherwise a read-path snapshot computation. This extends an already present (but limited) mutation in the restorableAgent != nil branch. The practical consequence is that sessionSnapshot() is not idempotent — the first call when shellActivityState == .promptIdle clears the surface-title snapshot, and a second call then observes different state — and the source of truth for "does this panel have a title-derived codex snapshot?" is now spread across three different call sites (updatePanelTitle, updatePanelShellActivityState, and sessionPanelSnapshot). These transitions belong in the updatePanelTitle/updatePanelShellActivityState handlers so the snapshot function is a pure reader of already-correct state.
File Used: .github/review-bot-rules/swift-architectural-rethink.md (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Workspace.swift`:
- Around line 378-395: When restorableAgent is nil, add handling so a stored
surface-title snapshot is cleared if the live title no longer parses as a Codex
restorable snapshot even when the hook state is .unknown: inside the existing
block that checks restoredAgentSnapshotsByPanelId[panelId], keep the current
branch that clears on shellActivityState == .promptIdle, and in the else-if
branch (the shellActivityState != .promptIdle branch that currently tries
CodexSessionTitleParser.restorableSnapshot(...)), add logic so that if the
parser returns nil AND the existing currentSnapshot is a surface-title snapshot
(use CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot)), you
remove restoredAgentSnapshotsByPanelId[panelId],
restoredAgentAutoResumePendingPanelIds.remove(panelId),
restoredAgentAutoResumeRunningPanelIds.remove(panelId), and
invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId); this
ensures stale title-derived snapshots are cleared when the live title stops
matching even if panelShellActivityStates[panelId] remains .unknown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 406dfdd7-e197-4081-9c88-203706c7aff2
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (5)
GhosttyTabs.xcodeproj/project.pbxprojSources/CodexSessionTitleParser.swiftSources/Workspace.swiftcmuxTests/RestorableCodexTitleTests.swifttests/test_session_relaunch_resumes_agent_sessions.py
| if restorableAgent == nil { | ||
| let currentSnapshot = restoredAgentSnapshotsByPanelId[panelId] | ||
| if shellActivityState == .promptIdle, | ||
| let currentSnapshot, | ||
| CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) { | ||
| restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId) | ||
| restoredAgentAutoResumePendingPanelIds.remove(panelId) | ||
| restoredAgentAutoResumeRunningPanelIds.remove(panelId) | ||
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | ||
| } else if shellActivityState != .promptIdle, | ||
| let titleRestorableAgent = CodexSessionTitleParser.restorableSnapshot( | ||
| fromTitle: panelTitles[panelId] ?? panel.displayTitle, | ||
| workingDirectory: panelDirectories[panelId] | ||
| ) { | ||
| restoredAgentSnapshotsByPanelId[panelId] = titleRestorableAgent | ||
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | ||
| } | ||
| } |
There was a problem hiding this comment.
Clear stale title-derived snapshots when the live title stops matching.
If hook state never comes back after restore, panelShellActivityStates[panelId] can stay .unknown. In that case this block only clears surface-title snapshots on .promptIdle, so once the panel title stops parsing as a Codex slug the old snapshot is kept and re-persisted on the next workspace save. That recreates the stale auto-resume loop this PR is trying to eliminate.
Suggested fix
if restorableAgent == nil {
let currentSnapshot = restoredAgentSnapshotsByPanelId[panelId]
if shellActivityState == .promptIdle,
let currentSnapshot,
CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) {
restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId)
restoredAgentAutoResumePendingPanelIds.remove(panelId)
restoredAgentAutoResumeRunningPanelIds.remove(panelId)
invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId)
} else if shellActivityState != .promptIdle,
let titleRestorableAgent = CodexSessionTitleParser.restorableSnapshot(
fromTitle: panelTitles[panelId] ?? panel.displayTitle,
workingDirectory: panelDirectories[panelId]
) {
restoredAgentSnapshotsByPanelId[panelId] = titleRestorableAgent
invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId)
+ } else if let currentSnapshot,
+ CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) {
+ restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId)
+ restoredAgentAutoResumePendingPanelIds.remove(panelId)
+ restoredAgentAutoResumeRunningPanelIds.remove(panelId)
+ invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if restorableAgent == nil { | |
| let currentSnapshot = restoredAgentSnapshotsByPanelId[panelId] | |
| if shellActivityState == .promptIdle, | |
| let currentSnapshot, | |
| CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) { | |
| restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId) | |
| restoredAgentAutoResumePendingPanelIds.remove(panelId) | |
| restoredAgentAutoResumeRunningPanelIds.remove(panelId) | |
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | |
| } else if shellActivityState != .promptIdle, | |
| let titleRestorableAgent = CodexSessionTitleParser.restorableSnapshot( | |
| fromTitle: panelTitles[panelId] ?? panel.displayTitle, | |
| workingDirectory: panelDirectories[panelId] | |
| ) { | |
| restoredAgentSnapshotsByPanelId[panelId] = titleRestorableAgent | |
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | |
| } | |
| } | |
| if restorableAgent == nil { | |
| let currentSnapshot = restoredAgentSnapshotsByPanelId[panelId] | |
| if shellActivityState == .promptIdle, | |
| let currentSnapshot, | |
| CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) { | |
| restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId) | |
| restoredAgentAutoResumePendingPanelIds.remove(panelId) | |
| restoredAgentAutoResumeRunningPanelIds.remove(panelId) | |
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | |
| } else if shellActivityState != .promptIdle, | |
| let titleRestorableAgent = CodexSessionTitleParser.restorableSnapshot( | |
| fromTitle: panelTitles[panelId] ?? panel.displayTitle, | |
| workingDirectory: panelDirectories[panelId] | |
| ) { | |
| restoredAgentSnapshotsByPanelId[panelId] = titleRestorableAgent | |
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | |
| } else if let currentSnapshot, | |
| CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot) { | |
| restoredAgentSnapshotsByPanelId.removeValue(forKey: panelId) | |
| restoredAgentAutoResumePendingPanelIds.remove(panelId) | |
| restoredAgentAutoResumeRunningPanelIds.remove(panelId) | |
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 378 - 395, When restorableAgent is nil,
add handling so a stored surface-title snapshot is cleared if the live title no
longer parses as a Codex restorable snapshot even when the hook state is
.unknown: inside the existing block that checks
restoredAgentSnapshotsByPanelId[panelId], keep the current branch that clears on
shellActivityState == .promptIdle, and in the else-if branch (the
shellActivityState != .promptIdle branch that currently tries
CodexSessionTitleParser.restorableSnapshot(...)), add logic so that if the
parser returns nil AND the existing currentSnapshot is a surface-title snapshot
(use CodexSessionTitleParser.isSurfaceTitleSnapshot(currentSnapshot)), you
remove restoredAgentSnapshotsByPanelId[panelId],
restoredAgentAutoResumePendingPanelIds.remove(panelId),
restoredAgentAutoResumeRunningPanelIds.remove(panelId), and
invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId); this
ensures stale title-derived snapshots are cleared when the live title stops
matching even if panelShellActivityStates[panelId] remains .unknown.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2b46dc8. Configure here.
| ) { | ||
| restoredAgentSnapshotsByPanelId[panelId] = titleRestorableAgent | ||
| invalidatedRestoredAgentFingerprintsByPanelId.removeValue(forKey: panelId) | ||
| } |
There was a problem hiding this comment.
Title snapshot overwrites richer hook-based restored snapshots
Medium Severity
When restorableAgent is nil (hook state missing) and the shell isn't at prompt idle, the else if branch unconditionally overwrites restoredAgentSnapshotsByPanelId[panelId] with a title-based snapshot — even when there's already a richer hook-based snapshot from a session restore. This replaces detailed launch command data (actual executable path, real arguments, environment) with hardcoded values ("codex" path and --dangerously-bypass-approvals-and-sandbox). A guard checking that no existing non-title snapshot is present would prevent this data loss.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2b46dc8. Configure here.
This branch was successfully deployed
1 participant
Summary
Regression
Verification
Fixes #3499
Note
Medium Risk
Changes session snapshot/restore behavior in
Workspaceby inferring/restoring Codex agent sessions from terminal titles and by tracking auto-resume state transitions, which could affect when agent resumes are persisted or cleared across relaunches.Overview
Adds support for persisting/restoring Codex sessions when the normal hook-based state is unavailable by parsing
codex-...session slugs from a terminal’s surface title and converting them intoSessionRestorableAgentSnapshots (newCodexSessionTitleParser).Updates
Workspacesnapshotting and shell-activity handling to only keep these title-derived Codex resumes while a command is running, clearing them when the shell returns topromptIdle, and introducesrestoredAgentAutoResumeRunningPanelIdsto avoid looping/stale auto-resumes. Adds unit/integration coverage (RestorableCodexTitleTests, updated relaunch test) and wires new sources into the Xcode project + Swift file budget.Reviewed by Cursor Bugbot for commit 2b46dc8. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Restore Codex sessions by parsing the session slug from the terminal title and persisting it as a restorable agent snapshot when hook state is missing. Resumes use the existing workspace path and clear on prompt return to prevent loops.
codex resume --dangerously-bypass-approvals-and-sandbox <session-id>using the stored snapshot; invalidate if the title changes.promptIdleto avoid looping on normal exits or stale sessions.Written for commit 2b46dc8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests