Skip to content

Read app-pinned required checks from the head's check runs (#176) - #195

Merged
mchwang merged 9 commits into
mainfrom
fix/176-app-pinned-checks
Oct 21, 2026
Merged

Read app-pinned required checks from the head's check runs (#176)#195
mchwang merged 9 commits into
mainfrom
fix/176-app-pinned-checks

Conversation

@mchwang

@mchwang mchwang commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #176. Part of closing build step 4.

Problem

A required check pinned to a GitHub App (ruleset integration_id, classic app_id) always read as missing: gh pr view --json statusCheckRollup (gh 2.102.0) never returns a check run's app, and the test fake returned an app field real gh does not send.

Change

  • When any required check is pinned (and the rules are known), github/merge.ts asks GitHub for exactly what each pinned check needs, at the exact head, through gh api graphql: checkSuites(filterBy:{appId}) then checkRuns(filterBy:{checkName, checkType:LATEST}), one alias per pinned check, up to 25 per call; only an app with more than 100 suites is paged (up to 10 pages). The app filter also finds a private app's suite (GraphQL shows its app as null). The name filter is exact.
  • Unpinned checks still come from the rollup, with no extra call. A commit status never meets a pinned check here: whether GitHub lets an app's status do so is unverified, so this is a known fail-closed gap.
  • When stages stop each other, the inspection reports the first failure (failed.signal.reason), as the inspect comment says.
  • The read fails closed with its reason (gh's error, or codeboost's own when the answer is malformed, too large or changed between pages): GraphQL errors, malformed or unknown fields, a missing alias, another or no commit, more than 100 latest runs of the pinned name in a suite, more than 10 pages of an app's suites, or suites changing between pages. Its timeout keeps the inspection's own message.
  • Test fakes now return only fields real gh returns, with the version named beside them. Recordings nonadmin-ruleset (vite) and nonadmin-mixed (svelte, whose private-app pinned check now reads success) re-recorded at the same heads. New recordings: private-app-suites (svelte suites via GraphQL and REST), pinned-rerun (a real re-run, LATEST drops the failed run), pinned-large-suite (a vscode suite with more than 100 runs: 116 when last recorded).
  • Docs: guarded-merge.md, merge-handoff.md, design T12 note.

Validation

Validated head: f622c5e (CI and local gates below ran on it).

  • CI on f622c5e: test passed (run 39539638476); real-docker passed (run 39539638522).
  • Full unit suite without the Docker files (vitest run --project parallel) on f622c5e: 2501 passed, 1 skipped, 0 failed.
  • Focused merge suites (merge, merge-recorded, merge-stop-reasons) on f622c5e: 297 passed. Typecheck on f622c5e: pass.
  • Fail-without-fix checks (each at the commit that added the test): removing the first-failure line fails the stop-reasons test (5ec3b0c, re-checked at f622c5e by round 9); removing the unknown-rules skip fails its test (5ec3b0c); the round-1 merge.ts fails 33 tests including the svelte, re-run and vscode recordings (5ec3b0c); the old cursor/page order fails the page-10 test (4f88972).

Historical counts (intermediate checkpoints, superseded by the above): 455 merge-suite tests at 75efb01; 477 at 5ec3b0c, 429d9c2 and 40717e1; full suite 2497 passed at 5ec3b0c.

Not proven: real paging past 100 suites of one app, more than 100 latest runs of one name in a suite, more than 25 pinned checks (fakes only); whether an app's commit status meets a pinned check (fails closed); a token without the Checks permission (fails closed). The re-run and vscode recordings come from repos whose rules pin nothing, so their rule and PR answers are faked.

Review record

  • Self-review (author subagent): done before commit.
  • Independent review round 1 (fresh general-purpose agent, base main / head 75efb01). Findings and outcome (fixed in 5ec3b0c unless noted):
    • M1: private-app runs always read missing (GraphQL app: null). Fixed with the server-side app filter; svelte now reads success.
    • M2: suites with more than 100 runs in the wild failed the whole inspection. Fixed with the name filter; vscode recording.
    • L1: latency inside the 6 s fresh inspection. Query narrowed (vite: ~1.0 s, 2.8 KB). Declined: running it in parallel with the rule reads, because the pinned set is known only after both rule reads, and a parallel stage would add another stop path on every exit for one call's time.
    • L2: rules stage could now reject and break first-stop ordering; read ran with unknown rules. Fixed (skip when unknown; first failure reported); stop-reasons test.
    • L3: doc inaccuracies (codeboost's own errors, commit-status claim, svelte cause). Fixed.
    • L4: an unreal exit-0 GraphQL-error fake; missing cases. Fake labelled defensive; exit-1 shape matches gh; tests added for the page limit, unknown rules and a real LATEST re-run.
  • Independent review round 2 (fresh general-purpose agent, base main / head 5ec3b0c): no critical, high or medium issues; the query checked live against GitHub (exact names, private apps, unknown app/commit, exit-1 errors, LATEST with re-runs, filtered paging, 25-alias cost). Low findings, fixed in 429d9c2:
    1. inspectPinned fakes did not match gh. Now real 404 shape and branch fields; version and omissions noted.
    2. Stop-reasons and requiredTestState fakes left out output silently. Real stdout or a note; the "Something went wrong" text is labelled not reproduced live.
    3. RecordedGh comment wrong for the three new fixtures, and their script uncommitted. New scripts/record-pinned-gh.ts; comment fixed. Re-recording grew the vscode suite from 101 to 116 runs; the test now checks 116.
    4. "Up to 10 pages" cannot fit the 6 s inspection. Kept the cap and documented the inspection deadline as the real limit (it fails closed with the timeout); a cap tuned to one deadline would be fragile under load, and the 12 s display inspection fits more.
    5. Docs: T12 evidence names the replacing test; the long guarded-merge paragraph is split, with a new "Required checks pinned to an app" section. Build step 4 status lines are left for a docs refresh after Show the merge job's step list and let a person cancel the merge alone (D16) #194, Read app-pinned required checks from the head's check runs (#176) #195 and Keep the rebase conflict resolver's trust-read timeout over a later stop (#174) #197 merge.
  • Independent review round 3 (fresh general-purpose agent, base main / head 429d9c2): no critical, high or medium issues; all five recordings reproduced from their scripts; live gh 2.102.0 checks matched. Findings, fixed in 40717e1 (item 3 in this body):
    1. Low: the stop-order comment, docs and fake described an order runWithInput cannot produce. Reworded (gh exited, pipes closed after the stop); fake models that order; the test still fails without the first-failure line.
    2. Low: new fakes left out real gh output silently. Now built like runWithInput (stderr, newline, stdout; bodies with documentation_url), UNPROTECTED_BRANCH from the recording, the abort's reason on timeout; one shared note names gh 2.102.0 and what the fakes leave out.
    3. Low: PR body said 101 runs. Fixed (116 when last recorded).
    4. Nit: page-limit wording now separates the 6 s fresh and 12 s display inspections; "read" → "parsed" in a test comment.
  • Independent review round 4 (fresh general-purpose agent, merge base aced300 / head 40717e1): no critical, high or medium issues; rounds 1–3 verified; live checks (fork suites excluded from the base repository's read, latency 3.3–3.4 s of 6 s). Nits, fixed in 673b9d1 (comment and docs only; merge-recorded passes):
    1. A test comment said the re-recorded files' other answers did not change; svelte's mergeable did. Noted.
    2. The recordings' date in guarded-merge.md omitted the 2026-10-21 re-record. Added.
    3. A token without the Checks permission was not in the "Not checked" list. Added (fails closed).
  • Independent review round 5 (fresh general-purpose agent, merge base aced300 / head 673b9d1): no findings at critical, high, medium or low; rounds 1–4 verified (live: an app with no suites, case-sensitive names). One nit, fixed in 30c3c18: a next page without a cursor was reported with the page-limit reason; it is now "GitHub returned incomplete check runs." (defensive test added). merge, merge-recorded, merge-stop-reasons pass.
  • Independent review round 6 (fresh general-purpose agent, merge base aced300 / head 30c3c18): no findings at critical, high, medium or low; rounds 1–5 verified; live checks repeated. One nit, fixed in 4f88972: on the 10th page a missing cursor still got the page-limit reason. The cursor is now checked first; a page-10 case was added and fails with the old order (checked). The 3 merge test files pass.
  • Independent review round 7 (fresh general-purpose agent, merge base aced300 / head 4f88972): nothing critical, high or medium; rounds 1–6 verified; live: LATEST drops the failed re-run, GraphQL cost 1–25 points per call (worst case about 875 per 30-minute job). Findings, fixed in aa23839:
    1. Low: one fake rollup (the invalid-app-identity test) still returned app, which gh never sends. Removed; no app field is left in any rollup fake.
    2. Nit: guarded-merge.md said the first stage's failure is always reported; the deadline can replace a failure that is not gh's own exit. Scoped. The 3 merge test files pass.
  • Independent review round 8 (fresh general-purpose agent, merge base aced300 / head aa23839): nothing critical, high or medium; rounds 1–7 verified. Findings, fixed:
    1. Low: per-call latency was too optimistic (measured 2–3 s for a full page of 100 suites on cli/cli and next.js). Code comment and guarded-merge.md corrected in f622c5e: about one full page fits a fresh inspection, three or four the display one; it still fails closed.
    2. Low: this body said the full suite was not re-run because only fakes and docs changed, but 30c3c18 and 4f88972 changed code. The full non-Docker suite was re-run at f622c5e (see Validation).
  • Independent review round 9 (fresh general-purpose agent, merge base aced300 / head f622c5e): clean for the code, tests, fakes, recordings and docs; rounds 1–8 verified; live timings support the latency wording. Two nits in this body's record (stale CI state; items not naming their commit), fixed by rewriting the Validation section above (no code change).

Before Copilot (head f622c5e, validated head for this request)

Copilot round 1 (head f622c5e)

Copilot (copilot-pull-request-reviewer): "Approval recommended", no open issues. No change made.

Review-lesson audit (merge gate)

No new rule is needed; each finding is covered by an existing rule or is a one-off.

  • Covered by CLAUDE.md "A fake of an external tool must give … each part of the answer the code reads … as the real tool gives it": round 1 L4 (exit-0 GraphQL error fake), round 2 L1 and L2 (inspectPinned, stop-reasons and requiredTestState fakes), round 3 L2 (fakes leaving out output), round 7 L1 (the last rollup app field). Also the issue's own cause (the app field the merge fake returned).
  • Covered by CLAUDE.md "Treat every behavioural claim … as something to verify": round 1 L3 (docs on codeboost's own errors, the commit-status claim, the svelte cause), round 2 nit 4 and round 8 L1 (page limit and latency), round 3 L1 (a stop order run-with-input cannot produce), round 4 nits (re-record notes), round 7 nit (first-failure claim scoped).
  • Covered by CLAUDE.md "Report focused tests, affected-suite tests, the full suite … as separate gates" and "label cited commits": round 3 L3, round 8 L2, round 9 nits (stale counts and commits in this body).
  • Covered by AGENTS.md "Preserve the original timeout, cancellation, and shutdown reason through every layer": round 1 L2 (the rules stage rejecting and the first stop reason).
  • One-off (GitHub API behaviour, now pinned by recordings and tests): round 1 M1 (GraphQL hides a private app's app; filter on the server instead), round 1 M2 (suites with more than 100 runs in the wild; filter by name), round 1 L1 (latency of the extra call), rounds 5–6 nits (the missing-cursor reason and its order against the page limit).

🤖 Generated with Claude Code

mchwang and others added 3 commits October 20, 2026 18:52
@mchwang @claude
Read app-pinned required checks from the head's check runs (#176)
75efb01 
A required check that a ruleset (integration_id) or classic protection
(app_id) pins to a GitHub App was matched through check.app.databaseId in
gh's statusCheckRollup. gh 2.102.0 never asks GitHub for a run's app, so
the check always read as missing, even when it passed.

When any required check is pinned to an app, the inspection now also reads
the check runs of the exact head the PR read returned, through gh api
graphql: each check suite with its app's databaseId and the suite's latest
run of each name. A pinned check matches only runs of its name from a
suite of its app; a commit status, or a run whose suite GitHub names no
app for, never matches it. Checks any app may report are still read from
the rollup alone. The read fails closed and keeps its reason: a gh
failure, errors or malformed fields, another commit, a suite with more
than 100 runs, more than 1,000 suites, or suites that change between
pages fail the inspection; a deadline keeps the inspection's timeout.

The test fakes no longer give rollup entries an app, and answer the new
read in the shape gh 2.102.0 prints. The vitejs/vite and sveltejs/svelte
recordings were made again at the same heads with the new read: vite's
three checks pinned to GitHub Actions now read as passing, and svelte's
pinned check reads as missing because GitHub names no app for its suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Ask GitHub for each pinned check's runs by app and name (#176 review)
5ec3b0c 
Round 1 of the independent review of PR #195 found that the read of every
check run on the head, with each suite's app, misread real repositories:

- A private app's suite has `app: null` in GraphQL, so its runs never met
  the check pinned to it (sveltejs/svelte's vite-ecosystem-ci).
- A suite with more than 100 runs failed the whole inspection, though the
  pinned check was elsewhere (microsoft/vscode has one with 101).

The read now asks GitHub, per pinned check, for the suites of its app
(checkSuites filterBy appId, which also returns a private app's suite)
and, in each, the latest run of its name (checkRuns filterBy checkName,
checkType LATEST). Each check is one alias; up to 25 go in one call, and
a check whose app has more than 100 suites is paged up to 10 pages. The
read still fails closed on any overflow left in that narrowed query.

The read is skipped when the rules are unknown, so it cannot replace
that blocker. When stages stop each other, the inspection now reports
the first stage's failure, even when a stopped read then ends with a gh
exit of its own.

Tests: the fake answers the new query as GitHub does, filtering by app
and name; the exit-0 GraphQL error case is labelled defensive. New:
several checks per call, more checks than one call holds, paging to
exactly the page limit, a pinned check with the rules unknown, and a
stop-reason case with a pinned check. The vite and svelte recordings
were made again (svelte now reads as passing), and new recordings show
a re-run inside one suite (codeboost PR 151), a suite with 101 runs
(vscode PR 343099) and the private app's suite in GraphQL and REST.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Match test fakes to real gh and document the pinned-check read (#176 …
429d9c2 
…review)

Round 2 of the independent review of PR #195 found five low issues:

- The fakes in inspectPinned, the stop-reason test and requiredTestState
  now answer as gh 2.102.0 does: the protection read's 404 as a
  CommandFailed with GitHub's body on stdout, the branch read with its
  protection, and a GraphQL error with GitHub's response on stdout. Each
  notes the fields it leaves out and that the gateway does not read them.
- scripts/record-pinned-gh.ts remakes pinned-rerun, pinned-large-suite
  and private-app-suites, and recorded.ts says which script writes which
  files and what each holds. Remade today, vscode's suite now has 116
  runs (101 earlier); the test checks the new count.
- The docs and the gateway's comment say the inspection's deadline, not
  the 10-page cap, is the real limit on paging: a head with more suites
  than fit fails closed with the inspection's timeout.
- guarded-merge.md's long Gate paragraph is split, and the pinned-check
  read has its own short section. The T12 evidence names the removed test
  as removed and points to its replacement; build step 4's status lines
  are left for the refresh after the parallel PRs merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 21, 2026
Merged
Merged
mchwang and others added 6 commits October 20, 2026 20:01
@mchwang @claude
Describe the stop order runWithInput gives, and name what fakes leave…
40717e1 
… out (#176 review)

Round 3 of the independent review of PR #195 found that the code
comment, the stop-reason test and guarded-merge.md described a stopped
read that exits with its own error after the stop. runWithInput cannot
give that: once it sees the abort before gh exits, it rejects with the
abort's reason. The real case is a gh that had already exited while its
pipes closed after the stop. All three now say so, and the test's fake
models that order. It still fails without the first-failure line.

The new fakes now use gh 2.102.0's shapes: the 401 and 404 bodies with
documentation_url and runWithInput's stdout suffix, the unprotected
branch read with its protection (UNPROTECTED_BRANCH), and the abort's
reason for a stopped read. Notes name what the fakes leave out.

guarded-merge.md says the fresh inspection's deadline fits a few pages
and the 12 s display inspection about nine.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Note svelte's changed mergeable, the re-record date and the Checks-pe…
673b9d1 
…rmission gap (#176 review round 4)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Report a next page without a cursor as an incomplete answer (#176 rev…
30c3c18 
…iew round 5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Check the cursor before the page limit, so a 10th page without one is…
4f88972 
… incomplete too (#176 review round 6)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Drop the last rollup app field gh never sends, and scope the first-fa…
aa23839 
…ilure claim (#176 review round 7)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang @claude
Give the measured per-call time of the pinned-check read (#176 review…
f622c5e 
… round 8)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang
mchwang marked this pull request as ready for review October 21, 2026 04:21
@mchwang
mchwang requested a balanced review from Copilot October 21, 2026 04:21
Copilot AI reviewed Oct 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No blocking defects were identified, and regression coverage supports the matching and fail-closed behavior.

No open issues 🎉

What changed in this PR

Fixes #176 by reading app-pinned required checks from the exact PR head, enabling guarded merges when those checks pass.

Changes:

  • Adds filtered, bounded GraphQL reads while preserving fail-closed behavior.
  • Covers app matching, pagination, reruns, timeouts, and failure ordering.
  • Updates recordings, recording scripts, and implementation documentation.
File Description
test/​merge.test.ts Tests pinned-check matching and failure handling.
test/​merge-stop-reasons.test.ts Tests preservation of the first stage failure.
test/​merge-recorded.test.ts Verifies behavior against recorded GitHub responses.
test/​fixtures/​gh/​recorded.ts Clarifies recording provenance.
test/​fixtures/​gh/​private-app-suites.json Records private-app visibility differences.
test/​fixtures/​gh/​pinned-rerun.json Records latest-run filtering after a rerun.
test/​fixtures/​gh/​pinned-large-suite.json Records filtering within a large suite.
test/​fixtures/​gh/​nonadmin-ruleset.json Adds pinned-check responses for Vite.
test/​fixtures/​gh/​nonadmin-mixed.json Adds private-app check responses for Svelte.
scripts/​record-pinned-gh.ts Adds reproducible pinned-check recordings.
scripts/​record-merge-gh.ts Updates the recording scope comment.
github/​merge.ts Reads pinned checks and preserves failure ordering.
docs/​implementation/​merge-handoff.md Documents pinned-check behavior during merge jobs.
docs/​implementation/​guarded-merge.md Explains filtering, limits, and failure behavior.
docs/​designs/​codeboost-plan-indexed-review.md Updates T12 evidence for the fix.

Review effort: Balanced

@mchwang
mchwang merged commit 302eac3 into main Oct 21, 2026
3 checks passed
mchwang added a commit that referenced this pull request Oct 21, 2026
@mchwang @claude
Mark build step 4 done: #174, #176 and the step list are closed (#194 …
891e6c3 
…review round 13)

Merges main (#195, #196, #197) and updates the status lines this PR
rewrites; tests that a held cancel refusal also survives Refresh.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mchwang mchwang mentioned this pull request Oct 21, 2026
Open
mchwang added a commit that referenced this pull request Oct 23, 2026
@mchwang @claude
Show the merge job's step list and let a person cancel the merge alon…
0913c8f 
…e (D16) (#194)

* Show the merge job's step list and let a person cancel the merge alone (D16)

Merge PR's job now records the design's seven steps (Fetch, Rebase, Push,
Re-run checks, GitHub checks, Already-fixed check, Merge) with each step's
state and start and end times, in its merge_jobs record. Preparation and
the guarded merge report their steps through display-only callbacks.

The review screen shows the list under the review strip with a Cancel
merge button. Cancelling stops the job and its preparation without
cancelling the task: the task keeps its status, its pull request stays
open, and Merge PR may be clicked again. It is refused once GitHub has
been asked to merge. A restart after a cancel settles the job cancelled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 1 of the merge step list

Give the server test's fake preparation stopForMergeCancel; poll at once
when a cancel is answered with a job that ended; rewrite only the elapsed
times each second; name the pending Cancel 'Cancel requested'; test a
cancel during the guarded merge's validation, during the job's first
review load and before shutdown, and older poll records after a cancel;
document where the list says a job stopped and when Cancel shows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 2 of the merge step list

Poll quickly again after a cancel, so its outcome shows within seconds of
a backed-off poll; settle a restart after both cancels as a cancelled
task; let screen readers read each step's word and time as text and
move focus off a hidden Cancel; test a merge cancel during the push;
document every case where no step reads where the job stopped, repeat
cancels and clock skew.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 3 of the merge step list

Let a stop win over an inspection that answers after it, poll at once
after an ended cancel answer unless a poll of this generation is in
flight, escape a step's fallback word, and say that Cancel explains
itself when the Merge step's write failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 4 of the merge step list

Poll again within 2 s when a cancel is answered with an ended or newer
job while a poll is on its way; move focus off Cancel whenever the bar
or the button is hidden; write why a disabled Cancel cannot stop the
merge beside it; give the restart row its cancelled case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 5 of the merge step list

Describe Cancel by its note only while the note is shown, test that focus
moves off a hidden Cancel (to Refresh when the merge control hides too),
and keep focus on the page when a failed load hides the merge controls.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 6 of the merge step list

Test that focus on Cancel moves to Refresh when a reload hides the merge
control or fails, and stop the elapsed-time ticker when a failed load
hides the step bar.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address Copilot review of the merge step list

Keep the step list of a job that ended queued through the queue polls;
ignore a cancel answer the poll already showed, follow a newer running
job it carries and discard polls of older jobs; show a refusal only
while the cancelled job is still shown running.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Tidy the cancel-answer doc bullet and say what clears the step list (#194 review round 8)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Say that a restart settles a cancelled merge as cancelled (#194 review round 9)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 10 of the merge step list

Refuse a merge-only cancel while the task is being cancelled, keep the
pending-cancel flag per job, update the Shutdown row and the #end
comment for a cancelled merge, name Cancel merge in the architecture
overview, note the approved D16 amendment beside D16, and test the
refusal banner for an enabled Cancel.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Keep a cancel refusal in the banner until the job's record changes (#194 review round 11)

Also clear only this cancel's own pending state, check the task-cancel
state the refusal test relies on, and qualify the repeat-cancel rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Show a held cancel refusal again on each poll, and list the step-list tests (#194 review round 12)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Mark build step 4 done: #174, #176 and the step list are closed (#194 review round 13)

Merges main (#195, #196, #197) and updates the status lines this PR
rewrites; tests that a held cancel refusal also survives Refresh.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* List D16's unbuilt failure banners and Queue row as missing; qualify onAsking (#194 review round 14)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address independent review round 15 of the merge step list

Add the step list's Waiting and Running marks to DESIGN.md (approved),
an AGENTS.md rule for focus when a background update hides the focused
control, cite #199 for the rest of D16 and DT7, save the waiting checks
only when they change, and correct the onAsking comment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Cite #199 on the build step 4 checklist line, word the checks-write comment, and add Skipped to DESIGN.md (#194 review round 16)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Add the Stopped and Unknown marks to DESIGN.md, record the step list's placement, and test focus when a reload shows the job ended (#194 review round 17)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test the older-job poll guard, the poll speed on a list-step change and the list surviving the timer (#194 review round 18)

Also log the step bar's placement in DESIGN.md and name the F6b queue
test in the handoff doc's test list.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Move focus off Merge PR and its blockers button when a reload hides them, and test each way (#194 review round 19)

Reads the focused control before hiding it, since hiding blurs it at
once. Also drops the redundant first save of the waiting checks and
widens DESIGN.md's step-bar log entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Decide Merge status's visibility once, so a running job's poll keeps focus on it (#194 review round 20)

Also test the fallback to Merge PR when a reload clears the last
blocker, and word the test fixture's revision note as order only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Move focus off a hidden merge control only after the render sets its final state (#194 review round 21)

Focus went to a still-disabled Merge PR (without a runner) and fell to
the page. renderMerge now records the focused control, renders, then
picks Merge PR only when shown and enabled, otherwise Refresh. The
AGENTS.md rule now says the fallback must be shown and enabled and a
control's visibility is decided once per update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Check Cancel's focus against the control focused before the update, and test the disabled fallback (#194 review round 22)

Also word the fallback as "not disabled (aria-disabled qualifies)" in the
rule and the comment, drop the second write of Merge status's
visibility, and document every focus move in merge-handoff.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Merge renderMergeSteps' doc comments, and say Cancel is marked unavailable, not disabled (#194 review round 23)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test the display-only onAsking guard and a queued job's Merge step, and keep Cancel pending until the poll answers (#194 review round 24)

Also replace the last "disabled Cancel" and "until Refresh" wording.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Keep Cancel unavailable until a poll stops reporting the cancelled job as running (#194 review round 25)

A poll already in flight could answer the job's older running record
and clear the wait early. The in-flight test now checks Cancel after the
held poll's answer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Read the in-flight Cancel state once, test that a replayed click does not call onAsking, and note restart downtime in step times (#194 review round 26)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test the older-record poll guard, an inspection answering after shutdown, Fetch failing for review, and a joined click (#194 review round 27)

Also name the merge step bar in DESIGN.md's Frame.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test on the real preparation path that Fetch is still running when a moved head stops it (#194 review round 28)

Also list round 27's tests in the handoff doc.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test that Merge stays waiting when the attempt is not saved, and Rebase and Push are skipped when nothing moved (#194 review round 29)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test a restart while a task cancel is stopping, a cancel refused after a failed ending write, and a held refusal ending when the job moves on (#194 review round 30)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test each merge step state's mark, word and DESIGN.md color (#194 review round 31)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test that a pending cancel stays with its own merge job (#194 review round 32)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Keep a cancel's wait when a poll answers for an older merge job (#194 Copilot review)

A poll that read an older job is discarded; it no longer ends the wait of a
newer job whose cancel was answered as ended first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Bind a cancel's wait to the job shown when its answer arrives (#194 Copilot review)

An older job's cancel answered late, carrying the newer job that has just
ended, no longer replaces that newer job's wait with its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Hold the ended record a cancel's answer carried (#194 review round 35)

Cancel stays requested for every record older than the ended job the
answer carried, so a poll read before that job ended no longer offers
Cancel again. The poll no longer clears the wait.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test a Refresh answered after a cancel; name Cancel merge in the running-merge refusal (#194 review round 36)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Wait for each cancel test's answer to be shown, not a fixed time (#194 review round 37)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Wait for Cancel's render, not a response or a fixed time, in the cancel tests (#194 review round 38)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Wait for a render or the next poll in every D16 no-change check (#194 review round 39)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Add the no-change race check rule to AGENTS.md (#194 review rounds 37-39)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Prove the step timer ran; widen the no-poll window past the poll delay (#194 review round 40)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Check an unavailable Cancel sends nothing over the poll-delay window (#194 review round 41)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Poll for the recorded cancel instead of reading it at once (#194 review round 42)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Keep the latest ended record when a cancel answers late (#194 Copilot review)

An answer is newer than the record shown, not always than the one held:
across three jobs, an older job's late answer replaced a newer ended
record and let Cancel be offered for that newer job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test that a later ended record replaces the held one (#194 review round 44)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Present and keep a newer ended job a late cancel answer carries (#194 Copilot review)

When the page already showed its own job ended, an answer carrying a newer
ended job was dropped: its outcome was never shown and its record was not
kept, so a Refresh read while it ran could offer Cancel. The answer now
keeps the record and asks the poll, which follows a later job also after
the shown job ended.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Poll until a newer ended job is shown; drop an older job's queue attempt (#194 review round 46)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test the cancel-answer path alone, polling's stop, and a later job's own queue attempt (#194 review round 47)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Drop an older job's queue attempt when a full reload keeps a newer job (#194 Copilot review)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Keep observations made while a merge click is on its way over its answer (#194 Copilot review)

A late cancel answer could adopt the click's job, a cancel of it could be
accepted, and then the click's older answer replaced that record, offering
Cancel again. The click's answer is now reconciled like a full response.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Keep an ended job's view against a poll read earlier; keep a followed job on a racing click refusal (#194 review round 50)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Follow an ended job's own attempt by action ID; drop only earlier or foreign answers (#194 review round 51)

Round 50's guard dropped every queue answer for an ended job unless it ended
queued, so a job that ended submitted with its attempt in GitHub's queue never
reached Merged. A refused click now keeps any running job the page follows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Follow its own attempt only for a job that ended queued or submitted (#194 review round 52)

A blocked or failed job drops a copy of its own attempt kept while it asked
GitHub; a submitted job follows its attempt on every path; a refused click
carrying a later ended record presents it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Leave a refused click's queued or submitted end to the poll; test the guard's ended-state check (#194 review round 53)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Test that a refused click never presents an older ended record (#194 review round 54)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

App-pinned required checks always read as missing

2 participants

@mchwang