-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: improve using-git-worktrees with pre-flight checks and TDD tests #288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: improve using-git-worktrees with pre-flight checks and TDD tests #288
Conversation
Addresses nested worktree creation and path resolution issues. Enhancements: - Pre-Flight Checks section: detect if in worktree, detect bare repo - Absolute path requirement in Create Worktree section - Updated Quick Reference with new situations - Updated Red Flags with explicit counters Prevents production failures: - Nested worktree at .worktrees/fix/.worktrees/main - Wrong location from relative paths - Confusion with bare repositories Co-Authored-By: Claude <noreply@anthropic.com>
Provides reusable test scenarios for: - Nested worktree creation prevention (test-nested-worktrees.md) - Absolute path enforcement (test-path-resolution.md) - Bare repository handling (test-bare-repos.md) Includes CREATION-LOG.md documenting RED-GREEN-REFACTOR process followed to address post-mortem failures with TDD methodology. Each test includes: - Pressure scenario setup - Expected baseline failure - Expected success with skill - Verification steps - Relation to production incidents Co-Authored-By: Claude <noreply@anthropic.com>
Enhanced description with symptom keywords (nested worktrees, wrong paths, bare repository confusion) for better skill discovery. Removed workflow summary from description per writing-skills CSO guidance: descriptions should only describe WHEN to use (triggers/symptoms), not WHAT the skill does (workflow). Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds comprehensive documentation for the git-worktrees skill: a CREATION-LOG describing a TDD approach, an expanded SKILL.md with pre-flight checks and absolute-path handling, and three new test/pressure documents for nested worktrees, path resolution, and bare-repo scenarios. Changes
Sequence Diagram(s)(omitted — changes are documentation and tests; no new multi-component control flow to visualize) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@skills/using-git-worktrees/SKILL.md`:
- Around line 20-29: Replace the brittle root-only check that uses `[ -f .git ]`
with a root-independent check using `git rev-parse --git-dir` (capture its
output into a variable like `git_dir`) and test whether that path is a file to
detect a worktree; if it is a file, derive the main repository path from
`git_dir` (strip the trailing `/.git`) into `main_repo` and `cd "$main_repo"` as
before. Update the block that currently defines `main_repo` and checks `.git` to
use `git_dir=$(git rev-parse --path-format=absolute --git-dir 2>/dev/null)` then
`if [ -f "$git_dir" ]; then ...` so the detection works from any subdirectory.
In `@skills/using-git-worktrees/test-path-resolution.md`:
- Around line 56-66: The pre-flight check using a file test on .git is fragile;
replace it with a root-independent comparison of git directories by calling git
rev-parse --path-format=absolute --git-dir and git rev-parse
--path-format=absolute --git-common-dir, assign them to git_dir and
git_common_dir, and if they differ emit a warning (e.g., "Already in a worktree.
Navigate to main repo first.") so the doc/example reliably detects being inside
a worktree from any subdirectory.
Replace fragile [ -f .git ] check with robust git-dir vs git-common-dir comparison. The old method only worked at repository root; new method works reliably from any subdirectory. Also corrected test documentation that mischaracterized the limitation as intended behavior. Addresses CodeRabbit feedback on PR obra#288. Co-Authored-By: Claude <noreply@anthropic.com>
Replace fragile [ -f .git ] check with robust git-dir vs git-common-dir comparison. The old method only worked at repository root; new method works reliably from any subdirectory. Also corrected test documentation that mischaracterized the limitation as intended behavior. Addresses CodeRabbit feedback on PR obra#288. Co-Authored-By: Claude <noreply@anthropic.com>
|
Can you please share a human-written description of what problem you were having and how these changes you made solved the problem? Also, the test documents don't look a lot like our standard testing methodology. Can you talk me through what was going on there? |
Skill(using-git-worktrees) Improvements
Summary
Adds pre-flight checks, TDD test scenarios, and enhanced CSO description to using-git-worktrees skill based on production incident learnings.
Problem
Production failures revealed three gaps in the using-git-worktrees skill:
.worktrees/feature/created worktree at.worktrees/feature/.worktrees/new/instead of.worktrees/new/../.worktrees/from within worktree caused wrong-location creationgit pullin bare repo root, unclear about primary workspace locationSolution
1. Pre-Flight Checks (commit 1)
.gitfile vs directory)2. TDD Test Scenarios (commit 2)
test-nested-worktrees.md- Pressure test for nested creationtest-path-resolution.md- Verify absolute path enforcementtest-bare-repos.md- Bare repository handlingCREATION-LOG.md- Documents TDD process (RED-GREEN-REFACTOR)3. Enhanced CSO Description (commit 3)
Testing
All improvements validated with pressure testing:
Impact
Files Changed
skills/using-git-worktrees/SKILL.md- Pre-flight checks + absolute paths + CSOskills/using-git-worktrees/test-nested-worktrees.md- New test scenarioskills/using-git-worktrees/test-path-resolution.md- New test scenarioskills/using-git-worktrees/test-bare-repos.md- New test scenarioskills/using-git-worktrees/CREATION-LOG.md- TDD process documentationSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.