Repository navigation
Hold the site menu's "Update to latest trunk" while an update, install or build runs, and say why - #611
Merged
Conversation
… and say why The site menu's item was always enabled, and startTrunkUpdate returns early while an update, an install or a build is running, so choosing it then did nothing and said nothing. The reason is decided in site-menu.cjs (updateHeldReason), asked by the update's own guard and shown on the item as its description, so the guard and the menu cannot disagree. The stale notice's button, which was disabled with no sentence, is held with the same reason. A terminal held by anything else is still refused in beginTrunkUpdate with a line in the terminal. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zaerl
added a commit
that referenced
this pull request
Oct 4, 2026
Brings in the held "Update to latest trunk" menu item (#611). The update hook's call in the site view takes both changes: `updateHeld` from there, and the two functions that bring the terminal up from here. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
zaerl
added a commit
that referenced
this pull request
Oct 4, 2026
…to it (#612) ## Why First part of #558. In the redesign the terminal, the logs and the mail leave the foot of each site's page for a tray along the bottom of the window, as the prototype has them. This pull request builds the tray and moves the terminal into it; the logs and the mail follow in their own pull requests and stay on the page until then. ## What changes **The tray.** Between the page and the footer, closed when the window opens. The footer has a button for it, "Toggle Terminal", pressed while the tray is open; the tray's own Close button puts the focus back on that button. Its top edge is dragged with a pointer or moved with the arrow keys (Home is its smallest, End its largest), from 200px up to half the window. It takes its room from the page, so nothing on the page is under it, and a confirmation in the corner stands clear of it. **The terminal.** The tray is the window's and what is in it is a site's: every site's view fills the tray's slot with its own terminal, hidden unless the site is the open one. A terminal keeps what was printed in it through a close and a look at another site. It is fitted to the tray, as many columns and rows as there is room for, where it was a fixed 80 by 12. **When the tray opens by itself.** On the page the terminal was always in view, and the app leaned on that. So the tray opens on the terminal in the cases whose only word is there: - an install or a build that fails with its output in the terminal (setup, an update, an applied patch); - an action refused because a command is running, which is a line printed there; - an update that cannot fetch, and an apply whose hand-off to the build watch fails. It does not open for a run the contributor stopped, for a failure whose output is in the logs (the build that runs ahead of the watch), or on another site than the one that failed; that site's terminal comes up when the site is opened. The copy that said "the Terminal below" now says "the Terminal". **Not in this pull request:** - The logs and the mail in the tray. - Remembering the tray's height or whether it was open across restarts. The prototype remembers the height; the window's other arrangements (the sites list, the details) are not remembered either. - A slide animation as the tray opens. - The user guide, which still describes a terminal panel on the page: #561 retakes the screenshots and the text. ## How to test this Platforms: any. The journeys below drive every part of this on macOS and Windows. From the repository root, after `npm ci` and `npm run build:once`: ``` npx playwright test --project=journeys tray.spec terminal.spec ``` `tests/e2e/journeys/tray.spec.js` is new, nine journeys: - closed until the footer opens it; shows the open site's own terminal; each terminal survives a close and a look at another site; the page and a confirmation are not under it; - the edge moves by keyboard and by pointer, stays inside its limits, a drag that loses the pointer is over, and the terminal is fitted (rows and columns) at every size; - a failed build, an update that cannot fetch and a refusal each bring the terminal up; a stopped build, a failure whose output is in the logs, a failure on another site and a site deleted mid-build do not. I removed or broke 26 behaviours one at a time; 25 turned a journey red, and the one that did not was a redundant clamp, which is gone. `tests/unit/tray.test.cjs` and `tests/unit/terminal-hints.test.cjs` hold the decisions (limits, keys, the terminal's grid, which run's end shows the terminal). **Worth a look by hand** (any platform, the current head): open a site, press the icon at the left of the footer, drag the tray's top edge, type `help`, switch site and back, close it. Then choose **Update to latest trunk** on a site with no remote, or let a build fail: the tray opens on the reason. **What must not have happened:** a site's terminal losing its output when the tray is closed or another site is opened; the tray opening for something the contributor did on purpose (a stop); anything on the page ending up under the tray. **What cannot be tested by hand here:** "Stop setup" during a real first setup, which needs a clone and an install that take minutes. The stop is decided in one place for every run, and a journey covers it through Ctrl+C. ## Risks and limitations - Review: 2 passes, 13 findings fixed here, 4 left as follow-ups, 1 design question. Details below. - **The footer's button is an icon with a tooltip, as in the prototype, and the copy now says "the Terminal" without saying where it is.** A failure opens the tray, so nobody is left looking for the output of one. The sentences that are not failures ("Run npm run build in the Terminal", "the Terminal shows what it is doing") point at a tray the contributor has to find. The second reviewer suggested a visible "Terminal" label beside the icon; I have left that as a design decision. - **A build's output is no longer always in view.** During a long setup or update the page shows the steps and the terminal stays closed unless it is opened. - Erasing typed input breaks once the line wraps. It did before; a terminal that follows the window's width makes it easier to reach in a narrow window. - The hint sentences under the terminal are still not translatable, as before. ## Related Part of #558. Mail HTML rendering is unchanged and tracked separately. --- <details> <summary>Design decisions and alternatives considered</summary> - **In the page's flow, not laid over it.** The prototype lays the tray over the foot of the page and pads the page by its height. Here it is a row of the window's column, so the page is simply shorter: nothing to pad, and the page's own scrolling and its sticky details need no knowledge of the tray. - **Never unmounted.** A terminal is drawn in one element for as long as it lives, and the prototype's tray, which mounts only what is selected, would cost every other site its terminal. The tray and every site's panel stay in the document, hidden. - **The terminal is made when the site's view mounts and drawn when first shown.** What a site prints before then is kept in the terminal's buffer. Its element arrives through the tray's slot a render later, so it is state and not a ref. - **One reveal function per site**, which waits if the site is not the open one, rather than opening the tray on whatever site is in front. - **A run's caller says where its output goes** (`outputInTerminal`), and the runner knows whether it was asked to stop. Opening the tray only where the page points at the terminal was considered and is the same thing with more places to keep in step. - **A splitter, not a button**, for the edge: it can be focused, moved with the arrow keys, and says how tall the tray is. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **Pass 1: 8 [fix here] · 3 [follow-up]. Pass 2 (on the fix-up): 3 [fix here] · 3 [follow-up]. 13 fixed (7 and 6), 4 left as follow-ups (the guide, deferred to #561, and pass 1's three), and 1 design question.** - **Review:** completed — a separate agent context, read-only; head `480ee0e` / base `bda9083`; lint and the tray's unit tests run, the journeys read and not run; 8 [fix here], 3 [follow-up]. - Fixed: the app said "the Terminal below" with the tray closed, and refusals were printed only there (the tray now opens for a failure or a refusal; decided with the maintainer); the tray's floor left the terminal no rows; no test checked the terminal's columns; one layout check read positions once; Home and End were the reverse of a splitter's, and the edge did not name its pane; a drag could get stuck; confirmations covered the open tray. - Deferred to #561: the user guide. - Follow-ups: wrapped input, the untranslated hints, a terminal that cannot move to a new element (no path does). - **Review:** completed — a second separate agent context, read-only; head `4e0cdff` / base `bda9083`; every caller of the two runners and of the kill traced; 3 [fix here], 3 [follow-up]. - Fixed: "Stop setup" opened the tray on a stop that was asked for; an update that fails before it runs anything said so only in the terminal; a site deleted mid-build opened the tray on the next site. - Fixed though marked follow-up: the journey measured a column off an element xterm keeps for itself; an assertion at the smallest height that could not fail; the decision moved into `terminal-hints.cjs` with a unit test, and the watcher says its output is not in the terminal. - Left as a design decision: a visible label on the footer's button. - **Since review:** `4e0cdff → d89b1a8` is those fixes, and `edd378a` merges `redesign/wpds` (#611), with one conflicting line in the site view resolved to keep both changes; lint, `npm test`, `npm run test:electron`, all 108 journeys, the small-window and slowed runs of the tray's journeys and the packaged smoke re-run on it. Then `f595b31`, after both journey jobs failed on `edd378a` in the new edge journey, for reasons of the journey's own: on Windows it measured the terminal before the site's status had brought the two hints under it, and on macOS its own measure of a column came out a hair wider than the terminal's on the runner's font. The journey now waits for the hints and counts columns from the room left over; no app code changed. </details> <details> <summary>Implementation notes</summary> - New: `src/renderer/tray.cjs`, `src/renderer/components/bottom-tray.jsx`, `tests/unit/tray.test.cjs`, `tests/e2e/journeys/tray.spec.js`. `runFailedInTerminal` is in `src/renderer/terminal-hints.cjs`. - The tray's height reaches the stylesheet as a property the component sets from an effect (`--app-tray-height` on the tray, `--app-tray-inset` on the document for the confirmations), since the lint rules forbid an inline `style`. - A dozen journeys now open the Terminal tray first; `ui.openTray`, `ui.tray` and `ui.trayToggle` are in `tests/e2e/helpers/ui.cjs`. - Local checks on macOS: `npm run lint` clean, `npm test` and `npm run test:electron` pass (1930 tests), `npm run test:e2e` 108 pass, `npm run test:e2e:packaged` 15 pass. </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The steps under "Worth a look by hand" show everything that changed. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
zaerl
added a commit
that referenced
this pull request
Oct 5, 2026
## Why #542 replaces the app's interface with the one in the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype), drawn with the WordPress Design System. It was built in slices on `redesign/wpds`, so that trunk, which releases are cut from, never held a half-redesigned window. The window is whole now: the shell, the page header and details, every card and dialog, and the tray. This pull request is how the branch reaches trunk. It lands before the last three sub-issues, on purpose. The settings dialog (#559) and dark mode (#560) add to a finished window and do not need a branch to hide in, and the guide (#561) is easier to rewrite against what is on trunk. They follow as pull requests against trunk. ## What changes Everything `redesign/wpds` holds; this pull request adds nothing of its own. Each slice was reviewed in its own pull request against that branch, and says there how to test it and what it leaves out. - **The shell** (#555): a sites list, the page area and a footer, in a larger window. #590 - **The page header and the details** (#556): the site's menu in the header, and what the checkout is in a sidebar that can be put away. #594 - **The site's page** (#557): the server and the build watch in the header and the details (#595); the ticket card, the apply card with a preview dialog, and the list of a site's tickets (#596, #597, #599); the create-site, rename and delete dialogs (#600, #601); confirmations as notices in the window's corner (#602); the page's notices, update card and setup checklist (#605); the terminal, logs, mail and the review dialogs painted with the design system's tokens (#606, #608); and lint rules that keep it so (#609, #610). - **The tray** (#558): along the bottom of the window, holding the terminal, the logs or the mail, with a count of unseen debug.log lines on the footer (#612, #613, #614, #615). - Along the way: #611 (the site menu's update item says why it is held), and three that tidy up before landing: #616 (a journey's race with the page's clock), #617 (comments the finished redesign made untrue) and #618 (the server's button by its present name in `AGENTS.md`). **What a contributor will notice first:** the terminal, the logs and the mail are no longer on the page but behind three buttons at the foot of the window; the terminal is dark text on light; the server and the build watch are started from the header; a patch is previewed in a dialog before it is applied; and what worked is said in the window's corner. **Not in this pull request:** - The settings dialog (#559). Until it, nothing about how the window is arranged is remembered across restarts: the details, the tray's height, the apply card's fold. - Dark mode (#560). The terminal reads its colours once, when it is made, and will need to follow the theme. - The user guide and its screenshots (#561), which describe the old interface. They are published from release tags, so they are not wrong until a release ships this; #561 is next, ahead of #559 and #560, for that reason. - How a mail is drawn, and how the window handles a link. Both are unchanged and tracked separately. ## How to test this Platforms: macOS and Windows, both. Buildkite builds signed artifacts for this pull request; use the build for its current head, and check that it is: a push to the branch makes earlier builds stale. The suite, from the repository root, after `npm ci`: ``` npm run lint npm test npm run test:e2e ``` 113 journeys walk every slice on macOS and Windows on each push. What they do not do is run a real site: a clone of `wordpress-develop`, an install, a build and a server are stood in for. The one test that does, `real-setup.yml`, is run by hand and was run on this branch at `548c020` (https://github.com/WordPress/contributor-toolkit/actions/runs/37298667056): a Core site on macOS and on Windows and a Gutenberg site on macOS were created, installed, built and served, and passed. **The Gutenberg site on Windows did not finish.** Its first attempt was cancelled after 74 minutes inside the journey, a minute short of the limit that would have failed it with evidence, so it says nothing about where it was; a second attempt was still running when this was written and its result is not in this description. The commits since `548c020` change a journey, comments and prose. **A pass by hand, on a real machine** (any platform; Apple Silicon for the server, which no test on a pull request starts for real). It follows the order a contributor meets things in: 1. Open the app with no sites. **Create new site**, a Core site, in a folder with a space in its path. The setup checklist runs its steps; the terminal's button at the foot of the window shows what each is doing. 2. Let the checklist's last step start the server, and finish WordPress's own setup in the browser. Back in the app the header says "Server running" and "Build watching". Stop the server and start it again: from **Server running** in the header, or with **Stop** and **Start** beside "Server" in the details. 3. Press the three buttons at the left of the footer in turn. Terminal: type `help`. Logs: the server's lines, the build watch's, and Debug.log. Email: where SMTP is listening. Drag the tray's top edge. 4. Link a ticket by its number. The card shows the ticket and its pull requests and patches. 5. Apply one of them. The preview dialog names the files; **Apply and rebuild** runs the steps on the card. 6. Edit a file under `src/`, then **Review & submit changes**. The diff is the edit and nothing else. 7. Create a second site, or open one from before, and move between the two from the sites list. Each has its own terminal, logs and mail. 8. **Update to latest trunk** from the site's menu. **What must not have happened:** work lost in a link, an apply, a switch or an update that did not say so; `node_modules` rebuilt where nothing changed; a failure said only in a tray that stayed closed; one site's output in another's tray; anything on the page under the tray. **What cannot be tested by hand, and what was not:** none of the slices' manual steps were run by a person. I looked at screenshots and drove the app through the journeys; everything local was on macOS, with Windows left to CI. This pass is the first one by hand. ## Risks and limitations - **It is one commit on trunk**, since the repository allows only squash merges. The slices' pull requests are its record. `redesign/wpds` should not be branched from after this: it will have diverged from trunk. - **Reviewed in slices, not as a whole.** Twenty-two pull requests, all but a two-word change to `AGENTS.md` reviewed by a separate reviewer context before they were opened, most in two passes; see below. The branch was then swept once for what falls between slices, which found comments and one unread field (#617) and the three points that follow. - **Three questions and two failures still use the system's own dialogs**: the discards ask through its confirm box, and a path that could not be copied and a server started before there is a build are said in its alert. #557 meant to replace them and did so for deletions only. - **Some dialogs are repainted, not rebuilt.** The review dialog and its destinations, the mail dialog and the question an update asks about loose edits have the design system's colours and type on the old component library's parts. - **A real Gutenberg setup on Windows is not shown to pass on this branch.** See "How to test this". It passed on trunk before the redesign, in 35 minutes; here it was past 46 on its second attempt. Whether that is the runner or the branch is not known. - **The guide describes the old interface** until #561. Do not cut a release before it. - **The bundle roughly doubled** (#590): the sites list brings `DataViews` and what it needs. Launch time was not measured. - **Less is always in view.** A build's output, a new line in debug.log and a caught mail are in a tray that may be closed. A failure opens the terminal, or the logs if nothing else is open; unseen debug.log lines are counted on the footer; a mail that arrives shows nowhere until its tray is opened, which was decided and is not an oversight. - **Translation is about half done.** The shell, the cards, the tray and most dialogs go through the translator. Roughly 200 strings in components and hooks do not yet (the review dialog and its destinations, the setup checklist, the logs, most of what the hooks say), nor about 150 in the modules they take sentences from. Strings that are new are in English in the shipped translations until they are translated. - **Focus is lost in a few places**: when the app swaps what the tray holds from under it, when a notice with a focused button goes, when a row's own action removes the row. - **Some failures are still worded by the main process** ("Error invoking remote method…"): a failed rename, a failed setup. - **Things carried over, not made by this:** a start that throws leaves the server's button dead until a restart; erasing typed input in the terminal breaks once the line wraps; a line written to debug.log while it is being attached to can be shown twice. - **#571 and #603 will conflict with trunk after this**, in the mail's journey and helpers and in the dev server's hook. - Each slice's own list has more, and says which of its limits were chosen. ## Related Closes #557. Closes #558. Part of #542, which stays open for #559, #560 and #561. #555 and #556 are closed already. --- <details> <summary>Design decisions and alternatives considered</summary> - **An integration branch, and one landing.** A window drawn half by the old view and half by the new one could not be released, and the slices were too many to hold back one by one behind a flag. Fixes and changes with no visible effect went to trunk first and were merged across, so this diff is the redesign and little else. - **Landing before #559 to #561.** The plan was to land when all seven sub-issues were in the branch. The remaining three do not change what is there: two add to it and one documents it. Holding a finished window on a branch for them only lengthens the time trunk and the branch can drift apart. - **What the prototype lacks is kept and redrawn**, and what it has somewhere else moves to where it puts it. Its labels are adopted. Where a slice departs from it, the slice says so. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **No review of the whole diff was run. The record is the slices': twenty-two pull requests into `redesign/wpds`, each but one reviewed before it was opened by a separate, read-only reviewer context following `.github/instructions/code-review.instructions.md`, most in two passes (the change, then its fix-up), with the counts and the findings in each pull request's own review outcome. The one is #618, two words in a line of prose.** - **Review:** completed, per slice — #590, #594 to #602, #605, #606, #608 to #617 (no review was run for #618); everything marked to fix in a slice was fixed in that slice, and what was left is in its Risks. - **Review:** completed, for leftovers only — a separate agent context, read-only; `redesign/wpds` at `548c020` against trunk; not a review of correctness, which the slices had: a search for dead CSS and code, comments that waited for a slice that has landed, what is left of the old component library and of the system's dialogs, strings not yet translatable, and repository docs that name the branch. Nothing found in the stylesheet, the dependencies or the docs; thirteen comments and one unread field, fixed in #617; the rest is in Risks above. - **CodeRabbit:** not run. Its check reads "Review skipped: automatic reviews are disabled" on this pull request and on the slices, so the slices' pre-PR reviews are the reviews of record, each for the head it names. - **Since review:** nothing was added to the branch outside a reviewed slice, apart from three merges of trunk (`da19ac6`, `3344172`, `23d46e7`). </details> <details> <summary>Implementation notes</summary> - `git diff origin/trunk...origin/redesign/wpds`: 148 files, about 14,800 lines added and 3,000 removed; about 6,300 of the added lines are tests. - The site's view, which was one component of five thousand lines, is spread over the four regions the layout has: the header, the cards, the details and the tray. Decisions are in `src/renderer/*.cjs` with unit tests; components draw them. - New for the suite: journeys per region under `tests/e2e/journeys/`, lint rules of the repository's own (`eslint-rules/`), and Stylelint over the window's stylesheet. - The main process changed in one place for the redesign's sake: the debug.log tail says which lines the file already held (#614). </details> <details> <summary>Screenshots or recording</summary> Not attached: I have no way to upload images from the command line. The prototype shows the design; the signed build shows the app. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
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.
Why
The site menu's Update to latest trunk item was always enabled, but
startTrunkUpdatereturns at once while an update, an install or a build is running. Choosing the item then did nothing and said nothing: no line in the terminal, no notice, no change on the page. The review standard names this shape: a control that swallows a click is a control that does nothing.What changes
The root cause is that the guard and the control did not share an answer: the hook knew the update could not start, and the menu was never told.
updateHeldReasoninsrc/renderer/site-menu.cjsis now the one answer to "can an update start":''when it can, otherwise a sentence naming what is in the way ("Wait for the trunk update to finish.", "Wait for the installation to finish.", "Wait for the build to finish."). The update leads, since its own install and build are running while it is.useTrunkUpdateasks it for its own guard and returns it asupdateHeld, so the guard and the controls cannot disagree.siteMenuItemstakes it and returns the item withdisabledand adescription; the menu draws the description under the item's name withMenu.ItemDescription, which the design system wires as the item'saria-describedby. A held item is still reachable with the keyboard.ReasonedUiButtonheld with the same reason, so there is no second inline answer.docs/guide/trunk-updates.mddescribed the old behaviour ("clicking it in that state simply does nothing") and now describes this one.Deliberately not in it: the case where the terminal is held by something that is not an install or a build (a typed
npm run lint, say). The item stays enabled there and the update is refused inbeginTrunkUpdatewith a line in the terminal, as before.How to test this
Platforms: any. Nothing here touches paths, spawning or line endings.
Covered by a journey.
a site with no build is built before it is watched…intests/e2e/journeys/build-watch.spec.jswalks the reported path: a Gutenberg site with no build, Start build watch so its build is running, then the Site actions menu. It asserts the item is disabled, that its accessible description and the text on it are the reason for a build, and that after the build ends the item is enabled with no description. Run it from the repository root, afternpm ciandnpm run build:once:It is red without the change: with the menu not told the update is held, it fails on
toBeDisabled("unexpected value "enabled""). Checked by removingupdateHeldfrom the menu's props, rebuilding and running that one test.What the journey cannot reach is a real install or build, since it stands in for the script runner. On a real site, with the build of this branch's current head (Buildkite, either platform):
Starting state: a WordPress Core site whose setup has finished, with nothing running.
npm run buildand press Enter.What must not have happened:
npm run lint), the item is still enabled, and choosing it on a clean tree prints "A command is already running. Press Ctrl+C to stop it." in the terminal, as before.Not tested by hand: the stale notice's button with its tooltip needs a site whose trunk is more than 14 days old and an install or build running on it; its reason comes from the same function and the same component the ticket actions use, and no journey reaches that pair of states. I ran the journeys and not a real
npm run build.Risks and limitations
[fix here]· 3[follow-up], one of them folded in. Details in the collapsed block.redesign/wpdsyet. The brief for this change described the terminal-held refusal as opening the tray on the Terminal (Tray shell, then terminal, logs, and email #558). On this base,beginTrunkUpdateonly prints the line;app-footer.jsxstill says "once there are trays". This PR leaves that path exactly as it found it, so it composes with the tray work whichever lands first.ticket-actions.cjsandwatch-activity.cjs. Here they are wrapped, because the menu is a translated area. Under a translated locale the same sentence will be translated on the menu and English on a ticket action until that area is wrapped.Related
No issue. The always-enabled item came with the site menu (#556).
Design decisions and alternatives considered
ReasonedUiButtonpairs a tooltip with a hidden description because a button has nowhere else to put a sentence. A menu item does:Menu.ItemDescriptionis the design system's own slot for it, it is visible without hovering, and it is wired toaria-describedbyby the menu itself.site-menu.cjs, beside the items it is said on, and that module is already translated.update-plan.cjswas the other candidate; it is not translated and is not where a sentence on a control belongs.Review outcome (required — see AGENTS.md)
0
[fix here]· 3[follow-up]— one folded in, two left as follow-ups.[follow-up]— the stale notice's button answered "can an update start" inline (disabled={installing || building}) and gave no reason. Folded in: it is held withupdateHeldthroughReasonedUiButton.[follow-up]— the same sentences are unwrapped inticket-actions.cjsandwatch-activity.cjs. Deferred: those modules are not translated yet; wrapping them belongs to that area's i18n pass. Named in Risks.[follow-up]— discard-then-refused with the terminal held by a typed command (see Risks). Deferred: predates this change, outside its scope, and needs its own reproduction.Checked and found sound by the reviewer: the guard and the menu read one value from one render; no new stuck state under the #507 rule (while the update waits on the watch the item was already refused by the guard, and Stop build watch still ends the wait);
Menu.ItemDescriptionis used within its contract and the item's name is still its label, so the exact-name locator matches; the unit tests and the journey'stoBeDisabledfail on the old code; every other journey andscripts/screenshots/shots.cjsopen the menu while idle.Deterministic layer, on the committed head:
npm run lintclean,npm test1923 pass, 0 fail, 2 skipped,npm run test:e2e99 passed (macOS only; Windows is CI's).bda9083before it was committed; outcome above.771c88echecked by the author: the notice's button moved toReasonedUiButton(finding 1), two comments reworded to say "the two controls", and the docs sentence tightened as the reviewer's style note asked. Lint, the fast suite and the journeys were rerun on771c88e. That delta was not re-reviewed by a separate context.Screenshots or recording
The held item, while a build runs (macOS, from the journey): the item greyed out, with "Wait for the build to finish." under "Update to latest trunk", and the other five items unchanged. I have the capture locally and could not attach it from the command line; the journey above reproduces it.
🤖 Generated with Claude Code