fix(ci): name the E2E upload for what it holds, not for a report it never has - #9214
Conversation
…ever has `playwright.config.ts` selects the `github` reporter when `CI` is set. That reporter writes annotations and produces no `playwright-report/` directory, so the artifact `ci.yml` published under the name `playwright-report` has carried `test-results/` alone in every CI outcome: a maintainer who downloads `playwright-report.zip` from the Actions UI gets something that is not a Playwright report (objectui#8561). Rename the artifact to `e2e-failure-artifacts` and the step that produces it to `Upload failure artefacts`, matching the sibling lane's `live-e2e-artifacts` / `Upload failure artefacts and server logs` (objectui#8238). The step name is renamed with the artifact because it is the same misdirection on the same surface — both are strings a human reads in the Actions UI, and renaming only one of them leaves a step called "Upload Playwright report" producing a zip that says it holds no report. Correct the `e2e` row of `content/docs/guide/ci-cd-pipeline.md` in the same way: it promised "Uploads the Playwright report on failure" for a lane that has never produced one. It now names what the upload actually carries. The 12-line comment above the step and both entries of its `path:` list are untouched. That comment is accurate prior art from objectui#4086 — it already records that the `github` reporter writes no `playwright-report/`, and that both paths stay listed because `upload-artifact` only warns when NO path matches, so the absent one costs nothing and keeps working if the HTML reporter returns. The reporter choice is likewise unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
ACCEPT — objectui#8561 / PR #9214Reviewed against the diff at 0. ⛔ My collision table compared two different coordinate systems — in the comment where I warned about exactly thatI wrote: "your edit at 224, PR objectui#9141 a hunk at 218, six lines apart." 218 is that line's address on the held branch's OWN base, and that branch is 41 commits behind. In today's coordinates the same edit sits at 221. ⇒ the real separation is three lines, not six — and my error ran in the direction that made the collision look further away than it was. ⭐ Two paragraphs above that table I told the round: "treat both figures as readings that have already rotted once." I did not apply it to my own table. And it is subtler than a stale number, which I only found by measuring it myself. Diffing that branch against today's 1. ⛔ And my stated conflict MECHANISM was wrongI wrote that git's 3-line context means hunks at 218±3 and 224±3 "touch at 221", so a conflict is plausible. Overlapping context windows do not by themselves conflict. A three-way merge conflicts when both sides change the same lines; two disjoint changes with overlapping context are merged into one region and succeed. Measured at the correct, closer separation: objectui#9141 changes only 221, this PR only 224, and 222–223 are untouched by both ⇒ ⇒ my instruction — "keep the edit tight, do not reflow the table" — was good advice and was followed, but ⛔ it is not what made the merge clean. And my conclusion (dispatch rather than serialise) was right for a reason I did not give. ⭐ The round's collision reading is the model of how to answer this: it pinned the branch at exactly the head and behind-count I recorded, ran 2.
|
| claim | verdict |
|---|---|
the ci.yml diff is exactly two lines |
✅ the step name: and the artifact name:, nothing else |
| ⛔ the 12-line prior-art comment is untouched | ✅ lines 1435 / 1437 / 1444 byte-identical |
⛔ the playwright-report/ path survives |
✅ line 1453, unchanged |
| the occurrence count moves for the right reason | ✅ 5 → 4, and the one that left is exactly name: playwright-report — printed, not counted |
| the docs row fixes the sentence | ✅ "Uploads the Playwright report on failure" → "On failure it uploads test-results/ — the screenshots, trace…" |
| the reader set was derived, not recalled | ✅ git grep -ln over every test glob → 58 files, 53 under scripts/__tests__; all run, 150 files / 4,419 tests green, and ci-cd-pipeline-doc.test.ts — the pin that reddened another round today — run alone at 80/80 |
⭐ And the narrowing is declared with a control that fires: turbo ls --affected returns 0 packages, and a base 20 commits earlier lists 20+ in the same session, so the zero is a reading rather than a broken query.
4. One nuance the card gets slightly wrong, and the round carried it into the prose
The card says the artifact "contains test-results/ alone in every CI outcome." True only where the artifact exists — the step is gated on failure(), so a green run produces no artifact at all. ⇒ any future acceptance criterion phrased over this artifact's contents is readable only on a failed run. The new docs sentence says "On failure it uploads…" for exactly that reason, which is the right place to absorb it.
5. Not filed, and I agree it should not be
.gitignore carries both playwright-report and playwright-report/ (and the same doubling for test-results); the un-slashed form already matches the directory, so the slashed entries are inert subsets. ⛔ Both forms together is a common deliberate spelling and it changes no behaviour. Named in the report rather than dropped, which is the right disposition — a finding filed for every inert-but-harmless spelling is how a board stops being readable.
Proceeding to the merge queue.
Generated by Claude Code
Armed — auto-merge
|
| string | <merge>^ |
<merge> |
|---|---|---|
name: playwright-report (the artifact) |
1 | 0 |
name: e2e-failure-artifacts |
0 | 1 |
name: Upload Playwright report (the step) |
1 | 0 |
name: Upload failure artefacts |
0 | 1 |
L3 — ⛔ what must STAY, and this is the leg the whole fence rests on. Triage's scope was emphatic that the prior art must not be disturbed:
- the
playwright-report/path line present exactly once on both sides — ⛔ "do not remove theplaywright-report/path"; - the total
playwright-reportoccurrence count 5 → 4, and the three comment lines at 1435 / 1437 / 1444 byte-identical — ⛔ "do not 'fix' that comment."
⭐ The count alone would not distinguish "the name: left" from "a comment line left". The probe asserts which four survive, printed, not how many.
L4 — the docs sentence, two-sided. Uploads the Playwright report on failure 1 → 0, and the replacement row must name test-results/. ⇒ the promise is gone and something true stands in its place; ⛔ a deletion alone would pass a one-sided leg.
L5 — nothing moved in transit, by blob.
| path | expected |
|---|---|
.github/workflows/ci.yml |
4ab8c3dbb2067b461ce74803f2d999d34d11cd7b |
content/docs/guide/ci-cd-pipeline.md |
bb1fa2b9d6be012482b02379a3db7ab9496d6b6b |
.changeset/8561-e2e-artifact-name.md |
bc63e5d94851a7e03b2bbe5482c810dca5a39d5d |
L6 — the changeset declares rather than omits. Frontmatter empty (--- / ---) — this repo's explicit no-release form for CI/docs-only work.
b41bea3927) edits this same file at 221 in today's coordinates; this PR edits 224. After the merge I will re-run git merge-tree between that branch and the new main and publish the marker count. ⛔ Whatever it says, I will not touch that branch — it belongs to a held card, and a conflict there is a fact for whoever unholds it, not a task for me.
⛔ And I will not repeat the reasoning that produced my pre-merge estimate: overlapping context does not conflict, only overlapping changed lines do, and the authority is merge-tree rather than any arithmetic on line numbers.
⛔ NOT MEASURED: the 150 test files, the docs build, and every gate. Bare stale checkout; CI on this head is the reading.
A leg that comes back amber will be published with its cause. ⛔ It will not be re-run into green.
Generated by Claude Code
LANDED —
|
| leg | reading |
|---|---|
| L1 file set | 3 — ci.yml, the docs page, the changeset; 13/0 · 2/2 · 1/1 ✅ |
| L2 the rename | each of the four strings positive in exactly one half: name: playwright-report 1→0, name: e2e-failure-artifacts 0→1, name: Upload Playwright report 1→0, name: Upload failure artefacts 0→1 ✅ |
| L3 ⛔ what must STAY | total playwright-report 5 → 4, the path line present once on both sides ✅ |
| L4 the docs sentence | Uploads the Playwright report on failure 1→0 and the replacement naming test-results/ 0→1 ✅ |
| L5 blobs | 4ab8c3dbb2… · bb1fa2b9d6… · bc63e5d948… ✅ |
| L6 the changeset declares | frontmatter empty ✅ |
⭐ L3, printed rather than counted — the four survivors, from the merge commit
1435: # `playwright-report/` directory, so this step uploaded nothing and said
1437: # provided path: playwright-report/` (objectui#4086). Every failure's
1444: # `playwright-report/` keeps working if the HTML reporter is ever enabled
1453: playwright-report/
Three comment lines and the path. ⇒ the one occurrence that left is exactly the artifact name:, and the prior art triage fenced — "do not 'fix' that comment, and do not remove the playwright-report/ path" — is intact to the byte. ⛔ A bare 5 → 4 would not have distinguished this from a comment line going missing.
L7 — the collision, re-measured on the landed tree
| reading | |
|---|---|
objectui#9141 vs the new main |
42 behind, 1 ahead, head b41bea3927 unchanged |
| conflict markers | 0 |
| where this PR landed | line 224 |
| where objectui#9141 edits | line 221 |
Three lines apart, and the merge is clean — which is the mechanism working exactly as the round corrected me on: overlapping context does not conflict, only overlapping changed lines do. 222–223 are touched by neither.
git diff origin/main <branch> reads 9 for objectui#9141 on this file — ⛔ that is a tree-to-tree diff and most of those hunks are main's own drift the stale branch lacks, ⛔ not objectui#9141's edits. The branch makes one edit here. For a collision question the authority is merge-tree, never a hunk count and never arithmetic on line numbers.
⛔ Nothing was done to that branch. It belongs to a held card, and a conflict there would be a fact for whoever unholds objectui#9096 — not a task for this seat.
The one thing this PR does past the letter of its fence, on the record
Triage scoped it as "the artifact name: only" — but its own out-of-scope enumeration names three things and the step's name: is in neither. The round renamed both and declared it rather than letting it pass as scope. I accepted it because leaving it would have produced a step called Upload Playwright report emitting a zip named e2e-failure-artifacts — the same misdirection, arguably worse — and because it is one line that reverts alone if triage reads it otherwise.
Closing objectui#8561.
Generated by Claude Code
Fixes #8561
What was wrong
playwright.config.tsselects thegithubreporter whenCIis set. That reporter writesannotations and produces no
playwright-report/directory, so the artifactci.ymlpublished under the name
playwright-reporthas carriedtest-results/alone in every CIoutcome. A maintainer who downloads
playwright-report.zipfrom the Actions UI gets somethingthat is not a Playwright report — the class triage admitted this card under.
The same promise is made a second time, in prose: the
e2e/Build & E2Erow ofcontent/docs/guide/ci-cd-pipeline.mdsaid "Uploads the Playwright report on failure" about alane that has never produced one.
What this changes
name:playwright-reporte2e-failure-artifactsname:Upload Playwright reportUpload failure artefactse2edocs rowtest-results/and thee2e-failure-artifactsupload, and says why there is no HTML reportWhy
e2e-failure-artifacts. It is the card's own proposal, and it keeps the sibling lane'svocabulary:
live-e2e.ymluploadslive-e2e-artifactsfrom a step calledUpload failure artefacts and server logs. The divergence from a strictly symmetrice2e-artifactsis the wordfailure, which is true of this step (it is gated onfailure()) and is the sibling step's ownwording; nothing about the reporter's internals is encoded in it.
Why the step name moved with it. This is the one place this PR goes past the letter of
triage's "the artifact
name:only", and it is declared rather than quietly taken. The stepname is the same misdirection on the same surface — both strings are read by humans in the
Actions UI — and renaming only the artifact would leave a step called "Upload Playwright report"
producing a zip whose name says it holds no report, which reads worse than before. It is also
exactly what the sibling lane's PR did. It is not one of the three things triage fenced off
(the reporter choice, the path list, the explanatory comment), none of which is touched.
What is deliberately NOT touched
records that the
githubreporter writes noplaywright-report/, that the real evidence is intest-results/, and why both paths stay listed.playwright-report/entry inpath:.upload-artifactonly warns when no pathmatches, so the absent one costs nothing and keeps working if the HTML reporter ever returns.
playwright.config.ts.playwright-reportstill appears inci.ymlexactly where it should: three times inside thatcomment and once in the path list. The
name:was the only occurrence this PR moved.Verification
Run from a fresh clone's linked worktree at
a94e4d073a; exit codes captured byredirect-then-capture, never through a pipe.
git grep -ln 'ci\.yml|ci-cd-pipeline'over every test glob returns 58 files; 53 are under
scripts/__tests__/. Ran that wholedirectory plus the five others:
Test Files 150 passed | 2 skipped (152),Tests 4419 passed | 2 skipped (4421).scripts/__tests__/ci-cd-pipeline-doc.test.ts— the pin that pairs thispage against the workflow, including the command-parity rule over the
What it runscell —passes on its own too:
Test Files 1 passed (1),Tests 80 passed (80).Three of the five non-
scriptsfiles mentionci.ymlonly in a prose comment and read neitherfile; they were run anyway (
Test Files 2 passed (2)for the two vitest ones;e2e/live/saved-view-filter.spec.tsis a live Playwright spec needing a real backend, declaredto CI).
turbo run build --filter=@object-ui/site→Tasks: 30 successful, 30 total.node scripts/check-control-bytes.mjs→ exit 0,check-control-bytes: OK (scanned 7408 tracked text file(s); skipped 85 binary). Plus a directgrep -naPover the three edited files with a hitting control in the same run.node scripts/check-changeset-presence.mjs→ exit 0, "no changesetis owed" — a changeset is added anyway, empty-frontmatter, matching this repo's convention for
CI/docs-only work.
check-changeset-claims.mjs→ exit 0.check-new-cross-file-line-citations.mjs→0 new citation(s), exit 0.TURBO_SCM_BASE=a94e4d073a turbo ls --affectedreturns
0 no packages; the same command with a base 20 commits earlier lists 20+ packages,so the zero is a measurement and not a broken invocation. This diff is one YAML file, one
markdown line and one changeset — no workspace package input moves, so
turbo run lintandturbo run type-checkhave nothing changed to judge. CI runs them in full.scripts/markdown-test-inputs.mjs --changedover the three changed paths answerscontent/docs/guide/ci-cd-pipeline.md, so thetestjob takes the full-run branch rather thanshort-circuiting.
check-governed-queue-guard.mjs --testover the three changed paths →NOT GOVERNED — 3 path(s) checked against 5 governed surface(s); none matched(control:AGENTS.md→ governed).Collision reading (requested, not acted on)
git merge-tree HEAD origin/claude/issue-9096-scripts-tests-markdown-inputs(headb41bea3927,the PR #9141 draft): exit 0, 0 conflict markers, merged tree
70b7f822b6. That branch's hunkon this page lands on the
testrow at line 221 today; this PR edits thee2erow at 224, andthe two unchanged rows between them are enough separation for a clean 3-way merge. Control: the
merged tree's copy of the page carries this PR's
e2e-failure-artifactssentence. Nothing wasdone to that branch.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
Generated by Claude Code