feat: in-repo .worktrees/ mode (rewrite of #388)#1129
Open
sasha-id wants to merge 10 commits intostablyai:mainfrom
Open
feat: in-repo .worktrees/ mode (rewrite of #388)#1129sasha-id wants to merge 10 commits intostablyai:mainfrom
sasha-id wants to merge 10 commits intostablyai:mainfrom
Conversation
Adds pure parsing helpers (isWorktreesDirIgnoredByGitignore, appendWorktreesEntry) and async IO wrappers (readGitignore, isWorktreesDirIgnored, addWorktreesDirToGitignore) used by the upcoming in-repo .worktrees/ mode to detect and offer to add a .gitignore entry before creating in-repo worktrees. Why pattern matching is intentionally narrow: only the four canonical hand-written forms (.worktrees, .worktrees/, /.worktrees, /.worktrees/) are detected. Implementing the full git ignore-rule grammar (globs, partial matches, etc.) is a non-goal — anything else falls through to the prompt, which is the safer default. Negation patterns (!.worktrees/) are explicitly skipped so a user-authored un-ignore can't accidentally suppress the prompt. readGitignore returns null on ENOENT (a missing .gitignore is a normal state, not an error) but rethrows other errors so EACCES/EIO surface to the caller. addWorktreesDirToGitignore is idempotent — it re-reads and re-checks before writing to make rapid successive calls safe.
Adds a sync helper that wraps `git rev-parse --is-bare-repository`,
matching the rest of repo.ts (sync because git rev-parse is fast and
upstream callers are not on a hot path).
Why a dedicated helper instead of letting callers shell out: the
upcoming gitignore:checkWorktreesIgnored IPC handler needs to short-
circuit for bare repos (no working tree to dirty), and other call
sites in the in-repo worktree flow gate on the same condition. A
named, tested function with documented error semantics ('not bare'
on any failure) keeps that gate consistent.
Adds a 'external' | 'in-repo' setting that determines where worktrees are physically created: - 'external' (default): sibling-of-repo directory, current behavior. Uses workspaceDir or repo's parent depending on nestWorkspaces. - 'in-repo': inside the repo at <repo>/.worktrees/<name>. Requires a .gitignore confirmation prompt before the first create so the entries don't pollute git status. Why default to 'external': it preserves the existing behavior for all current users. The new in-repo mode is opt-in via Settings → General; existing settings auto-pick up the default during the next read of getDefaultSettings. Updates the codex-accounts test settings fixtures to include the new field, since they construct full GlobalSettings literals. Other test files use partial mocks or the RuntimeStore narrowed shape, which gain the field in subsequent commits when they actually need it.
Adds an in-repo branch to computeWorktreePath that runs before both the WSL special case and the user-configured workspaceDir fallback. When worktreeLocation is 'in-repo', the worktree path is always <repo>/.worktrees/<sanitizedName>. Why in-repo runs first: it bypasses both the WSL workspace-dir override (irrelevant when worktrees live inside the repo) and the nestWorkspaces flag (also irrelevant). Skipping straight to this branch means the WSL branch never fires, even when the repo path is on a WSL filesystem. Path traversal is not re-validated here because sanitizeWorktreeName already strips path separators, leading/trailing dots, and any character outside [A-Za-z0-9._-], producing slugs that cannot escape the .worktrees/ directory. The new branch is covered by two unit tests asserting the path is stable regardless of nestWorkspaces and workspaceDir — without those, a regression that fell through to the external branch could still produce a valid-looking path. Widens RuntimeStore.getSettings() in orca-runtime.ts to include worktreeLocation, since it threads through to computeWorktreePath on the CLI create path. Updates test fixtures across runtime and worktree-logic tests with the new field.
Wires the gitignore utility introduced earlier (src/main/git/gitignore.ts)
through to the renderer:
- Two ipcMain handlers in worktrees.ts:
- gitignore:checkWorktreesIgnored — short-circuits to ignored=true for
folder/bare repos and missing repos. On read failure, fails OPEN
(ignored=false) so misconfiguration doesn't silently suppress the
prompt — a closed failure could leave thousands of untracked
worktree files in `git status` without the user knowing why.
- gitignore:addWorktreesEntry — rejects folder repos. Calls the
idempotent addWorktreesDirToGitignore helper.
- A `gitignore` namespace on the preload bridge that the renderer can
use as window.api.gitignore.checkWorktreesIgnored() etc. The TypeScript
surface is declared in api-types.d.ts (the canonical PreloadApi shape),
not in index.d.ts (the local-only types section), per the comment in
index.d.ts about where to put inherited keys.
Adds a two-button toggle ("External directory" / "In-repo .worktrees/")
above the existing Workspace Directory and Nest Workspaces fields,
and wraps those two fields in a `worktreeLocation === 'external'`
gate — they're meaningless in in-repo mode (worktrees live inside
each repo, not in workspaceDir).
The in-repo description tells the user that Orca will offer to add
`.worktrees/` to each repo's `.gitignore` on first create — the
prompt itself comes in the next commit (it requires the new
NewWorkspaceComposerModal/useComposerState integration).
Adds the picker to the settings search index so users searching
"in-repo", ".worktrees", "gitignore", etc. land on this setting.
When a worktree is created and the user has chosen in-repo mode in
settings, gate the createWorktree call on a check of the repo's
root .gitignore. If .worktrees/ is not already excluded, add it
silently and surface the change via a toast. If reading or writing
.gitignore fails, the create proceeds anyway with a warning toast —
a gitignore failure is non-fatal.
Why auto-add instead of an explicit prompt: the user opted into
in-repo mode in Settings, so silently doing the right thing
(keeping `git status` clean) is consistent with that intent. A
prompt on every first create would add a UX speed bump for a
near-universal preference. The IPC handler is idempotent, so
repeated calls are safe; the toast tells the user what changed
the first time.
The settings description is updated to be honest about the
behavior ("Orca will add ... automatically" instead of "offer
to add").
This logic is duplicated across submit (full-page composer) and
submitQuick (modal quick-create) rather than extracted, since both
sites depend on the same closure (settings, repoId, etc.) and a
shared useCallback would add an extra hook + dep array to maintain
without simplifying the call sites.
Adds the original design doc and the per-task implementation plan that drove this PR. Includes them now (rather than dropping them on rebase) so future maintainers can read the rationale behind: - Choosing 'external' / 'in-repo' as the WorktreeLocation enum rather than reusing nestWorkspaces or workspaceDir. - Putting the in-repo branch first in computeWorktreePath (so the WSL override never fires for in-repo mode). - The narrow set of canonical .gitignore patterns recognized by the parser (vs implementing the full ignore-rule grammar). - The fail-open behavior of gitignore:checkWorktreesIgnored. - Why path-traversal validation lives in sanitizeWorktreeName, not in the in-repo branch's path computation. Lives under docs/ to match the existing in-tree design notes.
Fixes two related bugs caught in review of the in-repo worktree mode. 1. **In-repo creation was broken end-to-end.** Both create paths (createLocalWorktree in worktree-remote.ts and createManagedWorktree in orca-runtime.ts) wrap computeWorktreePath with ensurePathWithinWorkspace against settings.workspaceDir. For in-repo mode, computeWorktreePath returns <repoPath>/.worktrees/<name> — which is NOT a descendant of workspaceDir — so the guard threw "Invalid worktree path" on every in-repo create. The earlier feature commit added the in-repo branch to computeWorktreePath but did not propagate the workspaceRoot change to the wrapper call sites. This commit completes that propagation: when worktreeLocation === 'in-repo', the path-traversal guard is now run against <repoPath>/.worktrees as the root, keeping the defense-in-depth check in place but with the correct boundary. 2. **Bare repos in in-repo mode silently created `<bare-repo>/.worktrees/` inside the bare object directory** — mechanically valid but convention-violating and almost never the user's intent. Both create paths now reject bare repos in in-repo mode with a clear error pointing the user to the Settings → General toggle. This mirrors how the code already rejects worktree creation for folder repos. Tests added in worktrees.test.ts and orca-runtime.test.ts pin both behaviors: - in-repo create succeeds and runs ensurePathWithinWorkspace against the .worktrees root, not workspaceDir - in-repo create on bare repos throws the bare-repo error before any git mutation happens - external mode on bare repos still works (the gate only fires for in-repo) Without the integration tests, the original review missed the broken-by-default state and only computeWorktreePath's pure-path behavior was covered. Pinning at the IPC handler layer ensures a future refactor can't regress one create path while leaving the other intact.
The previous comment claimed the re-check "must not duplicate the entry" — overstating the guarantee. The re-read makes the common case (rapid double-click) safe, but two truly concurrent writers can both observe `ignored: false` and both append, producing a duplicate `.worktrees/` line. Updated to acknowledge the narrow race and explain why we accept it: the duplicate is functionally harmless (git applies the first matching pattern), so a lockfile or atomic rename would be overkill for this low-risk path. Pure comment change; no behavior change.
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
2 participants
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.
Summary
Adds an "in-repo" worktree mode where worktrees live at
<repo>/.worktrees/<name>instead of an external workspace directory. Default stays'external'; the new mode is opt-in via Settings → General → Worktree Location.Replaces #388, which had drifted from upstream/main by ~700 commits and could not be rebased cleanly because PR #710 replaced
AddWorktreeDialog.tsx(which the originald7ce61b8modified) withNewWorkspaceComposerModal+useComposerState. Rather than force-push a heavily massaged history, the work is rewritten cleanly on top of current main as 10 logical commits.What's in the PR
gitignoreutility — narrow canonical pattern matching for.worktrees/. Idempotent IO wrappers; explicit comment about the TOCTOU race window (and why we accept it).isBareRepo()helper wrappinggit rev-parse --is-bare-repository.WorktreeLocationsetting with default'external'. Test fixtures updated.computeWorktreePath— runs before the WSL override, since in-repo worktrees inherit the repo's filesystem regardless.gitignore:checkWorktreesIgnored+gitignore:addWorktreesEntryIPC handlers + preload bridge. Read errors fail-open so misconfiguration doesn't silently suppress the safety net.GeneralPane, withWorkspace DirectoryandNest Workspaceshidden in in-repo mode (meaningless there)..worktrees/to.gitignoreon first in-repo create + confirmation toast. This deviates from the original PR's design — the originald7ce61b8rendered an inline confirmation dialog insideAddWorktreeDialog. Porting that touseComposerStatewould have meant ~150 lines of new state-machine code, and the IPC handler is idempotent, so silently doing the right thing is consistent with opt-in via Settings. The settings copy is updated to be honest about the behavior.createLocalWorktreeandcreateManagedWorktree. Bare repos in in-repo mode now reject with a clear error pointing to Settings → General.Test plan
gitignore.test.ts(24 tests) — pure parsing + IO wrapper coverage including empty file, missing file (ENOENT), already-ignored, negation patterns, comments, idempotent re-add.repo.test.ts(3 tests) —isBareRepohappy path + non-bare + thrown-from-rev-parse fallback.worktree-logic.test.ts—computeWorktreePathin-repo branch covered (3 new cases including the assertion that nestWorkspaces and workspaceDir are ignored in in-repo mode).worktrees.test.ts(13 tests, 3 new) — pins the renderer-side path-validation chain and bare-repo guard.orca-runtime.test.ts(31 tests, 1 new) — pins the CLI-side bare-repo guard.tsconfig.node.json/tsconfig.tc.web.json.<repo>/.worktrees/<name>exists and.gitignoreis updated.Notes for review
4f0342b6and399b7b2b: the original rewrite missed propagating theworkspaceRootchange toworktree-remote.tsandorca-runtime.ts, breaking in-repo creation end-to-end. New integration tests pin the regression.