Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Cmd as worktree command
participant Roles as roles module
participant Git as git/git2
participant FS as Filesystem
User->>Cmd: kin wt temp -b <new-branch> [start-point]
Cmd->>Cmd: parse args (new_branch?, target?)
Cmd->>Roles: ensure_temp_new_branch(repo, new_branch, target)
Roles->>Git: resolve start-point (requested or HEAD)
Roles->>Git: verify branch non-existence
Roles->>Git: create local branch from start-point
Roles->>FS: compute/verify temp worktree path
Roles->>Git: add worktree for new branch
Roles->>Roles: run on_create hooks
alt hooks succeed
Roles->>FS: write worktree metadata
Roles-->>Cmd: return success
else hooks fail
Roles->>Git: attempt delete_local_branch_if_tip_matches(new_branch, expected_tip)
Roles->>FS: remove created worktree path if present
Roles-->>Cmd: return aggregated error (may advise manual cleanup)
end
Cmd-->>User: exit (success|error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/cli_reference.md (1)
298-303:⚠️ Potential issue | 🟡 MinorSplit the
kin wt tempsynopsis into the two supported forms.Line 303 currently advertises
[<branch-or-start-point>]unconditionally, but only the-bpath treats the positional as a start point. Without-b, the implementation still interprets it as a branch name, so this synopsis is promising a mode the command does not actually support.📝 Suggested wording
-kin wt temp [-b <new-branch>] [<branch-or-start-point>] +kin wt temp [<branch>] +kin wt temp -b <new-branch> [<start-point>]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/cli_reference.md` around lines 298 - 303, The synopsis for the "kin wt temp" command is misleading: split it into the two supported forms so the docs reflect actual behavior—one form showing "kin wt temp -b <new-branch> [<start-point>]" (start-point only used with -b) and the other showing "kin wt temp <branch-or-start-point>" or "kin wt temp" (no -b, positional treated as branch name); update the "kin wt temp" line in the CLI reference to present these two distinct usages and clarify that the positional argument is a start point only when -b is present (reference the "kin wt temp" synopsis in the docs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worktree/roles.rs`:
- Around line 249-275: Replace the permissive
ensure_local_branch_exists_from_start_point(...) call with a strict creator that
returns whether it actually created the ref (e.g.
create_local_branch_from_start_point_strict(repo, branch, &start_point) ->
Result<bool>), use that boolean to know if we must roll back, and wrap the
subsequent steps (add_worktree, run_create_hooks, ctx.metadata.upsert,
ctx.metadata.save) in a failure path that deletes the branch if created by this
invocation; ensure rollback runs when add_worktree or run_create_hooks or
metadata save/upsert fail (and propagate the original error). Add an integration
test that runs the temp-branch flow (kin wt temp -b) with a deliberately failing
on_create hook to assert the branch is removed after the command errors.
---
Outside diff comments:
In `@docs/cli_reference.md`:
- Around line 298-303: The synopsis for the "kin wt temp" command is misleading:
split it into the two supported forms so the docs reflect actual behavior—one
form showing "kin wt temp -b <new-branch> [<start-point>]" (start-point only
used with -b) and the other showing "kin wt temp <branch-or-start-point>" or
"kin wt temp" (no -b, positional treated as branch name); update the "kin wt
temp" line in the CLI reference to present these two distinct usages and clarify
that the positional argument is a start point only when -b is present (reference
the "kin wt temp" synopsis in the docs).
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 36d68a63-c7d5-4669-b0cb-b70044735e08
📒 Files selected for processing (5)
README.mddocs/cli_reference.mdsrc/commands/worktree.rssrc/worktree/roles.rstests/worktree_temp_tests.rs
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worktree/roles.rs`:
- Around line 872-907: The rollback currently force-deletes the branch in
rollback_created_temp_branch by calling delete_local_branch(..., true); instead
capture the branch tip (commit OID) right after branch creation (in the code
path that sets branch_created) and store it (e.g., branch_tip_after_create),
then in rollback_created_temp_branch compare the current branch tip (resolve
branch to OID) to that saved OID and only call delete_local_branch without force
if the tip is unchanged; if the tip differs, do not delete and instead append a
rollback_errors entry instructing manual cleanup and include the current tip
info; also add a regression test that sets an on_create hook which makes a
commit then exits non-zero and assert that the branch is not force-deleted (and
that the test records the saved branch tip and reports manual cleanup when tips
differ).
- Around line 249-250: The race window lets another process create the ref after
ensure_local_branch_is_new(...) and
create_local_branch_from_start_point_strict(...) can return Ok(false), but the
current flow still proceeds to add_worktree(); change the control flow so that
create_local_branch_from_start_point_strict(repo, branch, start_point, ...)
returning Ok(false) is treated as a hard failure (return Err) instead of
continuing—do this wherever create_local_branch_from_start_point_strict is used
in this function (the block around ensure_local_branch_is_new,
resolve_requested_start_point, and the subsequent
create_local_branch.../add_worktree sequence) so we never call add_worktree()
when the branch was not actually created by this invocation.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 96a42eca-d637-4062-bd87-47b738264b86
📒 Files selected for processing (6)
README.mddocs/cli_reference.mdsrc/commands/worktree.rssrc/worktree/git.rssrc/worktree/roles.rstests/worktree_temp_tests.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/worktree/roles.rs`:
- Around line 911-918: The rollback uses delete_local_branch(repo, branch,
false) which calls the non-forced `git branch -d` and can leave branches when
the local start-point is divergent despite the OID match checked by
current_branch.get().target() == branch_tip_after_create; change the deletion to
an OID-guarded ref delete or call the force path (i.e., use
delete_local_branch(repo, branch, true) or implement a ref-delete-by-oid in
delete_local_branch that verifies branch_tip_after_create before forcing
removal) so the branch is removed when the tip matches branch_tip_after_create,
and add a regression test (based on
worktree_temp_b_failing_create_hook_rolls_back_created_branch) that simulates a
failing on_create hook with a divergent local start-point to ensure the new
behavior prevents stranded branches.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 86f20e68-c184-4311-84dd-33f59d316b63
📒 Files selected for processing (6)
README.mddocs/cli_reference.mdsrc/commands/worktree.rssrc/worktree/git.rssrc/worktree/roles.rstests/worktree_temp_tests.rs
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.
Stack
Summary by CodeRabbit
New Features
Bug Fixes
Documentation