-
Notifications
You must be signed in to change notification settings - Fork 114
SRE-515: Experimental; run auto-fixes in merge queue #8434
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,8 +122,48 @@ jobs: | |
| run: df -h | ||
|
|
||
| - name: Run ESLint | ||
| id: eslint | ||
| if: always() && steps.lints.outputs.eslint == 'true' | ||
| run: turbo run lint:eslint --filter "${{ matrix.name }}" | ||
| run: | | ||
| if [[ "${{ github.event_name }}" == "merge_group" ]]; then | ||
| # merge_group: fix first, then check | ||
| turbo run fix:eslint --filter "${{ matrix.name }}" || true | ||
| if ! turbo run lint:eslint --filter "${{ matrix.name }}"; then | ||
| echo '::error::ESLint failed even after auto-fix on merge_group for ${{ matrix.name }}.' | ||
| echo '## β ESLint (${{ matrix.name }})' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Auto-fix was applied but non-fixable ESLint issues remain.' >> $GITHUB_STEP_SUMMARY | ||
| exit 1 | ||
| fi | ||
| # Check if fix changed anything | ||
| if ! git diff --quiet; then | ||
| echo '## β οΈ ESLint (${{ matrix.name }})' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Auto-fixable ESLint issues were found and fixed in merge queue.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Note: ESLint fixes in the package job cannot be committed from here.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'The merge-queue ref may need a separate mechanism to persist these fixes.' >> $GITHUB_STEP_SUMMARY | ||
| echo '::warning::ESLint auto-fixes were applied for ${{ matrix.name }} but cannot be committed from the package job. Consider running fix:eslint on the PR branch before merge queue entry.' | ||
| fi | ||
| else | ||
| # pull_request / push: check first, auto-fix if needed | ||
| if ! turbo run lint:eslint --filter "${{ matrix.name }}"; then | ||
| echo '::notice::ESLint check failed for ${{ matrix.name }} β attempting auto-fix...' | ||
| turbo run fix:eslint --filter "${{ matrix.name }}" || true | ||
| if turbo run lint:eslint --filter "${{ matrix.name }}"; then | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Severity: medium Other Locations
π€ Was this useful? React with π or π, or π if it prevented an incident/outage. |
||
| echo '## β οΈ ESLint (${{ matrix.name }}) (auto-fixed)' >> $GITHUB_STEP_SUMMARY | ||
| echo 'CI detected fixable ESLint issues that were auto-resolved.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Please run `turbo run fix:eslint --filter "${{ matrix.name }}"` locally and commit the changes.' >> $GITHUB_STEP_SUMMARY | ||
| # Restore working tree so subsequent steps are not affected | ||
| git checkout -- . | ||
| else | ||
| echo '' | ||
| echo '' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| echo 'ESLint has non-fixable errors for ${{ matrix.name }}. Try running `turbo run fix:eslint --filter "${{ matrix.name }}"` locally to apply autofixes, then fix remaining issues manually.' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| git checkout -- . | ||
| exit 1 | ||
| fi | ||
| fi | ||
| fi | ||
|
|
||
| - name: Run TSC | ||
| if: always() && steps.lints.outputs.tsc == 'true' | ||
|
|
@@ -183,13 +223,15 @@ jobs: | |
| name: Global | ||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
| contents: write | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Severity: medium π€ Was this useful? React with π or π, or π if it prevented an incident/outage. |
||
| checks: write | ||
| pull-requests: write | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Clean up disk | ||
| uses: ./.github/actions/clean-up-disk | ||
|
|
@@ -243,27 +285,134 @@ jobs: | |
| fi | ||
|
|
||
| - name: Run yarn lint:markdownlint | ||
| id: markdownlint | ||
| if: ${{ success() || failure() }} | ||
| run: | | ||
| if ! yarn lint:markdownlint; then | ||
| echo '' | ||
| echo '' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| echo 'Try running `yarn fix:markdownlint` locally to apply autofixes.' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| exit 1 | ||
| if [[ "${{ github.event_name }}" == "merge_group" ]]; then | ||
| # merge_group: fix first, then check | ||
| yarn fix:markdownlint || true | ||
| if ! yarn lint:markdownlint; then | ||
| echo '::error::markdownlint failed even after auto-fix on merge_group.' | ||
| echo '## β markdownlint' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Auto-fix was applied but non-fixable issues remain.' >> $GITHUB_STEP_SUMMARY | ||
| exit 1 | ||
| fi | ||
| # Check if fix changed anything | ||
| if ! git diff --quiet; then | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Severity: low Other Locations
π€ Was this useful? React with π or π, or π if it prevented an incident/outage. |
||
| echo "MARKDOWNLINT_FIXED=true" >> $GITHUB_ENV | ||
| echo '## β οΈ markdownlint' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Auto-fixable markdownlint issues were found and fixed in merge queue.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'These fixes need to be committed to the merge-queue ref.' >> $GITHUB_STEP_SUMMARY | ||
| fi | ||
| else | ||
| # pull_request / push: check first, auto-fix if needed | ||
| if ! yarn lint:markdownlint; then | ||
| echo '::notice::markdownlint check failed β attempting auto-fix...' | ||
| yarn fix:markdownlint || true | ||
| if yarn lint:markdownlint; then | ||
| echo '## β οΈ markdownlint (auto-fixed)' >> $GITHUB_STEP_SUMMARY | ||
| echo 'CI detected fixable markdownlint issues that were auto-resolved.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Please run `yarn fix:markdownlint` locally and commit the changes.' >> $GITHUB_STEP_SUMMARY | ||
| # Restore working tree so subsequent steps are not affected | ||
| git checkout -- . | ||
| else | ||
| echo '' | ||
| echo '' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| echo 'markdownlint has non-fixable errors. Try running `yarn fix:markdownlint` locally to apply autofixes, then fix remaining issues manually.' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| git checkout -- . | ||
| exit 1 | ||
| fi | ||
| fi | ||
| fi | ||
|
|
||
| - name: Run yarn lint:format | ||
| id: biome_format | ||
| if: ${{ success() || failure() }} | ||
| run: | | ||
| if ! yarn lint:format; then | ||
| echo '' | ||
| echo '' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| echo 'Try running `yarn fix:format` locally to apply autofixes.' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| exit 1 | ||
| if [[ "${{ github.event_name }}" == "merge_group" ]]; then | ||
| # merge_group: fix first, then check | ||
| yarn fix:format || true | ||
| if ! yarn lint:format; then | ||
| echo '::error::Biome format check failed even after auto-fix on merge_group.' | ||
| echo '## β Biome format' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Auto-fix was applied but non-fixable formatting issues remain.' >> $GITHUB_STEP_SUMMARY | ||
| exit 1 | ||
| fi | ||
| # Check if fix changed anything | ||
| if ! git diff --quiet; then | ||
| echo "BIOME_FORMAT_FIXED=true" >> $GITHUB_ENV | ||
| echo '## β οΈ Biome format' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Auto-fixable formatting issues were found and fixed in merge queue.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'These fixes need to be committed to the merge-queue ref.' >> $GITHUB_STEP_SUMMARY | ||
| fi | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Biome diff check contaminated by prior markdownlint changesMedium Severity In the Additional Locations (1) |
||
| else | ||
| # pull_request / push: check first, auto-fix if needed | ||
| if ! yarn lint:format; then | ||
| echo '::notice::Biome format check failed β attempting auto-fix...' | ||
| yarn fix:format || true | ||
| if yarn lint:format; then | ||
| echo '## β οΈ Biome format (auto-fixed)' >> $GITHUB_STEP_SUMMARY | ||
| echo 'CI detected fixable formatting issues that were auto-resolved.' >> $GITHUB_STEP_SUMMARY | ||
| echo 'Please run `yarn fix:format` locally and commit the changes.' >> $GITHUB_STEP_SUMMARY | ||
| # Restore working tree so subsequent steps are not affected | ||
| git checkout -- . | ||
| else | ||
| echo '' | ||
| echo '' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| echo 'Biome format has non-fixable errors. Try running `yarn fix:format` locally to apply autofixes, then fix remaining issues manually.' | ||
| echo 'βΉοΈ βΉοΈ βΉοΈ' | ||
| git checkout -- . | ||
| exit 1 | ||
| fi | ||
| fi | ||
| fi | ||
|
|
||
| - name: Commit and push auto-fixes (merge_group) | ||
| if: ${{ github.event_name == 'merge_group' && (env.MARKDOWNLINT_FIXED == 'true' || env.BIOME_FORMAT_FIXED == 'true') }} | ||
| run: | | ||
| echo '## π§ Merge Queue Auto-Fix Commit Attempt' >> $GITHUB_STEP_SUMMARY | ||
| echo '' >> $GITHUB_STEP_SUMMARY | ||
|
|
||
| # Detect the merge-queue ref | ||
| MERGE_REF="${{ github.ref }}" | ||
| echo "Merge queue ref: $MERGE_REF" | ||
|
|
||
| # Configure git for the commit | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
|
|
||
| git add -A | ||
| git diff --cached --stat | ||
|
|
||
| if git diff --cached --quiet; then | ||
| echo "No changes to commit after auto-fix (unexpected)." | ||
| echo 'No changes detected after auto-fix β skipping commit.' >> $GITHUB_STEP_SUMMARY | ||
| else | ||
| git commit -m "ci: auto-fix markdownlint/biome formatting issues [merge-queue]" | ||
|
|
||
| # Attempt to push β this may fail if the merge-queue ref is read-only | ||
| if git push origin HEAD:"$MERGE_REF" 2>&1; then | ||
| echo 'β Auto-fix commit pushed to merge-queue ref successfully.' >> $GITHUB_STEP_SUMMARY | ||
| else | ||
| echo '' | ||
| echo '::warning::Could not push auto-fix commit to merge-queue ref.' | ||
| echo 'This is expected β GitHub merge queue refs may be read-only.' | ||
| echo 'The fixes were applied and checks passed, but the commit could not be persisted.' | ||
| echo '' >> $GITHUB_STEP_SUMMARY | ||
| echo 'β οΈ **Could not push auto-fix commit to merge-queue ref.**' >> $GITHUB_STEP_SUMMARY | ||
| echo 'GitHub merge queue refs are typically read-only. The auto-fixes were applied' >> $GITHUB_STEP_SUMMARY | ||
| echo 'and all checks passed in the fixed state, but the commit could not be persisted.' >> $GITHUB_STEP_SUMMARY | ||
| echo '' >> $GITHUB_STEP_SUMMARY | ||
| echo '**Feasibility note:** To persist auto-fixes in merge queue, consider:' >> $GITHUB_STEP_SUMMARY | ||
| echo '1. Using a GitHub App token with `contents: write` permission' >> $GITHUB_STEP_SUMMARY | ||
| echo '2. Pushing fixes to the source branch before merge queue entry' >> $GITHUB_STEP_SUMMARY | ||
| echo '3. Using a pre-merge-queue workflow that applies fixes to the PR branch' >> $GITHUB_STEP_SUMMARY | ||
| # Fail so the team can evaluate feasibility | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
||
| - name: Run yarn lint:package-json | ||
|
|
||


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.
In
merge_group,fix:eslintcan make the job pass while leaving the merge-queue ref unchanged (since these package-job fixes arenβt committed/pushed). That means the checks may be validating a working tree state that wonβt actually be merged.Severity: high
π€ Was this useful? React with π or π, or π if it prevented an incident/outage.