ci(#4303): a shard's results carry the attempt that produced them — a re-run can green the run again by rbuergi · Pull Request #4332 · Systemorph/MeshWeaver

rbuergi · 2026-09-14T17:20:02Z

Closes #4303.

The defect, re-measured live today

Consolidate test results — the repository's ONE required check — consolidated the previous
attempt's shard artefact after a rerun-failed-jobs, so a shard that re-ran green could not turn
the check green. Run 34823051999,
read again through the artefacts API at 17:0xZ today:

id 10338854604  size 297521  created 08:38:28Z   ← attempt 1, one failing test
id 10338734845  size 298484  created 08:49:55Z   ← attempt 2, 1454/1454 green

download-artifact de-duplicates a name collision by keeping the highest artefact ID, and IDs
are not monotonic across attempts — the older upload holds the higher one here. So the consolidate
read attempt 1 on attempt 2, and again on attempt 3. No re-run could green that run; only a new
head could.

The artefacts API answers id, name, size_in_bytes, created_at, digest and a workflow_run
block of {id, repository_id, head_repository_id, head_branch, head_sha} — and no attempt
(measured on the two rows above). Nothing but the producer can attribute an upload to an attempt,
and the name is its only channel.

The fix

Evidence carries its attempt in the name; transport overwrites.

	shard test results	shard build output
what it is	EVIDENCE the verdict is read from	TRANSPORT, superseded when rebuilt
name	`testResults-shard<N>-attempt<run_attempt>`	`build-output-<N>` + `overwrite: true`
consumer	`resolve-shard-attempts.py` keeps the highest attempt per shard	plain `name:` — only one can exist

A partial re-run legitimately MIXES attempts (five shards still on attempt 1, the re-run one on
attempt 2), so the rule is "each shard's newest attempt", never "the current attempt" — which would
throw away five sixths of the suite. created_at orders the two correctly and is still the wrong
instrument: it answers which upload is newer, not which attempt is this shard's outcome.

Same remedy as node-repo-module-pack.yml's lane key (Plugins#1077): when two producers can land one
name in one run, the discriminator goes INTO the name.

Would its output look different if the thing it checks were broken?

Yes, and each case was watched failing, not assumed:

mutation of the resolver	which case caught it
keep the OLDEST attempt (the production behaviour)	`shard1 attempt1 FAILED` — the symptom verbatim
lexical attempt order	attempt 9 kept over attempt 10
tolerate a name that is not attempt-scoped	no error raised
tolerate an attempt newer than the run's	no error raised
tolerate a shard outside the matrix	no error raised

and for the steward: first-listed-wins, lexical-attempt, a containment match on the shard index
(shard 1 stealing shard 11's artefact) and legacy-name-over-attempt-scoped each redden exactly one
of its four new assertions. The resolver's self-test runs one step before the resolution it
proves, inside collect-results, so an unproven resolver reds the required check itself.

Rate, with its denominator

700 dotnet-test.yml runs swept (2026-09-12T07:33Z → 2026-09-14T16:32Z): 15 ran a second
attempt, 9 of those carry a duplicated testResults-shard* name, and in 1 of the 9 the
ID-pick resolves to the older upload — the ID ordering decides it, and it is a coin toss. 0
carry a duplicated build-output-*: latent, not hypothetical, because rerun-failed-jobs re-ran
only shards, while a whole-workflow re-run re-runs build too.

Verification

python3 .github/scripts/resolve-shard-attempts.py --self-test → 21 assertions passed
python3 .github/scripts/merge-queue-steward.py --self-test → self-test PASSED (7 new rows)
check-workflow-timeouts.py --root . → 87 jobs checked, 0 violations, cap 45 min
check-workflow-yaml-keys.py, check-workflow-permission-pairing.py, check-workflow-shell.py
→ 0 violations / 0 live findings
MeshWeaver.Documentation.Test (link integrity + Architecture topic map) → 5/5 passed

Findings conserved as Doc/Architecture/ArtifactAttemptAttribution, linked from the Architecture
topic map.

Pairs-with: none — no public type or member leaves src/; the diff is CI workflow, two .github
scripts and documentation.

🤖 Generated with Claude Code

`Consolidate test results` downloaded the shard artefacts by the name pattern `testResults-shard*`. A run holds every attempt's uploads at once — a `rerun-failed-jobs` re-runs the failing shard and its upload lands BESIDE the one it replaces — and `download-artifact` resolves such a collision per NAME by keeping the highest artefact ID. Artefact IDs are not monotonic across attempts. Measured on run 34823051999 (#4292, head 75ae083), and re-read live today: id 10338854604 size 297521 created 08:38:28Z attempt 1, one failing test id 10338734845 size 298484 created 08:49:55Z attempt 2, 1454/1454 green 10338854604 is the higher ID, so the consolidate read attempt 1 and re-declared the failure the re-run had just cleared — on attempt 2, and again on attempt 3 three minutes later. That run could not be made green by any re-run; only a new head could. A required check whose recovery path is a no-op turns every unattributed failure into a forced new push. The artefacts API carries id, name, size, created_at, digest and the run's head_sha — and NO attempt, so nothing but the producer can attribute an upload to an attempt, and the name is its only channel. Shard results are now uploaded as `testResults-shard<N>-attempt<run_attempt>`, and `resolve-shard-attempts.py` keeps, per shard, the directory with the highest attempt, renames it to the canonical name every downstream step reads, and prints which attempt it kept. A partial re-run legitimately mixes attempts (five shards on attempt 1, the re-run one on attempt 2), so this is "each shard's newest attempt", never "the current attempt". `created_at` would order these two correctly and is still the wrong instrument: it answers which upload is newer, not which attempt is this shard's outcome. Build outputs take the opposite remedy, deliberately: a shard's trx is EVIDENCE, so the superseded attempt must survive and stay attributable; a build output is TRANSPORT, and a consumer reading the previous attempt's bins would test code this attempt did not build. `overwrite: true` deletes the name it replaces, so the ambiguity cannot arise and consumers keep downloading by the plain name. Measured over 700 dotnet-test.yml runs (2026-09-12T07:33Z → 2026-09-14T16:32Z): 15 ran a second attempt, 9 carry a duplicated testResults-shard name, 1 of those 9 resolves to the stale upload; ZERO carry a duplicated build-output — latent, not hypothetical, since a whole-workflow re-run re-runs `build` too. The merge-queue steward reads a shard's trx to decide whether a queue failure is a catalogued flake, so it resolves the same way, by attempt number. Both self-tests were watched failing: the resolver's five new cases each fail against a plausible wrong implementation (oldest-attempt wins — which reproduces the production symptom verbatim; lexical attempt order, so 9 beats 10; a tolerated unscoped name; a tolerated future attempt; a tolerated out-of-matrix shard), and the steward's four new cases fail against first-listed-wins, lexical-attempt, a containment match on the shard index, and legacy-over-scoped. Closes #4303. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Paginate artifact results and update stale artifact references before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes CI artifact attribution so rerun shard results are consolidated using each shard’s producing attempt.

Changes:

Scopes test-result artifacts by shard and workflow attempt.
Resolves the newest attempt per shard and updates artifact handling.
Updates merge-queue evidence selection and documentation.

File summaries

File	Summary
`src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-14-a-re-run-of-a-failed-test-shard-can-clear-the-run.md`	Announces the CI rerun fix.
`src/MeshWeaver.Documentation/Data/Architecture/ArtifactAttemptAttribution.md`	Documents artifact-attempt attribution.
`src/MeshWeaver.Documentation/Data/Architecture.md`	Adds the architecture topic entry.
`.github/workflows/dotnet-test.yml`	Updates artifact naming and consolidation.
`.github/scripts/resolve-shard-attempts.py`	Resolves per-shard attempt results.
`.github/scripts/merge-queue-steward.py`	Selects shard evidence for queue decisions.

Review details

Suppressed comments (2)

.github/scripts/merge-queue-steward.py:515

For a pre-#4303 run with duplicate unscoped artifacts, legacy is assigned the same string and returned, so gh run download -n repeats the ID-based de-duplication defect this change fixes and can feed the steward stale evidence. Count matching legacy artifacts and fail closed when that name is ambiguous; use the fallback only when exactly one legacy artifact exists.

            elif name == f"testResults-shard{shard}":
                legacy = name
        return best[1] if best else legacy

src/MeshWeaver.Documentation/Data/Architecture/ArtifactAttemptAttribution.md:69

This new attempt-scoped name also makes existing operational guidance stale: MergeQueue.md:119 and DebuggingNativeCrashes.md:38 still tell readers to fetch testResults-shard<N>, but new uploads are named testResults-shard<N>-attempt<A> and the steward selects that name. A user following either guide cannot find the new artifact; update those references or explicitly document the attempt-suffixed lookup.

| name | `testResults-shard<N>-attempt<run_attempt>` | `build-output-<N>` |
| collision | cannot happen — the name is unique per attempt | cannot happen — `overwrite: true` deletes the name it replaces |
| consumer | `resolve-shard-attempts.py` keeps the highest attempt per shard | plain `name:` — only one can exist |

Files reviewed: 6/6 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

rbuergi · 2026-09-14T17:26:46Z

+        exists at all. Returns None when the run holds no artefact for this shard — the caller
+        then records `present=False`, which is already the safe direction (no attribution, no
+        re-queue), not a silent pass."""
+        artifacts = (self.api(f"actions/runs/{run_id}/artifacts?per_page=100") or {}).get("artifacts", [])


Valid, and fixed in 47bb9df — thank you.

A run holds 12 artefacts per attempt (6 build-output-* + 6 testResults-shard*), so one per_page=100 page runs out after the eighth attempt. Worse than "some artefact is missed": pages are ordered oldest-first, so the row a single-page read drops is exactly the NEWEST attempt's — the one this function exists to resolve. The steward would then classify a superseded trx, or none, precisely on the pull request that has been re-run the most.

It now reads through Gh.api(..., paginate=True) and flattens the page objects. The self-test double answers PAGED (a list of page objects, page_size deciding how many), so single-page reading is a case rather than an assumption:

check(paged.shard_artifact_name(1, "0") == "testResults-shard0-attempt2", "the newest attempt is read even when it is on a later artefact PAGE")

with the winning attempt on the second page. Watched failing against the exact implementation you flagged before keeping it:

single-page read: exit 1 FAIL the newest attempt is read even when it is on a later artefact PAGE

(There is one artefacts read in this function, at what was line 502; line 513 is the same call's result being scanned, so the single fix covers both.)

github-actions · 2026-09-14T17:25:38Z