feat(validation): enforce breaking acknowledgement - #653
Conversation
X-Lerian-Ref: 0x1
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Summary by CodeRabbit
WalkthroughThe pull-request validation workflows now run a mandatory breaking-change guard, enforce exact author acknowledgement, expose guard outputs, report blocking-check runtime failures, and add workflow regression coverage and documentation. ChangesBreaking Change Validation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
🔍 Lint Analysis
|
🛡️ CodeQL Analysis ResultsLanguages analyzed: ✅ No security issues found. 🔍 View full scan logs | 🛡️ Security tab |
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-validation.yml:
- Around line 343-349: The workflow regression test should extract the
acknowledgement literals from both the detector in “Detect breaking changes” and
the “breaking-change-comment” configuration, then assert they are identical.
Preserve the existing check for “breaking-change-acknowledgement” while adding
coverage for the second literal.
- Around line 136-144: Update the breaking-change-guard action reference in the
“Detect breaking changes” workflow step from v1 to a release that declares and
supports the acknowledgement-match-mode input, while preserving the existing
exact-visible-line configuration.
In `@docs/pr-validation.md`:
- Around line 17-20: Update the “Draft PR support” description in the
documentation to state that draft runs still write the step summary, while
deferring only PR comments, pr-validation-reporter, guard comments, and Slack
notifications until the PR is ready for review. Apply the same wording to the
repeated draft-support entry.
In `@src/validate/breaking-change-guard/test-workflow.py`:
- Around line 430-436: Update the tests around the breaking-change guard step
assertions to first extract the breaking-change-guard job, then call
extract_step on that job content rather than the full WORKFLOW. Apply this
scoping both to the “Detect breaking changes” lookup near the existing
assertions and to the lookup around line 515, reusing the job-extraction pattern
already used by
test_guard_checkout_has_full_history_without_persisted_credentials.
- Around line 342-377: Add an EnforcementTests live-mode test that calls
enforce(DETECTION_SUCCEEDED="false") and asserts a nonzero return code, covering
the fail-closed enforcement behavior. If dry-run behavior is intended to remain
non-blocking, add the corresponding dry-run test and assert it succeeds while
detection is false.
- Around line 148-163: Update execute_body to construct a minimal allowlisted
environment instead of copying os.environ, while retaining the explicitly
provided env values and the temporary GITHUB_OUTPUT path. Do not include
GITHUB_STEP_SUMMARY, and add coverage verifying snippet execution fails or
behaves correctly when a required variable is absent.
- Around line 125-136: Update extract_workflow_call_section to stop depending on
the next_section argument or sibling section order. After locating the
section_name marker, scan subsequent lines and end at the first non-empty line
whose indentation is 4 spaces or fewer, while preserving the section content for
nested lines.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7050c286-2303-4e20-bdde-310eec2ff8e6
📒 Files selected for processing (16)
.github/workflows/go-pr-validation.yml.github/workflows/js-pr-validation.yml.github/workflows/pr-validation.yml.github/workflows/self-pr-validation.ymldocs/go-pr-validation.mddocs/js-pr-validation.mddocs/pr-validation.mdsrc/notify/pr-validation-reporter/README.mdsrc/notify/pr-validation-reporter/action.ymlsrc/validate/breaking-change-guard/README.mdsrc/validate/breaking-change-guard/action.ymlsrc/validate/breaking-change-guard/detect.shsrc/validate/breaking-change-guard/test-workflow.pysrc/validate/breaking-change-guard/test.shsrc/validate/pr-checks-summary/README.mdsrc/validate/pr-checks-summary/action.yml
Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
|
@CodeRabbit approve |
✅ Action performedComments resolved and changes approved. |
GitHub Actions Shared Workflows
Description
Makes breaking-change detection mandatory for every caller of the PR validation, Go validation, and JS/TS validation workflows.
The guard runs for every pull request target branch, including drafts, and enforces through the existing
Blocking Checksstatus. There is no enable input, branch filter, configurable acknowledgement, or guard opt-out.A breaking PR passes only when its description contains this exact, visible, case-sensitive author acknowledgement line:
The acknowledgement records author awareness. It does not grant maintainer authorization.
Detection and enforcement fail closed for unsupported events, shallow history, action failures, malformed outputs, cancelled jobs, and blocking runtime failures. Feedback comments run separately with minimal write permissions and never control enforcement.
Reporter, summary, Slack, and all reusable workflow outputs now propagate guard and blocking-runtime state with fail-closed fallbacks.
Caller trigger configuration must include
editedso acknowledgement changes rerun validation andready_for_reviewso full validation reruns when a draft becomes ready.Type of Change
feat: New workflow or new input/output/step in an existing workflowfix: Bug fix in a workflow (incorrect behavior, broken step, wrong condition)perf: Performance improvement (e.g. caching, parallelism, reduced steps)refactor: Internal restructuring with no behavior changedocs: Documentation only (README, docs/, inline comments)ci: Changes to self-CI (workflows under.github/workflows/that run on this repo)chore: Dependency bumps, config updates, maintenancetest: Adding or updating testsBREAKING CHANGE: Callers must update their configuration after this PRBreaking Changes
None. The workflow remains on
v1by product decision. Callers must addeditedandready_for_reviewto theirpull_requestactivity types for complete revalidation.Testing
@this-branchor the beta tagCaller repo / workflow run: Local validation passed 42 workflow-state tests and 56 detector tests. Actionlint, ShellCheck, duplicate-safe YAML parsing, composite schema validation, JavaScript syntax checks, documentation checks, and diff checks also passed.
Related Issues
Related PRs: #649, #651, and #652.