Conversation
## Summary - Require source publishing to pass validation successfully. - Make audit path discovery fail on API execution errors. - Extend the fleet interface contract and regression coverage for both guards. ## Root Cause A job-level condition replaced GitHub Actions' implicit success gate. The audit helper also redirected API errors into an ordinary absent-path result. ## Validation - 728 Python tests - audit self-tests and spec validation - ruff, mypy, and coverage - actionlint, markdownlint, EditorConfig, CSpell, ShellCheck, and PSScriptAnalyzer - live fleet audit and repository configuration check Closes #842 Closes #843
There was a problem hiding this comment.
Pull request overview
Promotes the develop changes that make release publishing and audit path discovery fail closed (rather than fail open) by tightening the publish workflow gate, updating the spec/audit interface contract and selftests, and adding a regression test to prevent future drift.
Changes:
- Gate
publishon successfulvalidatein.github/workflows/publish-release.yml, and encode that requirement in the interface contract (spec/files.json) + selftests (spec/audit.py). - Make audit path discovery fail loudly on
gh apierrors by prefetching root and.githubdirectory listings inAUDIT.md. - Add
scripts/tests/test_release_guards.pyto regression-test both guards.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/files.json | Extends the publish-release interface contract to require the validation-success guard token in the publish job. |
| spec/audit.py | Updates interface selftest fixtures/cases to enforce and regression-test the new publish guard. |
| scripts/tests/test_release_guards.py | Adds executable regression coverage for the release validation gate and audit probe error handling. |
| AUDIT.md | Switches audit path discovery from a fail-open probe to fail-loud directory listing + local membership checks. |
| .github/workflows/publish-release.yml | Makes publish conditional on needs.validate.result == 'success' in addition to the plan output. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
## Summary - Separate the Dependabot Contents API read from Base64 decoding. - Fail immediately when either boundary fails. - Execute the documented probe against successful, API-failure, and decode-failure cases. ## Root Cause The declaration pipeline ran without `pipefail`, so a later successful command could mask an earlier API or decode failure. ## Validation - 728 Python tests - audit self-tests and spec validation - ruff, mypy, and coverage - actionlint, markdownlint, EditorConfig, CSpell, ShellCheck, and PSScriptAnalyzer Follow-up to #843 and the Copilot finding on #846.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
AUDIT.md:145
- The snippet is fenced as
sh, but these lines rely on bash-specific features (<<<here-strings and$'\n'). If a reader runs the command under POSIXsh(e.g.,dash), it will fail and the audit procedure won't be reproducible as written. Consider rewriting to POSIX-compatible stdin pipes (or otherwise clearly make it bash-only).
dependabot_yaml=$(base64 -d <<<"$dependabot_content") || exit 1
decl=$(grep -oE '^[[:space:]]*-?[[:space:]]*package-ecosystem:[[:space:]]*"?[a-z-]+' <<<"$dependabot_yaml" | grep -oE '[a-z-]+$' | sort -u)
root_paths=$(gh api "repos/<owner>/<repo>/contents?ref=<ground>" --jq '.[].path') || exit 1
github_paths=$(gh api "repos/<owner>/<repo>/contents/.github?ref=<ground>" --jq '.[].path') || exit 1
has() { grep -Fxq "$1" <<<"$root_paths"$'\n'"$github_paths"; }
## Summary - Label all Bash-dependent audit command blocks as `bash`. - Add regression coverage that rejects a POSIX `sh` label on these blocks. ## Root Cause The audit commands use process substitution, here-strings, and ANSI-C quoting, but their fence labels implied generic POSIX shell compatibility. ## Validation - 729 Python tests - audit self-tests and spec validation - ruff, mypy, and coverage - actionlint, markdownlint, EditorConfig, CSpell, ShellCheck, and PSScriptAnalyzer Follow-up to the suppressed Copilot finding on #846.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/tests/test_release_guards.py:1
- Add the standard
#!/usr/bin/env python3shebang used by the otherscripts/tests/*.pyfiles in this repo, so the new test file is consistent and can be run directly the same way.
"""Protect release and audit boundaries from fail-open regressions."""
## Summary - require the canonical `pypi-build-` artifact stem in both carried contracts - require the same stem in the default uploader and documented publisher consumer - reject the legacy `pypilibrary-build-` stem anywhere in tracked content ## Root Cause The artifact rename in #821 aligned the existing contracts and implementations, but no automated check bound those surfaces together. A later edit could therefore recreate the contradiction reported by #845. ## Impact Contract drift between the hub and downstream PyPI workflows fails in the unit-test gate before it can be carried into another repository. ## Verification - `python3 -m unittest scripts.tests.test_release_guards` - `python3 -m unittest discover -s scripts/tests` (730 tests) - `uvx ruff@latest check scripts/tests/test_release_guards.py` - `uvx ruff@latest format --check scripts/tests/test_release_guards.py` - `git diff --check` Closes #845
|
Suppressed finding from review round (1): AUDIT.md:145 says the sh fence contains Bash-only here-strings and dollar-quoted text. Fixed in 1cd5b84. The audit command fences that contain Bash syntax are labeled bash, and the regression test in scripts/tests/test_release_guards.py verifies the class. |
## Summary - add the standard Python shebang to the release-guard test - mark the test executable so the shebang is functional ## Root Cause The release-guard test joined `scripts/tests` without the entry-point convention used by every other test file in that directory. ## Impact The test runs directly as a script and remains available through unittest discovery. ## Verification - `./scripts/tests/test_release_guards.py` (4 tests) - `python3 -m unittest discover -s scripts/tests` (730 tests) - `uvx ruff@latest check scripts/tests/test_release_guards.py` - `uvx ruff@latest format --check scripts/tests/test_release_guards.py` - `git diff --check` Addresses a suppressed Copilot finding on #846.
|
Suppressed finding from review round (1): scripts/tests/test_release_guards.py:1 says the standard executable Python shebang is missing. Fixed in 9b6c01f. The file carries #!/usr/bin/env python3 with mode 100755, and direct execution runs all 4 release-guard tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
spec/audit.py:2447
- The "publish-release.yml stub missing the validate job" test fixture builds a publish job that declares
needs: [plan]but then referencesneeds.validate.result. In GitHub Actions,needs.<job>is only populated for jobs listed inneeds, so this stub is internally inconsistent and less representative of a real workflow shape.
Adjust the stub to keep the publish job’s needs list consistent with the if: expression (while still omitting the validate: job definition to exercise the missing-job finding).
" needs: [plan]\n"
" if: ${{ needs.validate.result == 'success' }}\n"
## Summary - include `validate` in the missing-job fixture's `needs` list - keep the `validate:` job definition absent so the fixture isolates that contract failure ## Root Cause The fixture referenced `needs.validate.result` without declaring `validate` as a dependency. It therefore modeled an additional workflow defect beyond the missing job it was designed to test. ## Impact The audit self-test exercises one deliberate defect and remains representative of a real caller shape. ## Verification - `python3 spec/audit.py --selftest` - `python3 -m unittest discover -s scripts/tests` (730 tests) - `uvx ruff@latest check spec/audit.py` - `uvx ruff@latest format --check spec/audit.py` - `git diff --check` Addresses a suppressed Copilot finding on #846.
|
Suppressed finding from review round (1): spec/audit.py:2447 says the missing-validate-job fixture references needs.validate without declaring that dependency. Fixed in bb4d603. The publish job lists needs: [plan, validate] while the validate job definition remains absent, so the self-test isolates the intended missing-job finding and still reports exactly once. |
Summary
Source
a08713a)af2c8ff)1cd5b84)87c3960)9b6c01f)bb4d603)Validation
Closes #842
Closes #843
Closes #845