Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughReplaces ad-hoc worktree discovery with a structured registry produced by ChangesWorktree Records & Command Consumers
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (gtr)
participant Cmd as Command (list/clean/copy)
participant Core as Core helper (list_worktree_records)
participant Git as Git CLI
participant FS as Filesystem
User->>Cmd: invoke "gtr list / clean --merged / copy --all"
Cmd->>Core: request worktree records (repo_root)
Core->>Git: git worktree list --porcelain
Git-->>Core: porcelain output
Core->>Core: parse porcelain -> emit TSV records (is_main,path,branch,status)
Core-->>Cmd: records stream (tab/blank-delimited)
Cmd->>Cmd: filter, sort, present or decide removal
Cmd->>Git: (on remove) git worktree remove / git branch delete
Cmd->>FS: remove worktree path (preRemove/postRemove hooks)
Cmd-->>User: printed table / dry-run / confirmation / result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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)
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 `@lib/core.sh`:
- Around line 225-228: list_worktree_records currently emits raw tab-delimited
fields so a path containing a literal TAB corrupts parsing; change
list_worktree_records to output a safe, NUL-delimited format (emit each field
with printf '%s\0' per record) instead of raw TSV, and update all callers that
parse its output with IFS=$'\t' to read NUL-delimited fields (use read -r -d ''
or while IFS= read -r -d '' ...) so paths with spaces/special chars/tabs are
preserved; reference the list_worktree_records function and any callers that use
IFS=$'\t' when making the changes.
- Around line 297-299: The loop compares raw 'path' against normalized
'target_path' but never canonicalizes 'path', causing mismatches for symlinked
or differently-typed paths; inside the while-read loop (the block that reads
is_main, path, branch, record_status from list_worktree_records) produce a
canonical form of path (e.g., path_canonical via realpath/readlink -f or an
existing canonicalize helper) and then compare that canonicalized value against
target_path_canonical (and/or target_path) instead of the raw 'path' so
equivalent paths match correctly.
🪄 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: Enterprise
Run ID: 1235dc7d-8ffa-4e83-8549-903190f0734a
📒 Files selected for processing (8)
lib/commands/clean.shlib/commands/list.shlib/core.shtests/cmd_clean.batstests/cmd_copy.batstests/cmd_list.batstests/core_resolve_target.batstests/integration_lifecycle.bats
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 `@lib/core.sh`:
- Around line 668-675: The helper list_worktree_branches currently calls
_resolve_main_repo_root and ignores the caller-supplied base_dir/prefix, so
change list_worktree_branches to accept a repo_root parameter (e.g., repo_root)
instead of resolving it internally, remove the internal call to
_resolve_main_repo_root, and use the passed repo_root for worktree discovery;
then update all callers that already have repo_root (or can compute it once via
_resolve_main_repo_root) to pass that value into list_worktree_branches so the
function no longer depends on the current working directory.
- Around line 217-220: Canonicalize wt_path before comparing to repo_root so
symlinked or non-canonical paths match the main worktree: compute a canonical
form of wt_path (e.g., via realpath/readlink -f or cd "$wt_path"; pwd -P) into a
temporary variable and use that when setting is_main=1 (replace the direct
comparison [ "$wt_path" = "$repo_root" ] with a comparison against the
canonicalized path); leave _worktree_record_status and other variables
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8ecfb560-9718-4af6-b381-e025670d62a5
📒 Files selected for processing (4)
lib/commands/clean.shlib/commands/list.shlib/core.shtests/core_resolve_target.bats
✅ Files skipped from review due to trivial changes (1)
- tests/core_resolve_target.bats
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/commands/clean.sh
- lib/commands/list.sh
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/core.sh (1)
399-429:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t pass raw worktree paths back through TSV here.
This fallback now parses tab-safe records, but it returns
wt_pathinside a tab-delimited triple again. Any registered worktree whose path contains a tab will be parsed correctly bylist_worktree_records()and then broken here whenunpack_target()splits onIFS=$'\t', corrupting_ctx_worktree_pathand_ctx_branchfor everyresolve_worktree()caller. Use the same structured handoff as the inventory code, or escape/unescape the path before crossing theresolve_target/unpack_targetboundary. As per coding guidelines, "**/*.sh: Always quote paths to handle spaces and special characters; avoid unguarded globbing".🤖 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 `@lib/core.sh` around lines 399 - 429, The code returns raw wt_path in a tab-delimited triple which breaks unpack_target() (and thus resolve_worktree()) when paths contain tabs; before printing the triple from this block (and from list_worktree_records handoff), escape any literal tab characters in wt_path (e.g. replace $'\t' with a safe escape like \\t) and ensure you quote variables in the printf, and then update unpack_target() to unescape \\t back to a real tab when parsing into _ctx_worktree_path/_ctx_branch; alternatively adopt the same structured handoff used by the inventory code (escape/unescape or a different delimiter) so that list_worktree_records, resolve_worktree, and unpack_target all agree on the encoding for wt_path.
🤖 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.
Duplicate comments:
In `@lib/core.sh`:
- Around line 399-429: The code returns raw wt_path in a tab-delimited triple
which breaks unpack_target() (and thus resolve_worktree()) when paths contain
tabs; before printing the triple from this block (and from list_worktree_records
handoff), escape any literal tab characters in wt_path (e.g. replace $'\t' with
a safe escape like \\t) and ensure you quote variables in the printf, and then
update unpack_target() to unescape \\t back to a real tab when parsing into
_ctx_worktree_path/_ctx_branch; alternatively adopt the same structured handoff
used by the inventory code (escape/unescape or a different delimiter) so that
list_worktree_records, resolve_worktree, and unpack_target all agree on the
encoding for wt_path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0cc6af4f-7537-4007-b2c5-e14cdb52280e
📒 Files selected for processing (3)
lib/commands/copy.shlib/core.shtests/integration_lifecycle.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integration_lifecycle.bats
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 `@lib/core.sh`:
- Around line 351-357: The escape routine _tsv_escape_field is producing
double-escaped sequences for tabs/newlines (\\\\t/\\\\n) so unescape yields
backslash+letter instead of actual control chars; change the replacements to
produce single backslash escapes that the unescape logic expects: keep the
backslash-first replace (value=${value//\\/\\\\}) but replace tabs with $'\\t'
(value=${value//$'\t'/$'\\t'}) and newlines with $'\\n'
(value=${value//$'\n'/$'\\n'}) so the produced sequences are "\t" and "\n" and
roundtrip correctly with the corresponding unescape implementation.
🪄 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: Enterprise
Run ID: fc33283a-e1f7-491b-9cb3-8a05755a1537
📒 Files selected for processing (2)
lib/core.shtests/core_resolve_target.bats
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/core.sh (1)
351-357:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winTSV escape produces double-escaped sequences, breaking roundtrip.
The escape replacements use
\\\\t/\\\\nwhich in bash produces\\t/\\n(two backslashes + letter). The unescape function expects single-escaped\t/\n. This causes tab characters to become literal\tafter roundtrip, corrupting paths containing tabs.Proposed fix
_tsv_escape_field() { local value="$1" value=${value//\\/\\\\} - value=${value//$'\t'/\\\\t} - value=${value//$'\n'/\\\\n} + value=${value//$'\t'/\\t} + value=${value//$'\n'/\\n} printf "%s" "$value" }🤖 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 `@lib/core.sh` around lines 351 - 357, _tsF function _tsv_escape_field currently replaces tabs/newlines with four-backslash sequences (\\\\t/\\\\n) producing literal backslash-backslash-t in output; change the replacements to insert a single backslash plus letter by using ANSI-C quoted replacements: replace value=${value//$'\t'/\\\\t} with value=${value//$'\t'/$'\\t'} and value=${value//$'\n'/\\\\n} with value=${value//$'\n'/$'\\n'} so _tsv_escape_field produces \t and \n (single-escaped) that match the unescape logic; keep the backslash escaping value=${value//\\/\\\\} as-is.
🤖 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.
Duplicate comments:
In `@lib/core.sh`:
- Around line 351-357: _tsF function _tsv_escape_field currently replaces
tabs/newlines with four-backslash sequences (\\\\t/\\\\n) producing literal
backslash-backslash-t in output; change the replacements to insert a single
backslash plus letter by using ANSI-C quoted replacements: replace
value=${value//$'\t'/\\\\t} with value=${value//$'\t'/$'\\t'} and
value=${value//$'\n'/\\\\n} with value=${value//$'\n'/$'\\n'} so
_tsv_escape_field produces \t and \n (single-escaped) that match the unescape
logic; keep the backslash escaping value=${value//\\/\\\\} as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c8b716e-788a-41b9-baf0-15a713571abb
📒 Files selected for processing (2)
lib/core.shtests/core_resolve_target.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/core_resolve_target.bats
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 `@lib/core.sh`:
- Around line 225-228: The printed worktree inventory currently emits the raw
wt_path (printf "path %s\n" "$wt_path") which breaks parsing if the path
contains newlines; update the exporter (function/list_worktree_records or the
block producing "path ...") to escape the path using the existing TSV escaper
(e.g. call _tsv_escape_field on wt_path) before printing, ensure all
printf/printf-like calls quote the variable, and then update consumers/parsers
to unescape the parsed path (use _tsv_unescape_field on the value extracted via
${line#path } in list, clean, resolve_target, and worktree_status) so the
transport is safe end-to-end.
🪄 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: Enterprise
Run ID: 9c639e7d-ddba-48f1-b12c-d783f54874c8
📒 Files selected for processing (2)
lib/core.shtests/core_resolve_target.bats
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/core_resolve_target.bats
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.
Summary
git worktree list --porcelaininventory helper for registered worktreesgit gtr listconsume that inventory so nested worktrees are shown by their real path and fake parent rows disappearclean --mergediterationFixes #177.
Validation
git diff --checkshellcheck bin/gtr bin/git-gtr lib/*.sh lib/commands/*.sh adapters/editor/*.sh adapters/ai/*.sh./scripts/generate-completions.sh --checkbats tests/cmd_list.bats tests/core_resolve_target.bats tests/integration_lifecycle.bats tests/cmd_copy.bats tests/cmd_clean.batsbats tests/Summary by CodeRabbit
New Features
Bug Fixes
Tests