Skip to content

fix(copy): bound includeDirs discovery - #191

Merged
helizaga merged 3 commits into
mainfrom
tommy/fix-include-dirs-find
Aug 12, 2026
Merged

fix(copy): bound includeDirs discovery#191
helizaga merged 3 commits into
mainfrom
tommy/fix-include-dirs-find

Conversation

@helizaga

@helizaga helizaga commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

  • resolve literal gtr.copy.includeDirs paths with direct filesystem checks instead of full-tree find walks
  • bound single-star directory globs to their explicit path depth while keeping ** recursive
  • preserve legacy recursive basename matching while batching all unavoidable fallbacks into one scan
  • resolve directory patterns once per source so git gtr copy --all does not repeat discovery for every target
  • document the glob behavior and add regression coverage for scan avoidance and --all

Root cause

copy_directories ran an unbounded find for every slash-containing pattern and for missing shallow basename patterns. git gtr copy --all repeated that discovery for each target, multiplying the cost in repositories with large dependency trees or nested worktrees.

Literal paths now take the O(1) path and explicitly bounded globs only inspect the depth they can match. Bare basenames retain their legacy recursive fallback when no root match exists, but multiple fallbacks share one full-tree walk.

Validation

  • shellcheck bin/gtr bin/git-gtr lib/*.sh lib/commands/*.sh adapters/editor/*.sh adapters/ai/*.sh
  • ./scripts/generate-completions.sh --check
  • /bin/bash -n lib/copy.sh lib/commands/copy.sh lib/commands/help.sh
  • Bash 3.2 runtime smoke for nested literal and legacy basename resolution and copying
  • bats tests/copy_safety.bats (46/46)
  • bats tests/cmd_copy.bats (13/13)
  • all BATS suites except tests/cmd_clean.bats (456/456)
  • GitHub Actions bats tests/ against current main (490/490)

On local macOS, the full suite stops on cmd_clean --merged uses nested registered worktree path; the same assertion fails on untouched main and is unrelated to this change. The full suite passes in GitHub Actions.

Closes #189

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1cfcb603-70b7-479e-83ab-20d5a041a966

📥 Commits

Reviewing files that changed from the base of the PR and between 4a11d4a and 587bebb.

📒 Files selected for processing (3)
  • docs/configuration.md
  • lib/copy.sh
  • tests/copy_safety.bats
🚧 Files skipped from review as they are similar to previous changes (2)
  • docs/configuration.md
  • lib/copy.sh

Walkthrough

Directory include patterns now resolve once from the source with bounded matching. Resolved directories are reused for each target worktree. Documentation and tests cover literal, single-wildcard, and recursive patterns.

Changes

Directory copy resolution

Layer / File(s) Summary
Pattern resolution and matching
lib/copy.sh, tests/copy_safety.bats, docs/configuration.md, lib/commands/help.sh
Directory patterns resolve relative to the source. Literal paths avoid find; * and ? searches use bounded depth; ** searches recursively.
Resolved directory copy flow
lib/commands/copy.sh, lib/copy.sh, tests/cmd_copy.bats
cmd_copy resolves include directories once, then reuses them for each target while preserving copy behavior and dry-run handling.
Configuration examples
README.md
The includeDirs example includes packages/*/generated.

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

Possibly related PRs

Suggested reviewers: natoboram, averyjennings

Poem

I hop through paths with careful feet,
Find each directory, neat and sweet.
One search prepares the worktree row,
Then many targets copy and go.
Recursive stars now guide my way. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #189 by bounding glob discovery, using direct literal checks, preserving recursive patterns, and resolving directories once per source.
Out of Scope Changes check ✅ Passed The implementation, documentation, and regression tests are directly related to the includeDirs discovery objectives in issue #189.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: limiting discovery for includeDirs during copy operations.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tommy/fix-include-dirs-find

Comment @coderabbitai help to get the list of available commands.

@helizaga
helizaga merged commit f97ac02 into main Aug 12, 2026
4 checks passed
@helizaga
helizaga deleted the tommy/fix-include-dirs-find branch August 12, 2026 08:13
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.

copy_directories: unbounded find from repo root for includeDirs patterns (127x slowdown from one slash pattern)

1 participant