Skip to content

Preserve Pi resume identity across repeated restores - #9399

Merged
austinywang merged 2 commits into
mainfrom
issue-9380-pi-restore-nothing-to-restore
Aug 2, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-9380-pi-restore-nothing-to-restore

Conversation

@austinywang

@austinywang austinywang commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Preserve Pi managed-session identity when hooks publish UUIDs and process discovery resolves timestamp-prefixed JSONL paths.
  • Match registry-owned agent kinds by raw value, covering .pi and .custom("pi").
  • Use shared identity matching in liveness reconciliation, session restore, lifecycle cleanup, Dock and workspace snapshots, and restorable-session indexing.

Confirmed cause

This is a third cause, not the fresh-discovery race or stale CMUX_SURFACE_ID.

After a Pi restore, the Pi hook publishes a UUID while PiSessionLocator resolves that same running session as its timestamp-prefixed JSONL path. Hook parsing also represents the kind as .pi, while registry scanning produces .custom("pi"). Exact ID and enum comparisons caused reconciliation to treat the live restored Pi process as a different session and discard its binding.

Tagged polling observed the binding disappear after about 7 seconds while Pi remained alive, matching the reconciliation cadence. Claude, Codex, and Grok retain stable native session IDs and did not show this conversion.

Regression coverage

  • The test-only commit proves reconciliation deleted a live Pi binding after UUID-to-path resolution.
  • It also proves session restore rejected the same Pi session across UUID and JSONL path representations.
  • Both new tests failed at the test-only commit and passed after the fix.

Validation

  • The focused cmux-unit suite passed 5 tests with 0 failures on the final PR HEAD.
  • The tagged Debug build succeeded and its isolated socket returned the selected workspace.
  • No local XCUITests were run.

Closes #9380

Summary by CodeRabbit

  • Bug Fixes
    • Improved agent session restoration and resume behavior when session identifiers appear in different formats.
    • Better recognizes sessions represented by detected file paths, including Pi and OMP sessions.
    • Prevents valid resume bindings from being discarded unnecessarily.
    • Adds safeguards to avoid applying restored sessions or bindings to the wrong agent type or session.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 97874e8d-2312-4a0d-8745-18c54e0d6a46

📥 Commits

Reviewing files that changed from the base of the PR and between 3818ea3 and fb08b7c.

📒 Files selected for processing (7)
  • Sources/AgentResumeLiveness.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/RestorableAgentSession.swift
  • Sources/SurfaceResumeBindingSnapshot+ManagedSessionIdentity.swift
  • Sources/Workspace+AgentLifecycle.swift
  • Sources/Workspace.swift
  • cmuxTests/WorkspaceIsStaleAgentHookBindingTests.swift

📝 Walkthrough

Walkthrough

The change adds canonical agent-session identity matching for raw UUIDs and .jsonl session paths. Restore, lifecycle, liveness, and snapshot logic now use this matching. Pi restore and reconciliation tests cover UUID-to-path resolution.

Changes

Agent session identity

Layer / File(s) Summary
Managed session identity utilities
Sources/SurfaceResumeBindingSnapshot+ManagedSessionIdentity.swift
Adds canonicalization for agent kinds and session IDs. Raw UUIDs and Pi or OMP session paths can now match.
Runtime binding and liveness matching
Sources/AgentResumeLiveness.swift, Sources/DockSplitStore+SessionSnapshot.swift, Sources/Workspace+AgentLifecycle.swift, Sources/Workspace.swift
Runtime session checks use normalized agent kinds and ManagedAgentSessionIdentity.sessionIDsMatch(...).
Restore candidate matching and Pi validation
Sources/RestorableAgentSession.swift, cmuxTests/WorkspaceIsStaleAgentHookBindingTests.swift
Restore matching canonicalizes session IDs. Pi fixtures and tests cover scanner reconciliation and session restore from UUID-based bindings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • manaflow-ai/cmux#8619: Modifies agent-resume liveness and restore-reconciliation paths.
  • manaflow-ai/cmux#8321: Modifies session identity canonicalization in RestorableAgentSession.swift and Workspace.swift.
  • manaflow-ai/cmux#9397: Modifies session restore and liveness logic involving agent-kind and session-ID matching.

Suggested reviewers: lawrencecchen, azooz2003-bit


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Swift Actor Isolation ❌ Error New ManagedAgentSessionIdentity is a pure top-level helper without nonisolated, yet it is called by Workspace nonisolated restore APIs at lines 990 and 1029. Mark the helper enum and its static members nonisolated, then verify the nonisolated restore call sites compile under Swift 6 concurrency checking.
Cmux Swift Package Boundaries ❌ Error Foundation-only ManagedAgentSessionIdentity logic remains in app Sources and is reused by liveness, restore, Dock, and lifecycle code; no package target isolates it. Extract the enum into a small Packages/macOS/CmuxManagedSessionIdentity target with public ManagedAgentSessionIdentity, then keep the SurfaceResumeBindingSnapshot extension as app glue.
Cmux No Ambient Global State ❌ Error Sources/SurfaceResumeBindingSnapshot+ManagedSessionIdentity.swift:5 adds a caseless enum whose API is static helpers, matching the rule's prohibited namespace pattern; callers use it across product... Replace the static namespace with a constructable, injectable identity matcher, or keep narrowly scoped helpers private/fileprivate and place ownership at the session-index or workspace seam.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (21 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: preserving Pi session identity across repeated restores.
Description check ✅ Passed The description explains the cause, implemented changes, regression coverage, and validation; the missing checklist and demo video are non-critical.
Linked Issues check ✅ Passed The changes address issue #9380 by matching Pi UUIDs with JSONL paths and normalizing .pi and .custom("pi") agent kinds.
Out of Scope Changes check ✅ Passed The implementation and regression tests remain focused on Pi session identity, restoration, reconciliation, and related lifecycle handling.
Cmux Swift Blocking Runtime ✅ Passed The production diff adds only synchronous string/UUID identity matching and guard logic; it introduces no semaphores, waits, sleeps, delayed dispatch, polling, sync queues, timers, or locks.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only Pi session identity and related tests; no browser, WebKit, socket-worker, processV2Command, or policy code changed, so it does not introduce or worsen browser automation debt.
Cmux Expensive Synchronous Load ✅ Passed The production diff adds only UUID/string identity matching; it adds no synchronous loader, I/O, JSON parsing, directory scan, or main-actor history load. The sole new load call is test-only.
Cmux Cache Substitution Correctness ✅ Passed The production diff only adds canonical session-ID matching and raw-kind comparisons; it does not replace any fresh authoritative read with a cached value in persistence or snapshot paths.
Cmux No Hacky Sleeps ✅ Passed The patch changes only six Swift source files; it introduces no TypeScript, JavaScript, shell, or build/runtime script delays covered by this check.
Cmux Algorithmic Complexity ✅ Passed The production diff adds linear string/UUID normalization and a fixed two-kind Set lookup; matchingHookEntry scans only three candidates. No scalable nested scan or repeated collection sort/filter...
Cmux Swift Concurrency ✅ Passed The diff adds only synchronous session-identity matching; it adds no DispatchQueue, Task, Combine, completion-handler, async, or await usage, and existing marker counts are unchanged.
Cmux Swift @Concurrent ✅ Passed The diff adds only synchronous, pure identity helpers and comparisons; it changes no async isolation or @concurrent annotations, and the existing heavy loader retains Task.detached.
Cmux Swiftpm Lockfiles ✅ Passed The PR changes only Swift source and test files; it changes no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project package-reference files.
Cmux Swift Logging ✅ Passed The production diff adds identity matching and normalization only; it adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data diagnostics.
Cmux User-Facing Error Privacy ✅ Passed The production diff only adds internal session-identity normalization and comparisons; it adds no user-facing errors, alerts, command output, or recovery text.
Cmux Full Internationalization ✅ Passed The PR changes only session-identity logic in six Swift files; it adds no user-facing text, localization keys, catalogs, web messages, metadata, or locale data.
Cmux Swiftui State Layout ✅ Passed The diff adds session-identity logic only. It adds no new ObservableObject/@published, @Observable, GeometryReader, lazy/list row store reference, or render-time state mutation.
Cmux Architecture Rethink ✅ Passed The patch is a small correctness fix with a pure shared identity matcher and canonical SessionKey; it adds no timing, blocking, observer, mutable cache, side channel, or lifecycle owner.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff only changes managed-session identity logic; it adds no window or close-shortcut code. scripts/lint_auxiliary_window_close_shortcuts.py also passes.
Cmux Source Artifacts ✅ Passed All seven changed paths are hand-written Swift source or regression tests; no logs, caches, build output, scratch directories, or copied artifacts enter the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The production diff adds only runtime session-identity matching and callers; it adds no DEBUG/test guard, test-named member, accessor seam, or visibility widening.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9380-pi-restore-nothing-to-restore

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/RestorableAgentSession.swift`:
- Around line 928-933: Normalize agent kinds using their canonical raw value
throughout SessionKey, PanelKindKey, PanelIDKindKey, and matchingHookEntry so
.pi and .custom("pi") compare identically; remove the pre-check in the matching
path that rejects equivalent kinds before sessionIDsMatch. Add a regression
covering a .pi hook entry matching a .custom("pi") detected snapshot for the
same UUID/path session.
🪄 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 Plus

Run ID: 00a1a8bc-ab56-417c-81ea-26f1a2131144

📥 Commits

Reviewing files that changed from the base of the PR and between 175127e and 3818ea3.

📒 Files selected for processing (7)
  • Sources/AgentResumeLiveness.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/RestorableAgentSession.swift
  • Sources/SurfaceResumeBindingSnapshot+ManagedSessionIdentity.swift
  • Sources/Workspace+AgentLifecycle.swift
  • Sources/Workspace.swift
  • cmuxTests/WorkspaceIsStaleAgentHookBindingTests.swift

Comment thread Sources/RestorableAgentSession.swift
@austinywang
austinywang force-pushed the issue-9380-pi-restore-nothing-to-restore branch from 3818ea3 to fb08b7c Compare August 2, 2026 21:46
@austinywang
austinywang merged commit e7ca40e into main Aug 2, 2026
6 checks passed
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.

Pi restore says 'nothing to restore' for a real, current session

1 participant