From ed05283023b2c118d5e68c3af74f64760b8c5141 Mon Sep 17 00:00:00 2001 From: Colt Frederickson Date: Thu, 22 Oct 2026 14:29:05 -0600 Subject: [PATCH 1/2] Add auto formatting to rust and typescript (if it's supported) --- .github/workflows/rust-ci.yaml | 69 +++++++++++++++-- .github/workflows/typescript-ci.yaml | 108 ++++++++++++++++++++++++++- 2 files changed, 167 insertions(+), 10 deletions(-) diff --git a/.github/workflows/rust-ci.yaml b/.github/workflows/rust-ci.yaml index ecedf21..c8f9626 100644 --- a/.github/workflows/rust-ci.yaml +++ b/.github/workflows/rust-ci.yaml @@ -1,5 +1,6 @@ # tag-version: v3 -# CI for Rust projects. This compiles (using sccache), runs tests, and checks formatting. +# CI for Rust projects. This compiles (using sccache), runs tests, and applies cargo fmt + cargo sort +# auto-fixes (pushing them back to the PR branch when possible). # Customization: # - all customization should be available via inputs. If anything isn't available, try to add it to this as an input instead of copying and modifying directly @@ -79,7 +80,7 @@ on: description: "AWS secret key to access our Rust tool cache S3 bucket." required: false WORKFLOW_PAT: - description: "GitHub PAT used to check out sql-smelter when smelter_required is true." + description: "GitHub PAT used to check out sql-smelter when smelter_required is true, and to push cargo fmt/cargo sort auto-fix commits back to PR branches. If absent, formatting falls back to check-only mode." required: false env: @@ -253,24 +254,78 @@ jobs: echo '${{ secrets.LEEROY_PRIVATE_SSH_KEY }}' > "${HOME}/.ssh/id_ed25519" chmod 600 "${HOME}/.ssh/id_ed25519" ssh-keyscan github.com > "${HOME}/.ssh/known_hosts" + - name: Determine if auto-fix push is possible + id: push_mode + env: + EVENT_NAME: ${{ github.event_name }} + IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + HAS_PAT: ${{ secrets.WORKFLOW_PAT != '' }} + # Forks can't be pushed to and push events have no PR branch to fix, so both check only. + run: | + if [ "$EVENT_NAME" = "pull_request" ] && [ "$HAS_PAT" = "true" ] && [ "$IS_FORK" != "true" ]; then + echo "can_push=true" >> "$GITHUB_OUTPUT" + else + echo "can_push=false" >> "$GITHUB_OUTPUT" + fi - uses: actions/checkout@v7 with: path: ${{ env.UNDER_TEST_FOLDER }} + # The branch tip rather than the merge ref, so the auto-fix commit can be pushed back. + ref: ${{ steps.push_mode.outputs.can_push == 'true' && github.head_ref || '' }} + token: ${{ steps.push_mode.outputs.can_push == 'true' && secrets.WORKFLOW_PAT || github.token }} - uses: IronCoreLabs/rust-toolchain@v1 with: components: rustfmt working-directory: ${{ env.UNDER_TEST_FOLDER }} - - run: ${{inputs.cargo_command_env_vars}} cargo fmt --all -- --check - working-directory: ${{ env.UNDER_TEST_FOLDER }} - name: Install cargo-sort uses: taiki-e/install-action@cargo-sort - - run: | + - name: Apply cargo fmt and cargo sort auto-fixes + if: steps.push_mode.outputs.can_push == 'true' + id: autofix + working-directory: ${{ env.UNDER_TEST_FOLDER }} + run: | if grep -q '\[workspace]' Cargo.toml ; then - ${{inputs.cargo_command_env_vars}} cargo sort --check --workspace + SORT_ARGS=--workspace + fi + ${{inputs.cargo_command_env_vars}} cargo fmt --all + # --no-format keeps this to what --check actually fails on; its whitespace + # normalization is only a warning there. + ${{inputs.cargo_command_env_vars}} cargo sort --no-format $SORT_ARGS + if [ -n "$(git status --porcelain --untracked-files=no)" ]; then + echo "changed=true" >> "$GITHUB_OUTPUT" else - ${{inputs.cargo_command_env_vars}} cargo sort --check + echo "changed=false" >> "$GITHUB_OUTPUT" fi + - name: Commit and push auto-fixes, then fail + if: steps.autofix.outputs.changed == 'true' working-directory: ${{ env.UNDER_TEST_FOLDER }} + env: + HEAD_REF: ${{ github.head_ref }} + # Failing after the push is what makes the unformatted commit visible; the push alone + # would land a green run and nobody would know. + run: | + git config user.email ops@ironcorelabs.com + git config user.name "Leeroy Travis" + git add -u + git commit -m "Apply cargo fmt and cargo sort auto-fixes" + git push origin "HEAD:$HEAD_REF" + echo "::error::cargo fmt/cargo sort auto-fixes were applied and pushed. CI will re-run on the new commit. Failing this run so the unformatted commit is visibly flagged." + exit 1 + - name: Check cargo fmt + if: steps.push_mode.outputs.can_push != 'true' + working-directory: ${{ env.UNDER_TEST_FOLDER }} + run: ${{inputs.cargo_command_env_vars}} cargo fmt --all -- --check || { echo "::error::Formatting check failed. Run 'cargo fmt --all' locally and commit the result."; exit 1; } + - name: Check cargo sort + if: steps.push_mode.outputs.can_push != 'true' + working-directory: ${{ env.UNDER_TEST_FOLDER }} + run: | + if grep -q '\[workspace]' Cargo.toml ; then + SORT_ARGS=--workspace + fi + ${{inputs.cargo_command_env_vars}} cargo sort --check $SORT_ARGS || { + echo "::error::Cargo.toml ordering check failed. Run 'cargo sort' locally and commit the result." + exit 1 + } clippy: name: Clippy diff --git a/.github/workflows/typescript-ci.yaml b/.github/workflows/typescript-ci.yaml index 4881352..0dfec95 100644 --- a/.github/workflows/typescript-ci.yaml +++ b/.github/workflows/typescript-ci.yaml @@ -1,5 +1,7 @@ # tag-version: v1 -# CI tasks for Node (TypeScript, JavaScript) repositories. +# CI tasks for Node (TypeScript, JavaScript) repositories. This runs tests, and for repos with +# `format` and `format:check` scripts, applies prettier auto-fixes (pushing them back to the PR +# branch when possible). # Customization: # - all customization should be available via inputs. If anything isn't available, try to add it to this as an input instead of copying and modifying directly @@ -64,8 +66,8 @@ on: description: "Keys to use when decrypting optional ironhide files." required: false WORKFLOW_PAT: - description: "GitHub PAT to use when checking out an earlier version of the branch for coverage." - required: true + description: "GitHub PAT to use when checking out an earlier version of the branch for coverage, and to push prettier auto-fix commits back to PR branches. If absent, formatting falls back to check-only mode." + required: false jobs: test: @@ -133,3 +135,103 @@ jobs: if: ${{ inputs.additional_steps != '' }} shell: bash run: ${{ join(inputs.additional_steps, '; ') }} + + # Unlike scalafmt or cargo fmt, prettier isn't guaranteed to be set up in a consumer repo, so + # repos opt out by omitting the scripts. This is a separate job so those repos skip the install entirely. + detect-format: + runs-on: ubuntu-24.04 + if: github.event_name != 'issue_comment' + outputs: + present: ${{ steps.check.outputs.present }} + steps: + - uses: actions/checkout@v7 + with: + sparse-checkout: package.json + sparse-checkout-cone-mode: false + - name: Look for prettier scripts + id: check + run: | + if jq -e '.scripts.format and .scripts["format:check"]' package.json > /dev/null 2>&1; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "present=false" >> "$GITHUB_OUTPUT" + echo "::notice::Skipping prettier formatting: package.json needs both a 'format' and a 'format:check' script." + fi + + # Not folded into `test`: that job is matrixed over node versions, so an auto-fix step there + # would race itself on push. + format: + runs-on: ubuntu-24.04 + needs: detect-format + if: needs.detect-format.outputs.present == 'true' + env: + NODE_OPTIONS: ${{ inputs.node_options }} + UNDER_TEST_FOLDER: typescript-repo + steps: + - name: Determine if auto-fix push is possible + id: push_mode + env: + EVENT_NAME: ${{ github.event_name }} + IS_FORK: ${{ github.event.pull_request.head.repo.fork }} + HAS_PAT: ${{ secrets.WORKFLOW_PAT != '' }} + # Forks can't be pushed to and push events have no PR branch to fix, so both check only. + run: | + if [ "$EVENT_NAME" = "pull_request" ] && [ "$HAS_PAT" = "true" ] && [ "$IS_FORK" != "true" ]; then + echo "can_push=true" >> "$GITHUB_OUTPUT" + else + echo "can_push=false" >> "$GITHUB_OUTPUT" + fi + - uses: actions/checkout@v7 + with: + path: ${{ env.UNDER_TEST_FOLDER }} + # The branch tip rather than the merge ref, so the auto-fix commit can be pushed back. + ref: ${{ steps.push_mode.outputs.can_push == 'true' && github.head_ref || '' }} + token: ${{ steps.push_mode.outputs.can_push == 'true' && secrets.WORKFLOW_PAT || github.token }} + - name: Set yarn mode + if: ${{ inputs.modern_yarn }} + run: corepack enable + - uses: actions/setup-node@v7 + with: + node-version: ${{ fromJSON(inputs.test_matrix_node_version)[0] }} + cache: yarn + cache-dependency-path: ${{ env.UNDER_TEST_FOLDER }}/yarn.lock + - name: Install additional dependencies + if: ${{ inputs.additional_system_deps != '' }} + run: | + sudo apt update + sudo apt install ${{ inputs.additional_system_deps }} + # Prettier comes from the lockfile, not a global install, so it can't disagree with the + # developer's version about the same files. + - name: Install modules + run: yarn + working-directory: ${{ env.UNDER_TEST_FOLDER }} + - name: Apply prettier auto-fixes + if: steps.push_mode.outputs.can_push == 'true' + id: autofix + working-directory: ${{ env.UNDER_TEST_FOLDER }} + run: | + yarn format + if [ -n "$(git status --porcelain --untracked-files=no)" ]; then + echo "changed=true" >> "$GITHUB_OUTPUT" + else + echo "changed=false" >> "$GITHUB_OUTPUT" + fi + - name: Commit and push auto-fixes, then fail + if: steps.autofix.outputs.changed == 'true' + working-directory: ${{ env.UNDER_TEST_FOLDER }} + env: + HEAD_REF: ${{ github.head_ref }} + # Failing after the push is what makes the unformatted commit visible; the push alone + # would land a green run and nobody would know. + run: | + git config user.email ops@ironcorelabs.com + git config user.name "Leeroy Travis" + git add -u + git commit -m "Apply prettier auto-fixes" + git push origin "HEAD:$HEAD_REF" + echo "::error::prettier auto-fixes were applied and pushed. CI will re-run on the new commit. Failing this run so the unformatted commit is visibly flagged." + exit 1 + - name: Check prettier + if: steps.push_mode.outputs.can_push != 'true' + working-directory: ${{ env.UNDER_TEST_FOLDER }} + run: yarn format:check || { echo "::error::Formatting check failed. Run 'yarn format' locally and commit the result."; exit 1; } From 27a140b66db3bad4d8cd97a52466570c827a1999 Mon Sep 17 00:00:00 2001 From: Murph Murphy Date: Thu, 22 Oct 2026 15:13:07 -0600 Subject: [PATCH 2/2] Address auto-format review: guard install changes, skip hooks, gate coverage on PAT --- .github/workflows/rust-ci.yaml | 8 +++----- .github/workflows/typescript-ci.yaml | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/.github/workflows/rust-ci.yaml b/.github/workflows/rust-ci.yaml index c8f9626..86ca878 100644 --- a/.github/workflows/rust-ci.yaml +++ b/.github/workflows/rust-ci.yaml @@ -260,7 +260,7 @@ jobs: EVENT_NAME: ${{ github.event_name }} IS_FORK: ${{ github.event.pull_request.head.repo.fork }} HAS_PAT: ${{ secrets.WORKFLOW_PAT != '' }} - # Forks can't be pushed to and push events have no PR branch to fix, so both check only. + # On fork PRs, head_ref names a branch in the fork. run: | if [ "$EVENT_NAME" = "pull_request" ] && [ "$HAS_PAT" = "true" ] && [ "$IS_FORK" != "true" ]; then echo "can_push=true" >> "$GITHUB_OUTPUT" @@ -288,8 +288,7 @@ jobs: SORT_ARGS=--workspace fi ${{inputs.cargo_command_env_vars}} cargo fmt --all - # --no-format keeps this to what --check actually fails on; its whitespace - # normalization is only a warning there. + # cargo sort --check fails only on ordering; formatting is a warning there. ${{inputs.cargo_command_env_vars}} cargo sort --no-format $SORT_ARGS if [ -n "$(git status --porcelain --untracked-files=no)" ]; then echo "changed=true" >> "$GITHUB_OUTPUT" @@ -301,8 +300,7 @@ jobs: working-directory: ${{ env.UNDER_TEST_FOLDER }} env: HEAD_REF: ${{ github.head_ref }} - # Failing after the push is what makes the unformatted commit visible; the push alone - # would land a green run and nobody would know. + # Fails after the push so the unformatted commit is flagged. run: | git config user.email ops@ironcorelabs.com git config user.name "Leeroy Travis" diff --git a/.github/workflows/typescript-ci.yaml b/.github/workflows/typescript-ci.yaml index 0dfec95..b12ccea 100644 --- a/.github/workflows/typescript-ci.yaml +++ b/.github/workflows/typescript-ci.yaml @@ -66,7 +66,7 @@ on: description: "Keys to use when decrypting optional ironhide files." required: false WORKFLOW_PAT: - description: "GitHub PAT to use when checking out an earlier version of the branch for coverage, and to push prettier auto-fix commits back to PR branches. If absent, formatting falls back to check-only mode." + description: "GitHub PAT used to check out sql-smelter when smelter_required is true, to check out an earlier version of the branch for coverage, and to push prettier auto-fix commits back to PR branches. If absent, coverage is skipped and formatting falls back to check-only mode." required: false jobs: @@ -76,6 +76,7 @@ jobs: env: NODE_OPTIONS: ${{ inputs.node_options }} UNDER_TEST_FOLDER: typescript-repo + HAS_WORKFLOW_PAT: ${{ secrets.WORKFLOW_PAT != '' }} strategy: matrix: node_version: "${{ fromJSON(inputs.test_matrix_node_version) }}" @@ -123,7 +124,7 @@ jobs: - name: Run tests run: yarn run test - name: Check test coverage - if: ${{ inputs.run_coverage && github.base_ref != '' }} + if: ${{ inputs.run_coverage && github.base_ref != '' && env.HAS_WORKFLOW_PAT == 'true' }} uses: joshuali925/Jest-Coverage-Diff@custom-directory with: fullCoverageDiff: false @@ -136,8 +137,7 @@ jobs: shell: bash run: ${{ join(inputs.additional_steps, '; ') }} - # Unlike scalafmt or cargo fmt, prettier isn't guaranteed to be set up in a consumer repo, so - # repos opt out by omitting the scripts. This is a separate job so those repos skip the install entirely. + # Repos opt out by omitting either script; a separate job lets them skip the install. detect-format: runs-on: ubuntu-24.04 if: github.event_name != 'issue_comment' @@ -146,6 +146,7 @@ jobs: steps: - uses: actions/checkout@v7 with: + ref: ${{ github.event.pull_request.head.sha }} sparse-checkout: package.json sparse-checkout-cone-mode: false - name: Look for prettier scripts @@ -174,7 +175,7 @@ jobs: EVENT_NAME: ${{ github.event_name }} IS_FORK: ${{ github.event.pull_request.head.repo.fork }} HAS_PAT: ${{ secrets.WORKFLOW_PAT != '' }} - # Forks can't be pushed to and push events have no PR branch to fix, so both check only. + # On fork PRs, head_ref names a branch in the fork. run: | if [ "$EVENT_NAME" = "pull_request" ] && [ "$HAS_PAT" = "true" ] && [ "$IS_FORK" != "true" ]; then echo "can_push=true" >> "$GITHUB_OUTPUT" @@ -200,8 +201,6 @@ jobs: run: | sudo apt update sudo apt install ${{ inputs.additional_system_deps }} - # Prettier comes from the lockfile, not a global install, so it can't disagree with the - # developer's version about the same files. - name: Install modules run: yarn working-directory: ${{ env.UNDER_TEST_FOLDER }} @@ -210,6 +209,11 @@ jobs: id: autofix working-directory: ${{ env.UNDER_TEST_FOLDER }} run: | + if [ -n "$(git status --porcelain --untracked-files=no)" ]; then + git status --porcelain --untracked-files=no + echo "::error::Installing modules modified the tracked files above. Run 'yarn' locally and commit the result." + exit 1 + fi yarn format if [ -n "$(git status --porcelain --untracked-files=no)" ]; then echo "changed=true" >> "$GITHUB_OUTPUT" @@ -221,14 +225,13 @@ jobs: working-directory: ${{ env.UNDER_TEST_FOLDER }} env: HEAD_REF: ${{ github.head_ref }} - # Failing after the push is what makes the unformatted commit visible; the push alone - # would land a green run and nobody would know. + # Fails after the push so the unformatted commit is flagged. run: | git config user.email ops@ironcorelabs.com git config user.name "Leeroy Travis" git add -u - git commit -m "Apply prettier auto-fixes" - git push origin "HEAD:$HEAD_REF" + git -c core.hooksPath=/dev/null commit -m "Apply prettier auto-fixes" + git -c core.hooksPath=/dev/null push origin "HEAD:$HEAD_REF" echo "::error::prettier auto-fixes were applied and pushed. CI will re-run on the new commit. Failing this run so the unformatted commit is visibly flagged." exit 1 - name: Check prettier