ci: fix MODULE.bazel.lock gate and regenerate the lock file

[openroad-ci]

Every push to master on the public repo has failed since 2026-08-04 (example), reported by @gadfort

Root cause

Two independent problems.

1. MODULE.bazel.lock is stale. Running bazelisk mod deps --lockfile_mode=update on master produces a 10 insertion / 2 deletion diff, all under facts > @@rules_python+//python/extensions:pip.bzl%pip > dist_hashes > https://pypi.org/simple — new entries for markdown-it-py 4.2.0, pygments 2.20.0, rich 15.0.0 and setuptools 83.0.0.

The drift surfaced with #11058 (bazel: add yamlfix support to fix_lint and lint_test, merged 2026-08-04 21:18 UTC; the run 2.5 h earlier was green and it was the only merge in between). That PR changed bazel/requirements.in and bazel/requirements_lock_3_13.txt, which invalidates the rules_python pip extension. Re-evaluating it re-queries pypi.org/simple for the hashless transitive hubs (pgv_pip_deps, grpc_python_dependencies — the requirement file has been generated without hashes warnings in the CI log) and records the newer releases. The regenerated lock file was never committed.

2. The pull request gate never looked at the lock file. github-actions-check-bazel-lock.yml did:

cp MODULE.bazel MODULE.bazel.before
bazelisk mod deps --lockfile_mode=update
if ! diff MODULE.bazel.before MODULE.bazel ; then

MODULE.bazel compared against itself — MODULE.bazel.lock is what --lockfile_mode=update rewrites. Blind since #10690 (2026-06-19) replaced the working bazelisk mod deps --lockfile_mode=error with this. The original check would have caught #11058.

Why master cannot self-heal: the auto-format job on master regenerates the lock and pushes, but master is protected:

remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: - Changes must be made through a pull request.

So the job is red on every push that produces any diff, and the fix never lands.

Changes

• Regenerate MODULE.bazel.lock.
• github-actions-check-bazel-lock.yml: compare MODULE.bazel.lock, and on failure emit a ::error file=MODULE.bazel.lock:: annotation naming bazelisk mod deps --lockfile_mode=update so the developer sees how to regenerate it. git diff --exit-code also prints the offending patch.
• github-actions-on-master-push.yml: drop the lock file regeneration, since its push can never succeed on a protected branch. The lock is now gated on pull requests only.

Note

The pip dist_hashes facts track the live PyPI index, so any pull request that touches bazel/requirements*.txt can pick up unrelated lock churn. The gate now tells the developer exactly what to run, which is the intended workflow.

[gemini-code-assist]

Code Review

This pull request updates the MODULE.bazel.lock file to add new versions and hashes for several Python packages, including markdown-it-py (4.2.0), pygments (2.20.0), rich (15.0.0), and setuptools (83.0.0). There are no review comments, and I have no feedback to provide.

[sombraSoft]

FYI @maliberty @hzeller

[oharboe]

@maliberty @hzeller
The underlying problem is that requiring developers to generate MODULE.bazel.lock locally is flawed for two reasons:

1. Environment dependency: It reduces quality of life for developers that don't have the exact machine setup required to produce the correct lockfile.
2. Supply-chain security: Accepting a user-provided lockfile bypasses the security mechanism. A human reviewer can review changes to URLs and versions in MODULE.bazel, but they can have no opinion on the cryptographic hashes in MODULE.bazel.lock. We may trust the authentication of the submitter, but we have no way to know if the submitter's machine has been compromised, or if an AI submitted a PR with a third-party library that it thought was good with the best of intentions.

The source of truth for the review is MODULE.bazel. The lockfile itself must be generated in a trusted environment.

Instead of checking the user's lockfile, CI should ignore MODULE.bazel.lock changes from the PR, resolve the dependencies on OpenROAD infrastructure, and handle the merge by generating the lockfile server-side. There are several ways to automate this during the merge process to avoid transferring PR ownership.

[sombraSoft]

@oharboe Your point 2 holds up. I tested it against the gate in this PR by tampering with MODULE.bazel.lock on master and running bazelisk mod deps --lockfile_mode=update:

Tampered field	Bazel	Gate
moduleExtensions[*].generatedRepoSpecs — URL and sha256 of prometheus_metrics_model	exit 0, values left untouched	passes
facts > pip > dist_hashes — a PyPI wheel hash	exit 0, left untouched	passes
registryFileHashes — a BCR source.json hash	ERROR: Checksum was d6da852f… but wanted cccc…	fails

And the tampered spec is genuinely honoured:

WARNING: Download from https://evil.example.invalid/client_model-v0.6.1.tar.gz failed: Unknown host: evil.example.invalid

Bazel fetched the attacker's URL. A module extension is only re-run when its inputs change (usages, bzl transitive digest, recorded files and env). Edit only the recorded results and every digest still validates, so nothing is recomputed, and the check sees no diff.

So the split is: the registry portion of the lock is tamper-evident, because recorded hashes are verified against what BCR actually serves. Extension-generated repo specs and pip dist_hashes are trust-on-first-write — no reviewer can audit them, and no lockfile mode verifies them.

Two qualifications:

1. This is not a regression introduced here. The --lockfile_mode=error form that preceded Make the MODULE.bazel.lock CI more actionable #10690 is equally blind — both modes check staleness, not authenticity.
2. On the environment-dependency argument, regenerating on my machine reproduced CI byte-for-byte. The instability is time, not machine setup: dist_hashes tracks the live PyPI index for the hashless transitive hubs (pgv_pip_deps, grpc_python_dependencies), which is what caused the drift this PR is cleaning up. A contributor can regenerate correctly today and be stale tomorrow through no fault of their own.

A smaller step toward what you're describing

Rather than generating the lock server-side at merge, the PR's lock can simply be denied any trust during the check — seed the resolution from the base branch's lock instead:

- name: Regenerate from the trusted lock file
  run: |
    git fetch --depth=1 origin ${{ github.base_ref }}
    git checkout FETCH_HEAD -- MODULE.bazel.lock
    bazelisk mod deps --lockfile_mode=update

- name: Compare against the lock file in the pull request
  run: |
    if ! git diff --exit-code ${{ github.sha }} -- MODULE.bazel.lock ; then
      echo "::error file=MODULE.bazel.lock::MODULE.bazel.lock does not match the \
        resolution performed by CI. Regenerate with 'bazelisk mod deps \
        --lockfile_mode=update' and commit the result."
      exit 1
    fi

Entries supplied by the pull request are then never reused: where the PR leaves extension inputs alone, the base branch's entries are reused and any tampering shows up as a diff; where the PR does change them, they are re-resolved on OpenROAD CI. That closes all three rows in the table above while keeping the ordinary regenerate-and-commit workflow, and it needs no PR ownership transfer or write-back.

The cost is more spurious failures: resolving from the base lock can pick up PyPI index drift the contributor never saw locally, and they will have to regenerate again to clear it. Worth it, in my view, but it is a real tax.

Happy to fold this into this PR, or to keep this one as the pure fix — master has been red since 2026-08-04 — and follow up separately. Preference?

[oharboe]

Personally, I have no strong preference on how this specific PR is handled—I am able to do my work either way. My main concern is the long-term security of the project, the quality of life for contributors and reviewers, and not least, those who provide the CI infrastructure for us (which is like unappreciated household chores that are only noticed when they aren't done).

While resolving from the base lockfile and comparing it to the PR's lockfile definitely closes the security gap, it preserves the "regenerate and commit" workflow. I believe that is exactly the workflow we should be abandoning.

Subjecting contributors to a "tax" of spurious CI failures simply because the PyPI index drifted between their local generation and the CI run is a tax we don't need to levy. It is essentially hipster programming—forcing contributors to perform a manual, awkward dance to satisfy the linter for no discernible difference in the final result.

My next pet project is to advocate for shifting all of this busywork to automation that runs after the maintainers take delivery of the PR, before it goes into master.

Maintainers should be able to review the human intent (MODULE.bazel changes, or C++ logic), approve the PR, and take delivery of it. Then, an automated process mechanically generates the MODULE.bazel.lock, applies clang-format (since the contributor might not have the exact required version), applies black, and merges the result. Looking not so far into the future, we will likely have AI applying coding nits during this phase too.

This model is safer (security artifacts are generated strictly in a trusted environment), and it massively improves the quality of life for everyone involved. We shouldn't enforce a workflow that causes developers pain just to arrive at the exact same deterministic output our CI infrastructure could have securely generated for them automatically.

[oharboe]

Another reason to move to a model where automation takes over after the maintainers approve the human intent is to avoid the "AI nit war" and fuzzing input convergence issue.

If I submit a PR, get feedback containing AI-generated nits, fix them locally, and ask for another review, the AI review is run again. Because LLM responses inherently have some variance (fuzzing), and often have a maximum depth of nits they will report per iteration, I just get a new batch of nits. What's worse, the maintainer might be using a different AI reviewer than I am using locally.

Humans then have to sit in the middle and manually evaluate if this cycle of nitpicking is actually converging, and each individual nit has to be reviewed for validity. I don't think this teaches the PR submitter or the reviewer much of anything.

I would much prefer a workflow where the reviewer uses their AI to grind down enough nits that they are satisfied the core logic is sound and safe to go into master. Then, the maintainers take delivery of the PR, and the automated system handles the rest (generating MODULE.bazel.lock, applying whitespace/formatter fixes, and applying any final automated AI coding nits) completely transparently to the contributor.

The contributor can then simply look at what automated nitpicking was applied on top of their PR after the fact, rather than being forced to act as a human middleman typing out mechanically generated fixes just to satisfy a bot before the merge.

[sombraSoft]

@oharboe You've convinced me, and I'd like to withdraw the trusted-seed suggestion.

Checking how the drift actually propagates made the case for you. Bazel only re-queries the PyPI index when a module extension's inputs change, so the lock sits stable until someone touches bazel/requirements*.txt, pip.parse, or bumps rules_python — and that contributor then inherits every package that drifted since the last refresh. #11058 added yamlfix and was handed markdown-it-py, pygments, rich and setuptools for its trouble. The cost doesn't fall on whoever caused it or on anyone who can act on it; it falls on whoever next happens to brush against the extension. Making that a contributor's responsibility isn't a small tax, it's an arbitrary one, and my proposal would have entrenched it.

Your broader point — that this class of work belongs to automation running in a trusted environment rather than to people — is right, and it generalises past the lock file to clang-format, black, buildifier and tclfmt. All of them are convergent, verifiable by re-running, and teach nobody anything when typed out by hand.

The one boundary I'd want drawn explicitly when we design it: mechanical, convergent artifacts can safely be generated after approval, because re-running reproduces them and a reviewer loses nothing by not having read them. AI nits are a different animal — they change semantics, they're sometimes wrong, and applying them post-approval means behavioural edits land in master under an approval that never saw them. Same trust-boundary reasoning you're applying to the lock, just pointing the other way. Advisory before merge, never auto-applied, and the convergence problem shrinks a lot if the reviewer runs once per pull request instead of on every push.

I'll merge this PR as it stands, purely to fix the current behaviour — regenerate the lock, compare MODULE.bazel.lock instead of MODULE.bazel, and drop the push that branch protection will never accept. Nothing here forecloses the model you're describing.

[oharboe]

@sombraSoft 👍

Yes, AI nits is a different animal indeed. It does come up here w.r.t. where does the maintainer take delivery of a PR.

I'm thinking AI nitpicking fixing is faster and easier for the maintainer to do locally and that it is false economy for everybody to try to get the PR author to do this work based upon a slow and painful feedback loop between three AIs (local AI for PR author, Gemini and local AI for maintainer) and 2+ humans having opinions about the work. A maintainer and his AI will handle this just fine. The PR author can submit a followup PR if some Chesterton Fence was torn down too quickly.