Skip to content

fix: preserve explicit worktree workspace membership - #4301

Merged
JJLiebig merged 5 commits into
herdrdev:masterfrom
akbash-bot:akbash/4293-preserve-worktree-membership
Sep 20, 2026
Merged

JJLiebig merged 5 commits into
herdrdev:masterfrom
akbash-bot:akbash/4293-preserve-worktree-membership

Conversation

@akbash-bot

@akbash-bot akbash-bot commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Issue

After a shell in a repository workspace enters a linked checkout, opening that worktree can turn the repository workspace into the worktree's workspace. Removing the worktree then closes the repository workspace and kills processes in its other tabs.

Problem

Checkout lookup accepted a shell's current directory even when the workspace was explicitly assigned to a different checkout. It could select the parent before the actual worktree workspace and overwrite the parent's assignment.

How did we fix it?

Herdr now matches an explicitly assigned workspace only to its assigned checkout. Workspaces without an explicit assignment retain directory-based discovery, including discovery from subdirectories.

Verification

Before the fix, an isolated CLI reproduction closed the parent and killed a second-tab process. Afterward, opening selected the real child, removing the parent was rejected, and removing the child preserved the parent's process. A regression test also failed before and passed afterward.

Local just check, Linux CI, and macOS CI passed. Both review bots completed without code findings. Windows CI failed in an unrelated remote-bridge test because its descendant PID file was missing.

refs #4293

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: fc4b06a4-d1ad-4af8-893d-3badd9f923f8

📥 Commits

Reviewing files that changed from the base of the PR and between 4c8f911 and ba335ea.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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: 70876116-69ff-4e2d-a5b0-150bd47842a6

📥 Commits

Reviewing files that changed from the base of the PR and between f77b753 and 4c8f911.

📒 Files selected for processing (1)
  • src/app/api/worktrees.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The change makes open_workspace_idx_for_checkout treat existing worktree membership as authoritative. Non-matching memberships no longer fall back to Git-space or terminal-cwd checks. A new integration test covers shell navigation, refreshed Git metadata, workspace listing, repeated open behavior, and membership preservation.

Suggested reviewers: ogulcancelik

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 4c8f9

No unresolved issue is established; the change is ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 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 describes the main change: preserving explicit workspace membership for linked worktrees.
Description check ✅ Passed The description directly explains the workspace membership bug, the fix, and its verification results.
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.

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 Sep 17, 2026

Copy link
Copy Markdown

@kangal-bot I will review pull request #4301.

✅ 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 Sep 17, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the focused lookup change preserves explicit membership while retaining fallback discovery for unassigned workspaces.

Summary

This PR prevents shell navigation and refreshed Git metadata from overriding a workspace’s explicit worktree membership.

  • Treats recorded worktree membership as authoritative during checkout-to-workspace lookup.
  • Retains directory- and Git-metadata-based discovery for workspaces without explicit membership.
  • Adds regression coverage for selecting the actual linked-worktree workspace while preserving the parent workspace and its membership.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Lookup checkout workspace] --> B{Explicit worktree membership?}
  B -->|Yes| C{Assigned checkout matches?}
  C -->|Yes| D[Select workspace]
  C -->|No| E[Skip workspace]
  B -->|No| F[Check Git metadata and identity cwd]
  F -->|Matches| D
  F -->|No match| E
Loading

Reviews (4) · Last reviewed commit: "Merge branch 'master' into akbash/4293-p..."

@akbash-bot

Copy link
Copy Markdown
Collaborator Author

Regarding CodeRabbit’s docstring-coverage warning: this changes a private lookup helper and adds a regression test, not a public API. The inline comment documents the membership-precedence rule; adding docstrings solely to meet the generated coverage percentage would not clarify this fix.

@akbash-bot
akbash-bot force-pushed the akbash/4293-preserve-worktree-membership branch from 2269665 to 2b4e44b Compare September 17, 2026 14:55
@akbash-bot

Copy link
Copy Markdown
Collaborator Author

The regression fixture now uses the checkout path returned by Git, matching the real create/open flow on macOS and Windows. Full local checks pass again. Please review the updated head.

@coderabbitai review
@greptileai

@akbash-bot
akbash-bot force-pushed the akbash/4293-preserve-worktree-membership branch from 2b4e44b to f77b753 Compare September 17, 2026 15:06
@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

@akbash-bot I will review the updated head of pull request #4301.

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

@JJLiebig

JJLiebig commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Sep 17, 2026

Copy link
Copy Markdown

@JJLiebig I will review pull request #4301.

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

@JJLiebig

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

@JJLiebig I will review pull request #4301.

⚠️ Action not completed

No files to review.

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.

@JJLiebig
JJLiebig self-requested a review September 20, 2026 14:42

@JJLiebig JJLiebig left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Validated locally on two machines, lgtm

@JJLiebig
JJLiebig merged commit 6c62707 into herdrdev:master Sep 20, 2026
7 checks passed
kaushikgopal added a commit to kaushikgopal/herdr that referenced this pull request Sep 21, 2026
Merged upstream/master b88e811..2836010. Zero conflicts: all six
overlapping files split into disjoint regions (mouse.rs navigator
scrollbar/row-click work, state.rs navigator fields and close-tab
confirmation, config/model.rs clear_pane keybinding, config-reference.json
keys.clear_pane, AGENTS.md detection policy, tests/mod.rs mod close_tab).
No src/protocol/wire.rs change, so no protocol conflict.

Notable upstream fixes absorbed:
- distinguish agent completion from startup and session changes (herdrdev#4457)
- honor droid scrollback clear requests in panes (herdrdev#4432)
- refresh forwarded ssh agents after reconnect (herdrdev#4443)
- index navigator tabs and panes per endpoint (herdrdev#4426)
- preserve session layouts across shutdown and restore failures (herdrdev#4400)
- confirm closing the last tab in the tui (herdrdev#4409)
- reject terminal-less attach before starting a session (herdrdev#4395)
- drain event subscriptions and report history loss (herdrdev#4225)
- preserve explicit worktree workspace membership (herdrdev#4301)
- keep workspace navigation visible in terminal theme (herdrdev#4408)
- preserve delayed mouse reports with confirmed keyboard input (herdrdev#4247)
- detect codex status with custom interrupt keys (herdrdev#4196)
- detect kiro status from live controls and osc signals (herdrdev#4372)
- recover the cursor during animated redraws (herdrdev#4404)
- stabilize windows cursor redraws and settle deadlines (herdrdev#4389)
- show every agent and terminal in the go to picker (herdrdev#4384)
- add configurable pane screen and scrollback clearing (herdrdev#4383)
- reveal selected agent when cycling the sidebar (herdrdev#4355)
- preserve request ids in socket error responses (herdrdev#4353)
- enable ssh compression for remote connections (herdrdev#4340)
- correct grok activity detection with custom or disabled osc signals (herdrdev#4337)
- preserve windows mouse capture during refresh (herdrdev#4319)
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.

4 participants