fix(publication-base): a run that withheld its publication is walked past, never the baseline - #5850
Conversation
…past, never the baseline Measured on MeshWeaver.Plugins main run 36396191949 (the first settle-PR merge): the baseline a push narrows from is the newest SUCCESSFUL main push, and the source merge before it (c6fa2b5b, own=false) was successful while publishing nothing on purpose. Taken as the baseline, the settle merge would narrow to its lock-only diff and publish none of that merge's modules (this time the change was a doc and core CD happened to bake the tree, so nothing was lost — by luck). New --withheld-marker NAME: a successful run carrying an artifact of that name is walked past like a failed one; the history union from the older baseline carries its changes. Self-test: 4 new cases incl. the not-withheld control and 'off HEAD's line still stops the walk'. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…k it (--withheld-marker) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![systemorph-com[bot]](https://avatars.githubusercontent.com/u/77832550?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a withheld-publication predicate to node-repo-publication-base.py's baseline(): a successful main push that deliberately published nothing (detected by a marker artifact, --withheld-marker) is walked past like a failed run, so the next publishing run's git log ..HEAD keeps that merge's modules in scope; without the flag behaviour is unchanged. The off-HEAD break is preserved (withheld or not, off-line stops the walk as a full build). Verified the baseline() control-flow inversion is semantically identical to the old code plus the withheld skip, that the withheld run's id lands in between (feeding the MAX_UNSETTLED full-build guard conservatively), that the four new self-test cases cover the meaningful branches, that carries_marker URL-encodes the name and surfaces ArtifactStoreError rather than misreading a broken store as 'not withheld', and that the marker check runs only after the ancestor check so it fires at most once per on-line success. Doc addition in ModuleVersioning.md matches the code. No blocking findings; one question about the uploader-side reliability of the marker contract (the Plugins half follows) and one nit.
Findings: 0 blocking · 0 should-fix · 1 question · 1 nit
Internal review of 977390cb77e1b70622cf61dc635d773f123a2052 — GLM-5.3, posted by the control plane. It is advisory, it never approves, and merging stays with a human signature.
| runs = gh_json(endpoint)["workflow_runs"] | ||
| if not isinstance(runs, list): | ||
| raise ValueError("workflow_runs is not a list") | ||
| withheld = (lambda run: carries_marker(a.repo, run["id"], a.withheld_marker, |
There was a problem hiding this comment.
question — Correctness of the whole mechanism rests on the uploading side (the Plugins settle-locks half, follow-up) unconditionally uploading the marker on every own=false run. A withheld run that fails to upload its marker silently becomes the next baseline again and loses that merge's publications — the exact bug this fixes. Worth asserting in the follow-up's own tests that the marker upload happens on every non-publishing success path (including runs that hit the scope job but skip publication, and ones that skip the scope job entirely).
There was a problem hiding this comment.
Agreed that the guarantee lives on the uploading side, so here is how the Plugins half (settle-locks) closes each path. Every one ends in either a marker or a run that is not green, and a run that is not green is never a baseline:
own=falsecomputed: the marker upload step runs withif: steps.own.outputs.own == 'false',if-no-files-found: error, and nocontinue-on-error. A failed upload fails the job, so the run is red and not a baseline.- The
ownstep itself fails (--settlered, network): the job is red, so the run is red. Also,ownis empty, somodules-floorpublish /publish-bake/tag-modules(allown == 'true') publish nothing. settle-locksnever runs: itneeds: [change-set]. Ifchange-setfailed or was cancelled, the run is red or cancelled, not a success.- The run reaches the scope job but withholds publication: that is only the
own=falsepath above, becauseown=trueis the only condition these jobs add. A run that publishes nothing because nothing was selected genuinely published everything owed, and is a correct baseline.
So there is no green, unmarked, withholding run. The first withheld run after the follow-up lands will be checked for the marker artifact before I call it done.
| return json.loads(out.stdout) | ||
|
|
||
|
|
||
| def carries_marker(repo, run_id, name, artifact_store="", expected_store_id=None): |
There was a problem hiding this comment.
nit — carries_marker's gha branch ignores expected_store_id. Presumably intentional (the GitHub-native path has no alternate store identity to check), but if the expect-store-id discipline is meant to be uniform, a one-line comment saying so would help.
There was a problem hiding this comment.
Intentional: on the GitHub-native path the artifact API is scoped to this repository's runs, so there is no second store whose identity could be confused. attested_inputs' gha branch ignores it for the same reason. Not adding the comment in this PR, to avoid restarting CI for prose; the uniform discipline is enforced where it can apply, the declared-store branch.
Test Results (shard 0) 1 files 1 suites 3m 8s ⏱️ Results for commit 977390c. |
Test Results (shard 3)461 tests 461 ✅ 55s ⏱️ Results for commit 977390c. |
Test Results (shard 1)1 701 tests 1 701 ✅ 4m 41s ⏱️ Results for commit 977390c. |
Test Results (shard 4) 3 files 3 suites 6m 36s ⏱️ Results for commit 977390c. |
Test Results (shard 2)770 tests 579 ✅ 7m 8s ⏱️ Results for commit 977390c. |
Test Results (shard 5) 5 files 5 suites 14m 26s ⏱️ Results for commit 977390c. |
Test Results 17 files 17 suites 36m 57s ⏱️ Results for commit 977390c. |
1 participant
Measured
MeshWeaver.Plugins now publishes only from settled commits: a source merge whose
manifest.locks main has not settled yet publishes nothing (own=false), and the settle PR's merge publishes that tree (Plugins #2481,Hosting/PullRequestDrain.md). The first settle merge proved the flow (settle PR #2487 opened by the bot → green → auto-merged → main run 36396191949own=true, taggedHosting/v1.42.2), and exposed this gap:node-repo-publication-base.pytakes the newest successful main push as the baseline a push narrows from (git log <baseline>..HEAD). The source merge's run (c6fa2b5b, run 36393485221) was successful while publishing nothing on purpose. Taken as the baseline, the settle merge's run narrows to the lock-only diff and publishes none of that merge's modules. This time nothing was lost only by luck: the change was a doc page, and the platform's CD had baked that tree anyway (bake-scopebaselinec6fa2b5b).Fix
baseline()takes awithheld(run)predicate;--withheld-marker NAMEmakes it "the run uploaded an artifact called NAME" (read from the same declared store as the attestations, expired ones included — presence is the fact). A withheld successful run is walked past exactly like a failed one: the history union from the older baseline carries its changes into the run that does publish. A withheld run off HEAD's line still stops the walk (full build), never skips it. Default: no marker → unchanged behaviour for every caller.Self-test: 4 new cases (withheld walked past; only-withheld = full build; the not-withheld control; off-line still stops the walk) — 24 assertions pass. Doc:
ModuleVersioning.md→ "Who writes the lock".The Plugins half (settle-locks uploads the marker when
own=false; build-scope passes--withheld-marker) follows once this lands.Pairs-with: none — additive optional CLI flag;
baseline()gains a defaulted parameter.🤖 Generated with Claude Code