Repository navigation
PR card: a red the base already has reads "also red on base" - #23644
Conversation
The card and the chat header's chip now say which of a pull request's failing
checks fail the same way at the base branch's tip. That is the one thing a
reader needs before opening a log: a base red is not the pull request's, it does
not hold a merge (CLAUDE.md), and there is nothing in it to fix.
The host decides. `AgentChatPullRequestReader`'s batched GraphQL read now asks
for the base ref's own rollup beside the head's, through the same contexts
fragment, and `AgentPanePullRequest` compares them.
The comparison cannot be naive, which is what the tests are built around. A
commit's `statusCheckRollup` still lists the checks of a run that the
concurrency group cancelled when a newer run of the same workflow started on the
same commit, and those read as failures (`outcome` has no CANCELLED case on
purpose: a cancellation a person asked for is a red). Comparing the two lists
as they arrive would tell a reader "that red is base's" about a red that is
theirs, which is the expensive direction of wrong. So `latestRuns` keeps only
each workflow's highest run id before either side is read, and `failing` also
refuses a cancelled run as evidence even when it is the newest: it reached no
verdict. Erring this way can only fail to excuse a red, never excuse one
wrongly. The head's own superseded runs drop out the same way, so a cancelled
run no longer inflates the head's own failure count either.
A rollup with no run ids at all -- the single-link `gh pr view` path, which does
not read them and cannot see the base -- is left exactly as it was: nothing is
marked, and "not marked" never renders as "not yours".
On the page, the card's summary reads "98 of 100 checks passed, 2 failing, 1
also red on base", and when every red is the base's its dot drops from danger to
warning. Each row in the chip's popover says "also red on base" and drops to
warning too.
Red then green, with the trap as the fixture:
- `AgentPaneBaseRedsTests.aCancelledRunNeverExcusesAHeadRed` and seven more, run
as a line-for-line port of the new rules against the tests' own fixtures
(17/17; Swift cannot execute on this Linux host). The naive comparison reads
`onBase == true` on that fixture, the shipped rules read `false`.
- `webviews`: `bun test test/pr-card-content.test.tsx
test/chat-header-pull-request-status.test.tsx` fails 4 of 12 against the
unchanged sources ("2 failing" where "2 failing, 1 also red on base" belongs),
passes 12/12 with them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
GitHub Actions
Inactive
|
Owned by pr-cards on big-red · session |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Gallery before/after for
States
Pick other states with a Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on 6b8b7e4 to compare; do not merge until green; see lane-rules Tour of Path-selected interactions
Agent pane (before): PASS — 3 ok
Screenshots (3)1. Launch
Screenshots (3)1. Launch
Screenshots (15)1. Launch
Screenshots (15)1. Launch Tours are data in |
|
Bead |
|
Found 9 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Three defects an independent review found in the first cut, and the honest limits of what ships. `latestRuns` was applied to the pull request's own rollup as well as the base's. Two runs of one workflow on one commit are not always a supersession: a workflow with both a `push` and a `pull_request` trigger whose concurrency groups differ runs twice on the same head (mux.yml, release-rails.yml), and a dispatched or rescued re-run outranks the run beside it without cancelling it. Keeping only the newest could therefore drop a real red -- the card would say "2 of 2 checks passed" about a commit GitHub shows as red. It is a one-way measure, so only `failing` (the base tip) uses it now, where dropping the older of two live runs can only fail to excuse a red. `aSecondLiveRunOfOneWorkflowKeepsItsRed` pins that, and the head-side claim the first cut made is withdrawn: a cancelled, superseded run of the head's own workflow still reads as a red, as it did before this branch. Base reds were matched by check name alone. Job labels repeat across this repository's workflows (`test`, `web`, `build`, `check`, `comment`), so a base red called `test` from one workflow could excuse a head red called `test` from another -- the one direction the design promises never happens. Matching is now by `identity`: the workflow's id (stable across commits) with the check's name, and a context that belongs to no run matches only another context with no run. A base red that arrives from a different workflow than the head's is no longer marked, which is the safe direction of that trade. A card that refreshed lost its marks. `pr.inspect` reads a stale link through `perform`, which took the single-link `gh pr view` path -- the one that cannot see the base. Sixty seconds after a card gained "also red on base" it quietly went back to "2 failing". `perform` now reads through the same batched GraphQL as a reply's links, with a batch of one: the same single call to GitHub, and the only one of the two that reads the base. Also from the review: the card prefers the host's own count over the marked rows (the count covers the whole rollup, the row list is truncated at 50), with the rows as the fallback for a host that marks without counting; four fixtures the first cut was missing (two live runs, a same-named red of another workflow, a passing head check, a base check still running); and the string catalog is rebuilt on base's exact text, so its diff is 264 added lines and no reordering. Not fixed here, and now said plainly rather than claimed: the chat header's chip reads `gh pr view` in a folder, which has no base rollup, so its row marker cannot light up in the shipping app yet -- it is the same data shape, drawn where rows exist, and it turns on when that reader gains a base read. Verified again by the line-for-line port of the new rules against the tests' own fixtures: 16/16, and the naive comparison still reads `onBase == true` on the trap fixture where the shipped rules read false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
GitHub Actions
Inactive
GitHub Actions
Error
…ther-host read Three notes from the re-review, none of them behaviour the card shows. The query comment explained the run fields as something `latestRuns` reads, which is now base-only -- so a later reader could trim them from the head's rollup believing nothing used them, and every head identity would silently become just its name again, with no test failing. The comment names both readers now, and the query test counts the run selection twice (head and base) and asserts the base ref is asked for at all. Routing `perform` through `read(urls:)` moved the other-host fallback (`gh pr view <url>` for a GitHub Enterprise link) from the home folder to whatever the App inherited; `read(url:)` ran it in the home folder on purpose, since a pull request named by URL needs no repository. Both read `home` now. `read(url:)` has no production caller left. Its doc says why it stays -- it is the shape of the other-host read, and its test -- rather than leaving the next dead-code sweep to guess. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
cf413c8 to
d54aba4
Compare
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
GitHub Actions
Inactive
GitHub Actions
Inactive
…e reds Webview screenshots come from the gallery advisory run, so the two readings this change adds get fixture states of their own: one red the base shares out of two, which keeps the danger dot, and both reds shared, which drops it to warning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
GitHub Actions
Inactive
|
Base finding, not this PR's: It fails the same way on an unrelated PR of mine that touches no Swift at all (#23649, The same job also reports |
`agent-pane.url-chip#pull-request-lines-by-kind` has never rendered what it describes. Its play clicks the `+/-` row to pin the breakdown open, and a trusted click lands at the point the row occupied when the step ran -- but the card moves twice after it opens. The host's answer fills it in, which drops the row from 186 to 211, and the card's own entrance settles it from 211 back to 209 over about six frames. The click went to 194 while the row was at 217, nothing was pinned, and the state spent its five-second cap waiting for a breakdown that was never going to appear: - agent-pane.url-chip#pull-request-lines-by-kind: play: waitFor did not hold within 5000 ms That is red for every pull request whose files make CI render this entry, and it is red with the base's copy of the file: the same failure reproduces in a worktree that does not touch it (`bun health.ts --manifest ... --gallery-dir ...` on `618a90ba5f00`), so it is this state, not the base-red marks around it. `stillRow` holds the row's rounded top for twelve frames before the click, which is the row standing still rather than a gap between its two moves. The state now renders its breakdown -- Code, Tests, Mocks, Docs, Generated with their lines -- and reports ready in 2.6s instead of 7.3s, because it no longer burns five seconds failing. Red then green, `scripts/gallery-matrix/health.ts` over this entry's ten states (including the two this branch adds): before, `1 newly broken state(s)`, that line; after, `0 newly broken state(s)`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
GitHub Actions
Inactive
The row-is-still wait counted calls to its own condition, and `ctx.waitFor` runs that condition on every animation frame *and* on every batch the MutationObserver reports (playRunner.ts). The positioner writes inline styles while the card settles, so a dozen "held" counts could all land inside one or two frames -- the gap between the card's two moves, which is exactly what the wait is supposed to rule out. It would have failed the same opaque way as the bug it fixes. It now drives its own `requestAnimationFrame` loop, the way `samplePinned` (bottomStack.gallery.ts) and the scroll-thumb play do, so eight held frames are eight painted frames. A row that never stops moving gives up after 300 frames with a message that names the selector, instead of leaving the play parked until the stage times out. Green on this host, `scripts/gallery-matrix/health.ts` over the entry's ten states: `0 newly broken state(s)`, exit 0, and the state's own record reads `play status: pass`, both steps `pass` with no problems, ready in 1931 ms -- under the 2000 ms default budget (ready-budgets.json), where it was 7322 ms before this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cc0-fc9d-7382-af90-51f8726825bd&host=big-red
GitHub Actions
Inactive
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
|
Disclosed, out of scope for this PR: the card's Worth knowing: the gallery matrix cannot currently report this movement. Layout shift and anchors |
|
Merging with the Mac-side checks still queued, and saying so plainly. What is green on this exact head: So the new Swift tests have compiled but not yet executed anywhere. What stands behind them instead: |
|
Shipped in:
|
What
The pull request card and the chat header's chip now say which of a pull request's failing checks
fail the same way at the base branch's tip. That is the one thing a reader needs before opening
a log: a base red is not the pull request's, it does not hold a merge (CLAUDE.md),
and there is nothing in it to fix.
98 of 100 checks passed, 2 failing, 1 also red on base.this pull request's to fix.
also red on baseand drops to warning too.The host decides:
AgentChatPullRequestReader's batched GraphQL read now asks for the base ref'sown
statusCheckRollupbeside the head's, through the same shared contexts fragment, andAgentPanePullRequestcompares them. No extra request and no per-hover call — the base ref comesback in the one batched read the lane already makes.
The trap this is built around
A commit's
statusCheckRollupstill lists the checks of a run that the concurrency groupcancelled when a newer run of the same workflow started on the same commit, and those read as
failures (
outcomehas noCANCELLEDcase on purpose: a cancellation a person asked for is a red).Comparing the two rollups as they arrive would tell a reader "that red is base's" about a red that
is theirs — the expensive direction of wrong.
So:
latestRunskeeps only each workflow's highest run id, on both sides, before either is read.A context with no run (a commit status) cannot have been superseded, so it stays.
failingrefuses a cancelled run as evidence even when it is the newest: it reached noverdict, so a check whose only base run was cancelled stays the pull request's own red. Erring
this way can only fail to excuse a red, never excuse one wrongly.
head's failure count either.
gh pr viewpath, which does not read them andcannot see the base — is left exactly as it was. Nothing is marked, and "not marked" never
renders as "not yours".
Red then green
Built test-first, with the trap as the fixture.
AgentPaneBaseRedsTests(new, 8 tests) opens withaCancelledRunNeverExcusesAHeadRed: at thebase tip
Verify / scriptsfailed in run 100, which the concurrency group cancelled, and passedin run 101 of the same workflow; the head's failure of it must stay the head's own.
against the Swift tests' own fixtures: 17/17, including a regression check that
AgentChatPullRequestTests' existing fixture still readsfailing 3/5. A naive comparison readsonBase == trueon the trap fixture; the shipped rules readfalse.webviews:bun test test/pr-card-content.test.tsx test/chat-header-pull-request-status.test.tsx→ 4 of 12 fail against the unchanged sources (
"98 of 100 checks passed, 2 failing"where", 1 also red on base"belongs), 12/12 pass with them.Observed / Not run
Ran and saw, on this head:
bun test test/pr-card-content.test.tsx test/chat-header-pull-request-status.test.tsx— 12 pass,and the red above against the pre-change sources.
webviews/bun run check— exit 0 (Oxfmt, Oxlint + type check, layers, type scale, motion scale,dialog keys, xcstrings keys, protocol codegen).
scripts/web-lint.sh— passed.scripts/cmux-next/prepush-gate.py— no new violations.two new keys added, none lost.
Only read, not run here:
PR's own
fleet test-compilecheck cover them.webviews/test/ui-rules.test.tsfails onpages/chief-views/ChiefViewsPage.tsxandpages/apps/AppsPage.tsx, andscripts/lint-ios-package-conventions.sherrors onCmuxNextSidebar/Model/ChiefWorkspaceName.swift— none of those files are in this diff.Follow-ups (disclosed, not in this PR)
PullRequestMergestill readschecks.state, so a pull request whose only reds are the base'sdoes not yet get the Merge button. That is a behaviour change to the merge affordance and wants
its own PR.
What it does not mark (the conservative half of the trade)
Matching is by workflow and name, because job labels repeat across this repository's workflows.
The cost is that a base red reported by a different workflow file than the head's is not marked
at all. The repo mirrors a few checks that way —
CI fast guards(ci-fast-guards.yml on push vsci.yml on pull_request),
web(cmux-next-web.yml push vs ci.yml) — andworkflow_callcheckscarry their caller's workflow, which can differ between the base tip and a head. For PRs into
feat-cmux-nextthe usual base-red sources (cmux-next.yml, cmux-next-guards.yml,cmux-next-media.yml, backend.yml, rust-workspaces.yml, cmux-tui-spec.yml, test-ios.yml,
gallery-health.yml) carry both triggers in one file, so the common case matches. Everything here
errs the same way: it can fail to excuse a red, never excuse one wrongly. So "it didn't mark my
base red" is this trade, not a bug.
Two more honest limits:
gh pr viewin the chat's folder, whichhas no base rollup. The row marker and its string ship because they are the same data shape drawn
where rows exist; they turn on when that reader gets the
read(urls:)treatmentperformjustgot. Today only the card's batched read carries base reds.
contexts(first: 100)truncates on a busy base, andcheckListis truncated at 50 rows whilefailingOnBasecounts the whole rollup. Both fail safe (a red stays the pull request's), so thisis paging work, not a defect.
Review
Two rounds with an independent reviewer. The first found three real defects, all fixed in
dc48c4c:latestRunsapplied to the pull request's own rollup (which could drop a real red whena workflow legitimately runs twice on one commit), name-only matching across workflows, and a card
that lost its marks 60 seconds later because
pr.inspect's refresh took the base-lessgh pr viewpath. The second round passed and left three notes, fixed in
d54aba4: the query test now pins therun fields on both rollups (nothing else would have caught a later trim degrading the match),
the other-host fallback keeps the home folder it had before the reroute, and
read(url:)says whyit stays now that nothing in production calls it.
Capture
No capture: the Swift this changes is data plumbing —
AgentPanePullRequestcompares two checkrollups,
AgentChatPullRequestReaderasks for the base ref's — and draws nothing. The visiblechange is one line of text and one dot colour in the pull request card, a webview, so its evidence
is where webview evidence belongs: two new gallery states
(
agent-pane.url-chip→pull-request-base-reds,pull-request-all-reds-on-base) and a test thatasserts the exact rendered line,
98 of 100 checks passed, 2 failing, 1 also red on base, plus thedata-on-base="all"attribute that dims the dot. Before/after of those states, live from bothbuilds: https://cmux-lawrences-mac-mini.tail137216.ts.net:18796/compare/cmux-pr-card-base-reds/?entry=agent-pane.url-chip
(tailnet only.)
A gallery state this branch repairs
agent-pane.url-chip#pull-request-lines-by-kind(from #22951) has never rendered its breakdown.Its play clicks the
+/-row to pin the lines-by-kind list open, and a trusted click lands wherethe row was when the step ran -- but the card moves twice after it opens: the host's answer fills it
in (the row drops 186 -> 211) and the card's entrance settles it back (211 -> 209 over about six
frames). The click went to y=194 while the row sat at 217, so nothing was pinned and the state
burned its five-second cap on
waitFor did not hold within 5000 ms.It is red with the base's copy of the file -- reproduced in a worktree that does not touch it -- so
it is this state, not this branch. It is in this branch anyway because it is the same file and the
same entry: a separate PR would only conflict with this one.
stillRowholds the row's rounded topfor twelve frames before clicking. Over this entry's ten states (the eight on base plus the two
here),
scripts/gallery-matrix/health.tsgoes from1 newly broken state(s)to0, and the statereports ready in 2.6s instead of 7.3s because it no longer waits for something that cannot happen.
Changelog
Changed: a pull request card and the chat header's chip now say which failing checks are also red on the base branch, so a red nobody has to fix reads as one.
🤖 Generated with Claude Code
Agent session: big-red