Skip to content

fix(ci): name the E2E upload for what it holds, not for a report it never has - #9214

Merged
baozhoutao merged 1 commit into
mainfrom
claude/issue-8561-e2e-artifact-name
Sep 11, 2026
Merged

fix(ci): name the E2E upload for what it holds, not for a report it never has#9214
baozhoutao merged 1 commit into
mainfrom
claude/issue-8561-e2e-artifact-name

Conversation

@claude

@claude claude Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #8561

What was wrong

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 — the class triage admitted this card under.

The same promise is made a second time, in prose: the e2e / Build & E2E row of
content/docs/guide/ci-cd-pipeline.md said "Uploads the Playwright report on failure" about a
lane that has never produced one.

What this changes

surface before after
artifact name: playwright-report e2e-failure-artifacts
the step's name: Upload Playwright report Upload failure artefacts
the e2e docs row "Uploads the Playwright report on failure." names test-results/ and the e2e-failure-artifacts upload, and says why there is no HTML report

Why e2e-failure-artifacts. It is the card's own proposal, and it keeps the sibling lane's
vocabulary: live-e2e.yml uploads live-e2e-artifacts from a step called Upload failure artefacts and server logs. The divergence from a strictly symmetric e2e-artifacts is the word
failure, which is true of this step (it is gated on failure()) and is the sibling step's own
wording; 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 step
name 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

  • The 12-line comment above the step. It is accurate prior art from objectui#4086: it already
    records that the github reporter writes no playwright-report/, that the real evidence is in
    test-results/, and why both paths stay listed.
  • The playwright-report/ entry in path:. upload-artifact only warns when no path
    matches, so the absent one costs nothing and keeps working if the HTML reporter ever returns.
  • The reporter choice in playwright.config.ts.

playwright-report still appears in ci.yml exactly where it should: three times inside that
comment 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 by
redirect-then-capture, never through a pipe.

  • Readers of the two edited files, derived not guessed. git grep -ln 'ci\.yml|ci-cd-pipeline'
    over every test glob returns 58 files; 53 are under scripts/__tests__/. Ran that whole
    directory 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 this
    page against the workflow, including the command-parity rule over the What it runs cell —
    passes on its own too: Test Files 1 passed (1), Tests 80 passed (80).
    Three of the five non-scripts files mention ci.yml only in a prose comment and read neither
    file; they were run anyway (Test Files 2 passed (2) for the two vitest ones;
    e2e/live/saved-view-filter.spec.ts is a live Playwright spec needing a real backend, declared
    to CI).
  • Build Docs: turbo run build --filter=@object-ui/siteTasks: 30 successful, 30 total.
  • Control Byte Scan: node scripts/check-control-bytes.mjs → exit 0,
    check-control-bytes: OK (scanned 7408 tracked text file(s); skipped 85 binary). Plus a direct
    grep -naP over the three edited files with a hitting control in the same run.
  • Changeset Declaration: node scripts/check-changeset-presence.mjs → exit 0, "no changeset
    is owed"
    — a changeset is added anyway, empty-frontmatter, matching this repo's convention for
    CI/docs-only work. check-changeset-claims.mjs → exit 0.
  • Line citations: check-new-cross-file-line-citations.mjs0 new citation(s), exit 0.
  • Type Check / Lint — declared narrowing. TURBO_SCM_BASE=a94e4d073a turbo ls --affected
    returns 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 lint and
    turbo run type-check have nothing changed to judge. CI runs them in full.
  • Test shards will run: scripts/markdown-test-inputs.mjs --changed over the three changed paths answers
    content/docs/guide/ci-cd-pipeline.md, so the test job takes the full-run branch rather than
    short-circuiting.
  • Governed surface: check-governed-queue-guard.mjs --test over 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 (head b41bea3927,
the PR #9141 draft): exit 0, 0 conflict markers, merged tree 70b7f822b6. That branch's hunk
on this page lands on the test row at line 221 today; this PR edits the e2e row at 224, and
the 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-artifacts sentence. Nothing was
done to that branch.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr


Generated by Claude Code

…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

Copy link
Copy Markdown
Contributor

ACCEPT — objectui#8561 / PR #9214

Reviewed against the diff at 923fa9e1b3170a46b062b06851fa3cc7dccfb53a; ls-remote, the report's sha and the branch head agree. 13 0 changeset · 2 2 ci.yml · 1 1 the docs row.

0. ⛔ My collision table compared two different coordinate systems — in the comment where I warned about exactly that

I 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 main prints eight hunks in this file — 46, 221, 321, 508, 516, 557, 1686, 1699 — because a tree-to-tree diff also carries everything main has gained that the stale branch lacks. ⇒ ⛔ neither raw number answers "where does objectui#9141 edit": diff against its own base says one hunk at 218, diff against today's main says eight. The branch makes one edit to this file; 221 is where that edit lands today. The authority for the collision question was never a line number at all — it is merge-tree.

1. ⛔ And my stated conflict MECHANISM was wrong

I 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 bothmerge-tree exits 0 with zero markers.

⇒ 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 merge-tree, got 0, and proved the merge was real rather than vacuous by checking that the merged tree carries this PR's own sentence. An absence with a control that hits.

2. ⚠️ Triage's scope line disagrees with triage's own enumeration, and the round chose the enumeration

In scope: the artifact name: onlyOut of scope: the reporter choice, the path list, and the comment.

The step's own name: is in neither list. Renaming only the artifact would have left a step called Upload Playwright report emitting a zip named e2e-failure-artifacts — ⛔ the same misdirection on the same surface, and arguably worse than before.

The round renamed it to Upload failure artefacts, matching the sibling lane's precedent, and declared it in the PR body rather than letting it pass as scope. ⭐ I accept it. The enumeration is the operative fence; "only" is contradicted by triage's own list; the change is one line and reverts on its own if triage reads it otherwise. ⛔ That last clause is the reason this is acceptable rather than scope creep.

3. Verified in the tree

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

@baozhoutao
baozhoutao marked this pull request as ready for review September 11, 2026 22:45

Copy link
Copy Markdown
Contributor

Armed — auto-merge SQUASH, and the probe I will run afterwards, written first

Armed 2026-09-11T22:45:58Z on head 923fa9e1b3170a46b062b06851fa3cc7dccfb53a. ⓘ The arm call echoed method: MERGE; ⛔ not the authority — origin/main is linear, so the queue squashes.

The probe, posted BEFORE the merge

Every value extracted from the tree at 923fa9e1b and a94e4d073a just now. Legs scoped to <merge>^ <merge>, ⛔ never origin/main, and every read names an explicit ref — this seat's checkout is ~580 commits stale.

L1 — the file set. Exactly 3 paths: .github/workflows/ci.yml (2 2), content/docs/guide/ci-cd-pipeline.md (1 1), .changeset/8561-e2e-artifact-name.md (13 0).

L2 — the rename, four strings, each positive in exactly one half. ⛔ Not a count of "how many things changed" — the identity of what left and what arrived:

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 the playwright-report/ path";
  • the total playwright-report occurrence 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.

⚠️ L7 — the collision, re-measured on the landed tree. objectui#9141 (held, head 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

@baozhoutao
baozhoutao added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit b775500 Sep 11, 2026
37 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-8561-e2e-artifact-name branch September 11, 2026 23:17

Copy link
Copy Markdown
Contributor

LANDED — b775500af677ec876b5f6dad443028e7f9ed6fbf

Probe run scoped to b775500af6^ b775500af6 (parent a94e4d073a). All seven legs green.

leg reading
L1 file set 3ci.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.

⚠️ And the trap I fell into once today, stated so it does not get re-used: the hunk count from 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci.yml's E2E artifact is named playwright-report but never contains one — the name is the only thing a reader sees in the Actions UI

2 participants