Skip to content

feat: automated dry-run gate on every PR#11

Merged
nbrieussel merged 5 commits intomainfrom
feat/pr-dry-run-gate
Apr 14, 2026
Merged

feat: automated dry-run gate on every PR#11
nbrieussel merged 5 commits intomainfrom
feat/pr-dry-run-gate

Conversation

@nbrieussel
Copy link
Copy Markdown

Summary

Adds .github/workflows/pr-dry-run.yml — a required-status-check-ready gate that runs safe-settings in NOP (dry-run) mode on every PR targeting main and posts a summary comment.

Gate logic

Situation Check result Action
Dry-run OK + 0 changes ✅ pass Merge allowed
Dry-run OK + N changes ⚠️ pass with comment Human reviews whether diffs are intentional
Crash = known check_suite bug ⚠️ pass with warning Known pre-existing bug in safe-settings 2.1.17, non-blocking
Crash = unexpected error ❌ fail Blocks the PR

The check only fails hard on a real unexpected error. Intentional diffs (e.g. a PR that modifies settings.yml) remain visible via the PR comment for human review — they do not block.

Implementation details

  • Runs on ubuntu-24.04, 30-minute timeout, skips fork PRs (no secrets available)
  • Checks out the PR branch at its exact SHA, then fetches github/safe-settings@2.1.17 fresh (same pattern as the production sync workflow)
  • Uses npm ci (not npm install) for reproducible installs
  • PR comment is upserted (updated on re-push, not duplicated) using actions/github-script@v7
  • The known crash message Cannot read properties of undefined (reading 'check_suite') is detected and treated as a warning, not an error

Next steps

Once merged, this workflow itself will trigger on subsequent PRs — this PR is the first one that would have been gated. To wire it as a required status check: go to branch protection for main → add Safe-settings dry-run as a required status check.

Part of repo hygiene audit.

@github-actions
Copy link
Copy Markdown

⚠️ Dry-run: known NOP bug (check_suite)

safe-settings hit the pre-existing check_suite crash (safe-settings 2.1.17 bug).
This is not related to the changes in this PR — it is a known limitation of the
current version. The check is not blocking.

@nbrieussel
Copy link
Copy Markdown
Author

Validation

Type de changement : nouveau workflow CI déclenché par pull_request. Ne peut pas être testé formellement avant d'être sur main (le trigger pull_request ne s'active qu'une fois le fichier présent sur la base branch).

Analyse du code :

  • Utilise uniquement des actions officielles GitHub : actions/checkout@v4, actions/setup-node@v4, actions/github-script@v7
  • permissions: contents: read / pull-requests: write — moindre privilège ✅
  • Guard fork PR : if: github.event.pull_request.head.repo.full_name == github.repository
  • Secrets exposés uniquement au step sync, pas au step github-script
  • Le check ne bloque que sur erreur inattendue, pas sur le bug check_suite connu ✅

Un TODO post-merge : aligner SAFE_SETTINGS_VERSION sur 2.1.19 une fois PR #13 mergée (actuellement 2.1.17 dans ce workflow).

Verdict : ⚠️ Trop difficile à tester formellement avant merge, mais code lisible, patterns GitHub Actions standards, zéro dépendance tierce. Prête pour review humaine + approbation.

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 <noreply@anthropic.com>
@nbrieussel nbrieussel force-pushed the feat/pr-dry-run-gate branch from aa0adf7 to 5e609d3 Compare April 14, 2026 11:02
Nicolas Brieussel and others added 4 commits April 14, 2026 13:08
- "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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…" 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 <noreply@anthropic.com>
@nbrieussel nbrieussel merged commit eb7a082 into main Apr 14, 2026
1 check passed
@nbrieussel nbrieussel deleted the feat/pr-dry-run-gate branch April 14, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant