Skip to content

fix: keep session restore responsive during git discovery - #4945

Merged
ogulcancelik merged 1 commit into
herdrdev:masterfrom
minatoaquaMK2:issue/4926-launchd-session-restore
Oct 5, 2026
Merged

ogulcancelik merged 1 commit into
herdrdev:masterfrom
minatoaquaMK2:issue/4926-launchd-session-restore

Conversation

@minatoaquaMK2

@minatoaquaMK2 minatoaquaMK2 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Session restore reads Git metadata synchronously before the server enters its event loop. If a restored checkout's Git config or other metadata stalls, the server can accept socket connections but stop answering workspace, session, agent, and shutdown requests after logging the first spawned pane.

Restore workspace state with fallback labels and refresh optional Git metadata through the existing single detached worker, including headless servers and sidebars without Git tokens. Validate saved worktree memberships independently with up to four one-shot detached workers. Prevent worktree operations and group closes from using pending memberships, and apply results only when the workspace and saved membership still match. An invalidated membership emits the existing workspace update event.

refs #4926

Reproduction and validation

  • On macOS 27.0.1, saved three workspaces from a shell-launched server, held one checkout's .git/config read pending with a FIFO, and restored the session through an isolated launchd job. Before this change, the server stalled after the first pane spawn; a process sample located the blocking read inside restore-time Git discovery.
  • Used tuistory with the patched launchd server: all three workspaces appeared, switching workspaces and executing shell commands worked, workspace.list, session.snapshot, and agent.list responded, and server stop completed while the Git read remained blocked.
  • Added a Unix integration regression that fails before the fix and covers API readiness, shutdown, saved identity, and pending worktree operations with blocked metadata. A healthy saved membership completes independently of a blocked one. Unit tests cover background refresh without sidebar consumers, stale results, state invariants, update events, and all group-close paths before and after validation.
  • NEXTEST_TEST_THREADS=4 just ci: 3,726 Rust tests passed (12 repository skips), plus maintenance, architecture, and integration asset tests.
  • just check also reached Windows cross-lint after passing its native CI dependency. Cross-lint could not run because this host has no Windows SDK configured; the PR's native Windows CI supplies that coverage.
  • just docs-contract-test: 7 passed.

The reporter's original session file is unavailable, and an ordinary three-workspace launchd restore did not reproduce the hang. This fixes the confirmed blocking path that reproduces the reported symptoms; it does not establish that unavailable Git metadata caused the reporter's exact session to hang. No configuration, wire protocol, or persisted session format changes are needed.

Validation workers are bounded: permanently stalled reads retain their slots. The existing optional Git status worker remains serialized. This does not change synchronous discovery in worktree lookup when a workspace has no saved membership or cached metadata; that pre-existing path can still block on unavailable storage during explicit worktree operations.

@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: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c4c6ef37-621f-4c93-b025-4f78aeb19852
📥 Commits

Reviewing files that changed from the base of the PR and between e992378 and 6eef9b1.

📒 Files selected for processing (6)
  • src/app/api/panes.rs
  • src/app/api/tabs.rs
  • src/app/api/workspaces.rs
  • src/app/api/worktrees.rs
  • src/app/git_refresh.rs
  • tests/session_restore.rs

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


📝 Walkthrough

Walkthrough

Session restoration preserves saved worktree membership and defers Git metadata discovery to a background refresh. Worktree operations return worktree_operation_in_progress while the corresponding membership is pending validation. When validation rejects a still-matching membership, the app clears it, marks the session dirty, and emits a workspace update.

Suggested reviewers: ogulcancelik

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 6eef9

No concrete outstanding issue is established for this change. It appears ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6eef9

The change improves startup responsiveness while guarding operations that depend on restored worktree identity. No new security flaw was established in the reviewed paths. Residual uncertainty concerns indefinitely stalled validation and deployment access controls.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated scope is the restored workspace set within one server process and its existing filesystem and Git authority. Saved checkout paths feed metadata reads; dependent Git mutations are guarded while their saved membership is pending. Cross-tenant, network, and environment-wide exposure cannot be determined from the supplied deployment context.

Security Findings and Attack Paths

  • inferred — The explicit cwd creation route does not establish a newly introduced saved-membership trust bypass. Although it skips the pending-membership gate, it obtains repository identity and command paths from live Git discovery. Saved membership is captured for workspace association, not substituted into the Git command's source path. The route and completion logic predate this PR.

Trust Boundaries and Controls

  • observed — Pending saved membership is kept distinct from operational readiness through exact workspace-and-membership matching. Workspace-derived source resolution and removal check readiness before consuming saved paths, while asynchronous invalidation checks the same identity before altering state. These are provenance and lifecycle controls, not evidence of global caller authentication.

Resilience and Maintainability Implications

  • inferred — Moving metadata reads off the event loop improves failure containment compared with the base behavior. Containment remains bounded: four indefinitely blocked validation checks can prevent queued memberships from becoming ready. The gates continue denying dependent operations, but the implementation has no explicit timeout or worker-loss recovery transition.

Hardening Proposals

  • proposed — Consider exposing degraded validation status and defining bounded recovery for saturated or lost workers. Any recovery should keep saved-membership-dependent operations blocked until identity is established and avoid repeatedly spawning replacements for still-blocked reads.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.84% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 14 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: keeping session restore responsive during Git discovery.
Description check ✅ Passed The description explains the restore-time Git blocking issue, the changes made, and the reported validation.
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.
  • Fix all pre-merge checks with AI
  • 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.

@kangal-bot

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Defers git metadata discovery to avoid blocking session restore.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR moves restore-time Git discovery off the server event loop, validates saved worktree memberships through a bounded queue, and guards group-close operations while validation is pending.

  • The independent queue lets a healthy membership complete when another checkout’s metadata read stalls.
  • Regression coverage exercises API readiness, shutdown, membership updates, and the API group-close paths.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Restore workspaces with fallback metadata] --> B[Queue saved memberships]
    B --> C[Up to four validation workers]
    C --> D[Apply result if workspace and membership match]
    A --> E[Single detached Git status worker]
    D --> F[Release pending-operation guards]
Loading

Reviews (2) · Last reviewed commit: "fix: keep session restore responsive dur..."

Comment thread src/app/git_refresh.rs Resolved
Comment thread src/persist/restore.rs Resolved
@minatoaquaMK2
minatoaquaMK2 force-pushed the issue/4926-launchd-session-restore branch from e992378 to 6eef9b1 Compare October 5, 2026 11:10
@minatoaquaMK2

Copy link
Copy Markdown
Contributor Author

Both previous findings were addressed in 6eef9b1 and their threads have been resolved with implementation and regression-test details. The latest Linux, macOS, Windows, and ConPTY CI checks pass.

@coderabbitai review
@greptileai

Please re-review the current head, especially the independent bounded membership-validation queue and the guards covering all group-close paths.

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ogulcancelik
ogulcancelik merged commit e5443f0 into herdrdev:master Oct 5, 2026
7 checks passed
@ogulcancelik

Copy link
Copy Markdown
Collaborator

ty

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.

3 participants