Document edge cases for /run-skipped-ci workflow

[justin808]

Summary

• Documents edge cases for the /run-skipped-ci workflow in CONTRIBUTING.md
• Adds important notes about force-push and branch deletion behavior

Closes #2002

Test plan

• Documentation only change - no functional changes
• Verify markdown renders correctly in GitHub

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation
  • Added "Important Notes" to contribution guidelines covering CI behavior: how force-pushes can affect runs and a warning against deleting or force-pushing branches while workflows run.
  • Expanded developer install guidance for testing the generator: detailed steps to link the local library, run the installer in the test app, and publish/add the npm package locally to ensure proper installation and linking for tests.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds documentation to CONTRIBUTING.md: an "Important Notes" subsection under CI control commands documenting that /run-skipped-ci runs against the commit SHA at comment time (force-push caveat) and warning against deleting or force-pushing branches while workflows run. Also expands Install Generator guidance with step-by-step gem and yalc instructions.

Changes

Cohort / File(s)	Summary
Documentation: CONTRIBUTING updates CONTRIBUTING.md	Adds "Important Notes" under CI control commands explaining force-push behavior and branch-operation warnings for /run-skipped-ci. Expands Install Generator instructions with explicit steps: add gem path, run bundle, run generator, publish npm with yalc, and add the package to the test app.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

• Pure documentation changes; no code/signature edits.
• Review focus: accuracy and clarity of CI and install steps.

Possibly related PRs

• Fix generator tests #1706 — Related to generator testing and yalc-based linking for dev workflows.
• Simplify PR welcome messages #2009 — Related to CI control flow and messaging around /run-skipped-ci and full-ci label.
• Add PR comment command to trigger full CI suite and fix workflow issues #1974 — Related documentation updates for /run-skipped-ci and install-generator guidance.

Suggested reviewers

• Judahmeek
• AbanoubGhadban

Poem

🐰 I hopped through the docs and left a bright clue,
About force-pushes and branches — mind what you do!
Run the installer, link with yalc in the test,
Let CI finish its work — then push with zest.
Happy builds and green checks — a rabbit's review!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The PR includes substantial changes to Install Generator guidance in CONTRIBUTING.md that are unrelated to the linked issue #2002 about /run-skipped-ci edge cases.	Remove the Install Generator changes or address them in a separate PR focused on generator documentation improvements.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly summarizes the main change: documenting edge cases for the /run-skipped-ci workflow, which matches the primary objective and content of the PR.
Linked Issues check	✅ Passed	The PR successfully documents both edge cases from issue #2002: force-pushed PRs and branch deletion behavior with clear explanatory notes in CONTRIBUTING.md.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch jg/doc-run-skipped-ci

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26d5d1c and 84c3f9f.

📒 Files selected for processing (1)

• CONTRIBUTING.md (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• CONTRIBUTING.md

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: claude-review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review ✅

This is a documentation-only PR that adds important clarifications to the /run-skipped-ci workflow usage. The changes are excellent and accurately describe the workflow behavior.

✅ Strengths

1. Accurate technical documentation: The notes correctly describe how the workflow behaves:

   • Line 83-84 in .github/workflows/run-skipped-ci.yml shows the workflow captures pr.data.head.sha at the time of comment
   • This confirms the "Force-pushes" note is technically accurate

2. Addresses real edge cases: Based on issue CI: Document edge cases for /run-skipped-ci workflow #2002, these are actual scenarios that could confuse users

3. Clear and concise: The warning messages are brief but informative

4. Good placement: The notes are positioned right after the /stop-run-skipped-ci section, making them easy to find

📝 Minor Suggestions (Optional)

While the current documentation is clear, consider these minor enhancements for even better clarity:

1. Add a practical example to make the edge case more concrete:

   - **Force-pushes:** The workflow runs against the commit SHA at the time of the comment. If you force-push after commenting, the workflow will test the old commit. 
     *Example: Comment on commit `abc123`, then force-push `def456` → CI tests `abc123`. Solution: Comment `/run-skipped-ci` again.*

2. Cross-reference the workflow behavior to help advanced users:

   - **Force-pushes:** The workflow runs against the commit SHA at the time of the comment (captured in `.github/workflows/run-skipped-ci.yml:83`). If you force-push after commenting, the workflow will test the old commit. Comment again to test the new commit.

However, these are truly optional - the current documentation is already clear and follows the project's style of being concise.

🎯 Conclusion

LGTM (Looks Good To Me) ✅

This PR:

• ✅ Is documentation-only (no functional changes)
• ✅ Accurately describes workflow behavior
• ✅ Addresses a real user confusion scenario
• ✅ Follows the project's documentation style
• ✅ Closes issue CI: Document edge cases for /run-skipped-ci workflow #2002

The markdown will render correctly in GitHub, and the changes align perfectly with the project's contributing guidelines.

---

Review performed by Claude Code

[claude]

Code Review

Overview

This PR adds important documentation about edge cases for the /run-skipped-ci workflow. The changes are clear, helpful, and align well with the repository's documentation standards.

✅ Strengths

1. Addresses Real User Confusion: The documented edge cases directly address the scenarios outlined in issue CI: Document edge cases for /run-skipped-ci workflow #2002, particularly around force-pushes and branch operations during CI runs.

2. Well-Positioned: The "Important Notes" section is placed appropriately after the command documentation, making it easy to find when users are learning about the workflow.

3. Clear and Concise: The explanations are brief but informative, using bold labels to highlight key concepts.

📝 Suggestions for Improvement

1. Force-Push Explanation Could Be More Accurate

The current text states:

"If you force-push after commenting, the initial workflow run will test the old commit, but subsequent pushes will automatically run full CI because the label persists."

Issue: This might slightly confuse users about the behavior. The wording "subsequent pushes will automatically run full CI" could be misread as referring to the same workflow run continuing.

Suggested rewording:

- **Force-pushes:** The `/run-skipped-ci` command adds the `full-ci` label to your PR and triggers workflows against the commit SHA at the time of the comment. If you force-push after commenting, the initial workflow run will still test the old commit. However, the `full-ci` label persists, so any subsequent pushes will automatically trigger full CI on the new commits.

This makes it clearer that:

• The label addition is the key mechanism
• The initial run tests the old SHA
• The label ensures future pushes get full CI

2. Consider Adding a "When This Matters" Context

The current documentation tells users to avoid force-pushing during workflow runs, but doesn't explain when this actually becomes a problem. Consider adding:

- **Branch operations:** Avoid deleting or force-pushing branches while workflows are running, as this may cause checkout failures. Wait for workflows to complete before performing branch operations, or be prepared to re-run the `/run-skipped-ci` command on the new commit.

This addition:

• Explains the actual failure mode ("checkout failures" vs just "failures")
• Provides a recovery path (re-run the command)

3. Minor: Consider Cross-Referencing Related Documentation

The PR description mentions this closes #2002, which has excellent context about these edge cases. Consider adding a reference for maintainers:

#### Important Notes

<!-- See issue #2002 for detailed edge case analysis -->

- **Force-pushes:** ...
- **Branch operations:** ...

🎯 Alignment with CLAUDE.md Guidelines

• ✅ Documentation placed appropriately (top-level CONTRIBUTING.md is correct)
• ✅ Uses clear markdown formatting
• ✅ Concise and focused on user needs
• ✅ Follows existing documentation patterns in the file

🧪 Testing Verification

As noted in the test plan, this is a documentation-only change with no functional changes. The markdown renders correctly (verified via the diff).

Final Verdict

LGTM with minor suggestions ✨

The core documentation is solid and addresses a real user need. The suggestions above are optional refinements to make the edge cases even clearer, but the PR is mergeable as-is.

Great work documenting these edge cases before they confuse users! This is exactly the kind of proactive documentation that prevents support questions.

---

Review conducted with attention to CLAUDE.md guidelines and React on Rails documentation standards.