Repository navigation
fix(gen-manifests): hermetic self-test — fixtures never inherit the caller's PR event - #5847
Conversation
…t the caller's PR event On the first settlement PR (MeshWeaver.Plugins#2487) the validate lane's canonical self-test went red: _lock_base / is_manifest_lock_settlement_pr read GITHUB_EVENT_NAME / GITHUB_EVENT_PATH from the REAL run, so every main-owned fixture took the settlement branch. --self-test now runs with those (and GITHUB_BASE_REF, MW_LOCK_BASE) removed, restoring them after. Reproduced locally with the settle PR's event payload: main's copy fails the two main-owned cases, this copy passes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![systemorph[bot]](https://avatars.githubusercontent.com/u/77832550?s=60&v=4){kind=small}
There was a problem hiding this comment.
The change makes the canonical gen-manifests.py --self-test hermetic: main() pops the four run-context variables its fixtures must not inherit (GITHUB_EVENT_NAME, GITHUB_EVENT_PATH, GITHUB_BASE_REF, MW_LOCK_BASE) around the self_test() call and restores them in a finally. The whole diff is present (+13/-1, no cuts).
What I checked: the pop/restore mechanics (the k in os.environ guard keeps the comprehension KeyError-free; try/finally restores on both the return and an exception path; a variable that was absent stays absent); the scope (isolation applies only to the --self-test arm, so --check's env-driven behavior — including the settlement-PR acceptance path is_manifest_lock_settlement_pr -> check_generated_lock_settlement — is unchanged); that the four names cover every environment read of the two functions the comment names, cross-checked against the identical machinery landed in MeshWeaver.Plugins#2481; and the causality — on the settle PR's run (Plugins#2487) the predicate was true for every fixture, so check_untouched delegated to check_generated_lock_settlement, which is exactly the two observed 'main-owned' fixture failures quoting the settlement message. No security surface: no injection, no secrets, no access-control change; the settlement predicate itself is not altered, only shielded during the self-test; nothing in the diff or PR text addresses the reviewer.
What I could not verify: the fixtures' own expectations — the diff shows only these two hunks and the mesh does not carry the full file, so 'this copy passes the full self-test' rests on the author's verification and this PR's CI; and which copy of the script the failing step executes (one question below).
Findings: 0 blocking · 0 should-fix · 1 question · 0 nit
Internal review of e2959ae2f8dd6c2e0c6c22731f00f0e29c1924a2 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| # `is_manifest_lock_settlement_pr` read GITHUB_EVENT_NAME / GITHUB_EVENT_PATH / MW_LOCK_BASE, | ||
| # so on the settlement PR's own run (MeshWeaver.Plugins#2487) every fixture took the | ||
| # settlement branch and the self-test went red over the environment, not the code. | ||
| saved = {k: os.environ.pop(k) for k in _SELF_TEST_ENV_ISOLATED if k in os.environ} |
There was a problem hiding this comment.
question — The failing job ran in MeshWeaver.Plugins (validate / Validate node repos on the settle PR #2487), and that repo carries a vendored scripts/gen-manifests.py with the identical --self-test arm and the same env-reading functions (landed in Plugins#2481; its KNOWN_ARGS includes --self-test) — so the vendored copy has this same leak. The body calls the failing step 'the canonical self-test' and says 'Pairs-with: none', but the stated verification — running main's copy with the settle PR's exported event payload — cannot distinguish the two copies, because they are the same code: it reproduces the failure either way. Can you confirm the validate step fetches and runs this core copy (e.g. through scripts/platform-script.py, which the script's own usage block names) rather than the vendored one? If any Plugins job can reach the vendored --self-test inside a pull-request environment, this isolation needs porting there too, or the settle PR stays red.
There was a problem hiding this comment.
Confirmed — the failing step runs THIS core copy, not the vendored one. The job was validate / Validate node repos (job 108829103360), failing step "The manifest checker can fail — --resolve, the trunk baseline and the config contract (self-test)", which is node-repo-validate.yml:554: python3 "$RUNNER_TEMP/gen-manifests.py" --self-test — the canonical fetched from Systemorph/MeshWeaver at the lane's scripts ref (main for Plugins), not scripts/gen-manifests.py.
And no Plugins job runs the vendored --self-test: git grep "gen-manifests.py --self-test" over Plugins main's .github/ finds nothing (its repo-gates job, which calls the vendored copy's check_manifests through validate-repos.py, went green on #2487).
The vendored copy still carries the same leak as dead code in CI, so I'll re-sync it to this canonical in the Plugins follow-up anyway: it is kept code-identical to the canonical, and check-resolver-copy.py reports any drift.
Test Results (shard 2)770 tests ±0 579 ✅ ±0 7m 8s ⏱️ -1s Results for commit e2959ae. ± Comparison against base commit 003b70d. This pull request removes 25 and adds 9 tests. Note that renamed tests count towards both. |
Test Results (shard 5) 5 files ±0 5 suites ±0 14m 15s ⏱️ -4s Results for commit e2959ae. ± Comparison against base commit 003b70d. This pull request removes 4 and adds 2 tests. Note that renamed tests count towards both. |
Test Results 17 files ±0 17 suites ±0 36m 6s ⏱️ -47s Results for commit e2959ae. ± Comparison against base commit 003b70d. This pull request removes 29 and adds 11 tests. Note that renamed tests count towards both. |
The first settlement PR the Plugins
settle-locksjob opened (Systemorph/MeshWeaver.Plugins#2487, App-authored, lock-only) went red invalidate / Validate node repos— not on its lock diff, but in the canonical self-test step:Root cause:
_lock_baseandis_manifest_lock_settlement_prreadGITHUB_EVENT_NAME/GITHUB_EVENT_PATHfrom the real run, so on the settlement PR every fixture took the settlement branch. Fix:--self-testruns withGITHUB_EVENT_NAME,GITHUB_EVENT_PATH,GITHUB_BASE_REF,MW_LOCK_BASEremoved (restored afterwards).Verified: with the settle PR's event payload exported, main's copy fails exactly those two cases; this copy passes the full self-test. On any ordinary PR the predicate is false, which is why no other run saw it.
Pairs-with: none — self-test harness only.
🤖 Generated with Claude Code