v1.0.0#42
Conversation
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR updates branch-directory slugging, push controls, initialization cleanup, agent seeding, installation and uninstall behavior, JSON output, smoke-test coverage, CI wiring, contributor guidance, README documentation, and v1.0.0 release notes. Changesgit-trees update
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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 `@git-trees`:
- Around line 141-150: Update the agent-seeding failure path around the visible
write logic and its caller cmd_init so a genuine failure to create AGENTS.md
removes the newly initialized container before returning nonzero. Preserve the
successful and concurrent-creation cases, and ensure cleanup occurs only for the
failed initialization path so retries can run normally.
- Around line 485-506: Update the worktree porcelain parsing and rows
construction to use git worktree list --porcelain -z with NUL-delimited records,
preserving raw path bytes—including newlines—before CSV/JSON encoding. Adjust
tab, newline, and NUL boundary handling so fields remain correctly separated,
then keep the existing jesc JSON escaping in the awk output.
- Around line 168-180: Update the positional-argument handling in the init
option parser to reject a second target instead of overwriting or accepting it.
When a positional argument is already set, print the init usage to stderr and
return failure; preserve the existing handling for a single target and option
validation.
In `@install.sh`:
- Around line 25-36: Update the uninstall loop in install.sh to track targets
found independently from successfully removed targets: when git-trees exists,
mark it as discovered before attempting rm, and report any rm failure. Use the
discovered-target count for the “no git-trees found” message, retain successful
removal reporting, and exit nonzero if any existing target could not be removed.
In `@README.md`:
- Around line 120-125: Update the README convenience curl installation snippet
to create ~/.local/bin before downloading git-trees. Add a mkdir -p step before
the curl command, while preserving the existing download and chmod commands.
- Around line 223-255: The README’s `add` documentation overstates that upstream
setup and pushing always occur. Update the opening `add` paragraphs to qualify
these statements as applying by default or when pushing is enabled, while
preserving the existing `--no-push` and `TREES_NO_PUSH` behavior describing an
unset upstream when the remote branch is absent.
- Around line 75-84: Label the fenced directory-layout block in the README with
the text language by changing its opening fence to ```text, while leaving the
tree contents unchanged.
In `@tests/smoke.sh`:
- Around line 95-96: Update the smoke suite’s Git setup to avoid reading or
writing the user’s global configuration: set temporary author and committer
identity variables and configure Git to use a temporary global config file under
TMP. Remove the git config --global fallback commands while preserving the test
identity values.
- Around line 375-381: Add a positive assertion after the `add --no-push`
invocation to verify that the `solo` worktree and its local branch were created
successfully, such as checking the worktree path or branch reference. Keep the
existing negative remote/upstream assertions and push-command output check
unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f92d4440-8b21-41c7-845f-2ccf7e2cc08f
📒 Files selected for processing (10)
.claude/agent-notes.md.github/workflows/ci.ymlAGENTS.mdAGENTS.md.templateCHANGELOG.mdREADME.mdgit-treesinstall.shlint.ymltests/smoke.sh
💤 Files with no reviewable changes (1)
- lint.yml
| if (set -C; cat -- "$TREES_AGENTS_TEMPLATE" > "$1/AGENTS.md"); then | ||
| echo "wrote $1/AGENTS.md" >&2 | ||
| return 0 | ||
| fi | ||
| [ -e "$1/AGENTS.md" ] || [ -L "$1/AGENTS.md" ] && return 0 | ||
| # The write failed. Benign if another process won the race and created it; | ||
| # a real failure (read error, full disk) otherwise. | ||
| if [ -e "$1/AGENTS.md" ] || [ -L "$1/AGENTS.md" ]; then | ||
| return 0 | ||
| fi | ||
| echo "git trees: could not write $1/AGENTS.md" >&2 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Roll back the container when agent seeding fails.
A real write failure returns nonzero here, then cmd_init returns at Line 241 without removing the newly initialized directory. Retrying init subsequently fails on “already exists,” leaving manual cleanup as the only recovery.
Proposed fix
- _seed_agents "$dir" || return 1
+ if ! _seed_agents "$dir"; then
+ rm -rf "$dir"
+ return 1
+ fi🤖 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 `@git-trees` around lines 141 - 150, Update the agent-seeding failure path
around the visible write logic and its caller cmd_init so a genuine failure to
create AGENTS.md removes the newly initialized container before returning
nonzero. Preserve the successful and concurrent-creation cases, and ensure
cleanup occurs only for the failed initialization path so retries can run
normally.
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| --host) host="${2:-}"; host_set=1; shift 2 ;; | ||
| --dir) dir="${2:-}"; shift 2 ;; | ||
| # `shift 2` with one argument left fails and leaves $1 in place, so the | ||
| # while loop spins forever. Check arity before shifting. | ||
| --host) | ||
| [ $# -ge 2 ] || { echo "git trees init: --host needs a value" >&2; return 1; } | ||
| host="$2"; host_set=1; shift 2 ;; | ||
| --dir) | ||
| [ $# -ge 2 ] || { echo "git trees init: --dir needs a value" >&2; return 1; } | ||
| dir="$2"; shift 2 ;; | ||
| -*) echo "git trees init: unknown option $1" >&2; return 1 ;; | ||
| *) arg="$1"; shift ;; | ||
| esac |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject multiple init targets.
cmd_init owner/repo accidental silently initializes accidental, despite usage allowing one target. Reject a second positional argument and print usage. As per coding guidelines, “Every subcommand must validate its own arguments and print usage to stderr when validation fails.”
Proposed fix
- *) arg="$1"; shift ;;
+ *)
+ [ -z "$arg" ] || {
+ echo "git trees init: unexpected argument $1" >&2
+ return 1
+ }
+ arg="$1"; shift ;;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while [ $# -gt 0 ]; do | |
| case "$1" in | |
| --host) host="${2:-}"; host_set=1; shift 2 ;; | |
| --dir) dir="${2:-}"; shift 2 ;; | |
| # `shift 2` with one argument left fails and leaves $1 in place, so the | |
| # while loop spins forever. Check arity before shifting. | |
| --host) | |
| [ $# -ge 2 ] || { echo "git trees init: --host needs a value" >&2; return 1; } | |
| host="$2"; host_set=1; shift 2 ;; | |
| --dir) | |
| [ $# -ge 2 ] || { echo "git trees init: --dir needs a value" >&2; return 1; } | |
| dir="$2"; shift 2 ;; | |
| -*) echo "git trees init: unknown option $1" >&2; return 1 ;; | |
| *) arg="$1"; shift ;; | |
| esac | |
| while [ $# -gt 0 ]; do | |
| case "$1" in | |
| # `shift 2` with one argument left fails and leaves $1 in place, so the | |
| # while loop spins forever. Check arity before shifting. | |
| --host) | |
| [ $# -ge 2 ] || { echo "git trees init: --host needs a value" >&2; return 1; } | |
| host="$2"; host_set=1; shift 2 ;; | |
| --dir) | |
| [ $# -ge 2 ] || { echo "git trees init: --dir needs a value" >&2; return 1; } | |
| dir="$2"; shift 2 ;; | |
| -*) echo "git trees init: unknown option $1" >&2; return 1 ;; | |
| *) | |
| [ -z "$arg" ] || { | |
| echo "git trees init: unexpected argument $1" >&2 | |
| return 1 | |
| } | |
| arg="$1"; shift ;; | |
| esac |
🤖 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 `@git-trees` around lines 168 - 180, Update the positional-argument handling in
the init option parser to reject a second target instead of overwriting or
accepting it. When a positional argument is already set, print the init usage to
stderr and return failure; preserve the existing handling for a single target
and option validation.
Source: Coding guidelines
| printf '%s' "$rows" | awk -F'\t' ' | ||
| BEGIN { print "["; sep = "" } | ||
| function jesc(s, out, i, n, c) { | ||
| out = ""; n = length(s) | ||
| for (i = 1; i <= n; i++) { | ||
| c = substr(s, i, 1) | ||
| if (c == "\\") out = out "\\\\" | ||
| else if (c == "\"") out = out "\\\"" | ||
| else if (c in ctl) out = out ctl[c] | ||
| else out = out c | ||
| } | ||
| return out | ||
| } | ||
| BEGIN { | ||
| # Control characters are not legal raw in a JSON string. | ||
| for (i = 1; i < 32; i++) ctl[sprintf("%c", i)] = sprintf("\\u%04x", i) | ||
| ctl["\b"] = "\\b"; ctl["\f"] = "\\f"; ctl["\n"] = "\\n" | ||
| ctl["\r"] = "\\r"; ctl["\t"] = "\\t" | ||
| print "["; sep = "" | ||
| } | ||
| NF { | ||
| for (i = 1; i <= NF; i++) gsub(/"/, "\\\"", $i) | ||
| printf "%s {\"branch\":\"%s\",\"upstream\":\"%s\",\"track\":\"%s\",\"date\":\"%s\",\"state\":\"%s\",\"path\":\"%s\"}\n", \ | ||
| sep, $1, $2, $3, $4, $5, $6 | ||
| sep, jesc($1), jesc($2), jesc($3), jesc($4), jesc($5), jesc($6) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -uo pipefail
tmp=$(mktemp -d)
trap 'rm -rf "$tmp"' EXIT
repo="$tmp/repo"
git init -q "$repo"
git -C "$repo" config user.email test@example.invalid
git -C "$repo" config user.name test
: > "$repo/file"
git -C "$repo" add file
git -C "$repo" commit -qm init
wt=$tmp/$'worktree\nwith-newline'
git -C "$repo" worktree add -q -b newline-path "$wt" HEAD
printf '%s\n' 'Without -z:'
git -C "$repo" worktree list --porcelain
printf '%s\n' 'With -z:'
git -C "$repo" worktree list --porcelain -z | od -An -t x1
git --versionRepository: brightdigit/git-trees
Length of output: 533
🏁 Script executed:
#!/bin/bash
set -uo pipefail
printf 'Locate git-trees and relevant command references:\n'
fd -a '^git-trees$' . | sed -e 's#^\./##'
printf '\nRelevant occurrences:\n'
rg -n "worktree list --porcelain|awk -F|printf '%s', \"\\$rows\"|cmd_.*tree|cmd_.*output|--json|json" git-trees || true
echo
printf 'Inspect git-trees outline around candidate functions:\n'
ast-grep outline git-trees --match cmd --view expanded | sed -n '1,220p' || true
echo
printf 'Inspect git-trees lines 440-520:\n'
cat -n git-trees | sed -n '440,520p'Repository: brightdigit/git-trees
Length of output: 298
🏁 Script executed:
#!/bin/bash
set -u
printf 'Relevant occurrences:\n'
rg -n "worktree list --porcelain|awk -F|printf '%s', \\\"\\$rows\\\"|cmd_.*tree|cmd_.*output|--json|json" git-trees || true
echo
printf 'Inspect git-trees lines 440-520:\n'
cat -n git-trees | sed -n '440,520p'
printf '\nInspect command parsing/usage functions mentioning worktree/json/output:\n'
rg -n "usage|cmd_|worktree|json|output" git-trees | sed -n '1,160p'Repository: brightdigit/git-trees
Length of output: 218
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf 'Relevant occurrences:\n'
grep -nE 'worktree list --porcelain|awk -F|printf '\''%s'\'', "\$rows"|cmd_[A-Za-z_]+|json|output' git-trees || true
echo
printf 'Inspect git-trees lines 440-520:\n'
sed -n '440,520p' git-trees | nl -ba -v440
printf '\nInspect command parsing/usage functions mentioning worktree/json/output:\n'
grep -nE 'usage|cmd_|worktree|json|output' git-trees | sed -n '1,200p'Repository: brightdigit/git-trees
Length of output: 4147
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf 'Inspect cmd_list body:\n'
sed -n '448,522p' git-trees | cat -n
printf '\nBehavioral probe from repo root; preserve path containing literal tab and newline, but not NUL:\n'
tmp=$(mktemp -d -p "$PWD" git-trees-test-XXXXXX)
trap 'rm -rf "$tmp"' EXIT
repo="$tmp/repo"
git init -q "$repo"
git -C "$repo" config user.email test@example.invalid
git -C "$repo" config user.name test
: > "$repo/file"
git -C "$repo" add file
git -C "$repo" commit -qm init
printf '%s\n' 'Worktree path containing newline and tab:'
git -C "$repo" worktree add -q -b "newline\ttab-path" "$tmp/$'worktree\nwith\tchars'" HEAD
printf '%s\n' 'git worktier list --porcelain around new worktree:'
git -C "$repo" worktree list --porcelain | grep -A2 '^worktree '"$tmp/$'worktree\nwith\tchars'" || true
printf '\nCommand output for list --json contains newline path broken across rows?\n'
bash git-trees -C "$repo" list --json 2>/proc/null || true
printf '\nWithout --json around new worktree:\n'
bash git-trees -C "$repo" list 2>/proc/null | sed -n '/^[A-Z]/,/----/p' | grep -A2 "$tmp/$'worktree\nwith\tchars'" || trueRepository: brightdigit/git-trees
Length of output: 3878
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf 'Check git availability/version:\n'
if command -v git >/dev/null 2>&1; then git --version; else echo "git unavailable"; fi
printf '\nBehavioral probe with valid branch, path containing newline and tab:\n'
tmp=$(mktemp -d -p "$PWD" git-trees-test-XXXXXX)
trap 'rm -rf "$tmp"' EXIT
repo="$tmp/repo"
git init -q "$repo"
git -C "$repo" config user.email test@example.invalid
git -C "$repo" config user.name test
: > "$repo/file"
git -C "$repo" add file
git -C "$repo" commit -qm init
printf '%s\n' 'Worktree path containing newline and tab:'
git -C "$repo" worktree add -q -b "newline-tab-path" "$tmp/worktree$(printf '\n')with$(printf '\t')chars" "$repo"
printf '%s\n' 'git worktree list --porcelain for new worktree:'
git -C "$repo" worktree list --porcelain | sed "/$tmp\\/worktree/Np;q" | sed 's/^/P:'
printf '%s\\n' 'Hex bytes for worktree line:'
git -C "$repo" worktree list --porcelain | awk 'NR==2{for(i=1;i<=length($0);i++)printf "%02x ", sprintf("%d", substr($0,i,1)); print ""}'
printf '\nCommand output for list --json with newline/tab path:\n'
git trees -C "$repo" list --json
printf '\nCommand output for list without --json around new worktree:\n'
git trees -C "$repo" list | awk '
BEGIN{p=0}
/worktree/ {p=1}
p
' | sed -n '/^[A-Za-z][A-Za-z0-9\/._-]*$/,/--- /p'Repository: brightdigit/git-trees
Length of output: 1090
🌐 Web query:
Git worktree list --porcelain -z NUL delimited documentation
💡 Result:
When using the git worktree list command, the --porcelain option provides a machine-readable output format that is stable across different Git versions and user configurations [1][2]. To handle paths that contain newline characters or other unusual characters, you should combine --porcelain with the -z option [1][2]. When -z is specified, each line of the output is terminated with a NUL character (ASCII 0) instead of a standard newline character [1][2]. This is the recommended practice for robust script parsing of the worktree list output [1][2]. In the porcelain format: - Attributes are listed with a label and value separated by a space [1][2]. - Boolean attributes (such as bare and detached) appear as a label only when the value is true [1][2]. - The first attribute for every worktree record is always worktree [1][2]. - Records are separated by an empty line (or, if -z is used, the NUL terminator marks the end of each attribute line) [1][2].
Citations:
- 1: https://git-scm.com/docs/git-worktree
- 2: https://www.kernel.org/pub/software/scm/git/docs/git-worktree.html
Preserve raw porcelain paths before JSON encoding.
git worktree list --porcelain tokenizes worktree <path> by unquoted newlines, so a worktree path containing a newline is emitted as worktree /path\nwith-newline, splitting one record across two lines. Use git worktree list --porcelain -z and NUL-delimited parsing so raw path bytes are stored before writing CSV/JSON, and adjust the tab-delimited rows construction for NUL-delimiter/tab/newline boundaries.
🤖 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 `@git-trees` around lines 485 - 506, Update the worktree porcelain parsing and
rows construction to use git worktree list --porcelain -z with NUL-delimited
records, preserving raw path bytes—including newlines—before CSV/JSON encoding.
Adjust tab, newline, and NUL boundary handling so fields remain correctly
separated, then keep the existing jesc JSON escaping in the awk output.
| if [ -e "$d/git-trees" ]; then | ||
| rm -f "$d/git-trees" && echo "removed $d/git-trees" | ||
| if rm -f "$d/git-trees"; then | ||
| echo "removed $d/git-trees" | ||
| removed=1 | ||
| fi | ||
| fi | ||
| done | ||
|
|
||
| if [ "$removed" -eq 0 ]; then | ||
| echo "install.sh: no git-trees found in $*" >&2 | ||
| echo " if you installed elsewhere, pass it: ./install.sh --uninstall <dir>" >&2 | ||
| fi |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail uninstall when an existing target cannot be removed.
If rm fails, this reports that no file was found and exits 0, even though the executable remains installed. Track discovered targets separately, report removal failures, and exit nonzero.
Proposed fix
- removed=0
+ found=0 failed=0
for d in "$@"; do
- if [ -e "$d/git-trees" ]; then
- if rm -f "$d/git-trees"; then
+ if [ -e "$d/git-trees" ] || [ -L "$d/git-trees" ]; then
+ found=1
+ if rm -f -- "$d/git-trees"; then
echo "removed $d/git-trees"
- removed=1
+ else
+ echo "install.sh: could not remove $d/git-trees" >&2
+ failed=1
fi
fi
done
- if [ "$removed" -eq 0 ]; then
+ if [ "$found" -eq 0 ]; then
echo "install.sh: no git-trees found in $*" >&2
echo " if you installed elsewhere, pass it: ./install.sh --uninstall <dir>" >&2
fi
+ [ "$failed" -eq 0 ] || exit 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -e "$d/git-trees" ]; then | |
| rm -f "$d/git-trees" && echo "removed $d/git-trees" | |
| if rm -f "$d/git-trees"; then | |
| echo "removed $d/git-trees" | |
| removed=1 | |
| fi | |
| fi | |
| done | |
| if [ "$removed" -eq 0 ]; then | |
| echo "install.sh: no git-trees found in $*" >&2 | |
| echo " if you installed elsewhere, pass it: ./install.sh --uninstall <dir>" >&2 | |
| fi | |
| if [ -e "$d/git-trees" ] || [ -L "$d/git-trees" ]; then | |
| found=1 | |
| if rm -f -- "$d/git-trees"; then | |
| echo "removed $d/git-trees" | |
| else | |
| echo "install.sh: could not remove $d/git-trees" >&2 | |
| failed=1 | |
| fi | |
| fi | |
| done | |
| if [ "$found" -eq 0 ]; then | |
| echo "install.sh: no git-trees found in $*" >&2 | |
| echo " if you installed elsewhere, pass it: ./install.sh --uninstall <dir>" >&2 | |
| fi | |
| [ "$failed" -eq 0 ] || exit 1 |
🤖 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 `@install.sh` around lines 25 - 36, Update the uninstall loop in install.sh to
track targets found independently from successfully removed targets: when
git-trees exists, mark it as discovered before attempting rm, and report any rm
failure. Use the discovered-target count for the “no git-trees found” message,
retain successful removal reporting, and exit nonzero if any existing target
could not be removed.
1 participant
Summary by CodeRabbit
--no-push/TREES_NO_PUSHcontrols for upstream/remote push behavior (foraddandtrack).git treesdocumentation (usage, concepts, env vars/limitations) and addedv1.0.0release notes.tests/smoke.shend-to-end coverage and expanded CI checks (bash syntax, ShellCheck).install.shuninstall behavior and reporting.