Skip to content

Guard the release tests against masked test failures and other PyPI uploads - #648

Merged
MaxGhenis merged 3 commits into
masterfrom
claude/core-591-review-followups-ac1zqs
Nov 3, 2026
Merged

MaxGhenis merged 3 commits into
masterfrom
claude/core-591-review-followups-ac1zqs

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups from the independent review of #591 (#591 (comment)): P3 1, P3 6 and the actionlint note in section 8. No file under policyengine_core/ or .github/workflows/ changes.

Changes

  • The release tests reject three ways Test could pass after its tests fail (P3 1). test_test_job_cannot_pass_with_failing_tests fails if push.yaml's Test job or any of its steps has continue-on-error, if the step that runs make test has an if:, or if that step runs anything besides make test (for example || true). Publish needs: Test, so any of these would let a release whose tests failed reach PyPI. A parametrized test checks that the guard rejects each of the four mutations. The guard misses a shell: on the test step that ignores its script, a - prefix on the Makefile's test recipe and PYTEST_ADDOPTS: --collect-only on the job (review P3 5).
  • The PyPI upload check reads every workflow file (P3 6). test_only_push_publish_job_uploads_to_pypi reads every *.yaml/*.yml in .github/workflows/ and collects each job with a step that uses pypa/gh-action-pypi-publish@, or runs twine upload or uv publish. It asserts that no job but push.yaml's Publish uploads. A parametrized test adds each upload form to a copy of pr.yaml and checks that the scan finds it. Other publishers (poetry, flit, hatch, pdm and maturin publish), an upload inside a make target and a reusable workflow called from another repository pass the scan (review P3 1 and 2).
  • The actionlint config drops two dead ignores (section 8). .github/actionlint.yaml drops both ignores for the old "Deploy documentation" step, which Deploy the documentation site from its own release job #594 moved into the Docs job, and drops the now-empty paths: key. The header comment stays.
  • The changelog fragment names what the tests check.

Tests

  • uv run pytest -q -p no:cacheprovider tests/core/test_release_tagging.py: 16 passed before, 25 passed after.
  • tests/core/test_docs_deployment.py: 5 passed before and after.
  • File-level mutants on push.yaml / pr.yaml, restored after each one: M6 (job continue-on-error), M7 (step continue-on-error), M8 (make test || true), M9 (if: false on the test step) and M10 (a PyPI upload step in pr.yaml) each fail the new guard.
  • actionlint 1.7.12 with ShellCheck 0.11.0 and pyflakes 4.1.0 on PATH reports "Found 0 errors in 2 files" with the new config.

Independent review: #648 (comment)

🤖 Generated with Claude Code

…job uploads to PyPI

Publish waits on Test, so continue-on-error on Test or its steps, a
condition on the test step, or anything such as || true after make test
would let a release whose tests failed reach PyPI. The publishing check
also read push.yaml alone, so an upload in any other workflow file
would have bypassed it. Follow-ups to the review of #591 (P3 1 and 6).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwQq6Jzt9cKvgaMoYvZDfG
#594 moved that step out of Test into the Docs job, so both ignored
messages no longer occur; keeping them would hide them if they came back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LwQq6Jzt9cKvgaMoYvZDfG
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Ready for hub review
Head: 3620e8d55db72781919638b6dc6d0524bb181f97 (base master at 55cd1915cb)

Before pushing, I checked that neither claude/core591-release-guards nor this branch existed and that no open PR made this change. This PR had no commits or comments from anyone else.

Items

1. Test can't pass after its tests fail (review P3 1): FIXED

  • tests/core/test_release_tagging.py:392-406 assert_test_job_fails_when_tests_fail includes the reviewer's guard unchanged (no continue-on-error on Test or any of its steps). It adds two checks on the single step whose run contains make test: the step has no if: (catches M9), and its run stripped is exactly make test (catches M8, || true). The second check follows the existing tag-step check (:204). Neither fires on master: Test has no step-level if:, and its test step is exactly run: make test.
  • :409-412 test_test_job_cannot_pass_with_failing_tests applies that check to push.yaml.
  • :415-453 parametrized in-suite mutants (continue_test_job_on_error, continue_after_test_failure, ignore_test_failure, skip_tests) check that each mutation is rejected with its message.

2. Every workflow file is checked for PyPI uploads (review P3 6): FIXED

  • :344-360 pypi_publishing_jobs(dir) reads every *.yaml/*.yml in a directory. It returns (file, job) for each job with a step whose uses starts with pypa/gh-action-pypi-publish@, or whose run matches \b(twine\s+upload|uv\s+publish)\b.
  • :363-366 test_only_push_publish_job_uploads_to_pypi asserts the result for .github/workflows/ is [("push.yaml", "Publish")]. I added it as a new test beside test_publish_workflow_publishes_only_after_tests_pass (:315-341, unchanged).
  • :369-389 copies the workflows to tmp_path, adds each upload form (action, python -m twine upload, uv publish) to pr.yaml's Test, and asserts it's found.

3. Dead actionlint ignores (review section 8): FIXED

  • .github/actionlint.yaml now holds only its two header comment lines. I removed both ignores and the paths: key.
  • actionlint 1.7.12 (release binary), with ShellCheck 0.11.0 (shellcheck-py) and pyflakes 4.1.0 on PATH: Found 0 errors in 2 files both before and after (-verbose), exit 0.
  • A probe workflow triggered both [shellcheck] SC2086 and [pyflakes] 'os' imported but unused, which confirms both integrations ran.
  • To confirm actionlint reads this file, I appended invalid YAML to it, and it failed with could not parse config file ".../.github/actionlint.yaml". The comment-only file parses cleanly.

Changelog: changelog.d/core591-release-guards.changed.md (type changed, per github-prs.md:50-52).

Tests (each file in its own process)

test_release_tagging.py test_docs_deployment.py
master 55cd1915 16 passed 5 passed
head 25 passed 5 passed

uv run ruff format --check .: 371 files already formatted. ruff check on the test file passes.

Mutants

I made each change on the working copy of the workflow, ran both test files in separate processes, and restored the file from a saved copy afterwards. git status was clean at the end.

Mutant New guard Result
M6 continue-on-error: true on job Test test_test_job_cannot_pass_with_failing_tests FAILED 3 failed, 22 passed
M7 continue-on-error: true on "Run tests" FAILED 3 failed, 22 passed
M8 run: make test || true FAILED 4 failed, 21 passed
M9 if: false on "Run tests" FAILED 2 failed, 23 passed
M10 pypa/gh-action-pypi-publish@release/v1 step in pr.yaml Test test_only_push_publish_job_uploads_to_pypi FAILED 1 failed, 24 passed
unmodified — 25 passed

The extra failures in M6–M9 come from the in-suite mutant tests. They load the already-mutated file, so either the check fires before the expected message or find_test_step finds no exact make test step. test_docs_deployment.py gave 5 passed for every mutant.

CI

This is a draft, so CI hasn't run. No runs exist at the head.

Judgment calls

  • Branch name. This session's environment fixes the branch at claude/core-591-review-followups-ac1zqs, so I pushed there instead of claude/core591-release-guards. Its fragment uses the brief's slug, core591-release-guards.changed.md, because a / in the name would create a subdirectory.
  • Editing under .github/. The brief's limits forbid that, but item 3 names .github/actionlint.yaml explicitly, so I edited only that file. I made no committed change to anything under .github/workflows/.
  • Refactor. I put the reviewer's guard inside an assert_* helper with failure messages, the same pattern as assert_publish_tags_only_after_pypi_succeeds, so the in-suite mutants can match on messages. The behaviour is a superset of the reviewer's snippet.
  • The if: check covers only the test step. A condition on an earlier step, such as make install, would make make test fail rather than mask it.
  • Header comment. I kept it in actionlint.yaml rather than deleting the file, so the file's purpose stays documented.
  • What the upload scan misses. It doesn't follow reusable workflows (job-level uses:), composite actions or Makefile targets. Today make build is only a build (as the review found), and no job-level uses: exists.

Proposed follow-up

  • Extend the PyPI upload check to reusable workflows and local actions. Today the scan reads only step-level uses/run in .github/workflows/. A job-level uses: or a composite action under .github/actions/ that uploads would slip past it.

Generated by Claude Code

@MaxGhenis
MaxGhenis marked this pull request as ready for review November 3, 2026 19:25
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Hub review in progress

Head 3620e8d55db72781919638b6dc6d0524bb181f97, unchanged since the hand-off. These findings are interim. The final review will follow as a separate comment.

  • Tests, one file per process: head gives 25 passed (test_release_tagging.py) and 5 passed (test_docs_deployment.py). Master 55cd1915cb gives 16 and 5.
  • Mutants M6–M14 are each killed by test_test_job_cannot_pass_with_failing_tests (M6, M7, M8, M9, M11, M12, M13) or by test_only_push_publish_job_uploads_to_pypi (M10, M14). M15 survives, as the hand-off says.
  • Deleting each check from assert_test_job_fails_when_tests_fail fails exactly the in-suite mutant test that targets it.
  • Found so far (P3): poetry/flit/hatch/pdm/maturin publish and a Makefile target that uploads all pass the PyPI scan. test_pypi_upload_check_finds_upload_in_another_workflow raises IsADirectoryError if .github/workflows/ ever holds a subdirectory. A custom step shell: or a Makefile - prefix can still mask a failure.
  • Still to do: actionlint, the changelog, scope, CI.

Generated by Claude Code

@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Ready for hub review
VERDICT: APPROVE
3620e8d
METHODOLOGY_CHOICE: no

I reviewed head 3620e8d55db72781919638b6dc6d0524bb181f97. It is still the PR head, so nothing changed after the hand-off. The base is master 55cd1915cb.

The PR does all three items in the build brief. The new Test-job guard kills every masking mutant the brief named (M6–M9, M11–M13). The new PyPI scan kills M10 and M14. Deleting any one of the helper's three checks fails the in-suite mutant test aimed at it. actionlint 1.7.12 reports 0 errors with the comment-only config, and CI is green (see CI). I found nothing at P1 or P2. All five findings are P3, and none blocks the merge.

On the assignment: the PR leaves out nothing the build brief asked for. It differs from the brief in two places, both disclosed in the hand-off and neither of any consequence:

  • The head branch is claude/core-591-review-followups-ac1zqs, not claude/core591-release-guards.
  • The reviewer's guard is folded into an assert_* helper rather than appended verbatim. The continue-on-error expression is unchanged.

gh api --paginate repos/PolicyEngine/policyengine-core/pulls/648/files lists exactly three files: .github/actionlint.yaml, changelog.d/core591-release-guards.changed.md and tests/core/test_release_tagging.py. Nothing else under .github/ changed.

P3

1. The PyPI scan, its test name, the PR body and the changelog claim more than the code checks (tests/core/test_release_tagging.py:345, :363, PR body "Changes", changelog.d/core591-release-guards.changed.md).

  • PYPI_UPLOAD_COMMAND matches only twine upload and uv publish, and uploads_to_pypi also matches the pypa/gh-action-pypi-publish@ action. A step that runs poetry publish, flit publish, hatch publish, pdm publish or maturin publish passes. So does a step that calls make build after someone adds an upload to the Makefile, or a job that calls a reusable workflow from another repository.
  • Command: mutants on scratch copies, each restored afterwards, then python -m pytest -q -p no:cacheprovider tests/core/test_release_tagging.py. Each of the following gave 25 passed:
    • X9–X13: a run: poetry|flit|hatch|pdm|maturin publish step in pr.yaml's Test job;
    • X8: twine upload dist/* added to the Makefile's build: target.
  • Today none of these exists. make build is python -m build (Makefile:24-25). No job has a job-level uses:, and .github/actions/ doesn't exist.
  • Trusted publishing (Publish to PyPI with trusted publishing #610) limits uploads to push.yaml in the pypi environment, according to push.yaml:120-121. I couldn't check PyPI's side of that. So this is a gap in a test's coverage, not a live path to PyPI.
  • Suggested code change (the scan still returns [("push.yaml", "Publish")] on this tree, and the poetry publish mutant then fails 4 tests):
    PYPI_UPLOAD_COMMAND = re.compile(
        r"\b(twine\s+upload|(uv|poetry|flit|hatch|pdm|maturin)\s+publish)\b"
    )
  • Suggested text for the PR body bullet header: "Every workflow file is checked for the common PyPI upload steps (P3 6)."
  • Suggested text for the changelog fragment: "The release tests now fail if the push workflow's Test job could pass after its tests fail, or if a workflow job other than Publish uploads to PyPI with the PyPI publish action, twine upload or uv publish, and the actionlint config drops two ignores for a step that no longer exists." If you adopt the regex above, list those publishers too.
  • The test name test_only_push_publish_job_uploads_to_pypi can stay, with a comment line above the assertion: # Recognises the pypa publish action, twine upload and uv/poetry/flit/hatch/pdm/maturin publish in steps; not Makefile targets or remote reusable workflows.

2. The hand-off says the scan misses reusable workflows. That holds only for remote ones (hand-off "Judgment calls" and "Proposed follow-up").

  • GitHub's docs say "you locate reusable workflows in the .github/workflows directory of a repository. Subdirectories of the workflows directory are not supported" (Reuse workflows).
  • So a local reusable workflow's upload step is scanned in its own file. X1 shows this: M15 plus a .github/workflows/publish.yaml that runs uv publish gives 4 failed, 21 passed, including test_only_push_publish_job_uploads_to_pypi. M15 alone, which calls a file that doesn't exist, passes (25 passed), as the hand-off says.
  • Suggested follow-up text: "Extend the PyPI upload check to reusable workflows called from other repositories (uses: owner/repo/.github/workflows/x.yml@ref) and to composite actions. Local reusable workflows already live in .github/workflows/ and are scanned."

3. test_pypi_upload_check_finds_upload_in_another_workflow crashes if .github/workflows/ ever holds a subdirectory (tests/core/test_release_tagging.py:378-379).

  • WORKFLOWS.iterdir() yields directories too, and shutil.copy raises on one.
  • Command: mkdir .github/workflows/sub && cp .github/workflows/pr.yaml .github/workflows/sub/x.yaml, then pytest. Output: all three parametrized cases fail with E IsADirectoryError: [Errno 21] Is a directory: '.../.github/workflows/sub'. The tree was restored afterwards.
  • GitHub ignores such a directory, as the docs quote above says, so this is unlikely. The fix is one line. With a subdirectory present it gives 25 passed, and ruff format --check passes:
        shutil.copytree(WORKFLOWS, tmp_path, dirs_exist_ok=True)
    This replaces the two-line for path in WORKFLOWS.iterdir(): shutil.copy(...) loop.

4. The exact-match check is the right strictness, but its message doesn't say what to do (tests/core/test_release_tagging.py:406, and :400-402).

  • make test -j4 fails with test step runs more than make test. Under the default bash -e {0} (see "The rule" below), make test -j4, make test; true and a multi-line script are all safe. But make test | tee log masks a failure, because the default shell sets no pipefail, and so do || true and || exit 0. An exact match is the simplest rule that catches all of these, so I'd keep it.
  • The message doesn't explain the rule. A maintainer who hits it could delete the assertion instead of updating it. Recommended text:
        assert test_step["run"].strip() == "make test", (
            "test step runs more than make test: anything after it (|| true, | tee, "
            "|| exit 0) can hide a failure; if the command changed on purpose, update "
            "this check to the new exact command instead of removing it"
        )
  • The locator "make test" in step.get("run", "") (:401) also matches make test-country-template. A maintainer who splits the template tests into their own step gets expected one test step, found [4, 5]. I checked this by inserting such a step into the parsed job. Matching step.get("run", "").split()[:2] == ["make", "test"] would avoid that. Recommend only.

5. Masks the guard still misses (none exists today, and no change is needed in this PR).

  • X5: a custom step shell: bash -c true {0} runs true instead of the script. Gives 25 passed.
  • X7: a - prefix on the Makefile test recipe line makes make ignore the failure. Gives 25 passed.
  • X6: PYTEST_ADDOPTS: --collect-only on the job means no tests run. Gives 25 passed.
  • An assert "shell" not in test_step would close X5 cheaply. The other two sit outside the workflow file.

The rule (section 1)

The proxy blocks docs.github.com from this session, both through curl and the fetch tool. I read the same pages' source from the github/docs repository's main branch on raw.githubusercontent.com. The URLs below are the published pages for that source.

  • jobs.<job_id>.steps[*].continue-on-error: "Prevents a job from failing when a step fails. Set to true to allow a job to pass when this step fails." The contexts page adds: "When a continue-on-error step fails, the outcome is failure, but the final conclusion is success." Its context-availability table lists jobs.<job_id>.steps.continue-on-error with contexts github, needs, strategy, matrix, job, runner, env, vars, secrets, steps, inputs, so the key accepts an expression such as ${{ true }}.
  • jobs.<job_id>.continue-on-error: "Prevents a workflow run from failing when a job fails. Set to true to allow a workflow run to pass when this job fails." It accepts github, needs, strategy, vars, matrix, inputs. The docs don't say what result a job that needs it sees. The guard rejects the key whatever its value, so that question doesn't matter here.
  • jobs.<job_id>.needs: "If a job fails or is skipped, all jobs that need it are skipped unless the jobs use a conditional expression that causes the job to continue." Status check functions: "A default status check of success() is applied unless you include one of these functions."
  • jobs.<job_id>.steps[*].if: "You can use the if conditional to prevent a step from running unless a condition is met." The page doesn't say in so many words that a skipped step leaves the job green. The guard rejects any if: on the test step, so it doesn't depend on that.
  • Supported shells: on Linux with shell unspecified, the command is bash -e {0}. With shell: bash it is bash --noprofile --norc -eo pipefail {0}. Exit codes: "fail-fast behavior is enforced using set -e for both sh and bash. When shell: bash is specified, -o pipefail is also applied", and "sh-like shells exit with the exit code of the last command executed in a script".

What follows for push.yaml's Test job, which sets no shell or defaults:

Route Masks a failure? New guard
continue-on-error: true or ${{ true }}, on the job or any step (M6, M7, M11) yes catches it
if: that skips the test step (M9, M13) yes, by not running it catches it
make test || true, || exit 0, | tee log yes (set -e exempts || lists; no pipefail) catches it (exact match)
make test; true (M12), multi-line make test then true (X3) no: set -e stops at make test catches it anyway (conservative)
job defaults.run.shell: bash {0} (X4) no: the last command is make test passes, correctly
custom step shell: that ignores the script (X5) yes misses (P3 5)
Makefile - prefix, or env that skips tests (X7, X6) yes misses (P3 5)

None of the misses exists in this repository today.

Checked and sound

  • Tests, one file per process (python -m pytest -q -p no:cacheprovider <file>):

    test_release_tagging.py test_docs_deployment.py
    master 55cd1915cb 16 passed 5 passed
    head 3620e8d55d 25 passed 5 passed
  • Mutants. Each ran on the working copy and was restored with git checkout -- . && git clean -fdq .github. git status --porcelain was empty after each and at the end. Results for test_release_tagging.py:

    Mutant Result Killed by
    M6 job continue-on-error: true 3 failed, 22 passed test_test_job_cannot_pass_with_failing_tests
    M7 step continue-on-error: true 3 failed, 22 passed same
    M8 make test || true 4 failed, 21 passed same
    M9 step if: false 2 failed, 23 passed same
    M10 pypi-publish action in pr.yaml Test 1 failed, 24 passed test_only_push_publish_job_uploads_to_pypi
    M11 step continue-on-error: ${{ true }} 3 failed, 22 passed test_test_job_cannot_pass_with_failing_tests
    M12 make test; true 4 failed, 21 passed same
    M13 step if: ${{ false }} 2 failed, 23 passed same
    M14 uv publish in new release.yml 4 failed, 21 passed test_only_push_publish_job_uploads_to_pypi
    M15 pr.yaml job uses: ./.github/workflows/publish.yaml 25 passed none (expected; the target file doesn't exist)
    X15 SHA-pinned pypi-publish action 1 failed, 24 passed test_only_push_publish_job_uploads_to_pypi
    X16 python -m twine upload in new .yml 4 failed, 21 passed same
    unmodified 25 passed —

    The extra failures in M6–M9 and M11–M13 are the in-suite mutant tests, which load the already-mutated file, as the hand-off explains. In M14 and X16 they are the three copy tests, which see the extra upload. test_docs_deployment.py gave 5 passed for every mutant except M14 and X16, where it gave 6. That file evidently collects one case per workflow file, so the new release.yml adds a case.

  • The in-suite mutants (:415-452) reach their own checks.

    • continue_test_job_on_error and continue_after_test_failure hit the first assertion, which they target.
    • ignore_test_failure passes the continue-on-error check and the if check, then fails the exact match.
    • skip_tests passes the first check and fails the if check.
    • Deleting each check from assert_test_job_fails_when_tests_fail in turn (restored after each):
      • the if check: 1 failed, [skip_tests];
      • the exact match: 1 failed, [ignore_test_failure];
      • the continue-on-error check: 2 failed, [continue_test_job_on_error] and [continue_after_test_failure].
  • The hand-off's line numbers hold: :344-360, :363-366, :369-389, :392-406, :409-412, and :315-341 unchanged. The exceptions: the file has 452 lines, so the mutant tests end at :452, not :453. And :204 is the tag-step exact match the hand-off cites.

  • PyPI scan file pattern. *.yaml and *.yml, non-recursive. That matches GitHub's rule that workflows in subdirectories aren't supported (quoted in P3 2). X14 (a nested file) doesn't trip the main test, correctly.

  • actionlint. The 1.7.12 release binary (actionlint -version: 1.7.12 … built with go1.26.1), with ShellCheck 0.11.0 and pyflakes 4.1.0 (shellcheck-py, pyflakes from PyPI) on PATH:

    • A probe workflow produced both [shellcheck] and [pyflakes] findings.
    • With the shipped config, actionlint -verbose gives Found 0 errors in 2 files, exit 0.
    • With paths: [ appended to the config, it fails with could not parse config file ".../.github/actionlint.yaml": ... yaml: line 3: did not find expected node content. So actionlint reads the file, and the comment-only version parses.
    • With the file removed, it gives exit 0 with no output. The comment-only file behaves exactly like no file.
    • On push.yaml from before Deploy the documentation site from its own release job #594 (7e608613^), the head config reports exactly the two removed ignore targets, at 79:13 and 82:11, so those ignores matched only the removed step.
    • pr.yaml:25-40 runs actionlint from the rhysd/actionlint 1.7.12 image digest in the checked-out workspace with -color only. It finds .github/actionlint.yaml the same way the binary does, and CI's "Lint workflows" job passed.
    • Deleting the file would be equally correct. Keeping the two comment lines points the next person at the config format, so I'd leave it.
  • Changelog fragment. changed is a configured towncrier type (pyproject.toml, [[tool.towncrier.type]] directory = "changed"), and github-prs.md:50-52 files tooling under changed, as the earlier workflow fragments (trusted-publishing.changed.md and others) did.

    • The name before the type is free text to the tooling. Towncrier takes the type from the suffix, and .github/bump_version.py:28-32 reads the category from the stem's last part. Either way the bump is a patch.
    • No other fragment shares the name, and changelog.d/ holds no other fragment at the head.
    • The <branch> naming is a convention, not something the release depends on. The text's last clause is accurate. Its middle clause overclaims (P3 1).
  • Scope. Nothing under policyengine_core/ or .github/workflows/ changed.

  • CI. Pull request run 41702042594 at 3620e8d55db72781919638b6dc6d0524bb181f97 is completed with conclusion success. All 19 jobs are success: Lint, Lint workflows, Check changelog fragment, 8 Test jobs (ubuntu and windows, Python 3.11–3.14) and 8 "Test Core and country packages" jobs. It is the only run at this SHA.

What I couldn't check

  • docs.github.com itself: the proxy refuses it. I read the page sources from github/docs main instead, which may differ slightly from what the site serves.
  • What a job that needs a job with continue-on-error: true sees. The docs don't say, and the guard makes it moot.
  • PyPI's trusted-publisher configuration (the "only from push.yaml in the pypi environment" claim in push.yaml:120-121).
  • The push workflow itself, which runs only on master in the canonical repository.

Generated by Claude Code

The fragment said the release tests fail if any workflow other than
Publish uploads to PyPI, but the check recognizes only
pypa/gh-action-pypi-publish, twine upload and uv publish, so a step that
runs poetry publish passes it (review P3 1). The fragment now names the
three upload forms and the three Test-job checks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis merged commit 28ac7aa into master Nov 3, 2026
19 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

US + core hub merge audit, core#648 at 38c462b1a82c80ea5da872fffc2c6c1473addfa1 (squash): Guard the release tests against masked test failures and other PyPI uploads.

  • Review: an independent review in a separate Claude Code session approved 3620e8d55d with five P3 findings and no P1 or P2: Guard the release tests against masked test failures and other PyPI uploads #648 (comment)
    • Its mutants M6 to M14, X15 and X16 failed the new tests. M15, a job-level uses: of a file that doesn't exist, passed, as the build hand-off said it would.
    • Deleting each of the guard's three checks failed the in-suite mutant test aimed at it.
    • actionlint 1.7.12 reported 0 errors with the comment-only config.
  • Change after the review: 38c462b1a8 rewrites only the changelog fragment (P3 1). The old text said the tests fail if any workflow other than Publish uploads to PyPI; the new text names the three upload forms and the three Test-job checks. The hub checked each clause against assert_test_job_fails_when_tests_fail and uploads_to_pypi at the head. It also rewrote the description's bullet headers to match the code, added the limits that P3 1, 2 and 5 found, and dropped a stale line about CI on drafts.
  • Follow-ups for a later core PR:
    • P3 1: recognize poetry, flit, hatch, pdm and maturin publish, and say in a comment what the scan covers;
    • P3 2: scan reusable workflows called from other repositories, and composite actions;
    • P3 3: copy the workflows with shutil.copytree, since today's loop crashes if .github/workflows/ holds a subdirectory;
    • P3 4: give the exact-match assertion a message that says what to do, and stop the test-step locator matching make test-country-template;
    • P3 5: reject shell: on the test step.
  • Classification: CORE_SEMANTICS no. The PR changes release tests, the actionlint config and the changelog, not package code.
  • PE-US core gate: the hub skipped it, since no package code changes.
  • Gates the lander checked at merge:
    • every check passes at the head;
    • MERGEABLE;
    • not a draft;
    • no CHANGES_REQUESTED review;
    • the merge pins the head (--match-head-commit).
  • Axiom: n/a; the PR changes no policy.

🤖 Generated with Claude Code

@MaxGhenis
MaxGhenis deleted the claude/core-591-review-followups-ac1zqs branch November 3, 2026 20:08
MaxGhenis added a commit that referenced this pull request Nov 3, 2026
* Extend the release tests' PyPI scan and Test-job guard (core#648 review)

The independent review of #648 found uploads the scan missed (other
publishers, Makefile recipes, remote reusable workflows, composite
actions), a crash on a subdirectory under .github/workflows, a test-step
locator that also matched make test-country-template, and masks the
Test-job guard missed (a custom shell, PYTEST_ADDOPTS, a - prefix on a
make test recipe line).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Restore uv.lock, which a local uv sync changed

This PR changes only the release tests and its changelog fragment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants