From 5e609d3d6428d27ffed9785ccbca1ccea35d16f7 Mon Sep 17 00:00:00 2001 From: Nicolas Brieussel Date: Tue, 14 Apr 2026 13:02:41 +0200 Subject: [PATCH 1/5] =?UTF-8?q?refactor:=20simplify=20dry-run=20gate=20?= =?UTF-8?q?=E2=80=94=20fail=20on=20changes,=20no=20JS=20parsing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove 80 lines of embedded github-script JS: upsert comment logic, known-bug detection, repo name extraction. Replace with two plain bash steps: run NOP with tee, grep for "There are changes for branch" and fail if found. Output is visible directly in Actions logs. Also drop pull-requests: write permission (no PR comment posted). Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/pr-dry-run.yml | 68 ++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 .github/workflows/pr-dry-run.yml diff --git a/.github/workflows/pr-dry-run.yml b/.github/workflows/pr-dry-run.yml new file mode 100644 index 0000000..04d605f --- /dev/null +++ b/.github/workflows/pr-dry-run.yml @@ -0,0 +1,68 @@ +name: Dry-run gate + +on: + pull_request: + branches: [main] + +permissions: + contents: read + +jobs: + dry-run: + name: Safe-settings dry-run + runs-on: ubuntu-24.04 + timeout-minutes: 30 + # Do not run on fork PRs — secrets are not available there + if: github.event.pull_request.head.repo.full_name == github.repository + env: + SAFE_SETTINGS_VERSION: 2.1.17 + SAFE_SETTINGS_CODE_DIR: ${{ github.workspace }}/.safe-settings-code + + steps: + - name: Checkout PR branch + uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + + - name: Checkout safe-settings app + uses: actions/checkout@v4 + with: + repository: github/safe-settings + ref: ${{ env.SAFE_SETTINGS_VERSION }} + path: ${{ env.SAFE_SETTINGS_CODE_DIR }} + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "20" + cache: npm + cache-dependency-path: ${{ env.SAFE_SETTINGS_CODE_DIR }}/package-lock.json + + - name: Install dependencies + run: npm ci + working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} + + - name: Run dry-run (NOP) + run: | + set -o pipefail + npm run full-sync 2>&1 | tee /tmp/dry-run.log + working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} + env: + GH_ORG: ${{ vars.SAFE_SETTINGS_GH_ORG }} + APP_ID: ${{ vars.SAFE_SETTINGS_APP_ID }} + PRIVATE_KEY: ${{ secrets.SAFE_SETTINGS_PRIVATE_KEY }} + GITHUB_CLIENT_ID: ${{ vars.SAFE_SETTINGS_GITHUB_CLIENT_ID }} + GITHUB_CLIENT_SECRET: ${{ secrets.SAFE_SETTINGS_GITHUB_CLIENT_SECRET }} + WEBHOOK_SECRET: ${{ secrets.WEBHOOK_SECRET }} + ADMIN_REPO: admin + DEPLOYMENT_CONFIG_FILE: ${{ github.workspace }}/deployment-settings.yml + FULL_SYNC_NOP: "true" + LOG_LEVEL: debug + + - name: Fail if config changes detected + run: | + if grep -q "There are changes for branch" /tmp/dry-run.log; then + echo "Config changes detected — review the dry-run output above." + grep "There are changes for branch" /tmp/dry-run.log + exit 1 + fi From 1c21a4590329bb3a927dcffe8e7af867575e2390 Mon Sep 17 00:00:00 2001 From: Nicolas Brieussel Date: Tue, 14 Apr 2026 13:08:54 +0200 Subject: [PATCH 2/5] fix: crash fails job, diffs are informational (continue-on-error) - "Run dry-run" keeps set -o pipefail: npm crash = job fails (desired) - "Report config changes" gets if: always() + continue-on-error: true: runs even after a crash, but finding diffs does not block the PR - warning annotation surfaces detected changes without blocking merge Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/pr-dry-run.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-dry-run.yml b/.github/workflows/pr-dry-run.yml index 04d605f..ba9c332 100644 --- a/.github/workflows/pr-dry-run.yml +++ b/.github/workflows/pr-dry-run.yml @@ -59,10 +59,15 @@ jobs: FULL_SYNC_NOP: "true" LOG_LEVEL: debug - - name: Fail if config changes detected + # Runs even if the previous step crashed, so changes are always surfaced. + # continue-on-error: finding diffs is informational, not a merge blocker — + # a human must review but the PR is not blocked. + - name: Report config changes + if: always() + continue-on-error: true run: | if grep -q "There are changes for branch" /tmp/dry-run.log; then - echo "Config changes detected — review the dry-run output above." + echo "::warning::Config changes detected — human review required before merging" grep "There are changes for branch" /tmp/dry-run.log exit 1 fi From 2783eb9e8f62c721e1c731fccb752eeed66b1c44 Mon Sep 17 00:00:00 2001 From: Nicolas Brieussel Date: Tue, 14 Apr 2026 13:20:50 +0200 Subject: [PATCH 3/5] fix: remove set -o pipefail, check_suite crash is expected and accepted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without pipefail, tee's exit code (always 0) wins — npm crashing with the known check_suite NOP bug no longer fails the step. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/pr-dry-run.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/pr-dry-run.yml b/.github/workflows/pr-dry-run.yml index ba9c332..0dc2467 100644 --- a/.github/workflows/pr-dry-run.yml +++ b/.github/workflows/pr-dry-run.yml @@ -43,9 +43,7 @@ jobs: working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} - name: Run dry-run (NOP) - run: | - set -o pipefail - npm run full-sync 2>&1 | tee /tmp/dry-run.log + run: npm run full-sync 2>&1 | tee /tmp/dry-run.log working-directory: ${{ env.SAFE_SETTINGS_CODE_DIR }} env: GH_ORG: ${{ vars.SAFE_SETTINGS_GH_ORG }} From 30e6414ecb90b3cb8a723625be7f9898758c8c2a Mon Sep 17 00:00:00 2001 From: Nicolas Brieussel Date: Tue, 14 Apr 2026 13:24:22 +0200 Subject: [PATCH 4/5] fix: also print NOPCOMMAND entries alongside detected changes NOPCOMMAND lines contain the actual diff (existing vs expected state) and are logged just before "There are changes for branch". A single grep -E alternation surfaces both without added complexity. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/pr-dry-run.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-dry-run.yml b/.github/workflows/pr-dry-run.yml index 0dc2467..d1fb7fe 100644 --- a/.github/workflows/pr-dry-run.yml +++ b/.github/workflows/pr-dry-run.yml @@ -66,6 +66,6 @@ jobs: run: | if grep -q "There are changes for branch" /tmp/dry-run.log; then echo "::warning::Config changes detected — human review required before merging" - grep "There are changes for branch" /tmp/dry-run.log + grep -E "NOPCOMMAND|There are changes for branch" /tmp/dry-run.log exit 1 fi From 1a3d34657c14d2201f9c9971a19f32b67f71d300 Mon Sep 17 00:00:00 2001 From: Nicolas Brieussel Date: Tue, 14 Apr 2026 13:27:43 +0200 Subject: [PATCH 5/5] fix: use grep -A 2 to show diff details after each "There are changes" line The change details (JSON diff + description) are logged on the 2 lines immediately after "There are changes for branch", not before it. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/pr-dry-run.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-dry-run.yml b/.github/workflows/pr-dry-run.yml index d1fb7fe..6f9e4d0 100644 --- a/.github/workflows/pr-dry-run.yml +++ b/.github/workflows/pr-dry-run.yml @@ -66,6 +66,6 @@ jobs: run: | if grep -q "There are changes for branch" /tmp/dry-run.log; then echo "::warning::Config changes detected — human review required before merging" - grep -E "NOPCOMMAND|There are changes for branch" /tmp/dry-run.log + grep -A 2 "There are changes for branch" /tmp/dry-run.log exit 1 fi