Repository navigation
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
🚧 Files skipped from review as they are similar to previous changes (19)
📝 WalkthroughWalkthroughAdds an optional 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/renderer/src/components/sidebar/WorktreeVisibilityDialog.agent-worktrees.test.tsx (1)
192-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the layout-test rationale.
The 12-line comment walks through implementation details already conveyed by the test. Keep a one-line note that happy-dom cannot validate geometry and E2E covers it.
Suggested change
- // Why: happy-dom computes no real layout — getBoundingClientRect and - // scrollWidth/clientWidth all report 0 (verified empirically) — so this - // cannot assert actual truncation or that the row stays inside the - // dialog. It can only guard the specific utility classes a real browser - // needs for that to happen, which is exactly what shipped broken - // silently before: `flex-1` on the label defeats truncate (flex-basis:0 - // forces it to claim all row slack instead of shrinking), and the two - // ancestor `div`s between the label and DialogContent's grid need - // min-w-0 or their intrinsic content width — the full untruncated path — - // inflates the shared grid column and drags the sibling non-Orca row's - // width along with it. The e2e harness is the only real guard for the - // geometry itself; see tests/e2e/zz-agent-scratch-visibility-*.spec.ts. + // happy-dom cannot validate layout geometry; E2E covers actual truncation.As per coding guidelines, “Comments must be concise, limited to non-obvious information, and preferably one line; do not explain obvious code or walk through implementation details.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 93c3e4c1-02bf-4679-8707-1ed9bfee572e
📥 Commits
Reviewing files that changed from the base of the PR and between 505967e and 2fbebce722d8cb5938d2fd93bccf2c81f166f69b.
📒 Files selected for processing (21)
docs/agent-scratch-worktree-visibility.mdsrc/main/ipc/repos.tssrc/main/persistence.test.tssrc/main/persistence.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/repo-update-schema.tssrc/preload/api-types.tssrc/renderer/src/components/sidebar/WorktreeVisibilityDialog.agent-worktrees.test.tsxsrc/renderer/src/components/sidebar/WorktreeVisibilityDialog.tsxsrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/store/slices/repos.tssrc/shared/external-worktree-inbox.test.tssrc/shared/external-worktree-inbox.tssrc/shared/types.tssrc/shared/worktree-accounting-invariant.test.tssrc/shared/worktree-ownership.test.tssrc/shared/worktree-ownership.ts
Contributor
|
thanks for the PR. will give it some thought. potentially also just use fold it under "externalWorktreeVisibility" |
4 of 5 tasks
5 tasks done
Gldywn
added a commit
to Gldywn/orca
that referenced
this pull request
Aug 6, 2026
Keeping a discovered worktree hidden was a one-way door for agent scratch. The inbox baseline suppressed the notification, the repo-wide toggle can never reveal scratch by design (stablyai#9388), and the dialog reported nothing was there, so the worktree became unreachable from the UI. The visibility dialog now owns both kinds of non-Orca worktrees, each with the same anatomy: a repo switch that also covers worktrees created later, and a collapsible per-worktree list to show or hide one at a time. Agent worktrees answer to their own opt-in setting, so the non-Orca toggle still never reveals them; a shared test pins that guarantee. Hiding a whole kind drops that kind's explicit exceptions, otherwise an exception would outrank the switch that was just set, and both switches roll back if the refresh that follows them fails. Both switches also need an authoritative listing, because hiding derives the purge and the decision ledger from it. On a host Orca cannot reach, the dialog says so and offers a retry instead of purging against an empty list. Worktrees a legacy repo shows whatever the setting are left out of a kind's counts, since no switch on this screen can move them. A row counts as an exception when the visibility rule without imports would hide it, not when its path sits in the recorded list, because that list can lag behind reality. Row-level actions reuse the inbox import path so behavior and rollback stay identical, and the recovery list ignores the inbox baseline on purpose: the baseline suppresses a notification, it must not block recovery. The sidebar notices now speak the same vocabulary as the dialog and link straight into it instead of naming a menu. docs/non-orca-worktree-visibility.md writes down the model, the stablyai#9388 guarantee, and the exception rules. Supersedes stablyai#11730 and consolidates the overlapping work in stablyai#10560 (the agent visibility setting, whose field name and rollback shape are kept) and
Agent scratch worktrees (.claude/worktrees, .gsd-workspaces) have been hidden unconditionally since stablyai#9535. The suppression reaches every surface that could report it — sidebar, discovery card, inbox, and the Non-Orca worktrees dialog counts — so a worktree created by `claude -w` vanishes with no way to see that it exists or to get it back. Setting the repo to show non-Orca worktrees has no effect, because shouldShowWorktree returns on the agent-scratch branch before it reaches that check, and the only field that can override the gate (importedExternalWorktreePaths) is writable solely through an inbox that also excludes scratch. Add a per-repo agentWorktreeVisibility opt-in, default hide, so stablyai#9388's fan-out protection stays the default. Read it from the single scratch branch in shouldShowWorktree, and give the visibility dialog a second row that counts scratch worktrees and toggles the field. The row names the containing directory, or a location count when the worktrees span more than one, since that path is the diagnostic payload. Once a repo has opted in the row stays visible even at zero, so the opt-in is always reversible. isUserFacingExternalWorktree is deliberately untouched, so the discovery card and the new-worktree inbox keep excluding scratch; two sibling accessors serve the dialog instead. Adds an accounting invariant test: every worktree git reports is either unconditionally visible (selected checkout, explicit import, or orca-managed) or counted in exactly one accessor bucket. It catches a worktree being dropped from every bucket — the failure mode this fixes — and is what should stop a future path-list change from reintroducing it. The new row is localized in all five catalogs, so it reads in the same language as the non-Orca row directly above it. Verified against the running app with a real `.claude/worktrees/` worktree: hidden by default with no sidebar row, named in the dialog, revealed on Show without closing it, and persisted. That run is also what caught the path overflowing the dialog — DialogContent is a single-column grid, so the row needed min-w-0 for the path to truncate instead of inflating the shared column. Refs stablyai#9388, stablyai#9535
jeongph
force-pushed
the
fix/agent-scratch-worktree-visibility
branch
from
August 6, 2026 12:04
2fbebce to
ffcc55c
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
5 tasks done
Contributor
|
Thanks for this work — the agent-worktree visibility policy and recovery path landed in #13652, so this PR is now superseded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since #9535 (fixing #9388), worktrees under
<repo>/.claude/worktrees/**and<repo>/.gsd-workspaces/**are classifiedagent-scratchand suppressed unconditionally. That stops sub-agent fan-out from flooding the sidebar, which is the right default — but it left no way out and no trace.A worktree created by Claude Code's
claude -wlands in<repo>/.claude/worktrees/<name>. When a user starts parallel work there and wants to continue it in Orca, the worktree is simply gone:shouldShowWorktreereturns on theagent-scratchbranch before it reaches that checkNon-Orca worktreesdialog reports0 worktrees currently shownwhile the worktree sits on disk, because every surface funnels throughisUserFacingExternalWorktree, which excludes scratchimportedExternalWorktreePathsis the only field that overrides the gate, and its only writer is the new-worktree inbox — which also excludes scratchSo there is no in-app path to see that the worktree exists, let alone recover it. Orca even mints
worktreeMetafor these paths (withworkspaceStatus: "in-progress") while the UI reports0.Reported after upgrading to 1.4.155 from a pre-1.4.148 build: four worktrees across three repos disappeared, all of them visible on the prior build.
This is the gap #7912 asks to close, and the diagnosis matches the confirming comment on that issue —
externalWorktreeVisibility: 'show'stopped working as a workaround once theagent-scratchclassification began running ahead of the visibility check.Fix
A per-repo
agentWorktreeVisibilityopt-in, default hide, so #9388's protection stays the default and no migration is needed — an absent field reproduces today's behavior exactly.shouldShowWorktree's singleagent-scratchbranch now reads the field instead of returningfalse. Selected-checkout,orca-managed, and explicit-import branches keep their precedence.Non-Orca worktreesdialog gains a second row that counts scratch worktrees and toggles the field. It names the containing directory — or a location count when they span more than one — because that path is the diagnostic payload. Once a repo has opted in the row stays visible even at zero, so the opt-in is always reversible.isUserFacingExternalWorktreeis deliberately untouched, so the discovery card and the new-worktree inbox keep excluding scratch. Two sibling accessors serve the dialog instead.Accounting invariant
src/shared/worktree-accounting-invariant.test.tsasserts that every worktreegit worktree listreports is either unconditionally visible (selected checkout, explicit import, ororca-managed) or counted in exactly one accessor bucket. It catches a worktree being dropped from every bucket — the failure mode above — and is what should stop a futureAGENT_SCRATCH_PATH_PREFIXESchange from reintroducing it. (It does not catch a misclassification that retags a worktree into a different bucket; that is stated in the design doc.)Relation to #7912
#7912 asks for agent-created worktrees to be adopted rather than hidden. Against its checklist:
agent-scratchclass from fix: never surface coding-agent scratch worktrees in the sidebar #9535.externalWorktreeVisibilityfor every unrelated worktree — this PR, via a separateagentWorktreeVisibilitytoggle.git worktree liston a short TTL andrequireAuthoritativedoes not bypass it, so a worktree created outside Orca takes a few seconds to appear. Separate change.Non-goals
AGENT_SCRATCH_PATH_PREFIXESchange. Adding a location hides worktrees that are visible today — the very regression this makes recoverable — so it should not ride along.claude -winside a split pane leaves its agent row under the original worktree, because agent rows derive fromtabsByWorktreeand bind at pane creation (useWorktreeAgentRows.ts:47). Different subsystem, separate discussion.Verification
Automated:
src/shared, sidebar,persistence,i18nsuites: 3079 passing, output pristinepnpm run typecheck(all three projects): cleanpnpm run lint(includingverify-localization-catalog,audit-localization-coverage,check-max-lines-ratchet): cleanmobile-sidebar-onboarding-badge,SidebarToolbar,LinearAgentSkillSetupPrompt.reminder-toast) reproduce identically on the merge base and are untouched by this branchAgainst the running app, driven through the repo's Playwright Electron harness with a real
git worktree add <repo>/.claude/worktrees/…:agent-scratch,visible: false, no sidebar row — the reported bugagentWorktreeVisibilitypersists as'show'That run is also what caught the path overflowing the dialog.
DialogContentis a single-column grid, so a child at full content width inflates the shared column and stretches its siblings. Measured before and after:row0is the pre-existing non-Orca row — it was being stretched by ours and returns to normal. Fixed withmin-w-0on two containers so the path truncates instead.Reproducing it yourself
Wait a few seconds — the worktree scan caches
git worktree liston a short TTL, so a worktree created outside Orca takes a moment to appear. Then: project menu →Hide non-Orca worktrees/Show hidden worktrees→ the second row reports the count and its container →Show.Localization
The new row is filled in for all five catalogs, using each one's existing dominant terms, so it reads in the same language as the non-Orca row directly above it rather than falling back to English mid-dialog.
Design doc
docs/agent-scratch-worktree-visibility.md, force-added past thedocs/**gitignore entry, following the precedent in 70ff47e and 3efc930.Refs #7912, #9388, #9535
ELI5
Agent scratch worktrees (like Claude's .claude/worktrees) were always hidden with no way to open them in Orca. Repos can reveal those coding-agent worktrees when you need to continue work there.