Skip to content

bump-version pushes to main without re-running the hygiene check when the first push is rejected #678

Description

@logbie

Split out of #674 review (raised by CodeRabbit; verified against current main). Pre-existing in the bump-version job — #674 did not touch it.

The gap

The job runs the static hygiene check exactly once, before the first push attempt, with a comment that states the requirement plainly (.github/workflows/ci.yml):

      # This job writes directly to main, so it must prove hygiene and version
      # agreement itself, immediately before pushing (REPOSITORY_HYGIENE.md §8).
      - name: Static hygiene and version check before push
        run: python scripts/check_repo_hygiene.py --mode static

The push step then retries up to five times, and each retry builds a new commit on a new tree:

          for attempt in 1 2 3 4 5; do
            if git push origin HEAD:"$BRANCH"; then
              echo "Pushed version bump on attempt $attempt"
              exit 0
            fi
            echo "Push rejected on attempt $attempt (main advanced); re-bumping from latest tip..."
            git fetch origin "$BRANCH"
            git reset --hard "origin/$BRANCH"
            python scripts/bump_version.py --update-all
          done

bump_version.py commits internally (it is not run with --skip-git), so the retry does produce a real bump commit — the loop is functionally correct. But that commit is computed from a tree that now contains whatever landed on main concurrently, and it is pushed without re-running the check. So on any retry the job does not do what its own comment says it must: prove hygiene immediately before pushing.

Concretely, a concurrent merge that lands a hygiene violation — or a bump that produces version drift against the new tree — reaches main unchecked, and is only caught by the next run's gate, after it is already on the branch this job is supposed to protect.

Why it is not caught today

It needs a genuine race: another commit must land on main between this job's checkout and its push. That is rare, which is exactly why it will go unnoticed until it matters.

Suggested fix

Re-run the check after each re-bump, inside the loop and before the next push:

             git fetch origin "$BRANCH"
             git reset --hard "origin/$BRANCH"
             python scripts/bump_version.py --update-all
+            python scripts/check_repo_hygiene.py --mode static
           done

The step already runs under set -eu, so a failing check aborts the step rather than pushing unchecked — which is the behavior wanted. Worth confirming that is the intended outcome rather than "skip this attempt and retry", since the two differ: aborting leaves main un-bumped (recoverable on the next push to main), whereas retrying past a violation would defeat the gate.

Acceptance criteria

  • A retry attempt runs check_repo_hygiene.py --mode static against the re-bumped tree before pushing it.
  • A hygiene violation on a retry fails the job instead of pushing.
  • The non-retry path is unchanged (still one check before the first attempt).
  • Ideally a test at the shell level, in the spirit of scripts/test_publish_spaces.sh — that suite exists because release scripts otherwise only ever execute in production, which is the same hazard this loop has.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions