[maestro] Restore public Bzlmod module name and guard it#799
[maestro] Restore public Bzlmod module name and guard it#799haasonsaas wants to merge 3 commits into
Conversation
…acting
stripRustTestModules stripped every inline `mod tests { }` block regardless
of #[cfg(test)], so edits inside an unguarded inline test module were dropped
from the release diff and packageChangedSinceTag could return false even
though compiled package sources changed. Gate stripping on a preceding
#[cfg(test)] attribute so only test-only modules are stripped; unguarded
modules stay package-impacting and force the required version bump.
Adds inline regression coverage (unguarded = impacting, cfg(test)-gated =
ignored) and registers the release-train-drift guardrail in the regression
suite manifest.
Refs #604
The public evalops/maestro MODULE.bazel declared module(name = "evalops_maestro_internal"), leaking the internal module identity to public Bzlmod consumers and breaking downstream bazel_dep(name = "evalops_maestro") resolution. Restore the public name and add a repo-local guardrail that fails if MODULE.bazel regresses to the internal name. Representative feedback: PR #417 renamed module(name = ...) on the public mirror, flagged as a breaking Bzlmod change. Refs #394
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This PR changes mirrored Maestro source files in the public repo, but it does not link the matching private source-of-truth PR. Add one of these to the PR body, then re-run the check:
Mirrored files touched:
|
PR SummaryMedium Risk Overview Public Bzlmod identity: Release impact filter: Reviewed by Cursor Bugbot for commit fc9c973. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Discontiguous cfg skips test stripping
- Updated Rust test-module stripping to honor cfg(test) attributes across intervening blank/comment trivia and added a regression test for the inline-module case.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 8902225. Configure here.
|
Closing as superseded. The durable fix landed in the source-of-truth repo via maestro-internal#2830: the public Bzlmod module name is now rewritten to This PR's direct |
2 participants
Summary
Guardrail for #394 (
other-feedback). Manual review conclusion: convert this class into a repo-local guardrail.The public
evalops/maestroMODULE.bazel declaredmodule(name = "evalops_maestro_internal"), leaking the internal module identity to public Bzlmod consumers — any downstream workspace usingbazel_dep(name = "evalops_maestro", ...)stops resolving. This was introduced by the mirror sync in #417 and flagged by both cursor and codex review as a breaking Bzlmod change; no later commit restored it.Change
MODULE.bazelevalops_maestro_internal→evalops_maestro. Safe and self-contained — nothing in-repo self-references the module name (BUILD.bazel/.bzl), andMODULE.bazel.lockcarries only BCR dependency hashes (no root-module identity), so no lock change is needed.test/scripts/ci-guardrails.test.ts):keeps the public Bazel module name public-facing— assertsmodule(name = "evalops_maestro")and forbidsevalops_maestro_internal, so a future mirror sync that re-leaks the internal name fails CI.public-bazel-module-identityentry toscripts/guardrail-regression-suite.json.Acceptance (#394)
evalops_maestro_internal).test/scripts/ci-guardrails.test.ts(run byci-nx-tests.sh/ Nxmaestro:test).other-feedbackfingerprint closed/absent (happens after merge).Verification
bunx vitest --run test/scripts/ci-guardrails.test.ts— 172 passednode scripts/check-guardrail-regression-suite.mjs— covers 13 bug classesbunx biome check test/scripts/ci-guardrails.test.ts— cleanFollow-up (out of scope here)
If the internal→public mirror sync rewrites module names, the rewrite (
evalops_maestro_internal→evalops_maestro) belongs in the internal sync path (maestro-internal). The guardrail here will catch any sync that fails to do so.Closes #394.