Repository navigation
Conversation
juanmaguitar
added a commit
that referenced
this pull request
Aug 7, 2026
- git:apply-patch rejects a sitePath that is not a registered site before writing anything, the same gate sites:set-ticket uses (#8). - Persisting the revert record is now part of the apply transaction: if the store write fails the apply is undone, and if that also fails the result says the patch is applied-but-untracked rather than reporting a clean failure (#7). - git:preview-patch surfaces a worktree-inspection failure instead of reporting "no collisions" when it could not look (#9). - git:discard-changes clears the applied-patch record with the reset, so a later trunk-update network failure cannot leave a revert banner for a patch that is already gone (#6). - git:apply-patch is now a WIRED handler with real tests (guard, reverse lookup, delegation, done event, metadata) via fakeSettingsStore, instead of a NOT_REACHABLE hole (#13). Not gating git:preview-patch: it is read-only, and the registered-site check would add a store dependency to a handler that has none. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
juanmaguitar
added a commit
that referenced
this pull request
Aug 7, 2026
The #7 transactional branch — undo the apply when the revert record cannot be saved, and report applied-but-untracked when that undo also fails — had no coverage. Drive store.set to throw and assert both outcomes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
|
Closing as superseded. This branch predates the current architecture by ~100 commits — it targets the old single-project layout and the pre-modularisation |
zaerl
added a commit
that referenced
this pull request
Oct 2, 2026
…dialog (#597) ## Why #557 rebuilds what is inside the site's page, as the [prototype](https://github.com/jameskoster/wordpress-contributor-toolkit-prototype) draws it. This is item 4 of its nine, apply patch and PR checkout: the card a pull request or a patch file is brought into the checkout from, the preview of what one would change, and the banner of a pull request that is checked out. It was about 250 lines of inline markup in the site's view, in the old look. **This PR targets `redesign/wpds`, not `trunk`.** ## What changes - **The apply card is drawn with the design system**, as the prototype has it: a card that can be folded, with a tab for each way in on a project that takes patch files, "Pull request" and "Diff", and the pull request's field alone on one that does not. - **What a patch would change is shown in a dialog** before it is applied, where it was a block in the card: its files, whose work it would land on, anything that changes what applying it means, and the button that applies it. - **The applied patch, a failure with its breakdown, and the checked-out pull request are the design system's notices.** The ones that were alerts or status regions still are. - **It is a component of its own**, `apply-card.jsx`, with what it decides in `apply-card.cjs` and unit tests beside it. Its own strings are translatable. The dialog is in front of the page, and that changed four things about the flow: - **A failed apply drops its preview.** It used to stay on the card beside the error. A dialog would come back in front of the error, so the error is read on the card and the patch is chosen again to retry. - **The preview steps aside while the app asks what should become of loose edits on trunk**, and is kept for the answer that goes on with the checkout. Saying "Cancel" to that question drops it. - **A preview is shown by its own site.** Every site's view is in the window at once, and one read while the contributor had gone to another site would have opened over that one. It waits for its site. - **The dialog's button is held, with the reason, while a command runs in the terminal.** An apply is refused then with a line in the terminal, which the dialog is in front of. Also: - **The card stays open while it has something to say** (the steps of an apply, a patch that is applied, a failure) whatever was chosen, and goes back to that choice afterwards. It remembers which tab it was left on. - **The screenshot harness waits for the ticket's number**, where it waited for a heading that #596 removed. That was missed there. **Deliberately not in this PR:** - translating what the card passes through from older modules: what a pull request's preview and banner say, what an applied patch says, why a patch did not fit, and the steps of an apply; - the list of a site's other tickets under the card (item 5), and the note about uncommitted changes (item 9). ## How to test this Platforms: any. Use the Buildkite build for the current head of this branch. The journeys carry this. From the repository root, `npm run test:e2e` runs them all; `tests/e2e/journeys/apply-card.spec.js` is new: - a patch that was chosen is shown in a dialog named for it, with what it changes, and nothing is written; leaving the dialog by its button or by Escape applies nothing and gives the card back on the tab it was left on; applied from the dialog, the patch is on disk, and focus goes to the card's header; - a patch that does not fit says before anything is written whose work it would land on; refused, its preview goes, the folded card opens to say what went wrong, the ticket's rows can be read again, and once the failure is dismissed the card is folded again; - a preview read for one site is not shown over another, and is there when its own site is opened again; - a patch cannot be applied while a command runs in the terminal, and the dialog's button says why; the command over, the same dialog can apply; - saying "Cancel" to the question about loose edits leaves the checkout and the edit as they were and does not bring the preview back. `i18n.spec.js` gains the card and a patch file's preview in the pseudo-locale. `patch-apply`, `pr-checkout`, `watch-during-changes`, `review-changes`, `store-persistence`, `gutenberg-site` and `terminal` walk the same flows as before, now through the tabs and the dialog. What they cannot reach is how it looks, a real pull request from GitHub, and an install that takes minutes: **Starting state:** a Core site that has finished setup, with a ticket linked. 1. In "Apply a patch or PR", type a pull request's number and click "Apply PR". A dialog opens, titled "Apply PR #…", with the files it changes. Click "Cancel": the dialog closes and nothing has changed. 2. Click the "Diff" tab, then "Choose a .diff or .patch file…", and choose a patch. The dialog is titled for the file. Click "Apply and rebuild": the dialog closes and the card shows the steps, then a green notice with "Revert this patch". 3. Click the card's header. It does not fold while the notice is there. Click "Revert this patch", and once it is gone click the header again: it folds. **What must not have happened:** - A patch that does not apply leaves no dialog in front of the error. - Nothing is written to the checkout before "Apply and rebuild" is pressed. ## Risks and limitations - **A failed apply cannot be tried again from the same preview.** The patch is chosen again; a pull request that was typed is typed again, since the field is cleared when it is read. - **Much of what the card says is not translatable yet**: the sentences it passes through from `pr-checkout.cjs`, `applied-layer.cjs`, `apply-conflict.cjs`, `watch-activity.cjs` and `update-plan.cjs`. Two notices join two of those sentences with a space. The translation journey covers the card with nothing applied and a patch file's preview. - **In a breakdown, why a change did not fit is on its own line**, lowercase and without a full stop: it was written to follow "near … ·". - **While the question about loose edits is up, the spoken "next step" still says to apply or cancel**, though neither button is on screen. And the next-action ring is not drawn round the dialog, which is in front of everything already. - **Linking a ticket while that question is up clears it, and the preview comes back.** Only "Cancel" drops it. - **A preview that arrives while another dialog is open on the same site**, the review or the create-site dialog, opens on top of it. - **After a pull request is checked out the card is gone, and focus with it.** The focus fix holds for a patch file, and for a pull request while its apply runs. - **During an update, an install or a build the dialog's button is held with no stated reason**, as the old one was. The reason is given when something holds the terminal. - **The card's heading is inside its header's button**, as the design system's collapsible card has it. The card is a region named by that heading; moving by headings may not find it. - **Whether the card is folded, and its tab, are not remembered across restarts.** That belongs with the settings (#559). - **The guide pages still show the old card.** #561. - **About 1,400 lines added, 560 of them tests**, against about 410 removed. One pull request, because the card, its preview and the pull request's banner are one block of the site's view and one flow. - **The manual steps above were not run by a person.** I looked at screenshots of the card with nothing applied, on each tab, of the dialog for a patch file and for a pull request, of the steps, of an applied patch, of a failure with its breakdown, and of a checked-out pull request. - **Checked on macOS only**, with the journeys this touches also run in a window resized to the Windows runner's 1024×728. Windows itself is CI's. - **Pre-PR review, two passes: everything marked to fix here was fixed, five findings, two of medium weight; the follow-ups left are in this list.** The medium two were a preview that could open over another site, and an apply button that could do nothing with its reason out of sight. ## Related Part of #557 (item 4 of its nine) and #542. Fifth slice on `redesign/wpds`, after #590, #594, #595 and #596. --- <details> <summary>Design decisions and alternatives considered</summary> **The patch file is chosen with a button, not the prototype's file input.** A page is not told where a file is, and the app reads the patch in the main process. The button opens the system's own dialog, as before, under the "Diff" tab. **"Apply PR" and "Apply and rebuild" keep their words on Gutenberg**, where the prototype says "Check out PR" and "Check out and rebuild". The app's sentences about a pull request ("PR #7 is applied", "Revert this PR") come from modules this PR passes through; changing two buttons alone would leave the two vocabularies side by side. **The description is the prototype's**, "Test changes in this checkout. Your own changes are preserved." The old sentence, that a pull request is checked out with its author's commits and a patch file is applied as a removable layer, is the line of help under each tab. **A failure drops the preview rather than showing the failure in the dialog.** A failure can carry a breakdown by file and place, and up to four ways on. That is the card's to hold, and it was already drawn there. **The steps stay on the card.** The prototype's dialog shows a spinner on its button until the apply is done. An apply here can install and build, for minutes, and the card's list says which step it is on while the page stays usable. **Notices are told to say nothing by themselves.** The design system's notice reads its own content out, and #596 found that reading can fail silently. The ones here that were alerts or status regions keep those roles, which announce them; the two that said nothing before, the applied patch and the checked-out pull request, still say nothing. **`pr-state.cjs` is gone.** Its last use was the old pill in the preview; the badge's rule lives with the work-item card's since #596. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> Run as the repository's review procedure, with the judgement pass in a fresh context that took no part in the change. First pass on `ceba525` against `d5bbd47`; second pass on the fix-up `cab185f`; the head, `df655d2`, applies what the second pass asked for. **First pass: four findings to fix here, six follow-ups; two medium, none high.** It compared the removed block with the new components line by line and found every line of the applied patch's and the checked-out pull request's banners, the failure's buttons and their gates, and the alert and status roles carried over. It traced every reader of the preview after a failed apply and found none that needed it, and no path on which the dialog goes without an error or a question being shown. Fixed: - **A preview could open over another site** (medium). Every site's view renders its own dialog, and a dialog is drawn over the whole window: one read while the contributor had gone elsewhere opened there, with a button that applied to the site out of view. A preview is shown by its own site. - **"Apply and rebuild" could do nothing, with the reason behind the dialog** (medium). The refusal while something holds the terminal is older than the dialog, and was a line on the same page. The button is held and says why. - **Two decisions this change made were in the components**: when the dialog shows and when the card is open. They are in `apply-card.cjs`, with unit tests. - **Translation**: "Near" and the line it introduces are one sentence a translation can reorder; "Diff" says to translators that it is a noun; a string that was extracted and never shown is gone. Follow-ups done here too: focus goes to the card's header after an apply, where it fell to the top of the window; a place in a breakdown is keyed by where it comes; a weak assertion was moved and a helper widened; and "Cancel" on the question about loose edits drops the preview, where it would have brought the dialog back. **Second pass: one finding to fix here, five follow-ups; all low.** It read base-ui's handling of where focus goes and confirmed it always lands on the card's header and never on a field, that the reason and the terminal's lock are written together, and that the three new journeys read their absences after the page has rendered. Fixed: - **The reason named the terminal as where a command was running**, and not everything that holds the terminal prints there. It says something else is running on the site, and how to stop it. Follow-ups done here too: a dialog left without applying gives focus back to its opener whatever was pressed in it; the two-site journey waits for a positive sign before it reads that no dialog is shown; the helper that finds a way to a patch file no longer matches two unrelated buttons; an assertion that could not fail is gone. Left as follow-ups, all in Risks above: the breakdown's wording; the spoken next step and the ring during the question; linking a ticket during the question; a preview over another dialog of the same site; focus after a pull request's checkout; no stated reason during an update, install or build; and retrying from the same preview. **Checks, on macOS, on the head of this branch:** | Check | Result | | --- | --- | | `npx eslint . --max-warnings=0` | clean | | `npm test` | 1827 pass, 2 skipped | | `npm run test:electron` | 1828 pass, 1 skipped | | `npm run test:e2e` (journeys) | 84 of 84 | | `npm run test:e2e:packaged` | 15 of 15, on an unsigned `npm run pack:dir` build of the head | | Nine journey files that reach the card, in a 1024×728 window, the Windows runner's | 34 of 34 | **The tests' claims were audited by breaking each one** in the source and running the test that should notice. 27 breaks, each run on the head, and all 27 turn a journey red on a line that states the claim. | Break | Caught by | | --- | --- | | A pull request can be asked for with nothing typed | `apply-card` | | The preview is not named for its patch | `apply-card` | | The preview does not say how much the patch changes | `apply-card` | | The preview does not list the files | `apply-card` | | Leaving the preview does not drop it | `apply-card` | | The card forgets the tab it was left on | `apply-card` | | The card cannot be folded | `apply-card` | | A folded card stays folded over a failure | `apply-card` | | A failed patch keeps its preview | `apply-card` | | The preview does not say whose work the patch lands on | `apply-card`, `patch-apply` | | A failed patch does not say the checkout was left alone | `apply-card`, `patch-apply` | | A failure cannot be dismissed | `apply-card` | | A preview opens over whichever site is on screen | `apply-card` | | The dialog's button is not held while something holds the terminal | `apply-card` | | Saying no to the question leaves the preview | `apply-card` | | Focus is left on nothing when the dialog closes on an apply | `apply-card` | | The preview is in front of the question about loose edits | `pr-checkout` | | Reverting a pull request does nothing | `pr-checkout` | | The failure is not an alert | `patch-apply` | | Reverting a patch does nothing | `patch-apply` | | A Gutenberg site is offered a patch file | `gutenberg-site` | | The pull request field is not held during a build | `terminal` | | The card's title is not translatable | `i18n` | | A tab's help is not translatable | `i18n` | | The preview's button is not translatable | `i18n` | | The preview's way out is not translatable | `i18n` | | The preview's sentence is not translatable | `i18n` | The audit also changed the code once: the first version of the focus fix did nothing, which the journey written for it showed, and it was redone through the dialog's own option for it. </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.
wip