Repository navigation
Conversation
The review header shows the task's status, one run action the server would admit now (Start, Resume, Publish PR, Publish draft PR, Close pull requests), Stop for the item in progress, Retry, and Cancel task with an inline confirmation. Tasks waiting on a person get the design's banner; needs amendment offers Approve continuation and Review plan change. Review anyway and Continue have no server action yet (#206). The page polls GET /api/runner only while work is in progress, applies only the latest read or answer, skips reads while an action is in flight, and keeps an action ID for resends until a definite answer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Name the outcome when a publish or close ends, and when a replayed publish answer is already settled; name an item's end by its own attempt. - Offer Approve continuation, with its banner, for a queued or running task that has an approvable checkpoint, and not while the task is cancelling. - Ask for Refresh instead of sending when the runner reports a newer review than the page shows, and name Refresh in stale-state refusals. - Show "Stopping (not saved yet)" and "Needs restart" with its cause; stop polling for a marker only a restart clears. - Use the running mark from the merge step list and an icon in the banner. - Keep Review plan change usable while an action is in flight. - Clear a read-error note on the next read and a run-end note on Refresh. - Drop the safety finding from the needs-human banner: the runner view does not send it (#207). - Correct the docs that said the screen has no runner controls. - Tests: prove the dropped poll was handled before reading the state, cover poll and failure focus fallbacks, backoff, and each new behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Keep polling a running or queued task the server offers nothing for: a run is between steps (cleanup, the next item, the publish). - An unresolved marker hides Stop for its attempt and ends polling, also when its row stays pending or running. - Approve continuation waits for work in progress to end; Review plan change is the banner's main action, as the design gives it. - Refresh clears a note that asked for it. - Tests: count requests sent for "nothing is sent", publish messages in the publisher's formats, and tests for each fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A running or queued task the server offers nothing for may be between steps of a run, or may wait on a person in a way the view does not name (an expired budget, unapproved items, an untrusted issue). Poll it for five unchanged reads, then stop and ask for Refresh. Name an attempt's end only once its row has settled, not when an unresolved marker hides it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A running task offered Publish or Close is not "offered nothing": it is not polled, and no quiet note appears beside its button. - A cancel with no work left (held by a push whose outcome could not be read) offers Cancel task again, as the server's recovery needs, and is polled within the quiet bound instead of without end. - The quiet note has its own kind: a later read that shows work or a change clears it, Refresh clears it, and it keeps an earlier note about ended work. A lapsed cancel confirmation puts back the note it replaced. - Reword the quiet note: work can still be running unseen. - Tests read the real server with a plain fetch, not route.fetch, which can hit an earlier test's server on a reused port. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The cancel confirmation holds the note it replaced only while nothing clears that note's kind; a read or Refresh that clears it drops it too. - When a read hides Cancel task while its question is armed, the question goes with it. - No Cancel task while an unresolved marker holds a cancelled task: the server refuses a second cancel there. - The quiet note for a held cancel names Cancel task, which settles it. - Tests: the held-cancel fake carries the push marker the server reports, checks that nothing is sent once polling stops, and covers each fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Notes from background work (a read error, a run that ended, the quiet note) wait behind the cancel confirmation's question instead of replacing it, and show when it goes; the quiet note composes with an ended-work note even then. - The Plans screen's refresh clears notes that asked for Refresh, as the review Refresh does. - Word the resend hint as conditional: a changed task makes a new request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The single note with kinds and a held-back slot kept letting one note erase another (a read error erased an ended-work note; a stale-review refusal replaced the armed cancel question). The note is now three parts shown together, each set and cleared on its own: - review: the review on screen is out of date; only a review reload clears it. - status: the latest action answer, read error or quiet note; an ended attempt or a review reload replaces it, a good read clears a read error and a read showing a change clears the quiet note. - ask: the cancel confirmation's question while armed. Tests cover an ended-work note across a failed and a good poll and with the quiet note, a stale-review Start under the armed question, and a changed read after a review action clearing the quiet note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A review reload clears only runner notes written before its request was sent: every note write takes a sequence number. A Refresh whose review was read before an item ended keeps the note about that end. - Ended work is kept per kind in the review part (attempt, publish or close, cancel, stale review), so a failed poll cannot lose a publish outcome. - Only a stale review goes to the review part; a stale task state is a status note that the runner read after it makes current, and its text no longer asks for Refresh. - Tests: the held-review race, a later success after a task-state refusal, a publish outcome across a failed and a good poll; drop a quiet-note test whose read went through a reload that clears it anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A read a person or a review load asked for restarts the quiet count whatever it answers, so a failed read after the quiet note shows the error instead of the quiet note again. - Name a publish or close by its newly settled record, not only by the end of an active flag a read saw: it can start and end between two polls. - A resent cancel that a push holds says the push's outcome is being checked, not that running work is stopping. - Tests: a good poll clearing a read error under the armed question, the quiet note followed by the question in full, and each fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A publish or close key kept after a lost answer is dropped once a read shows that job settled, so the next press starts a new job instead of replaying the old outcome. A replayed answer that carries the settled record no longer repeats the outcome the review part already names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…review) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The needs-human banner names a spent time budget before the current attempt's older failure: a budget that ran out moves an idle task there without a new attempt. - Hide Cancel task while a merge is being submitted or sits in GitHub's queue, where the server refuses it; say which controls follow server flags and which the page offers from the task's state. - Tests: Close pull requests, the budget and diagnostic banner reasons, the needs-approval and approved-continuation banners, a stop not yet saved, Cancel task during a merge submission; fakes use values the server sends (action IDs, attempt IDs, a retry on a running task). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The needs-human banner names the budget as the cause only for an attempt the time limit stopped; otherwise it names the failure or the publish, and adds that the budget has run out once its deadline passed. A deadline set at start and never cleared said "ran out" for any later cause. - Offer Cancel task while the task is open again, and let the server's refusal explain a merge in progress: hiding it by the review's merge state went stale while the merge poll updated that state. The docs say so; the review's merge fake goes with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The time-limit banner test uses a spent deadline, as the server always has for a time-limit stop, so it fails if the "also run out" suffix is added there. - Test that Cancel task stays offered during a real merge submission and shows the server's refusal. - The docs say Cancel task is hidden while a cancel is already stopping its work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A merge closes or blocks the task, but no runner read followed a merge answer or a merge poll, so the header kept "In review" and offered Cancel task after a merge. renderMerge now reads the runner view when the merge attempt or job it shows changes. Tested with a real direct merge on a server that has a runner (fifteenth independent review). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A merge click reads the runner view when it settles, answered or not: a merge whose review reload failed leaves the shown merge state as it was, and the header would keep the task's old status. - Refresh, a review action, the Plans refresh and a merge click read the runner themselves, so the merge-change read is skipped during their render: one read per reload, not two. - Tests: a merge queue poll that sees the merge land, and a polled merge job that ends, each followed by a runner read (sixteenth independent review). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Test a merge whose review reload failed (mergeRefreshRequired): the header follows the task to merged although the merge state shown did not change. - Test that a Refresh showing a changed merge state reads the runner once. - Only the busy owner's endBusy clears the read-follows flag, and the comment says an earlier read can still let one harmless extra through. - The merge-job stub says which fields it leaves out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A new revision changes what the runner admits (approvals, a continuation), but the import and Apply reloads did not read the runner view, so Start stayed offered until a refused press. Both now read it, as Refresh does, and clear the runner notes their reload makes old; a revision saved whose review reload failed reads it too. Tests import a revision and apply a draft, each followed without Refresh (eighteenth independent review). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es (#200) The runner view's start, resume and publish flags follow the issue's trust, which the server learns from a loaded issue board and from a trust decision; neither read the runner view, so Start stayed offered for an untrusted issue, and did not return once a person trusted it. Both read it now. An ambiguous import or Apply outcome, whose revision may have been saved, reads it too. Tested with an issue gateway whose author is outside the repository (nineteenth independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Test the four "revision may have been saved" reads: an import or Apply whose answer was lost, and one whose review reload failed. - A review action's answer clears the notes about ended work, as Refresh does: it is the review reloaded. - The issue board reads the runner after any answer, also a lost one (the server's refresh goes on), and before the first runner read settled. - The fake issue gateway says what it leaves out. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The board's refresh is server-owned and goes on after an answer the page lost; the test runs the refresh to completion, drops the answer to the page, and checks that one read followed and Start went (twenty-first independent review). The two issue tests share one fake gateway. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A cancel requested during a pre-merge preparation that died mid-rebase leaves the task's rebase marker with no preparation running it; only recovery at the next start settles it, and the server refuses another cancel meanwhile. The page showed it as a cancel with no work left: it polled, said to press Cancel task again, and offered the button. Like an unresolved marker, it now says "Needs restart", offers no Cancel task and does not poll; a marker whose preparation runs stays work in progress (twenty-second independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A first cancel on a task that needs a restart is admitted and takes effect when the runner restarts and recovers; its answer said running work was stopping while the status said nothing could run. Stop on a pre-merge command check (an attempt of kind `check`) now says it stops the checks, not the item's run (twenty-third independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The restart-needed comment says a first cancel closes the task at once when the marker's attempt row has already settled; a test cancels under such a marker. - The two fake cancel answers say they leave out the bumped state version, which no later action reads there. - The 1280 px layout test shows the widest status text the strip can carry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
At DESIGN.md's minimum width a long task status made the strip's title and buttons wrap onto several lines and get clipped, while the width check still passed: flex absorbed the width by wrapping. The title and the status now give way with an ellipsis (the status's full text is its title; the banner and note say why), and the strip's buttons never shrink or wrap. The layout test shows the widest status and asserts that every visible child sits inside the strip's box, which fails when anything wraps (twenty-fifth independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The strip's ellipsis cut the end of the issue line, where the plan revision lives. The title and the revision are now separate spans: the title gives way (its full text is its tooltip), the revision never does. - The task banner names a restart the runner needs, so its reason is readable, not only in the status's tooltip; the stylesheet comment now holds. - The 1280 px test shows the widest labels (Publish draft PR, Confirm cancel, Requesting merge…), asserts every strip child is one line tall and the revision is whole. With the one-line rules deleted it fails on the wrapped title (twenty-sixth independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ut test (#200) - The issue row's flex split dropped the space before "· r1"; a gap restores it. - The banner's restart reason has tests: alone, and before a waiting state's text; the prefix leaves no trailing space without one. - The 1280 px test measures after the web font has loaded, gives the issue title a text that needs cutting, and asserts it is cut and the revision whole, so the title's ellipsis is proven on every run (twenty-seventh independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The tests reach the server on a connection of their own: a pooled one (Playwright's or Node's) can reach an earlier test's server on a reused port, or be closed between tests, and answer ECONNRESET. - The revision's leading space is a non-breaking space in its text, so copied and read-aloud text keep it; the gap rule goes. - The task banner's text is a live region, so a task that moves to a waiting state is announced. - The restart-needed comment names when a first cancel closes the task at once (storage, or preparation files after a saved terminal write). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The task status wraps its mark in a span hidden from assistive technology and reads the word, as the merge step list does. - The banner's text, the note and the status are rewritten only when their text changes, so a poll that changes nothing does not re-announce them; a test counts the rewrites over unchanged polls. - The docs and #206 name the third unbuilt waiting-state action, Review dependency change for needs approval. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The re-announce test shows a banner, a note and a status before it counts rewrites over unchanged polls; the note's guard is proven by the held review reload, the one path that re-renders a note as it is. - The status and the issue title carry a tooltip only while they are cut, so a screen reader does not get their text twice when nothing is cut. - A read that is not a poll clears the pending poll timer, so it cannot fire into a newer generation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The issue title's and the status's tooltips were measured before the runner's controls were placed, while the review view was hidden, and never again after a window resize, so a cut title could lack its tooltip and an uncut one keep it. One measurement now runs after every render has placed the strip and whenever either element's size changes, never while the view is hidden; the tooltip is set or absent, never empty. A test drives it through the real review answer: cut on first load, following a text change while cut, gone when the window widens, back after a Refresh made on the Plans screen (thirty-first independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A status at its 40% maximum, or a title cut beside one, keeps its width through a text change, so no size observer fires; the measurement after each render is what updates the tooltip then. The comment says so, and two tests pin it: a summary change beside a capped status, and a reason change beside a short title, each with the widths proven unchanged (thirty-second independent review). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
An approved continuation never stays in needs amendment (its approval moves the task to queued or running in the same transaction), so the banner text for that pair and the fixture that faked it go. A double type assertion in a test becomes an annotation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
Review recordEvery round: an independent review in a fresh agent (no author context) of the full diff against
Clean pass: round 34, base Declined, with reasons: the needs-human banner cannot show a safety finding (the runner view does not carry it; #207); a hidden Start cannot name its reason (the view carries no refusal; #206's states are the ones that need it); the status marks reuse DESIGN.md's existing meanings rather than adding one (the owner's call to record). Regression evidence: each fix's test was run against the code with the fix removed (a mutation) and failed, before the commit that carries it. Where a first mutation check turned out not to apply (rounds 25 and 26), the reviewer caught it and the check was redone with the removal proven. |
There was a problem hiding this comment.
🟡 Changes recommended
Refresh coordination, missed completion notices, and banner overflow can leave the review screen stale or its recovery controls inaccessible.
4 open issues
What changed in this PR
Adds review-screen controls for existing runner actions, keeping server guards authoritative and leaving the deferred waiting-state actions in #206 out of scope.
Changes:
- Adds task status, runner controls, waiting-state banners, and inline cancellation confirmation.
- Adds guarded action requests, bounded polling, and notices when the displayed review needs refreshing.
- Adds browser regression coverage and updates runner documentation.
| File | Description |
|---|---|
web/public/style.css |
Styles runner banners and keeps the review strip on one line. |
web/public/index.html |
Adds runner controls, status, banners, and notices. |
web/public/app.js |
Implements runner actions, polling, refresh coordination, and focus handling. |
test/browser/runner-controls.spec.ts |
Adds browser coverage for controls, failures, races, and layout. |
README.md |
Documents starting and resuming tasks from the review screen. |
docs/designs/codeboost-plan-indexed-review.md |
Updates delivery status and identifies deferred actions. |
docs/architecture.md |
Documents runner controls and remaining workflow gaps. |
Review effort: Balanced
- A trust change whose answer is lost reads the runner view again: the decision may have committed, and it decides what the runner admits. - The note about ended work names the latest attempt whose row settled since the earlier view, seen live or not: an attempt can start and end between two reads. The view lists the newest attempts oldest first, so the last match. - A runner read asked for while an action is in flight is owed and done once the action settles: the action's answer may be older than what asked for the read (a plan revision, a trust change), and would stick. - The banner's text shrinks and wraps within a long undeclared path, so Approve continuation and Review plan change stay within 1280 px. - The summary-change tooltip test keeps its claim on every platform: it checks the widths to within a pixel (Linux settles the status a quarter of a pixel under its cap), and its companion pins the per-render measurement with an exactly capped status. Each fix has a test that fails without it; the owed read's tests pause the page's clock so no poll can account for the read. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- Two attempts settle between two reads in the test, and the note names the later one: a `find` in place of `findLast` now fails it. - The owed read's two tests watch a one-second window with the page's clock paused after the read, so a read paid twice fails them. - The banner text's rules sit in one block. (Thirty-fifth independent review.) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #200.
What this adds
The review screen gets controls for the runner actions. Nothing changes on the server: every control sends an action
POST /api/runneralready accepts, and the screen offers only whatGET /api/runnersays the server would admit.In the review header
For a task that waits on a person, the full-width banner from the design's "Task states that wait on you": needs amendment (with Approve continuation and Review plan change), needs human, possibly already fixed, needs approval, and a restart the runner needs, named first. "Review anyway", "Continue" and "Review dependency change" have no server action yet: #206. The safety finding for needs human is not in the runner view: #207.
A note under the banner, in three parts shown together so none replaces another: what changed since the review on screen loaded (an item that ended, a publish or close that ended, a cancel that settled, a stale-review refusal; cleared by a review reload sent after it was written), the latest answer, refusal, read error or quiet note, and the cancel question while armed.
Reads and polling
GET /api/runneris polled only while work is in progress (2 s, backing off to 30 s), and a task with no named work (between steps, or a cancel with no work left) only for five unchanged reads, after which the note asks for Refresh.The review strip stays one line at the 1280 px minimum: the issue title and the status give way with an ellipsis and carry their full text as a tooltip only while cut; the plan revision and the buttons never shrink.
Decisions (product owner, 2026-10-23)
Screen only; existing actions only; status + one run action + Cancel task in the header, waiting states in the banner; poll only while work is in progress; inline cancel confirmation. The status marks reuse DESIGN.md's existing meanings; recording that in DESIGN.md's decisions log is the owner's call.
Tests
test/browser/runner-controls.spec.ts: 76 tests against a real server with a runner whose preparation fails or is held (astest/runner-start.test.tssets one up), a merge gateway, an issue gateway and a planning provider where a case needs them. States this runner cannot reach are served as the real server's answer with named fields changed, each saying what it leaves out. Every race the change handles has a test that proves the interleaving happened (held answers, dropped polls, lost answers, a reload read before an item ended), and every behaviour named in a comment has a test that fails without it (checked by mutation).Validation (each gate on the exact head e95f4a3, reported separately)
test/browser/runner-controls.spec.ts, 76 passed, run in full after every change.tsc --noEmitclean.vitest run --project parallel): 86 files, 2629 passed, 1 skipped (a skip the suite has on main too). The Docker-backed files were left to CI'sreal-dockergate, which runs them on this head; the branch changes onlyweb/public/*, this spec and docs.real-dockerpassed andtestfailed one browser test on Linux (a layout-rounding precondition in a new test, not the behaviour it tests); fixed in f0e92bf, with the four Copilot findings from the same head. Read back on the new head.Review record
Independent review in a fresh agent on the full diff, each round on the head named, until a pass found nothing: see the first comment for every round's base/head, findings and what changed; the clean pass that authorises the Copilot request is named there too.
🤖 Generated with Claude Code