Conversation
📝 WalkthroughWalkthroughChangesWorktree completion flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant item_inner
participant BackendDB
participant push_and_open_pr
participant Git
participant GitHubCLI
Caller->>item_inner: claim item
item_inner-->>Caller: worktree path and cd instruction
Caller->>item_inner: done item
item_inner->>BackendDB: mark claim done and load branch inputs
item_inner->>push_and_open_pr: process completed worktree
push_and_open_pr->>Git: check divergence and push branch
push_and_open_pr->>GitHubCLI: create PR
GitHubCLI-->>push_and_open_pr: optional PR URL
push_and_open_pr-->>item_inner: completion result
item_inner-->>Caller: done status and optional PR guidance
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/mcp_server.rs`:
- Around line 2561-2577: Update the completion flow around claim_done and
push_and_open_pr so ownership remains exclusive until the deterministic task
branch is fully published. Introduce or reuse a finishing lease/state, or move
the branch snapshot and publish operation before releasing the claim; only mark
the item reclaimable after publication succeeds, while preserving the existing
PR URL handling.
In `@src/worktree.rs`:
- Around line 200-219: Bound both the git push via run_git_in and the gh PR
creation command in the item-done flow with a deadline so stalled subprocesses
cannot hang indefinitely. Detect timeout or deadline expiration, log the failure
with the item context, and return None while preserving the existing
soft-failure behavior for normal command errors.
🪄 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: aff52800-45b9-4685-bc65-c0b415849f17
📒 Files selected for processing (2)
src/mcp_server.rssrc/worktree.rs
Comment on lines
+2561
to
+2577
| let (done, item, target_branch) = self.with_backend_db(|conn| { | ||
| let done = agentflare_backend::item::claim_done(conn, &item_id, &owner, now) | ||
| .map_err(map_backend_err)?; | ||
| Ok(serde_json::json!({"done": done, "item_id": item_id}).to_string()) | ||
| })? | ||
| let (item, target_branch) = if done { | ||
| let item = agentflare_backend::item::get(conn, &item_id).ok(); | ||
| let target_branch = item | ||
| .as_ref() | ||
| .map(|i| crate::worktree::resolve_target_branch(conn, i, &repo_root)); | ||
| (item, target_branch) | ||
| } else { | ||
| (None, None) | ||
| }; | ||
| Ok::<_, ErrorData>((done, item, target_branch)) | ||
| })??; | ||
| let pr_url = match (&item, &target_branch) { | ||
| (Some(item), Some(target)) => { | ||
| crate::worktree::push_and_open_pr(item, &repo_root, target) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Keep ownership exclusive until the branch is published.
Line 2562 marks the claim done before Line 2577 publishes task/<sequence_id>. Another agent can immediately reclaim the item and modify that same deterministic branch/worktree, letting this request push or PR the new claimant’s unfinished commits. Use a finishing lease/state, or snapshot/publish while ownership is still exclusive before releasing the claim.
🤖 Prompt for 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.
In `@src/mcp_server.rs` around lines 2561 - 2577, Update the completion flow
around claim_done and push_and_open_pr so ownership remains exclusive until the
deterministic task branch is fully published. Introduce or reuse a finishing
lease/state, or move the branch snapshot and publish operation before releasing
the claim; only mark the item reclaimable after publication succeeds, while
preserving the existing PR URL handling.
Comment on lines
+200
to
+219
| if let Err(e) = run_git_in(repo_root, &["push", "-u", "origin", &branch]) { | ||
| eprintln!("worktree: push skipped for item {}: {e}", item.id); | ||
| return None; | ||
| } | ||
| let body = format!("Auto-opened on `item done` for {}.", item.id); | ||
| match Command::new("gh") | ||
| .args([ | ||
| "pr", | ||
| "create", | ||
| "--base", | ||
| target_branch, | ||
| "--head", | ||
| &branch, | ||
| "--title", | ||
| &item.name, | ||
| "--body", | ||
| &body, | ||
| ]) | ||
| .current_dir(repo_root) | ||
| .output() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound the push and PR subprocesses.
Command::output() has no timeout. A stalled Git remote, credential helper, or gh request can leave item done hanging indefinitely despite the intended soft-failure behavior. Apply a deadline to both calls and return None after logging on timeout.
🤖 Prompt for 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.
In `@src/worktree.rs` around lines 200 - 219, Bound both the git push via
run_git_in and the gh PR creation command in the item-done flow with a deadline
so stalled subprocesses cannot hang indefinitely. Detect timeout or deadline
expiration, log the failure with the item context, and return None while
preserving the existing soft-failure behavior for normal command errors.
This was referenced Jul 14, 2026
4 tasks
3 tasks
getappz
added a commit
that referenced
this pull request
Jul 28, 2026
…eclaim errors (#357) * fix(asset): close staging symlink/TOCTOU bypass, log swallowed blob-reclaim errors asset attach now rejects symlinked staging files and reads size+content from one no-follow-opened handle instead of separate exists/stat/read calls. delete_disk_blob logs unlink failures (except NotFound) instead of discarding them silently, since the DB row is already gone and the orphaned file otherwise has no way to be rediscovered. Closes item #179. * address CodeRabbit review: reject non-regular staged files, fix stale comment FIFOs/devices/sockets shared symlink-metadata's fall-through Ok(_) => {} branch, so a named pipe dropped into staging would hang the attach thread on the blocking read once opened. Reject anything that isn't a regular file, same as the symlink check. Also corrects delete_disk_blob's comment: it's called from blob_store's insert-failure cleanup too, where no row was ever inserted, not just from blob_unref where one existed and is now gone.
This was referenced Aug 4, 2026
Merged
3 tasks
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
1 participant
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.
What
Closes the "done"-side half of
019f5cba(item(claim)already auto-created an isolated worktree; nothing closed the loop).worktree::push_and_open_pr: onitem(done), if the claimed item's worktree branch has commits ahead of its target branch, push it and open a PR (gh pr create) against the target. Deliberately does not auto-merge or remove the worktree — unreviewed code should never land on the target branch automatically, and the worktree needs to stay until the PR is actually reviewed and merged. Soft-fails on any git/gh error (push failure, noghinstalled, nothing to push) — the item's completion is already committed to the DB by the time this runs, so none of this should blockdone.nexthint onclaim/doneresponses: both now include a shortnextfield telling the calling agent what to do —claimsays to cd intoworktree_path,done(when a PR was opened) says to wait for review/merge before removing the worktree. Discussed live in the tracker item's thread: the agent shouldn't have to infer this from field presence alone.Verification
cargo test --bin agentflare: 415 passed, 0 failed (4 new tests: 2 inworktree.rscoveringpush_and_open_pr's soft-fail guards without touching a real remote, 2 inmcp_server.rscovering thenexthint and the no-new-commits done path)cargo clippy --all-targets -- -D warnings -A unsafe_code -A clippy::pedantic: cleancargo fmt --check: cleanclaimauto-created (.worktrees/task/31).Summary by CodeRabbit
New Features
Bug Fixes