Fix the apply panel's error/copy states, and apply a PR by URL - #142
Merged
juanmaguitar merged 3 commits intoAug 7, 2026
Merged
Conversation
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 05:19
1a3d1ae to
c5876e3
Compare
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 06:28
c5876e3 to
4cfd453
Compare
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 06:47
4cfd453 to
60fedc8
Compare
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 07:58
60fedc8 to
4569456
Compare
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 09:15
4569456 to
2d24609
Compare
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 09:41
2d24609 to
5052f1a
Compare
Three fixes from real testing plus one requested addition, all in the "Try
someone else's patch" panel:
- The apply error stayed on screen after Cancel. Cancel now clears it, so
dismissing a failed attempt leaves no stale warning behind.
- The panel's sub-copy ("Apply a .diff/.patch file…") contradicted itself
while a pull request preview was open. It is now hidden whenever a preview or
the apply chain is showing — the preview card speaks for itself — and the
idle copy names pull requests as well as files.
- The failure message ran two sentences together ("…no longer applies The
checkout was not changed."). A period is added when the reason does not end
in punctuation.
- New: paste a pull request URL (or number) and apply it directly, without it
having to be linked to the ticket. parsePrRef (pure, in patch-sources.cjs)
accepts a wordpress-develop PR URL or a bare number and rejects other repos,
other hosts and non-PR URLs; the number then rides the existing
previewPr → fetchPrDiff → preview → apply flow, so it shares the same trust
boundary as the linked-PR list.
Refs #109, #11, part of #110.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
'Try someone else's patch' undersold it once the panel also applied pull requests and the contributor's own downloaded files. The heading now names what the panel does. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Screenshots and other non-patch attachments were listed with a 'not a patch' label, which is noise in a panel whose only action is to apply one. The list now shows just .diff/.patch files; when a ticket has attachments but none are patches, it says so. The parser still returns every attachment — the latest indicator and its tests rely on the full set — so this is purely what the panel displays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
juanmaguitar
force-pushed
the
juanmaguitar/apply-panel-polish
branch
from
August 7, 2026 09:59
5052f1a to
82ee3b4
Compare
juanmaguitar
added a commit
that referenced
this pull request
Aug 7, 2026
Fixes #164. Stacked on #142 — review the [two-file diff](juanmaguitar/apply-panel-polish...juanmaguitar/remove-trac-ticket-on-creation), not the base. ## What and why The **Create WordPress Core site** modal asked for a Trac ticket alongside the site name and location. That is the moment someone is least likely to have an answer — they may be creating the site precisely so they can go find something to work on — and the field made the modal look longer and more demanding than it is. It was also a second entry point for an association the site row already handles, better: link, change and unlink, at any point in the site's life. This removes the creation-time shortcut and leaves that panel as the only way in. ## Changes - `src/renderer/index.jsx` — the field, its state, its resets and its creation-time validation are gone; `parseTicketRef` is no longer needed in the renderer (the panel validates in the main process), so the import narrows to `ticketUrl`. - `src/main.js` — with the renderer no longer passing one, the `options.tracTicket` branch of the `wordpress:setup` handler had no caller left. Removed rather than kept as dead code. `parseTicketRef` stays imported: `sites:set-ticket` still uses it. The post-creation path — `sites:set-ticket`, `trac-ticket.cjs`, the ticket panel in `SiteRow` — is byte-identical. ## Review Ran the pass in `.github/instructions/code-review.instructions.md`, judgement dimensions dispatched to a fresh context per `/self-review`. - `npm run lint` — clean. `npm test` — 337/337. - **0 [fix here] · 0 [follow-up].** No findings across the five dimensions. Three things the review verified rather than assumed, since they were the plausible failure modes: - Nothing else in `src/`, `test/` or the docs expects a ticket at creation time — no test exercised `wordpress:setup` with `options.tracTicket`, so the deletion orphans no assertion. - Dropping `tracTicket: null` from the optimistic `setSiteMeta` patch cannot resurrect a stale ticket when a site is re-created at the same path: `sites:forget` and `sites:delete` both delete the meta entry, and the renderer refreshes wholesale from the store before a re-create can spread it. - No test added, deliberately: this removes a feature rather than adding one, the removed main-process branch had none, and there is no renderer-component harness in the repo. The behaviour that survives — validating and persisting a ticket — is already covered by the `sites:set-ticket` tests and `test/trac-ticket.test.cjs`. ## Testing Open **Create WordPress Core site**: only Site name and Site location. Create a site, then link a ticket from its row and confirm it saves, opens in Trac and unlinks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <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.
Part of #110. Refs #109, #11. Stacked on #140 (base is that branch), atop #139 → #136 → #135 → #123. From hands-on testing of the stack.
Three fixes reported from testing
.diff/.patchfile…" while showing "PR #4496 changes 1 file". The sub-copy is now hidden whenever a preview or the apply chain is showing (the preview card speaks for itself), and the idle copy names pull requests as well as files.One requested addition
Apply a PR straight from a pasted URL or number, without it having to be linked to the ticket. A "Paste a pull request URL or number" input + "Apply PR" button in the panel's idle state.
parsePrRef(pure,src/patch-sources.cjs) accepts a wordpress-develop PR URL or a bare number and rejects other repos, other hosts, issue URLs and junk — including crafted..paths (URL normalisation collapses them, then the repo/anchor check rejects). The number rides the existingpreviewPr → fetchPrDiff → preview → applyflow, so a pasted PR shares the same trust boundary as the linked-PR list:fetchPrDiffhardcodes the repo and strips the number to digits, and the diff flows through the apply engine's path-traversal defence.Testing
test/patch-sources.test.cjs—parsePrRef: bare/#number, URL with trailing/filesand#…, scheme-less, wrong-repo rejection, and issue/foreign-host/junk rejects.npm test/npm run test:electron: 242/242 both runtimes.npm run lint: clean.Self-review (per AGENTS.md)
Judgement pass on fresh context: 0 findings across the five dimensions. It independently confirmed the
..-traversal and foreign-repo/host rejections, that the pasted-number path introduces no new trust boundary (samefetchPrDiff+ apply engine), that the punctuation ternary can't throw (error is truthy-guarded), and that hiding the sub-copy never strips the idle call-to-action.Pending your visual confirm in the running app for the three UI fixes and the paste-a-PR flow — these are your reported issues, verified here by tests + review.
🤖 Generated with Claude Code