Conversation
All 15 worktree git-subprocess timeouts in WorktreeProvider were hardcoded at 30000ms. Repos with heavy post-checkout hooks (lint, dependency install, submodule init) routinely exceed that budget and fail worktree creation. Consolidate them onto a single GIT_OPERATION_TIMEOUT_MS constant at 5 min. Generous enough to cover reported cases while still catching genuine hangs (credential prompts in non-TTY, stalled fetches). Chosen over the config-key approach in #1029 to avoid adding permanent .archon/config.yaml surface for a problem a raised default solves cleanly. If 5 min turns out to also be too tight for real-world use, we'll revisit. Closes #1119 Supersedes #1029 Co-authored-by: Shay Elmualem <12733941+norbinsh@users.noreply.github.com>
📝 WalkthroughWalkthroughA new shared constant Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/isolation/src/providers/worktree.ts (1)
1024-1029: Minor inconsistency: adjacentbranch -fstill uses10000.In the same fallback path,
branch -fat line 1025 retains a 10s timeout while the surroundingworktree addnow usesGIT_OPERATION_TIMEOUT_MS.branch -fis normally instantaneous, so this is not a correctness issue, but it's the one remaining hardcoded branch-op timeout insideWorktreeProviderand could confuse future readers about which operations are covered by the new ceiling. Consider either switching it toGIT_OPERATION_TIMEOUT_MSfor uniformity or adding a brief comment noting why it deliberately stays short. Non-blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/isolation/src/providers/worktree.ts` around lines 1024 - 1029, Replace the hardcoded 10000 timeout in the execFileAsync call that runs "git ... branch -f" with the GIT_OPERATION_TIMEOUT_MS constant for consistency with the adjacent "git worktree add" call; locate the execFileAsync invocation that uses repoPath, branchName, and startPoint (in WorktreeProvider / worktree.ts) and swap the numeric timeout to GIT_OPERATION_TIMEOUT_MS (or alternatively add a one-line comment explaining why this specific branch-op should remain short if you prefer to keep 10s).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/isolation/src/providers/worktree.ts`:
- Around line 1024-1029: Replace the hardcoded 10000 timeout in the
execFileAsync call that runs "git ... branch -f" with the
GIT_OPERATION_TIMEOUT_MS constant for consistency with the adjacent "git
worktree add" call; locate the execFileAsync invocation that uses repoPath,
branchName, and startPoint (in WorktreeProvider / worktree.ts) and swap the
numeric timeout to GIT_OPERATION_TIMEOUT_MS (or alternatively add a one-line
comment explaining why this specific branch-op should remain short if you prefer
to keep 10s).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94d42673-c963-4daa-b172-1b0146d37766
📒 Files selected for processing (1)
packages/isolation/src/providers/worktree.ts
Summary
WorktreeProviderhardcodedtimeout: 30000. Repos with heavy post-checkout hooks (lint, dependency install, submodule init) exceed that budget and fail worktree creation.GIT_OPERATION_TIMEOUT_MS = 5 * 60 * 1000constant at the top ofworktree.ts. 5 min is generous enough to cover realistic hook workloads while still catching genuine hangs (credential prompts in non-TTY, stalled fetches).@archon/git,MergedConfig, orGlobalConfig. Othertimeout:usages outsideWorktreeProviderare untouched.Why this instead of a config key (supersedes #1029)
#1029 proposed a
.archon/config.yamlworktree.timeoutoption. That approach:Raising the default covers the reported case with zero API surface. If 5 min later turns out to also be too tight in real-world use, the config key stays an option on the table.
UX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
GIT_OPERATION_TIMEOUT_MS(new local const)execFileAsynccall sites inworktree.tsNo inter-module connections changed.
Label Snapshot
risk: lowsize: XSisolationisolation:worktree-providerChange Metadata
fixisolationLinked Issue
Validation Evidence (required)
Security Impact (required)
NoNoNoNoCompatibility / Migration
Yes— behavior change is strictly "waits longer before timing out"NoNoHuman Verification (required)
30000remains inworktree.ts.Side Effects / Blast Radius (required)
'worktree.*_failed'log events fire on timeout-derived errors. No new monitoring needed.Rollback Plan (required)
git revert <commit>— single-file, single-constant change.Risks and Mitigations
Credit to @norbinsh for raising #1119 and scoping the problem clearly in #1029.
Summary by CodeRabbit