fix(policy): make the repository-policy self-test able to fail - #5
Merged
Conversation
A bare `[[ ... ]]` assertion inside a function does not abort under `set -e` in this script, so every assertion in `self_test` was unenforced: the suite printed "passed" and exited 0 even when a validator was broken outright. Verified on the prior revision that breaking `validate_branch` still passed. Route assertions through `expect_error_count` / `expect_error_count_min`, count failures, return nonzero from `self_test`, and call `self_test || exit 1` at the entry point. No change to the validation rules themselves. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
self_testassertion throughexpect_error_count/expect_error_count_min, count failures, return nonzero fromself_test, and callself_test || exit 1at the entry point.validate_branch,validate_header,validate_body) themselves.Context
The
cicheck in this repository runs onlybash -n repository-policy/validate.shplusbash repository-policy/validate.sh --self-test. A bare[[ ... ]]assertion inside a function does not abort underset -ehere, so every assertion inself_testwas decorative: the suite reached its finalprintfand exited 0 regardless of outcome.Verified against the prior revision
2af0536: rewritingvalidate_branchso it rejects every valid branch still printedRepository policy self-test passedand exited 0. This repository's own required check has therefore been vacuous since the suite was written, and any future change to the shared policy could regress silently.This PR was split from an abandoned larger change (former #4) that tried to add a workflow-tamper guard. That guard was dropped after an adversarial review showed a path-scoped guard cannot contain the threat it targeted — the check's behaviour is defined by too many ordinary repository files. This self-test fix is the part worth keeping, and it stands on its own.
Validation
bash -n repository-policy/validate.shpasses.bash repository-policy/validate.sh --self-testpasses on bash 3.2 (macOS) and 5.x.validate_branchmutated to reject valid branches, the suite now printsSELF-TEST FAILED: ...and exits 1 — where the same mutation passed silently before.Impact and rollback
Test-harness only; the validation behaviour enforced on pull requests is unchanged, and this repository's
cicheck can now actually fail when the validator regresses. Roll back by reverting the squash commit.Screenshots
Not applicable because this pull request changes a shell test harness only.