Add PR comment command to trigger full CI suite and fix workflow issues #1974
Conversation
|
👋 Thanks for opening this PR! 🚀 Running Full CI SuiteBy default, PRs run a subset of CI jobs for faster feedback (latest Ruby/Node versions only). To run the complete CI suite including all dependency combinations and skipped jobs, comment: This will trigger:
The full CI suite takes longer but ensures compatibility across all supported versions before merging. |
WalkthroughAdds GitHub Actions docs, enables manual Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant PR as Pull Request
participant GH as GitHub Actions API
participant Orchestrator as run-skipped-ci
participant Workflows as CI Workflows
User->>PR: Posts "/run-skipped-ci" comment
PR->>GH: issue_comment event -> trigger run-skipped-ci
GH->>Orchestrator: Start job
Orchestrator->>GH: Check commenter write permission
GH-->>Orchestrator: Permission result
alt denied
Orchestrator->>PR: Post denial comment
else allowed
Orchestrator->>PR: Add reaction & acknowledgment comment
Orchestrator->>GH: Read PR head_ref & head_sha
par Dispatch workflows
Orchestrator->>Workflows: Dispatch main.yml (workflow_dispatch)
Orchestrator->>Workflows: Dispatch examples.yml (workflow_dispatch)
Orchestrator->>Workflows: Dispatch pro-integration-tests.yml (workflow_dispatch)
Orchestrator->>Workflows: Dispatch pro-package-tests.yml (workflow_dispatch)
end
Orchestrator->>GH: Verify dispatched runs exist with head_sha
GH-->>Orchestrator: Verification results
Orchestrator->>PR: Post summary comment (success / failures)
alt any failures
Orchestrator->>GH: Exit job with failure
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Code Review - PR 1974: Add PR Comment Command to Trigger Full CI SuiteThanks for this contribution! This is a well-thought-out approach to managing CI complexity. Strengths
Critical Issues That Must Be Fixed1. SECURITY: Missing Comment Author ValidationPROBLEM: Any user (including external contributors) can trigger full CI by commenting /run-full-ci LOCATION: .github/workflows/run-skipped-ci.yml lines 13-16 IMPACT: Resource abuse, potential DoS, external contributors triggering Pro package tests RECOMMENDATION: Add permission check step to validate comment author has write access. Only maintainers should trigger full CI. 2. Error Handling: Silent FailuresPROBLEM: Workflow triggers wrapped in try-catch only log to console. Users won't see if a workflow fails to trigger. LOCATION: .github/workflows/run-skipped-ci.yml lines 70-104 IMPACT: False confidence that full CI is running when it's not RECOMMENDATION: Consolidate trigger steps and post comment when workflows fail to trigger 3. Race Conditions: Multiple CommentsPROBLEM: Posting /run-full-ci multiple times triggers duplicate runs IMPACT: Wasted resources, higher costs, confusing results RECOMMENDATION: Add concurrency controls with PR number as group identifier Other Issues
VerifiedAll four referenced workflows (main.yml, examples.yml, pro-integration-tests.yml, pro-package-tests.yml) correctly have workflow_dispatch enabled. SummaryVERDICT: Good feature, but needs security hardening before merge MUST FIX: User permission validation, concurrency controls, error handling feedback RISK LEVEL: Medium (security issues important but feature itself is low-risk) Overall valuable addition - with security fixes will be great DX improvement! |
Code Review - PR #1974SummaryThis PR adds a useful feature that allows triggering the full CI suite via a ✅ Strengths
🔍 Issues & Recommendations1. Critical: Error Handling in Workflow TriggersIssue: The workflow trigger steps use # Current implementation
try {
await github.rest.actions.createWorkflowDispatch({...});
console.log('✅ Triggered main.yml');
} catch (error) {
console.error('❌ Failed to trigger main.yml:', error.message);
}Problem: If a workflow fails to trigger, the user sees "🚀 Running full CI suite" but some workflows silently don't run. Recommendation:
2. Potential Race Condition with Matrix JobsIssue: The workflows being triggered ( # From main.yml lines 52-54
- ruby-version: '3.2'
node-version: '20'
dependency-level: 'minimum'These jobs check Recommendation:
3. Welcome Comment Triggers on All PRsIssue: The Recommendation: on:
pull_request:
types: [opened]
jobs:
welcome:
# Skip for dependabot and other bots
if: github.event.pull_request.user.type \!= 'Bot'This prevents noise on automated PRs. 4. Minor: Incomplete Comment Body CheckIssue: The trigger condition uses if: contains(github.event.comment.body, '/run-full-ci')This would trigger on comments like "Don't use /run-full-ci yet" or code blocks containing the command. Recommendation: if: |
github.event.issue.pull_request &&
(
startsWith(github.event.comment.body, '/run-full-ci') ||
contains(github.event.comment.body, '\n/run-full-ci')
)This ensures the command is at the start of a line. 📋 Suggestions
🧪 Test CoverageN/A - GitHub Actions workflows don't have traditional tests. The PR correctly documents that testing requires merging to master first. Post-Merge Testing Checklist:
🔒 SecurityNo issues identified. The PR properly:
⚡ PerformanceNo concerns. The workflow is lightweight and only triggers when needed. 📖 Code Quality
🎯 Overall AssessmentThis is a well-designed feature that will improve the development workflow. The main concern is the matrix job conditionals (issue #2) which could make the feature less useful than intended. I recommend verifying the conditional logic before or immediately after merge. Recommendation: Approve with minor fixes suggested above. Great work on the comprehensive documentation! 🎉 |
🔒 Security & Reliability Fixes AppliedI've addressed the critical security and reliability issues with a second commit: Security Enhancements✅ Permission Validation
✅ Concurrency Protection
Reliability Improvements✅ Better Error Handling
✅ Improved Command Matching
✅ Workflow Dispatch Support
✅ Bot Filtering
Documentation UpdatesThe
Note: These fixes won't be testable until the PR is merged to master, since |
This adds a `/run-full-ci` command that can be used in PR comments to run the complete CI suite including tests normally skipped on PRs. **New workflows:** - `run-skipped-ci.yml` - Triggers on `/run-full-ci` comment, dispatches all CI workflows - `pr-welcome-comment.yml` - Auto-comments on new PRs explaining the command **Features:** - Runs full test matrix (all Ruby/Node versions) - Runs example generator tests - Runs Pro package integration and unit tests - Posts acknowledgment comment with workflow links - Adds rocket reaction to trigger comment **Documentation:** - Added `.github/README.md` with comprehensive docs - Explains why subset CI exists (fast feedback) - Documents testing limitations (must be on master branch) - Lists all available workflows and their purposes This improves the PR workflow by letting contributors easily run full CI on-demand without waiting for merge to master. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This addresses critical security and reliability issues identified in the initial implementation of the /run-full-ci comment trigger. **Security Fixes:** - Add permission check to restrict command to users with write access - Prevents resource abuse from external contributors - Posts informative message to unauthorized users **Reliability Improvements:** - Consolidate workflow triggers with better error handling - Post detailed results comment listing succeeded/failed workflows - Fail job if any workflows fail to trigger - Add concurrency controls to prevent duplicate runs per PR - Improve comment matching to require command at line start **Workflow Updates:** - Update main.yml, examples.yml to run minimum deps on workflow_dispatch - Update Pro workflows to honor workflow_dispatch events - Skip welcome comment for bot PRs (dependabot, renovate) **Documentation:** - Document security controls and access restrictions - Document concurrency protection - Explain workflow_dispatch behavior These changes ensure the feature is secure, reliable, and provides clear feedback when workflows fail to trigger. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The previous approach using matrix.exclude with matrix context expressions caused actionlint errors because the matrix context is not available in the exclude section. **Fix:** - Move conditional logic to job-level if statements - Check both event type and dependency level in single expression - For regular PRs: only run latest versions - For master/workflow_dispatch: run all versions including minimum This properly filters matrix jobs without using unsupported contexts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
justin808
force-pushed
the
justin808/ci-comment-trigger
branch
from
4a91920 to
67ea4b4
Compare
✅ Actionlint Fixes AppliedFixed the actionlint errors that were failing CI. The issue was using Solution:
The workflows will now:
This should resolve the actionlint errors in CI. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/README.md (1)
11-13: Add language specifier to code fence.The Markdown linter requires a language specifier for code fences.
-``` +``` /run-full-ci -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/README.md(1 hunks).github/workflows/examples.yml(1 hunks).github/workflows/main.yml(3 hunks).github/workflows/pr-welcome-comment.yml(1 hunks).github/workflows/pro-integration-tests.yml(3 hunks).github/workflows/pro-package-tests.yml(3 hunks).github/workflows/run-skipped-ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
.github/workflows/main.yml
🪛 LanguageTool
.github/README.md
[uncategorized] ~96-~96: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...check-markdown-links.yml`** - Validates markdown links ## Workflow Permissions Most wo...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.18.1)
.github/README.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (9)
.github/workflows/pro-integration-tests.yml (1)
37-37: ✅ Consistent workflow_dispatch support added.The workflow_dispatch event support is correctly integrated across all three jobs. The absence of minimum-version gating (unlike main.yml/examples.yml) is appropriate since the Pro workflows don't use a dependency-level matrix.
Also applies to: 127-127, 307-307
.github/workflows/examples.yml (1)
43-45: ✅ Gating logic correctly prevents minimum-version runs on PRs.The compound condition effectively ensures that minimum-version tests only run on master/workflow_dispatch, not on regular PRs. This aligns with the performance goal of providing fast feedback on PRs while maintaining full coverage on manual/default branch runs.
.github/workflows/pr-welcome-comment.yml (1)
1-37: ✅ PR welcome workflow looks good.The workflow correctly filters bots, uses appropriate permissions, and provides clear guidance on triggering full CI. The trigger configuration (
pull_request.types: [opened]) prevents redundant comments on edits..github/workflows/main.yml (1)
44-46: ✅ Consistent workflow_dispatch and gating logic.The conditions correctly gate minimum-version runs to master/workflow_dispatch scenarios. The pattern is consistent with examples.yml and well-commented.
Also applies to: 132-133
.github/workflows/pro-package-tests.yml (1)
37-37: ✅ workflow_dispatch support correctly added to all three jobs.The changes are consistent with the Pro integration tests workflow and enable proper orchestration via the full CI trigger.
Also applies to: 127-127, 208-208
.github/workflows/run-skipped-ci.yml (4)
15-20: ✅ Command parsing correctly balances safety and convenience.The dual-pattern matching (
startsWith+ newline-prefixed contains) effectively prevents false positives from inline mentions while allowing the command on a new line. The logic is sound for typical user behavior.Minor edge case: commands with leading whitespace on a new line (e.g., " /run-full-ci") won't match, but this is acceptable for a PR comment command.
27-61: ✅ Permission check implementation is secure and user-friendly.The workflow correctly validates write access before proceeding, posts a denial message for unauthorized users, and handles API errors gracefully. This prevents resource abuse while maintaining good UX.
99-154: ✅ Workflow dispatch orchestration with proper error aggregation.The implementation correctly:
- Dispatches all four workflows in a loop with individual try/catch handling
- Tracks succeeded/failed results
- Posts an aggregate summary comment
- Fails the orchestrator job if any dispatch fails (prevents silent failures)
This provides good visibility into which workflows were triggered and any dispatch failures.
8-10: ✅ Concurrency control prevents duplicate runs and resource waste.Using the PR number as the concurrency group with
cancel-in-progress: trueensures that multiple/run-full-cicomments on the same PR cancel prior runs rather than queuing them. This is the correct pattern.
|
@claude review this PR |
|
Claude finished @justin808's task —— View job PR Review: Add PR Comment Command to Trigger Full CI SuiteReviewing PR #1974 by @justin808 SummaryThis PR adds a ✅ Strengths
🔍 Issues & Recommendations1. Documentation Error - CriticalLocation: Issue: The README references - **`run-full-ci.yml`** - Triggered by `/run-full-ci` comment on PRsFix: - **`run-skipped-ci.yml`** - Triggered by `/run-full-ci` comment on PRs2. Markdown Linting Issue - MinorLocation: Issue: Code fence missing language specifier (triggers markdownlint warning) Current: /run-full-ci Fix: ```bash
/run-full-ciThis would require capturing the run IDs from the dispatch responses, which may not be immediately available from the API. 4. Matrix Job VerificationStatus: ✅ Verified correctly implemented The conditional logic for minimum versions is correct:
The double condition pattern prevents minimum versions on regular PRs while allowing them on workflow_dispatch: if: |
(github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.run_dummy_tests == 'true') &&
(matrix.dependency-level != 'minimum' || github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch')This is excellent - it ensures 🧪 Testing ConsiderationsThe PR correctly documents that testing requires merging to master first due to GitHub's security model. After merge, testing should verify:
📊 Code Quality
🔒 Security AssessmentNo security issues identified. The implementation properly:
🎯 Overall AssessmentThis is a well-designed and well-implemented feature that significantly improves the development workflow. The author has addressed all major concerns from previous reviews:
Recommendation✅ Approve with minor documentation fix The only required fix is updating the workflow filename in the README (issue #1 above). The markdown linting issue (issue #2) should also be fixed to keep CI clean. The workflow status links suggestion (issue #3) is optional but would improve UX. Great work on this feature! The security and reliability improvements in the second commit demonstrate excellent attention to detail. This will be a valuable addition to the development workflow. 🎉 |
Changes the PR comment command to match the workflow filename for consistency. **Changes:** - Update workflow trigger to listen for `/run-skipped-ci` - Update welcome comment to show `/run-skipped-ci` - Update README documentation to reference `/run-skipped-ci` - Update workflow filename reference in docs The command name now clearly indicates it runs the normally-skipped CI jobs (minimum versions, examples, Pro tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
📝 Command Renamed for ConsistencyChanged the PR comment command from Rationale:
All documentation and workflows have been updated consistently. |
The previous approach of using matrix context in job-level if statements caused actionlint errors. Fixed by using environment variables and step-level conditionals instead. **Solution:** - Set SKIP_MINIMUM env var at job level using matrix context - Add conditional `if: env.SKIP_MINIMUM != 'true'` to all steps - First step exits early with helpful message on regular PRs - Minimum versions only run on master or workflow_dispatch This approach is actionlint-compliant and properly filters matrix jobs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
✅ Actionlint Errors Fixed (For Real This Time)Fixed the actionlint errors properly using environment variables and step-level conditionals. The Problem:
The Solution:
How It Works Now:
This should pass all CI checks now! |
Code Review - PR #1974Thank you for this well-thought-out PR! The implementation of the ✅ Strengths1. Excellent Documentation
2. Security Implementation
3. User Experience
4. Concurrency Protection
|
- Remove duplicate if conditions in examples.yml and main.yml that caused knip YAML parsing errors - Replace SKIP_MINIMUM_ON_PR env var pattern with cleaner matrix-level exclude approach - Add workflow verification in run-skipped-ci.yml to confirm workflows actually start - Improve error handling with detailed status reporting showing verified/pending/failed workflows - Fix actionlint validation errors This eliminates inefficient step-level conditionals and ensures workflows are properly filtered at the job level, making them more efficient and easier to maintain. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Code Review - PR #1974: Add PR comment command to trigger full CI suiteOverviewThis is a well-designed PR that adds a practical feature for triggering full CI runs on demand via PR comments. The implementation follows GitHub Actions best practices and includes good documentation. ✅ Strengths1. Excellent Documentation
2. Security Considerations
3. Good UX Design
4. Solid Error Handling
🔍 Areas for Improvement1. Workflow Matrix Exclusion Logic (Medium Priority)The exclude:
- ruby-version: ${{ github.event_name == 'pull_request' && github.ref != 'refs/heads/master' && '3.2' || '' }}
node-version: ${{ github.event_name == 'pull_request' && github.ref != 'refs/heads/master' && '20' || '' }}
dependency-level: ${{ github.event_name == 'pull_request' && github.ref != 'refs/heads/master' && 'minimum' || '' }}Issue: When the condition is false, this produces empty strings ( Better approach: Use conditional matrix includes or separate jobs: # Option 1: Use if conditions on jobs
jobs:
test-latest:
# Always run
test-minimum:
if: github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch'Files affected:
2. Race Condition in Workflow Verification (Low Priority)In await new Promise(resolve => setTimeout(resolve, 5000));Issues:
Suggestion: Add retry logic with exponential backoff or increase timeout with retries. File: 3. Command Name Inconsistency (Low Priority)The workflow and command use Suggestion: Consider renaming to 4. Missing Edge Cases (Low Priority)Force-pushed PRs: If someone force-pushes after commenting Suggestion: Add a note in the documentation or consider checking if the PR head SHA changed. Deleted branches: If a PR's branch is deleted while workflows are running, they'll fail. Not a blocker, but worth documenting. 5. Pro Package Tests Matrix (Question)The pro workflows ( if: github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.run_pro_tests == 'true'This means pro tests run on PRs if Files: 🧪 Testing ConsiderationsConcern: Cannot Test Before MergeThe PR description correctly notes that Recommendation:
Missing Test CoverageThis feature cannot have automated tests due to the
🎯 Performance ConsiderationsPositive:
Suggestion: Monitor GitHub Actions minutes usage after this is deployed to ensure the feature isn't being overused. 📝 Code QualityStrengths:
Minor Suggestions:
🔐 Security ReviewOverall: Secure ✅
📋 Documentation QualityThe
Minor suggestion: Add examples of when to use vs not use the command:
🎬 Final Recommendation✅ Approve with minor suggestions This PR is well-implemented and ready to merge. The workflow matrix exclusion logic needs attention (point #1 above), but you can address that based on testing results. Action Items Before Merge:
Action Items After Merge:
Great work on this feature! The documentation and security considerations are particularly well done. 🚀 Review generated with Claude Code following React on Rails code review standards from CLAUDE.md |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/README.md (2)
11-13: Add language specifier to fenced code block.Per Markdown linting standards (MD040), fenced code blocks should specify a language.
-``` +```bash /run-skipped-ci -``` +```
96-96: Capitalize "Markdown" as a proper noun.-- **`check-markdown-links.yml`** - Validates markdown links +- **`check-markdown-links.yml`** - Validates Markdown links
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/README.md(1 hunks).github/workflows/examples.yml(1 hunks).github/workflows/main.yml(3 hunks).github/workflows/pr-welcome-comment.yml(1 hunks).github/workflows/run-skipped-ci.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/pr-welcome-comment.yml
- .github/workflows/examples.yml
🧰 Additional context used
🪛 LanguageTool
.github/README.md
[uncategorized] ~96-~96: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...check-markdown-links.yml`** - Validates markdown links ## Workflow Permissions Most wo...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.18.1)
.github/README.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (8)
- GitHub Check: lint-js-and-ruby
- GitHub Check: build
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
🔇 Additional comments (8)
.github/workflows/main.yml (3)
14-14: ✅ Workflow dispatch trigger properly added.The addition of
workflow_dispatch:enables manual triggering and orchestration by therun-skipped-ci.ymlworkflow, which aligns with the PR objectives.
41-59: Verify actionlint compliance of matrix exclude logic.The current matrix
excludepattern uses context expressions within matrix field values, which may conflict with actionlint validation that the PR objectives mention was resolved. Per the PR description, the final fix should use an environment variable set at job level with conditional step gating.Please confirm whether this exclude pattern is actionlint-compliant or if it should be refactored to set
SKIP_MINIMUMat the job level and gate steps accordingly as described in the PR objectives.
130-149: Verify actionlint compliance and consistency with first job.The second job (
dummy-app-integration-tests) applies the same matrix exclude pattern. Ensure consistency with any fixes applied to the first job, and confirm actionlint compliance..github/workflows/run-skipped-ci.yml (5)
15-20: ✅ Command detection logic properly prevents false positives.The condition correctly uses
startsWith()for line start andcontains('\n/run-skipped-ci')to detect the command after newlines, preventing accidental triggers inside phrases or code blocks.
27-61: ✅ Permission check appropriately gates access.The workflow properly validates write-access permissions using the GitHub API and denies unauthorized users with a courteous message. The try/catch error handling prevents silent failures.
84-189: Verify robustness of workflow verification timing.The workflow uses a 60-second search window (line 132) after a 5-second delay to verify triggered workflows. If workflow dispatch API latency or queuing exceeds this window, verified workflows may be missed, and the status message would show them as "not yet queued" even though they eventually run.
Consider whether the 60-second window is sufficient for your typical queue times, or if this should be configurable/more conservative.
136-140: ✅ Workflow verification checks are thorough.The verification correctly matches on workflow path, head SHA, and
workflow_dispatchevent type, ensuring triggered workflows correspond to the correct PR branch and dispatch action.
187-188: ✅ Job failure on workflow dispatch errors prevents silent failures.The job correctly fails if any workflows fail to trigger, ensuring visibility of problems rather than silently queuing partial CI runs.
justin808
changed the title
…es (#1974) ## Summary This PR adds a `/run-skipped-ci` command that can be used in PR comments to trigger the complete CI suite, including tests that are normally skipped on PRs for faster feedback. It also fixes several workflow issues identified during development. ## What Changed **New GitHub Actions Workflows:** - `.github/workflows/run-skipped-ci.yml` - Listens for `/run-skipped-ci` comments and triggers all CI workflows with verification - `.github/workflows/pr-welcome-comment.yml` - Automatically comments on new PRs explaining the command **New Documentation:** - `.github/README.md` - Comprehensive documentation of all workflows and the comment command feature **Workflow Fixes:** - Fixed duplicate `if` conditions in `examples.yml` and `main.yml` that caused YAML parsing errors - Replaced inefficient `SKIP_MINIMUM_ON_PR` env var pattern with cleaner matrix-level `exclude` approach - Added workflow verification logic to confirm triggered workflows actually start - Improved error handling with detailed status reporting (verified/pending/failed workflows) ## How It Works 1. **Default PR behavior** - Runs subset of CI (latest Ruby/Node only) for fast feedback 2. **Full CI on demand** - Comment `/run-skipped-ci` on any PR to run: - Main test suite with all Ruby/Node version combinations - Example generator tests - Pro package integration tests - Pro package unit tests 3. **User feedback** - Bot reacts with 🚀, waits 5 seconds for workflows to queue, verifies they started, and posts detailed status ## Why This Approach By default, PRs run a limited CI suite to keep feedback loops fast: - Only latest versions (Ruby 3.4, Node 22, Shakapacker 9.3.0, React 19) - Skips minimum versions (Ruby 3.2, Node 20, Shakapacker 8.2.0, React 18) Before merging, we want to verify compatibility across all supported versions. This command makes it easy to run full CI without: - Waiting for merge to master - Manually triggering individual workflows - Modifying workflow files ## Technical Improvements ### Matrix Filtering Before: ```yaml env: SKIP_MINIMUM_ON_PR: ${{ matrix.dependency-level == 'minimum' && ... }} steps: - uses: actions/checkout@v4 if: env.SKIP_MINIMUM_ON_PR != 'true' # ❌ Step-level conditional ``` After: ```yaml matrix: include: [...] exclude: # ✅ Job-level filtering - more efficient - ruby-version: ${{ ... && '3.2' || '' }} dependency-level: ${{ ... && 'minimum' || '' }} ``` ### Workflow Verification The `/run-skipped-ci` command now: 1. Triggers all workflows via API 2. Waits 5 seconds for workflows to queue 3. Queries GitHub Actions API to verify workflows started 4. Reports detailed status showing which workflows are verified, pending, or failed ## Testing Notes **Important:** The comment-triggered workflow must be merged to `master` before it can be tested, because GitHub Actions only executes `issue_comment` workflows from the default branch (security feature). Once merged, test by: 1. Creating a test PR 2. Commenting `/run-skipped-ci` 3. Verifying all workflows trigger and status is reported accurately ## Inspiration Based on similar pattern from shakacode/react-webpack-rails-tutorial#687 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- Reviewable:start --> - - - This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/shakacode/react_on_rails/1974) <!-- Reviewable:end --> --------- Co-authored-by: Claude <[email protected]>
Summary
This PR adds a
/run-skipped-cicommand that can be used in PR comments to trigger the complete CI suite, including tests that are normally skipped on PRs for faster feedback. It also fixes several workflow issues identified during development.What Changed
New GitHub Actions Workflows:
.github/workflows/run-skipped-ci.yml- Listens for/run-skipped-cicomments and triggers all CI workflows with verification.github/workflows/pr-welcome-comment.yml- Automatically comments on new PRs explaining the commandNew Documentation:
.github/README.md- Comprehensive documentation of all workflows and the comment command featureWorkflow Fixes:
ifconditions inexamples.ymlandmain.ymlthat caused YAML parsing errorsSKIP_MINIMUM_ON_PRenv var pattern with cleaner matrix-levelexcludeapproachHow It Works
/run-skipped-cion any PR to run:Why This Approach
By default, PRs run a limited CI suite to keep feedback loops fast:
Before merging, we want to verify compatibility across all supported versions. This command makes it easy to run full CI without:
Technical Improvements
Matrix Filtering
Before:
After:
Workflow Verification
The
/run-skipped-cicommand now:Testing Notes
Important: The comment-triggered workflow must be merged to
masterbefore it can be tested, because GitHub Actions only executesissue_commentworkflows from the default branch (security feature).Once merged, test by:
/run-skipped-ciInspiration
Based on similar pattern from shakacode/react-webpack-rails-tutorial#687
🤖 Generated with Claude Code
This change is