Conversation
…orktrees on stale local refs create_worktree branched new task worktrees off whatever target_branch pointed to LOCALLY, with no git fetch anywhere in worktree.rs. If the main checkout hadn't synced recent merges, every new claim silently started from stale code. Now fetches origin/<target_branch> first and branches off that when reachable; soft-fails to the old local-ref behavior when there's no remote, we're offline, or the branch was never pushed (never blocks a claim on network reachability). New test clones a remote, lands a commit on it after the clone (so the local clone's own refs go stale), and asserts the resulting worktree is based on the fresh remote commit rather than the stale local one.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe worktree branch start point is fetched from and verified against the remote target branch when available, with fallback to the local target branch. A regression test validates remote-only commits are included when the local clone is stale. Worktree branch source
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/worktree.rs`:
- Around line 166-183: The fetch in the start_point selection must fail fast
instead of blocking on unreachable remotes or credential prompts. Update the
fetch invocation to use the existing timeout helper, or ensure it runs with
GIT_TERMINAL_PROMPT=0, while preserving the current soft-fallback to
target_branch when fetching or remote-ref verification fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bc2e204b-56fe-4bdd-8075-bc22047b3740
📒 Files selected for processing (1)
src/worktree.rs
Routed through run_output_timeout (already used for push/PR) instead of the plain blocking run_git_in — an unreachable remote or a credential prompt must not be able to hang a claim indefinitely.
* feat(mcp): consolidate 6 tools into action-dispatch pattern - Merge skill/search+load, gateway/search+execute, claim/5-actions, review/5-actions, artifact/6-actions, memory/6-actions into one consolidated #[tool(...)] method each with action dispatch - Reconcile with #182 (fetch target branch before branching) and #183 (deferred claim release, mark_completed split) * fix(mcp): finish artifact/skill/gateway test migration, fix consolidation regressions CI was red: ~35 test call sites in mcp_server.rs still called the old per-action methods (artifact_publish/get/list/delete/diff/search, skill_search/load, gateway_search/execute) that this consolidation removed. Migrated them all to the unified action-dispatch tools. Also fixes real bugs the consolidation introduced along the way: - artifact()'s get/diff/search arms called store methods with a signature they no longer have (get takes no version, diff's to is a required u32, and there's no store.search at all) — restored the pre-consolidation logic (get_version fallback, latest-version resolution, inline list+filter+snippet search). - review()'s "consensus" arm called crate::review::consensus with the wrong arguments entirely (a Connection/ids instead of findings+changed-lines); now loads findings and computes the diff/changed-lines first, matching "record"'s already-correct pattern. Also dropped record's dead base/head locals that shadowed the actual req.base/req.head usage. - memory()'s "curate" arm passed the outer "curate" action into CurateInput.action, so update/delete/pin/unpin could never be reached; added a dedicated curate_action field. - mark_completed's ownership check and completion write weren't in the same transaction, leaving a window for a concurrent release+claim by a different owner to complete the item out from under its new owner. Also ran cargo fmt across the branch (had never been formatted). * fix(fmt): collapse tool() signature onto one line per rustfmt
Fixes item #42: create_worktree branched new task worktrees off whatever target_branch pointed to LOCALLY in repo_root — there was no git fetch anywhere in worktree.rs. If the main checkout hadn't synced recent merges, every new claim silently started from stale code, risking wasted work and conflicts when the resulting PR opens. Concretely hit earlier this session — had to manually git pull --ff-only origin master after merging PRs #178/#179 because the local checkout had drifted.
Fix
Before git worktree add, create_worktree now runs git fetch origin <target_branch> (scoped to just that branch) and verifies origin/<target_branch> resolves; if so, branches off that instead of the bare local ref. Soft-fails to today's behavior (branch off the local ref as-is) when there's no origin remote, we're offline, or the branch was never pushed (common for a parent item's task/N branch) — never blocks a claim on network reachability, matching every other soft-fail in this file. Skipped entirely on the already_isolated_for fast-path (re-claiming an existing worktree — nothing to sync).
Test
New test: clones a remote repo, lands a commit on the remote after the clone (so the local clone's own master and origin/master both go stale), then asserts the resulting worktree is based on the fresh remote commit, not the stale local one.
432/432 tests pass (431 + 1 new), clippy clean (-D warnings -A unsafe_code -A clippy::pedantic), fmt clean.
Summary by CodeRabbit
HEADmatches the remote’s latest commit rather than a stale local ref.