Skip to content

fix: preserve trailing newlines in registered worktree paths - #256

Merged
shiqui merged 1 commit into
mainfrom
codex/fix-registry-path-newlines
Nov 6, 2026
Merged

shiqui merged 1 commit into
mainfrom
codex/fix-registry-path-newlines

Conversation

@shiqui

@shiqui shiqui commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

Preserve trailing newline bytes in registered worktree paths across target resolution, status lookup, sparse-checkout inheritance, cleanup, and listing. Keep list paths escaped through sorting and preserve command failure statuses when capturing decoded or canonical paths.

Motivation

When a registered worktree path ended in a newline and an unregistered sibling existed with that newline removed, git gtr go could return the unregistered sibling. The same truncation could confuse status matching, sparse-source selection, and cleanup.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Testing

  • Full BATS suite: 939 passed, 0 skipped, using isolated Git/XDG configuration; remaining files ran independently in parallel after the first 188 tests.
  • Added 13 regressions covering one/multiple trailing newlines, escaped identities, shortened siblings, stale entries, canonicalization failures, status, listing, cleanup, and sparse inheritance.
  • ShellCheck passed for CLI entry points, libraries, command handlers, and adapters.
  • Generated-completion check and git diff --check passed.
  • Both production and development entry points passed version smoke checks.

Manual Testing Checklist

Tested on: macOS, Bash 3.2.57, Git 2.50.1.

  • Create a worktree and inherit sparse patterns from a source ending in multiple newlines.
  • Compare git gtr go stdout byte-for-byte with the registered path.
  • Remove the worktree by escaped path identity and confirm the shortened sibling remains intact.

Linux, Windows, and Git 2.17 were not available for runtime validation. Existing older-Git fallback tests passed; no newer Bash or Git features were introduced.

Breaking Changes

None. Detached-branch representation and remote-tracking behavior are unchanged.

Checklist

  • Read CONTRIBUTING.md and followed project style.
  • Added regression tests and completed the full test suite.
  • Performed manual testing on macOS and noted platform limits.
  • No new external dependencies.
  • No documentation or completion changes needed for this behavior fix.

License Acknowledgment

This contribution is made under the terms of the Apache License 2.0.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed worktree commands so paths ending in newline characters are handled correctly during cleanup, navigation, listing, and sparse worktree creation.
    • Listings now preserve these paths consistently in human-readable, identifier, and machine-readable output.
    • Fixed path resolution to distinguish newline-ending paths from similar paths without trailing newlines.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The change preserves trailing newline bytes in registered worktree paths during canonicalization, decoding, resolution, sparse-source lookup, cleanup, and listing. Regression tests cover one- and two-newline paths, shortened siblings, and related command behavior.

Changes

Registered worktree paths

Layer / File(s) Summary
Preserve paths in resolution and status
lib/core.sh, tests/core_resolve_target.bats, tests/cmd_go.bats
Core canonicalization and registry decoding preserve trailing newlines during target resolution and status lookup. Tests cover exact path identity, status, and git gtr go output.
Preserve sparse-source paths
lib/core.sh, lib/commands/create.sh, tests/sparse.bats
Sparse-source lookup and creation preserve trailing newlines while resolving a source and inheriting its sparse pattern.
Preserve paths during cleanup
lib/commands/clean.sh, tests/cmd_clean.bats
Cleanup preserves registered path bytes during canonicalization and decoding. Tests check that cleanup distinguishes registered paths from shortened siblings.
Preserve paths in list output
lib/commands/list.sh, tests/cmd_list.bats
List output keeps paths escaped during buffering and sorting, then prints decoded paths or escaped IDs according to the output mode.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

Suggested reviewers: natoboram, helizaga, adamwett

Merge Risk: Minimal · up to ee96a

No actionable merge-blocking issue is established for this change; it is ready for normal merge checks.

Security architecture risk: Low · up to ee96a

The change reduces confusion between registered paths and similarly named siblings without adding removal privileges or weakening the reviewed cleanup checks. No introduced security concern was substantiated. Concurrent path replacement and cross-platform behavior remain unvalidated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The relevant exposure is the caller's repository and its registered worktrees, including registered paths outside the configured base directory, plus associated branch references. The change improves exact-path selection within that existing scope rather than introducing broader filesystem authority.

Trust Boundaries and Controls

  • observed — Cleanup preserves the active-worktree check and dirty/untracked-file checks unless force is requested. For PR/MR cleanup, branch identity and commit tip are revalidated after pre-remove hooks; force does not bypass that validation. Branch deletion uses the previously validated tip and does not dereference symbolic references.

Resilience and Maintainability Implications

  • inferred — Removal failure prevents subsequent branch deletion, and failed post-hook identity validation skips removal. Interruption can still leave partial cleanup, such as a removed worktree with its branch retained or an unlocked phantom awaiting pruning. Filesystem and registry changes are not covered by one transaction; these recovery limitations predate the PR and were not shown to worsen.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: preserving trailing newlines in registered worktree paths.
Docstring Coverage Passed Docstring coverage is 94.12% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 94.12% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 17 functions across 9 files.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks each path with care
No newline vanishes in air
The branches point where they belong
The list keeps every byte along
Then hops away, content and strong

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 26627fb and ee96aff.

Files selected for processing: 9

  • lib/commands/clean.sh
  • lib/commands/create.sh
  • lib/commands/list.sh
  • lib/core.sh
  • tests/cmd_clean.bats
  • tests/cmd_go.bats
  • tests/cmd_list.bats
  • tests/core_resolve_target.bats
  • tests/sparse.bats

Included review availability: This review used your included allowance. Your plan provides up to 100 included reviews per hour; 95 remain after this review.

@shiqui
shiqui merged commit 3fad7d4 into main Nov 6, 2026
5 checks passed
@shiqui
shiqui deleted the codex/fix-registry-path-newlines branch November 6, 2026 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant