Skip to content

Commit d3ddb49

Browse files
justin808claude
andauthored
Guard master docs-only pushes and ensure full CI runs (#2042)
## Summary This PR adds a safety mechanism to prevent docs-only commits on master from showing a misleading green checkmark when the previous commit has failing or incomplete CI workflows. Docs-only commits can still be pushed and will skip heavy CI jobs, but the guard action will fail if the previous commit wasn't fully validated. ### Key Features - **Reusable GitHub Action** (`.github/actions/ensure-master-docs-safety`) that validates previous master commit status before allowing docs-only CI skips to show green - **Applied to all workflows**: Change detection integrated into all 11 workflow files - **Smart docs-only detection**: The `ci-changes-detector` script now always emits outputs for docs-only cases - **Optimized performance**: Early-exit pagination, efficient API usage, proper deduplication ### How It Works 1. **On docs-only master push**: Heavy CI jobs are skipped, but guard action checks if previous commit's workflows are complete and passing 2. **If previous commit has failures or in-progress runs**: Guard action fails with actionable error message (commit is still pushed, but doesn't show green) 3. **If previous commit is clean**: Guard action passes, docs-only commit shows green checkmark 4. **On code changes**: Always run full CI suite regardless of previous commit status ### The Problem This Solves **Before**: Pushing a docs-only commit to master would skip all CI and show a green checkmark, even if the previous commit had failing tests. This created a false sense of security. **After**: Docs-only commits still skip heavy CI jobs (saving resources), but the guard action fails if the previous commit wasn't validated, preventing a misleading green checkmark. ### Improvements from Code Review **GitHub Action enhancements**: - ✅ Clarified API rate limit constraints in 7-day window documentation - ✅ Documented run_number tiebreaker behavior for out-of-order reruns - ✅ Explained why 'cancelled' workflows block docs-only skips - ✅ Improved deduplication using `run_number` instead of `created_at` for accurate re-run handling - ✅ Added clickable markdown links to failing workflows in error messages - ✅ Comprehensive documentation for 7-day lookback window rationale - ✅ Explicit handling of 'skipped' and 'neutral' workflow conclusions **Script improvements**: - ✅ Eliminated duplication in `ci-changes-detector` by using single CI_FLAGS array for both GITHUB_OUTPUT and JSON output - ✅ Reduced maintenance burden and prevented potential inconsistencies **Workflow improvements**: - ✅ Clear inline comments explaining conditional logic across all workflows - ✅ Consistent pattern: "Skip only if: master push AND docs-only changes" **Code quality**: - ✅ Improved knip comment clarity - ✅ All changes verified with RuboCop and Prettier ## Testing ### Automated Tests - ✅ RuboCop: 149 files inspected, no offenses detected - ✅ Prettier: All files formatted correctly - ✅ `ci-changes-detector` script tested locally with JSON output ### Manual Testing Scenarios - [ ] Docs-only commit with passing previous commit → should skip CI and show green - [ ] Docs-only commit with failing previous commit → should skip heavy jobs but guard fails (no green) - [ ] Docs-only commit while previous workflows running → guard should fail - [ ] Initial commit handling (github.event.before == '0000...') - [ ] Old commit > 7 days → should allow skip with informative message - [ ] Workflow re-runs → should use latest run_number - [ ] Skipped/neutral workflows → should not block - [ ] Code changes to master → full CI should always run ## Impact **For developers**: - Faster docs updates when master is clean (heavy CI jobs skipped) - Clear error messages with links when docs-only skip would hide failures - Prevents misleading green checkmarks on broken master **For CI/CD**: - Reduced CI costs for docs-only commits (when safe) - Ensures master branch status accurately reflects code validation - Prevents docs-only commits from hiding failing tests ## Example Scenarios ### Scenario 1: Safe docs-only commit ``` Commit A (code changes) → All CI passes ✅ Commit B (docs only) → Heavy jobs skipped, guard passes ✅ (green checkmark) ``` ### Scenario 2: Unsafe docs-only commit (prevented) ``` Commit A (code changes) → Tests fail ❌ Commit B (docs only) → Heavy jobs skipped, guard FAILS ❌ (no misleading green) ``` Developer must fix Commit A's failures before Commit B will show green. ### Scenario 3: Code change (always full CI) ``` Commit A (code changes) → Tests fail ❌ Commit B (code changes) → Full CI runs (guard not involved) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Safety guard for docs-only pushes to master that prevents misleading green checkmarks when previous commit has failures * Docs-only commits can skip heavy CI jobs when previous commit is validated * Optimized API pagination with early exit strategy for improved performance * Added clickable links to failing workflows in error messages for better developer experience * CI now exposes docs-only signals so heavy jobs (tests, builds, linting) can be skipped safely * **Bug Fixes** * Improved workflow deduplication logic using run_number instead of created_at * Eliminated duplication in ci-changes-detector script * **Documentation** * Added comprehensive comments explaining 7-day lookback window rationale and API rate limits * Documented handling of 'skipped' and 'neutral' workflow conclusions * Explained run_number tiebreaker behavior and 'cancelled' workflow treatment * Clear inline comments explaining complex conditional logic in workflows <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude <[email protected]>
1 parent ae8b1b1 commit d3ddb49

File tree

13 files changed

+425
-40
lines changed

13 files changed

+425
-40
lines changed
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
name: Ensure master docs-only skips are safe
2+
description: Fails if the previous master commit still has failing workflow runs when current push is docs-only
3+
inputs:
4+
docs-only:
5+
description: 'String output from ci-changes-detector ("true" or "false")'
6+
required: true
7+
previous-sha:
8+
description: 'SHA of the previous commit on master (github.event.before)'
9+
required: true
10+
runs:
11+
using: composite
12+
steps:
13+
- name: Check previous master commit status
14+
if: ${{ inputs.docs-only == 'true' && inputs.previous-sha != '' && inputs.previous-sha != '0000000000000000000000000000000000000000' }}
15+
uses: actions/github-script@v7
16+
env:
17+
PREVIOUS_SHA: ${{ inputs.previous-sha }}
18+
with:
19+
script: |
20+
const previousSha = process.env.PREVIOUS_SHA;
21+
22+
// Query workflow runs from the last 7 days to avoid excessive API calls.
23+
// Why 7 days? This balances API efficiency with practical needs:
24+
// - Most master commits trigger CI within hours, not days
25+
// - Commits older than 7 days are likely stale; better to allow the docs-only skip anyway
26+
// - Reduces pagination load on high-velocity repos
27+
// - GitHub API rate limits (1000 req/hr for Actions) make unbounded searches risky
28+
// For commits outside this window, we skip the check and allow the docs-only skip.
29+
const createdAfter = new Date(Date.now() - 1000 * 60 * 60 * 24 * 7).toISOString();
30+
31+
// Optimize pagination: use lower per_page and collect only what we need
32+
const workflowRuns = [];
33+
let foundAllRelevant = false;
34+
35+
for await (const response of github.paginate.iterator(
36+
github.rest.actions.listWorkflowRunsForRepo,
37+
{
38+
owner: context.repo.owner,
39+
repo: context.repo.repo,
40+
branch: 'master',
41+
event: 'push',
42+
per_page: 30,
43+
created: `>${createdAfter}`,
44+
sort: 'created',
45+
direction: 'desc'
46+
}
47+
)) {
48+
const pageRuns = response.data;
49+
const relevantInPage = pageRuns.filter((run) => run.head_sha === previousSha);
50+
51+
if (relevantInPage.length > 0) {
52+
workflowRuns.push(...relevantInPage);
53+
}
54+
55+
// Early exit: if we found relevant runs and now seeing different SHAs,
56+
// we've likely collected all runs for the previous commit
57+
if (workflowRuns.length > 0 && relevantInPage.length === 0) {
58+
foundAllRelevant = true;
59+
break;
60+
}
61+
}
62+
63+
if (workflowRuns.length === 0) {
64+
core.info(`No workflow runs found for ${previousSha} in the last 7 days. Allowing docs-only skip.`);
65+
return;
66+
}
67+
68+
// Deduplicate workflow runs by keeping only the latest run for each workflow_id.
69+
// This handles cases where workflows are re-run manually.
70+
// Use run_number as tiebreaker since created_at might be identical for rapid reruns.
71+
// Note: If workflows are manually re-run out of order, we use the highest run_number
72+
// which represents the most recent attempt, regardless of trigger order.
73+
const latestByWorkflow = new Map();
74+
for (const run of workflowRuns) {
75+
const existing = latestByWorkflow.get(run.workflow_id);
76+
if (!existing || run.run_number > existing.run_number) {
77+
latestByWorkflow.set(run.workflow_id, run);
78+
}
79+
}
80+
81+
// Check for workflows that are still running
82+
// We require all workflows to complete before allowing docs-only skip
83+
// This prevents skipping CI when the previous commit hasn't been fully validated
84+
const incompleteRuns = Array.from(latestByWorkflow.values()).filter(
85+
(run) => run.status !== 'completed'
86+
);
87+
88+
if (incompleteRuns.length > 0) {
89+
const details = incompleteRuns
90+
.map((run) => `- [${run.name} #${run.run_number}](${run.html_url}) is still ${run.status}`)
91+
.join('\n');
92+
core.setFailed(
93+
[
94+
`Cannot skip CI for docs-only commit because previous master commit ${previousSha} still has running workflows:`,
95+
details,
96+
'',
97+
'Wait for these workflows to complete before pushing docs-only changes.'
98+
].join('\n')
99+
);
100+
return;
101+
}
102+
103+
// Check for workflows that failed on the previous commit.
104+
// We treat these conclusions as failures:
105+
// - 'failure': Obvious failure case
106+
// - 'timed_out': Infrastructure or performance issue that should be investigated
107+
// - 'cancelled': Might indicate timeout, CI infrastructure issues, or manual intervention needed
108+
// Being conservative here prevents a green checkmark when the previous commit
109+
// might have real issues that weren't fully validated
110+
// - 'action_required': Requires manual intervention
111+
// We treat 'skipped' and 'neutral' as non-blocking since they indicate
112+
// intentional skips or informational-only workflows.
113+
const failingRuns = Array.from(latestByWorkflow.values()).filter((run) => {
114+
return ['failure', 'timed_out', 'cancelled', 'action_required'].includes(run.conclusion);
115+
});
116+
117+
if (failingRuns.length === 0) {
118+
core.info(`Previous master commit ${previousSha} completed without failures. Docs-only skip allowed.`);
119+
return;
120+
}
121+
122+
const details = failingRuns
123+
.map((run) => `- [${run.name} #${run.run_number}](${run.html_url}) concluded ${run.conclusion}`)
124+
.join('\n');
125+
126+
core.setFailed(
127+
[
128+
`Cannot skip CI for docs-only commit because previous master commit ${previousSha} still has failing workflows:`,
129+
details,
130+
'',
131+
'Fix these failures before pushing docs-only changes, or push non-docs changes to trigger full CI.'
132+
].join('\n')
133+
);

.github/workflows/detect-changes.yml

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ on:
2525
jobs:
2626
detect:
2727
runs-on: ubuntu-22.04
28+
permissions:
29+
contents: read
30+
actions: read
2831
outputs:
2932
docs_only: ${{ steps.changes.outputs.docs_only }}
3033
run_lint: ${{ steps.changes.outputs.run_lint }}
@@ -41,17 +44,11 @@ jobs:
4144
- name: Detect changes
4245
id: changes
4346
run: |
44-
# For master branch, always run everything
45-
if [ "${{ github.ref }}" = "refs/heads/master" ]; then
46-
echo "docs_only=false" >> "$GITHUB_OUTPUT"
47-
echo "run_lint=true" >> "$GITHUB_OUTPUT"
48-
echo "run_ruby_tests=true" >> "$GITHUB_OUTPUT"
49-
echo "run_js_tests=true" >> "$GITHUB_OUTPUT"
50-
echo "run_dummy_tests=true" >> "$GITHUB_OUTPUT"
51-
echo "run_generators=true" >> "$GITHUB_OUTPUT"
52-
exit 0
53-
fi
54-
55-
# For PRs, analyze changes
56-
BASE_SHA="${{ github.event.pull_request.base.sha || github.event.before }}"
47+
BASE_SHA="${{ github.event.pull_request.base.sha || github.event.before || 'origin/master' }}"
5748
script/ci-changes-detector "$BASE_SHA"
49+
- name: Guard docs-only master pushes
50+
if: github.event_name == 'push' && github.ref == 'refs/heads/master'
51+
uses: ./.github/actions/ensure-master-docs-safety
52+
with:
53+
docs-only: ${{ steps.changes.outputs.docs_only }}
54+
previous-sha: ${{ github.event.before }}

.github/workflows/examples.yml

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ on:
44
push:
55
branches:
66
- 'master'
7-
# Never skip on master - always run full test suite
7+
# Always trigger on master; docs-only detection handles skipping heavy jobs
88
pull_request:
99
paths-ignore:
1010
- '**.md'
@@ -19,6 +19,9 @@ on:
1919

2020
jobs:
2121
detect-changes:
22+
permissions:
23+
contents: read
24+
actions: read
2225
runs-on: ubuntu-22.04
2326
outputs:
2427
docs_only: ${{ steps.detect.outputs.docs_only }}
@@ -54,6 +57,12 @@ jobs:
5457
BASE_REF="${{ github.event.pull_request.base.sha || github.event.before || 'origin/master' }}"
5558
script/ci-changes-detector "$BASE_REF"
5659
shell: bash
60+
- name: Guard docs-only master pushes
61+
if: github.event_name == 'push' && github.ref == 'refs/heads/master'
62+
uses: ./.github/actions/ensure-master-docs-safety
63+
with:
64+
docs-only: ${{ steps.detect.outputs.docs_only }}
65+
previous-sha: ${{ github.event.before }}
5766

5867
setup-matrix:
5968
needs: detect-changes
@@ -79,7 +88,15 @@ jobs:
7988
needs: [detect-changes, setup-matrix]
8089
# Run on master, workflow_dispatch, OR when generators needed
8190
if: |
82-
github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.run_generators == 'true'
91+
!(
92+
github.event_name == 'push' &&
93+
github.ref == 'refs/heads/master' &&
94+
needs.detect-changes.outputs.docs_only == 'true'
95+
) && (
96+
github.ref == 'refs/heads/master' ||
97+
github.event_name == 'workflow_dispatch' ||
98+
needs.detect-changes.outputs.run_generators == 'true'
99+
)
83100
strategy:
84101
fail-fast: false
85102
matrix: ${{ fromJson(needs.setup-matrix.outputs.matrix) }}

.github/workflows/gem-tests.yml

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ on:
44
push:
55
branches:
66
- 'master'
7-
# Never skip on master - always run full test suite
7+
# Always trigger on master; docs-only detection handles skipping heavy jobs
88
pull_request:
99
paths-ignore:
1010
- '**.md'
@@ -21,6 +21,9 @@ on:
2121

2222
jobs:
2323
detect-changes:
24+
permissions:
25+
contents: read
26+
actions: read
2427
runs-on: ubuntu-22.04
2528
outputs:
2629
docs_only: ${{ steps.detect.outputs.docs_only }}
@@ -56,6 +59,12 @@ jobs:
5659
BASE_REF="${{ github.event.pull_request.base.sha || github.event.before || 'origin/master' }}"
5760
script/ci-changes-detector "$BASE_REF"
5861
shell: bash
62+
- name: Guard docs-only master pushes
63+
if: github.event_name == 'push' && github.ref == 'refs/heads/master'
64+
uses: ./.github/actions/ensure-master-docs-safety
65+
with:
66+
docs-only: ${{ steps.detect.outputs.docs_only }}
67+
previous-sha: ${{ github.event.before }}
5968

6069
setup-gem-tests-matrix:
6170
needs: detect-changes
@@ -77,9 +86,18 @@ jobs:
7786
7887
rspec-package-tests:
7988
needs: [detect-changes, setup-gem-tests-matrix]
80-
# Run on master OR when Ruby tests needed on PR
89+
# Skip only if: master push AND docs-only changes
90+
# Otherwise run if: on master OR Ruby tests needed
91+
# This allows docs-only commits to skip heavy jobs while ensuring full CI on master for code changes
8192
if: |
82-
(github.ref == 'refs/heads/master' || needs.detect-changes.outputs.run_ruby_tests == 'true')
93+
!(
94+
github.event_name == 'push' &&
95+
github.ref == 'refs/heads/master' &&
96+
needs.detect-changes.outputs.docs_only == 'true'
97+
) && (
98+
github.ref == 'refs/heads/master' ||
99+
needs.detect-changes.outputs.run_ruby_tests == 'true'
100+
)
83101
strategy:
84102
fail-fast: false
85103
matrix: ${{ fromJson(needs.setup-gem-tests-matrix.outputs.matrix) }}

.github/workflows/integration-tests.yml

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ on:
44
push:
55
branches:
66
- 'master'
7-
# Never skip on master - always run full test suite
7+
# Always trigger on master; docs-only detection handles skipping heavy jobs
88
pull_request:
99
paths-ignore:
1010
- '**.md'
@@ -20,6 +20,9 @@ on:
2020

2121
jobs:
2222
detect-changes:
23+
permissions:
24+
contents: read
25+
actions: read
2326
runs-on: ubuntu-22.04
2427
outputs:
2528
docs_only: ${{ steps.detect.outputs.docs_only }}
@@ -51,9 +54,15 @@ jobs:
5154
echo "docs_only=false" >> "$GITHUB_OUTPUT"
5255
else
5356
BASE_REF="${{ github.event.pull_request.base.sha || github.event.before || 'origin/master' }}"
54-
script/ci-changes-detector "$BASE_REF"
57+
script/ci-changes-detector "$BASE_REF"
5558
fi
5659
shell: bash
60+
- name: Guard docs-only master pushes
61+
if: github.event_name == 'push' && github.ref == 'refs/heads/master'
62+
uses: ./.github/actions/ensure-master-docs-safety
63+
with:
64+
docs-only: ${{ steps.detect.outputs.docs_only }}
65+
previous-sha: ${{ github.event.before }}
5766

5867
setup-integration-matrix:
5968
needs: detect-changes
@@ -77,9 +86,19 @@ jobs:
7786
7887
build-dummy-app-webpack-test-bundles:
7988
needs: [detect-changes, setup-integration-matrix]
80-
# Run on master, workflow_dispatch, OR when tests needed on PR
89+
# Skip only if: master push AND docs-only changes
90+
# Otherwise run if: on master OR workflow_dispatch OR dummy tests needed
91+
# This allows docs-only commits to skip heavy jobs while ensuring full CI on master for code changes
8192
if: |
82-
github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.run_dummy_tests == 'true'
93+
!(
94+
github.event_name == 'push' &&
95+
github.ref == 'refs/heads/master' &&
96+
needs.detect-changes.outputs.docs_only == 'true'
97+
) && (
98+
github.ref == 'refs/heads/master' ||
99+
github.event_name == 'workflow_dispatch' ||
100+
needs.detect-changes.outputs.run_dummy_tests == 'true'
101+
)
83102
strategy:
84103
matrix: ${{ fromJson(needs.setup-integration-matrix.outputs.matrix) }}
85104
runs-on: ubuntu-22.04
@@ -154,7 +173,15 @@ jobs:
154173
needs: [detect-changes, setup-integration-matrix, build-dummy-app-webpack-test-bundles]
155174
# Run on master, workflow_dispatch, OR when tests needed on PR
156175
if: |
157-
github.ref == 'refs/heads/master' || github.event_name == 'workflow_dispatch' || needs.detect-changes.outputs.run_dummy_tests == 'true'
176+
!(
177+
github.event_name == 'push' &&
178+
github.ref == 'refs/heads/master' &&
179+
needs.detect-changes.outputs.docs_only == 'true'
180+
) && (
181+
github.ref == 'refs/heads/master' ||
182+
github.event_name == 'workflow_dispatch' ||
183+
needs.detect-changes.outputs.run_dummy_tests == 'true'
184+
)
158185
strategy:
159186
matrix: ${{ fromJson(needs.setup-integration-matrix.outputs.matrix) }}
160187
runs-on: ubuntu-22.04

.github/workflows/lint-js-and-ruby.yml

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ on:
44
push:
55
branches:
66
- 'master'
7-
# Never skip on master - always run full test suite
7+
# Always trigger on master; docs-only detection handles skipping heavy jobs
88
pull_request:
99
paths-ignore:
1010
- '**.md'
@@ -20,6 +20,9 @@ on:
2020

2121
jobs:
2222
detect-changes:
23+
permissions:
24+
contents: read
25+
actions: read
2326
runs-on: ubuntu-22.04
2427
outputs:
2528
docs_only: ${{ steps.detect.outputs.docs_only }}
@@ -54,10 +57,27 @@ jobs:
5457
BASE_REF="${{ github.event.pull_request.base.sha || github.event.before || 'origin/master' }}"
5558
script/ci-changes-detector "$BASE_REF"
5659
shell: bash
60+
- name: Guard docs-only master pushes
61+
if: github.event_name == 'push' && github.ref == 'refs/heads/master'
62+
uses: ./.github/actions/ensure-master-docs-safety
63+
with:
64+
docs-only: ${{ steps.detect.outputs.docs_only }}
65+
previous-sha: ${{ github.event.before }}
5766

5867
build:
5968
needs: detect-changes
60-
if: github.ref == 'refs/heads/master' || needs.detect-changes.outputs.run_lint == 'true'
69+
# Skip only if: master push AND docs-only changes
70+
# Otherwise run if: on master OR lint needed
71+
# This allows docs-only commits to skip linting while ensuring full CI on master for code changes
72+
if: |
73+
!(
74+
github.event_name == 'push' &&
75+
github.ref == 'refs/heads/master' &&
76+
needs.detect-changes.outputs.docs_only == 'true'
77+
) && (
78+
github.ref == 'refs/heads/master' ||
79+
needs.detect-changes.outputs.run_lint == 'true'
80+
)
6181
env:
6282
BUNDLE_FROZEN: true
6383
runs-on: ubuntu-22.04

0 commit comments

Comments
 (0)