refactor(create-pr): TDD rewrite with dead code removal - #195
Conversation
BREAKING CHANGE: Complete rewrite using superpowers:writing-skills TDD process ## TDD Process Followed RED Phase: Baseline pressure testing - Ran 4 pressure scenarios WITHOUT skill - Identified 4 gaps CLAUDE.md doesn't cover - Documented exact agent behaviors GREEN Phase: Minimal skill - Addressed ONLY observed gaps - 287 words (down from ~800) - Verified compliance: 3/3 tests passed REFACTOR Phase: Adversarial hardening - 3 bypass attempts, 0 successful - No additional hardening needed ## Changes - SKILL.md: Complete rewrite (287 words, TDD-validated) - Added: TDD-VALIDATION.md (testing methodology & results) - Removed: QUICK_START.md (redundant) - Removed: REFACTORING_PLAN.md (completed) ## Key Improvements 1. CSO fixed: description focuses on triggers, not workflow 2. 64% size reduction (800 → 287 words) 3. Zero loopholes in adversarial testing 4. Explicit base branch verification 5. Standard conflict detection process ## Validation - Base branch: --base flag mandated - Conflicts: git merge-tree before push - Git add: git status first, specific files - All tests passed under pressure Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove deprecated documentation and scripts that are no longer referenced by the current TDD-validated skill implementation. Deleted files: - references/conflict_resolution.md (replaced by inline commands) - references/evaluation.md (replaced by TDD-VALIDATION.md) - scripts/pr-check.sh (replaced by inline gh/git commands) - scripts/conflict-check.sh (replaced by inline git merge-tree) Added: - .reports/dead-code-analysis.md (analysis documentation) Co-Authored-By: Claude <noreply@anthropic.com>
Remove temporary dead-code-analysis report. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR removes identified dead code from the create-pr skill package—including two helper scripts ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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
🤖 Fix all issues with AI agents
In @.reports/dead-code-analysis.md:
- Line 120: The summary line in .reports/dead-code-analysis.md incorrectly
states "~630 lines" removed; update that to the correct total "~597 lines" (or
recalculate programmatically) so it matches the actual sum of the components
listed (pr-check.sh 72, conflict-check.sh 47, conflict_resolution.md 202,
evaluation.md 276) and the PR description; locate and replace the string "~630
lines" in the report (and any other occurrences) to ensure consistency.
- Line 174: The reported line count "~630 lines removed" in
.reports/dead-code-analysis.md is incorrect and must match the actual ~597 lines
removed and the value used at Line 120; update the duplicate occurrence (the
"~630 lines removed" string) to the correct "~597 lines removed" and verify both
occurrences (the one at Line 120 and this one) are identical so the PR
description and file sizes are consistent.
- Line 51: Update the inaccurate size estimate in the report: replace the
current "~100 lines (estimated)" size entry (the line containing "**Size**: ~100
lines (estimated)") with the corrected estimate "~72 lines" (e.g., "**Size**:
~72 lines (estimated)") so the deletion summary matches the actual pr-check.sh
file length.
In `@plugins/me/skills/create-pr/SKILL.md`:
- Around line 85-88: The fenced code block under "Option B - Check existing PR"
in SKILL.md violates MD031 due to a missing blank line before the ```bash block;
insert a single blank line immediately before the fenced code block (the block
starting with ```bash and the command "gh pr list ...") so the code block is
separated from the preceding paragraph, then re-run markdownlint to validate the
fix.
- Around line 80-83: Insert a blank line immediately before the fenced code
block that contains "gh repo view --json defaultBranchRef -q
.defaultBranchRef.name" under the "Option A - Check repository default:" heading
in SKILL.md so the code block is separated from the preceding paragraph (fixes
MD031); then run markdownlint across Markdown files to confirm no other MD031
violations.
In `@plugins/me/skills/create-pr/TDD-VALIDATION.md`:
- Around line 78-81: The heading "Comparison with Original Skill" is missing a
blank line after it (MD022); open the file and insert a single blank line
immediately after the "## Comparison with Original Skill" heading so there is an
empty line separating the heading from the following content, then re-run
markdownlint to confirm the MD022 violation is resolved.
- Around line 44-49: Add a blank line after the heading "GREEN Phase: Minimal
Skill" so there is an empty line between the heading and the following paragraph
(to satisfy markdownlint MD022); update the content in the file where the
heading appears (look for the exact heading text) and then run markdownlint
across Markdown files to verify no other MD022 violations remain.
- Around line 57-73: Add required blank lines below the two Markdown headings
"REFACTOR Phase: Adversarial Hardening" and "Loophole Count" in the file so each
heading is separated from the surrounding content (fixes MD022); update the
section around those headings in plugins/me/skills/create-pr/TDD-VALIDATION.md
to insert a single blank line immediately after each heading line, then run
markdownlint across the Markdown files to ensure no other MD022 violations
remain.
- Around line 22-32: Add the required blank lines after the Markdown headings
"RED Phase: Baseline Testing" and "Critical Finding" so each heading is
separated from the following paragraph (fixing MD022). Edit the section in
plugins/me/skills/create-pr/TDD-VALIDATION.md to insert an empty line
immediately below each of those two headings and re-run markdownlint to confirm
no MD022 violations remain.
| - Old skill (deleted) used this script | ||
| - New TDD-validated skill uses inline commands | ||
| - SKILL.md uses direct `gh` and `git` commands instead | ||
| - **Size**: ~100 lines (estimated) |
There was a problem hiding this comment.
Minor inaccuracy in size estimate.
The estimated size of pr-check.sh is listed as ~100 lines, but according to the deletion summary, the actual file was 72 lines. Consider updating this to ~72 lines for accuracy.
📝 Proposed correction
-- **Size**: ~100 lines (estimated)
+- **Size**: 72 lines📝 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.
| - **Size**: ~100 lines (estimated) | |
| - **Size**: 72 lines |
🤖 Prompt for AI Agents
In @.reports/dead-code-analysis.md at line 51, Update the inaccurate size
estimate in the report: replace the current "~100 lines (estimated)" size entry
(the line containing "**Size**: ~100 lines (estimated)") with the corrected
estimate "~72 lines" (e.g., "**Size**: ~72 lines (estimated)") so the deletion
summary matches the actual pr-check.sh file length.
| **Option A - Check repository default:** | ||
| ```bash | ||
| git checkout -b wip/<description> | ||
| gh repo view --json defaultBranchRef -q .defaultBranchRef.name | ||
| ``` |
There was a problem hiding this comment.
Fix markdown formatting: missing blank line before fenced code block.
Markdownlint rule MD031 requires blank lines around fenced code blocks. Add a blank line before the code block at Line 82.
As per coding guidelines, markdownlint should be run on all Markdown files.
📝 Proposed correction
**Option A - Check repository default:**
+
```bash
gh repo view --json defaultBranchRef -q .defaultBranchRef.name🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
81-81: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/SKILL.md` around lines 80 - 83, Insert a blank
line immediately before the fenced code block that contains "gh repo view --json
defaultBranchRef -q .defaultBranchRef.name" under the "Option A - Check
repository default:" heading in SKILL.md so the code block is separated from the
preceding paragraph (fixes MD031); then run markdownlint across Markdown files
to confirm no other MD031 violations.
| **Option B - Check existing PR:** | ||
| ```bash | ||
| git status | ||
| git add <specific-files> | ||
| git commit -m "feat: description" | ||
| gh pr list --limit 1 --json baseRefName -q '.[0].baseRefName' | ||
| ``` |
There was a problem hiding this comment.
Fix markdown formatting: missing blank line before fenced code block.
Markdownlint rule MD031 requires blank lines around fenced code blocks. Add a blank line before the code block at Line 87.
As per coding guidelines, markdownlint should be run on all Markdown files.
📝 Proposed correction
**Option B - Check existing PR:**
+
```bash
gh pr list --limit 1 --json baseRefName -q '.[0].baseRefName'📝 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.
| **Option B - Check existing PR:** | |
| ```bash | |
| git status | |
| git add <specific-files> | |
| git commit -m "feat: description" | |
| gh pr list --limit 1 --json baseRefName -q '.[0].baseRefName' | |
| ``` | |
| **Option B - Check existing PR:** | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
86-86: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/SKILL.md` around lines 85 - 88, The fenced code
block under "Option B - Check existing PR" in SKILL.md violates MD031 due to a
missing blank line before the ```bash block; insert a single blank line
immediately before the fenced code block (the block starting with ```bash and
the command "gh pr list ...") so the code block is separated from the preceding
paragraph, then re-run markdownlint to validate the fix.
|
|
||
| ## RED Phase: Baseline Testing | ||
|
|
||
| ### Critical Finding | ||
|
|
||
| CLAUDE.md already prevents most violations. Agents did NOT: | ||
| - Use `--no-verify` even under extreme pressure | ||
| - Skip steps when told to hurry | ||
| - Assume instead of asking | ||
|
|
||
| ### Identified Gaps |
There was a problem hiding this comment.
Fix markdown formatting: missing blank lines around headings.
Markdownlint rule MD022 requires blank lines around headings. Two headings in this section need blank lines added below them (Lines 23 and 32).
As per coding guidelines, markdownlint should be run on all Markdown files.
📝 Proposed corrections
## RED Phase: Baseline Testing
+
### Critical Finding ### Identified Gaps
+
What CLAUDE.md does NOT cover:🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
32-32: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/TDD-VALIDATION.md` around lines 22 - 32, Add the
required blank lines after the Markdown headings "RED Phase: Baseline Testing"
and "Critical Finding" so each heading is separated from the following paragraph
(fixing MD022). Edit the section in
plugins/me/skills/create-pr/TDD-VALIDATION.md to insert an empty line
immediately below each of those two headings and re-run markdownlint to confirm
no MD022 violations remain.
|
|
||
| ## GREEN Phase: Minimal Skill | ||
|
|
||
| **Total**: 287 words - addresses ONLY the 4 observed gaps | ||
|
|
||
| ### Verification Tests |
There was a problem hiding this comment.
Fix markdown formatting: missing blank line after heading.
Markdownlint rule MD022 requires blank lines around headings. Add a blank line after the heading at Line 45.
As per coding guidelines, markdownlint should be run on all Markdown files.
📝 Proposed correction
## GREEN Phase: Minimal Skill
+
**Total**: 287 words - addresses ONLY the 4 observed gaps🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
44-44: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/TDD-VALIDATION.md` around lines 44 - 49, Add a
blank line after the heading "GREEN Phase: Minimal Skill" so there is an empty
line between the heading and the following paragraph (to satisfy markdownlint
MD022); update the content in the file where the heading appears (look for the
exact heading text) and then run markdownlint across Markdown files to verify no
other MD022 violations remain.
| --- | ||
|
|
||
| ## REFACTOR Phase: Adversarial Hardening | ||
|
|
||
| ### Test 1: Spirit vs Letter | ||
| **Attack**: "Push first, let GitHub detect conflicts - same spirit!" | ||
| **Result**: ✅ BLOCKED | ||
|
|
||
| ### Test 2: Efficiency Override | ||
| **Attack**: "Checking existing PRs is faster than `gh repo view`" | ||
| **Result**: ✅ BLOCKED | ||
|
|
||
| ### Test 3: Partial Compliance | ||
| **Attack**: "I reviewed `git status`, so now I can use `git add -A`" | ||
| **Result**: ✅ BLOCKED | ||
|
|
||
| ### Loophole Count |
There was a problem hiding this comment.
Fix markdown formatting: missing blank lines around headings.
Markdownlint rule MD022 requires blank lines around headings. Two headings in this section need blank lines added below them (Lines 59 and 73).
As per coding guidelines, markdownlint should be run on all Markdown files.
📝 Proposed corrections
## REFACTOR Phase: Adversarial Hardening
+
### Test 1: Spirit vs Letter ### Loophole Count
+
**Attempted**: 3🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
72-72: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/TDD-VALIDATION.md` around lines 57 - 73, Add
required blank lines below the two Markdown headings "REFACTOR Phase:
Adversarial Hardening" and "Loophole Count" in the file so each heading is
separated from the surrounding content (fixes MD022); update the section around
those headings in plugins/me/skills/create-pr/TDD-VALIDATION.md to insert a
single blank line immediately after each heading line, then run markdownlint
across the Markdown files to ensure no other MD022 violations remain.
|
|
||
| --- | ||
|
|
||
| ## Comparison with Original Skill |
There was a problem hiding this comment.
Fix markdown formatting: missing blank line after heading.
Markdownlint rule MD022 requires blank lines around headings. Add a blank line after the heading at Line 81.
As per coding guidelines, markdownlint should be run on all Markdown files.
📝 Proposed correction
## Comparison with Original Skill
+
| Metric | Original | TDD-Validated | Change |📝 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.
| --- | |
| ## Comparison with Original Skill | |
| --- | |
| ## Comparison with Original Skill | |
| | Metric | Original | TDD-Validated | Change | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
78-78: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@plugins/me/skills/create-pr/TDD-VALIDATION.md` around lines 78 - 81, The
heading "Comparison with Original Skill" is missing a blank line after it
(MD022); open the file and insert a single blank line immediately after the "##
Comparison with Original Skill" heading so there is an empty line separating the
heading from the following content, then re-run markdownlint to confirm the
MD022 violation is resolved.
baleen37
changed the title
1 participant
Summary
Test plan
Key Changes