Comparing changes

#459) ## Why "Apply" on a linked pull request downloads it as a diff and runs `git apply` on the ticket branch's working tree. To Git those hunks are the contributor's own edits; the next park rolls them into the ticket's single WIP commit, and from then on nothing can separate the author's work from the contributor's, which is why Review & submit refuses with "PR #7701 is part of this checkout". The case is made in #458; this is the first of a small stack that makes "Apply" fetch the pull request's head and check out a `pr/<number>` branch instead, so the branch is the record and `git status` tells the truth. ## What changes A new module, `src/pr-checkout.js`, shipped dark the way #383 shipped the Git binary: nothing calls it yet, so it can be reviewed and tested on its own. - **Fetch** `refs/pull/<number>/head` from the site's own `origin` through the existing `fetchBranch`. GitHub serves that ref for every pull request, fork or not, and Git resolves the short name `pull/N/head` on the remote, so no refspec plumbing was needed. The partial clone's filter applies, so the fetch brings commits and trees; the blobs come at checkout. - **Describe** what the head changes: its own diff measured from the merge base with trunk (what the author changed, not a month of other people's commits), and whether `package-lock.json` differs from the current checkout. Both are reads of the object store; neither fetches. - **Check out**: put `pr/N` at the head and switch through `switchToBranch` in `ticket-branches.js`, so a ticket being left is parked as today, a dirty trunk is refused as today, and a checkout that dies part-way is tagged for the switch marker as today. A `pr/N` the app did not make is refused, not adopted. A head that moved on GitHub is followed unless edits sit on the branch. - **Leave** by the same switch, with any edits parked as one WIP commit whose parent is the pull request's head. That parent is the whole point: Git keeps the author's commits and the contributor's edits apart, and a patch cut from the branch holds only the edits. Two new reads in `git-read.cjs` (`mergeBase`, `changedPathsBetween`), the `pr/` namers in `ticket-branches.js`, and "PR #N" as what a switch says on screen. `blobOid` now reads the tree entry rather than the object, which also takes a lazy fetch out of the trunk update's lockfile check. Deliberately not in this PR: the IPC handlers, the store record, the renderer, the journey and the docs. Those are the next PRs of the stack. Trac attachments and local patch files keep `git apply` and the applied-patch layer; moving them to `patch/` branches is a follow-up issue. ## How to test this Platforms: any. There is no user-visible surface yet, so the demonstration is the suite. From the repository root: ``` node --test tests/unit/pr-checkout.integration.test.cjs ``` It builds an origin on disk that serves `refs/pull/7/head` the way GitHub does, clones a partial site from it over `file://`, and walks: fetch by short name (the clone stays partial), the file list and the install decision, checkout from a ticket and from trunk, a dirty trunk refused with no branch left behind, edits on the pull request parked on its head, the return to the ticket byte for byte, a pull request that moved on GitHub with and without edits, a branch the app did not make, a checkout that dies on `index.lock` and its retry, and every bad number or commit id. **What must not have happened:** trunk committed to; a second WIP commit stacked on a ticket; `node_modules` touched by a checkout; a blob downloaded to answer "does the lockfile move" (the suite asserts the blob is still missing after the read). ## Risks and limitations - Fetch cost on a slow connection: the head's blobs arrive in one batch at checkout, before Git prints "Updating files", so the checkout is silent for the download. The IPC PR logs a line before it; there is no second progress parser. - A pull request based on a release branch is measured against trunk's merge base, so its file list also carries that branch's own commits. Documented in the module; the app does not read the base the pull request declares on GitHub. - A detached HEAD sitting on the pull request's commit (only a user's own client makes one) reads as no branch, so `already-checked-out` does not fire and the switch runs without a park. Pre-existing in `switchToBranch`, reachable from one more direction now. - Nothing in the app deletes a `pr/` branch; `deleteTicketBranch` refuses non-ticket refs. Branches accumulate until a "forget pull request" action exists (follow-up). - The diff is 945 lines, 560 of them tests. The module is 330; I kept it in one PR because the tests are the argument that the module is right. ## Related Part of #458. Sets up #290 (a pull request tried as its author wrote it), which the renderer PR closes. --- <details> <summary>Design decisions and alternatives considered</summary> - **Beside `ticket-branches.js`, not inside it.** That module's invariants are stated for `ticket/` refs and `trunk`, and a pull request's branch breaks the second one on purpose: its base is a foreign commit that moves when the author pushes, and it starts with the author's commits. The new module composes `switchToBranch` and the primitives and never redefines a park or a checkout. - **The recorded head is the branch's `baseOid`.** That one choice is what lets every existing path (park, `patchBaseOid`, discard to base, the generic switch) work on `pr/N` unchanged, and it is why `leavePullRequest` refuses without a head rather than letting the park fall back to trunk. - **Refuse a hand-made `pr/N` rather than adopt it.** Adopting would let the next park write a WIP commit onto a mentor's branch. - **Edits on the pull request are parked, not refused.** The issue's "what it drops" was "the PR plus my edits" as one indistinguishable checkout; a WIP commit parented on the head is distinguishable, and a contributor debugging a pull request does make edits. - **`fetchBranch` unchanged.** Git resolves `pull/N/head` on the remote in the order `<name>`, `refs/<name>`, `refs/tags/<name>`, `refs/heads/<name>`; no refspec parameter was needed. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 3 [fix here] · 4 [follow-up] · 3 style notes — all 8 substantive findings fixed in f44798a, 1 left as a limitation. - Fixed: a failed checkout threw without saying the ref was created or moved, so the IPC handler could not have recorded the head and the next attempt was a dead end; the rollback of a created branch could replace the `dirty-trunk` refusal with a delete error; `blobOid` lazy-fetched the lockfile blob on a partial clone (confirmed empirically: missing before the call, present after) and a `GIT_NO_LAZY_FETCH` run would have invented `needsInstall` from a `missing`; `leavePullRequest` without a head parked onto trunk; commit ids reached `branch` and `update-ref` unvalidated; a third copy of the `pr/` regex; a false comment on `listTicketBranches`; `needsInstall` and `kind: 'deleted'` untested from the pull request's side. - Deferred: detached HEAD on the pull request's commit (pre-existing in `switchToBranch`, listed under Risks). - **Review:** completed — fresh subagent context against `.github/instructions/code-review.instructions.md`; reviewed head da35d22 / base trunk (0620fbd); `npm run lint` clean, `npm test` 1331/1331 on that head. - **Since review:** `da35d22 → f44798a` checked: the answers above, `npm run lint` clean, `npm test` 1334/1336 (2 skipped as on trunk), `npm run test:electron` 1336/1336. - CodeRabbit: to be requested once the PR is open. </details> <details> <summary>Implementation notes</summary> - `changedPathsBetween` runs with `GIT_NO_LAZY_FETCH=1` like `mergeTree`: a two-tree diff needs no blob, so a fetch there would mean the command asked a question the trees could not answer. It passes no `windowsArgs`: neither `core.autocrlf` nor `core.longpaths` applies to a comparison that touches no worktree. - `blobOid` via `ls-tree`: the same command `treeEntryMode` already ran, now shared as `treeEntry`. `cat-file --batch-check` resolved `<oid>:<path>` and then asked the object's type and size, which on a `blob:none` clone is a promisor fetch. - Error shape of a failed checkout: `stage`, `from`, `to` from `switchToBranch`, plus `ref`, `headOid`, `created`, `moved` (and `cleanupError` when the rollback itself failed). Same idea as `rebaseOntoTrunk`'s `movedTo`. - Codes the module throws: `bad-pr-number`, `bad-oid`, `already-checked-out`, `pr-branch-exists`, `pr-has-edits`, `not-on-pr`, `no-pr-head`; `dirty-trunk` and `no-such-branch` come through from `switchToBranch`. </details> <details> <summary>Screenshots or recording</summary> Nothing on screen changes in this PR; the surface arrives with the renderer PR. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QiLLJuuRWRMVFuaWv4q1tp <details> <summary>Stack review update (2026-09-14)</summary> CodeRabbit raised two valid findings: Git read failures could be mistaken for a missing lockfile, and pull-request head OIDs reached Git without validation. Both were reproduced with regression tests and fixed in `4286baa`. Validation: focused integration suite 17/17, full Node suite 1,336 passed with 2 expected skips, lint and `git diff --check` clean. **0 remaining [fix here] · 0 [follow-up].** </details> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

## Why The checkout module in #459 needs an IPC contract and a per-branch record before the Apply panel can use it. Without the branch-aware handler adjustments, updating trunk while trying a PR parks its work but never returns to it: the return condition recognises only ticket numbers. ## What changes Adds preview, checkout and return handlers, with named preload methods and correlated log/done events. The PR's recorded head is its base for parking work; an interrupted checkout records the head immediately and retries the same destination without fetching or parking partial files. Status reports the PR, ticket lists omit PR branches, and submission refuses a checked-out PR while ordinary backup export remains available. The renderer is not connected yet. Trac attachments and local patch files retain their existing apply/revert flow. This is the second part of the stack, based on #459. ## How to test this Platforms: any, current head. This adds no new clickable UI. From the repository root, run: ```sh node --test tests/unit/ipc-wiring.test.cjs tests/unit/preload-listeners.test.cjs tests/unit/pr-checkout-renderer.test.cjs npm run test:electron ``` The wiring suite covers checkout and return, guard refusals, partial-checkout recovery, deletion of the return ticket, preserved ticket context, submission restrictions and backup export. The trunk-return regression and the install decision when restoring parked PR lockfile edits were both confirmed failing before their fixes. Preload tests cover an immediate refusal arriving before invoke resolves, listener cleanup and overlapping run correlation. **What must not have happened:** a retry parks partial files; a return drops the PR's recorded base; updating trunk leaves the site on trunk; a PR author's commits are submitted as the contributor's work. A real clone, slow blob download, Windows file locks and the visible Apply journey are not exercised by this IPC layer. The renderer PR adds the journey. ## Risks and limitations - Preview fetches from the site's own origin; checkout fetches again, so a preview is not a promise of an immutable remote head. Recovery instead completes the already recorded destination. - PR branches remain on disk. No deletion UI is introduced. - The status field `hasEdits` describes a parked WIP commit; loose worktree edits remain available through the existing change count for the renderer to combine. - Files and Trac attachments still use the applied-patch record. This change does not resolve their attribution limitations. - No UI changes in this PR; the next stack layer connects the new methods. ## Related Part of #458. Based on #459. The renderer layer completes #290. --- <details> <summary>Design decisions and alternatives considered</summary> Use the existing switch marker, Git child tracking and progress formatter. A PR record is additive under the existing branches map; store changes are computed inside changeSiteMeta. The original return target is retained when a PR is applied again. Preload subscribes before invoke and correlates early events once the id arrives, so a fast validation refusal cannot leave the UI waiting. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 0 [fix here] · 0 [follow-up]. No findings across the five dimensions. - **Review:** completed by fresh Codex subagent `/root/review_pr_ipc`, against base f44798a, including the six functional files committed at the time as 15984c9. After #459 gained its Windows assertion fix, `git range-diff` confirmed the rebased functional commit e97fbe0de4601c17c314d435899793838e128e38 is equivalent. - **Since review:** replaced a nested ternary in the reviewed lockfile destination calculation with equivalent conditionals to satisfy lint, then added the packaged-smoke API-key expectation in ec96bfb. Both changes are mechanical coverage updates; focused recovery, lockfile and packaged-smoke tests passed. - **Validation:** npm test: 1,396 passed, 2 skipped; Electron: 1,398 passed; final lint clean. - **CodeRabbit:** not run: its check reports success, but its comment says automatic reviews are disabled. The completed local review plus the recorded equivalence and mechanical follow-up above cover the current head ec96bfb. </details> <details> <summary>Stack review update (2026-09-14)</summary> Rebased onto the reviewed #459 fix as `7c706f3`. `git range-diff` matched both original commits exactly, so this layer has no functional delta; it only inherits the validated OID and Git-error handling below it. CodeRabbit did not produce inline findings for this PR. **0 remaining [fix here] · 0 [follow-up].** </details>

## Why The Apply panel currently turns a GitHub pull request into an unattributed diff on the active ticket branch. A first-time contributor can test that code, but loses the author's commit history and cannot safely tell the PR's work apart from edits made afterward. The checkout primitives and IPC introduced by #459 and #460 need a contributor-facing flow. ## What changes Pull requests now preview their state and changed files, then check out the fetched GitHub head as a local `pr/<number>` branch. Ticket or trunk work is parked before the switch, edits made on top of the PR stay with its local copy, and **Back to ticket #N** or **Back to trunk** restores the previous work. If the remote PR moves while that local copy has edits, the app offers **Return to saved copy** instead of overwriting it. The review screen attributes edits to the active PR and prevents submitting the author's commits as the contributor's own work. Applying a local or Trac `.patch`/`.diff` deliberately keeps the existing layer model: it modifies the active branch and remains tracked through the existing applied-patch state. ## How to test this **Platform:** either macOS or Windows. Run against Buildkite's artifact for current head `a55df1e85595b8de6309371fbd83c987def0f081`, once available. The layer-4 journey `tests/e2e/journeys/pr-checkout.spec.js` drives the contributor path and is red without this change. From the repository root, run: ```sh npx playwright test --project=journeys tests/e2e/journeys/pr-checkout.spec.js ``` It verifies ticket → PR → local PR edits → ticket, returning to a saved local copy after the upstream PR moves, and continuing into a PR checkout after discarding loose trunk work. The existing patch journey also remains green and verifies that `.patch` files still use the applied-layer model: ```sh npx playwright test --project=journeys tests/e2e/journeys/patch-apply.spec.js ``` **What must not have happened:** ticket work must not be lost or mixed into `pr/<number>`; local PR edits must not be overwritten when the upstream head moves; PR author commits must not become submit-able as the contributor's work; applying a `.patch`/`.diff` must not change to branch checkout semantics; `node_modules` must not be rebuilt unless `package-lock.json` changed. The journey uses a local Git remote and cannot exercise GitHub availability, a real `wordpress-develop` clone, a long `npm install`, or an OS path containing spaces. The draft's signed artifact will be used for that manual pass. ## Risks and limitations A `pr/<number>` branch is retained on disk after leaving it; there is no deletion control in this change. Preview and checkout are separate fetches, so the remote head can move between them; the checkout records the head it actually fetched. A changed remote head plus local edits intentionally returns to the saved copy until those edits are discarded. Patch-file attribution and revert limitations remain those of the existing patch-layer model. The fresh self-review found four issues across two passes; all four were fixed before this PR. The final pass found 0 [fix here] and 0 [follow-up]. ## Related The complete stack, including the refined Apply/Revert wording, remembered applied PRs and review fixes, is available in #463. Use that top branch to test the combined feature. Fixes #458. Fixes #290. Stacked on #460, which is stacked on #459. --- <details> <summary>Design decisions and alternatives considered</summary> The GitHub pull-request ref `refs/pull/<number>/head` is fetched into a named local `pr/<number>` branch. A named branch avoids detached HEAD and gives later contributor edits a stable home while preserving the PR author's commits. Patch files do not identify a durable remote branch, so `.patch` and `.diff` files continue to be applied to the active branch and recorded as a reversible layer. The UI says **Back to ticket #N** or **Back to trunk** because leaving a PR restores the branch that was active before it; it does not run `git revert` against the PR commits. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> 4 [fix here] · 0 [follow-up] — all 4 fixed. Final result: 0 [fix here] · 0 [follow-up]. - **Review:** completed — fresh-context self-review against base `ec96bfbab450debe3c23ac6b3f33e228e836d659` and the full working-tree diff that became head `a55df1e85595b8de6309371fbd83c987def0f081`; the initial pass found three issues: dirty-trunk continuation routed a PR through ticket parsing, a moved upstream PR gave no path back to a locally edited copy, and submission refusal always named a ticket. All were fixed with regression coverage. - **Re-review:** completed — found one low-severity placement issue when a deleted return ticket redirected the active PR to trunk; fixed tests-first by making explicit `pullRequest.returnTo` authoritative. Final re-review found no findings across architecture, security, performance, cross-platform behavior, or tests. - **Since review:** the reviewed working tree was committed without further source changes as `a55df1e85595b8de6309371fbd83c987def0f081`. - **Validation:** `npm run lint`; `npm test` (1407 passed, 2 expected Electron skips); `npm run test:electron` (1409 passed); `npx playwright test --project=journeys` (25 passed); `npm run docs:build`; `git diff --check`. </details> <details> <summary>Implementation notes</summary> The renderer calls the streaming preview, checkout, and leave APIs from #460. The main process recognizes a moved PR whose local branch has edits and switches back to that saved tip without fetching over it. Documentation now uses “apply” for patch files and “check out” for pull requests throughout the contributor flow. </details> <details> <summary>Screenshots or recording</summary> Pending capture from the signed Buildkite artifact for this draft's current head. That artifact is also the first build that contains the complete three-PR stack. </details> <details> <summary>Stack review update (2026-09-14)</summary> CodeRabbit raised four findings. Three were already addressed in `a55df1e`; the remaining high-severity finding was valid: publishing `retryPrSwitchRef.current` during render could expose a callback from a discarded React render. A regression test failed before the fix, and `068cd6b` now publishes the callback from `useLayoutEffect`. Validation: full Node suite 1,410 passed with 2 expected skips, lint and `git diff --check` clean. **0 remaining [fix here] · 0 [follow-up].** </details>

…kets (#463) ## Why Leaving a ticket with a PR applied and returning to it reopened the ticket branch instead of the saved PR. A real older WordPress PR also exposed 1,784 generated Gutenberg files as the contributor's changes, and reviewing a large added file failed with `Maximum call stack size exceeded` while the screen claimed there was nothing to send. ## What changes Tickets remember their applied local PR across unlink, switching tickets and app restart. **Revert this PR** clears that association and restores the ticket work; edits on the PR stay with its local copy. Restoring a PR pauses the watcher, installs dependencies only when the lockfile changes, and rebuilds the site. The single highlighted banner stays inside the ticket card, using **PR #N is applied** and **Revert this PR**. Existing Apply wording and the `.patch`/`.diff` mechanism remain. Other source actions are hidden while a PR is applied. Ticket actions remain disabled, with a visible reason, until applying or reverting and its rebuild finish. Generated `/gutenberg/` files remain ignored when an older PR's `.gitignore` is checked out. The review excludes generated additions already parked by older app versions, while retaining changes to files present in the original base. Updating `diff` from 5.2 to 8.0.4 fixes large-file patch generation. Failed reviews show an error rather than an empty-state claim, and PR reviews name the PR as their comparison base. ## How to test this **Platforms:** macOS and Windows, on this PR's **current head**. The top PR's code includes #459, #460 and #461. No signed Buildkite artifact has been verified for this head yet. From the repository root: ```sh npm test npm run test:electron npm run test:e2e ``` `tests/e2e/journeys/pr-checkout.spec.js` covers apply → edit → unlink → restart → relink → switch tickets → revert. It verifies that saved PR edits and the original ticket edits return separately, that a restored PR actually rebuilds its assets, and that an explicit revert stops automatic PR restoration. Its regression cases were run red before the fixes. The failed-review journey verifies that a read error cannot become “nothing to send.” The IPC regressions cover untracked and previously parked generated files, a 150,000-line text addition, and conditional installation on PR restoration. The existing patch journeys verify apply/revert and persistence for `.patch` files. **What must not happen:** PR edits must not overwrite ticket work; restoration must not replace the saved copy with a new GitHub head; unchanged lockfiles must not trigger an install; generated Gutenberg downloads must not appear as the contributor's work. **Local macOS package:** built unsigned from `c71fbb7`, with all 15 packaged smoke tests passing. The packaged main process, renderer bundle and ticket-action module match the current build byte-for-byte. **Real-repository check performed on macOS:** an isolated local clone of the reported `wordpress-develop` PR #8906, including the old WIP commit with all 1,784 generated files, opens Review with zero contributor changes and no exception. The original site was not modified. **Remaining manual coverage:** a signed Windows build, live GitHub fetching, long real dependency installs, OS file dialogs and real build-watch processes. The automated journeys use a local Git remote and small npm scripts; their passing result does not claim those external checks. ## Risks and limitations Generated files already stored in old WIP commits are not deleted or rewritten; the review excludes additions under `/gutenberg/` absent from the original base. The app retains local PR branches after reverting. PR edits still cannot be submitted as a new contribution through this flow. An already-unlinked ticket from an older app version has no saved active-PR association; apply that PR again once. The independent review found four issues in PR restoration and its integration; all were fixed and re-reviewed. Review record below. ## Related Stacked on #461 → #460 → #459. Part of #458. This PR preserves the existing stack commits. <details> <summary>Review outcome</summary> - **Review:** completed — independent Codex agent `review_final_stack`; reviewed base/HEAD `a55df1e85595b8de6309371fbd83c987def0f081` plus the complete pending diff. Four runtime findings fixed across re-review: restored-PR build/install, missing new-ticket destination, cancellation handler, and terminal ownership. One terminal test fixture correction applied. Final outcome: **0 remaining [fix here] · 0 [follow-up]**. - **Since review:** committed as `9b49879`; the commit contains the reviewed pending work, including the final fixture correction and screenshot, then `9b49879 → 6709d44` reviewed independently: only two test fixtures changed to use explicit Git identity after Windows CI exposed reliance on host configuration. No runtime changes in that delta. The subsequent `6709d44 → c71fbb7` delta was reviewed independently with zero findings: publish ticket-switch callbacks in a layout effect and keep ticket actions blocked through the entire PR/patch operation. The latter fixes a real Windows journey failure where Unlink was clickable before Revert had finished; its new unit regression was red before the fix and green afterward. - **CodeRabbit:** completed a review of `9b49879` and raised one actionable finding (callbacks assigned to a ref during render). Fixed in `c71fbb7` with a layout effect and independently re-reviewed. Subsequent green CodeRabbit statuses say **Review skipped**, so the independent review is the review of record for the final head. Validation on `c71fbb7`: Node 1,417 passed, 2 expected skips; Electron 1,419 passed; all 29 journeys passed in one run; 15 local packaged smoke tests passed; lint clean. Final-head CI is green on macOS and Windows: unit suites, all journeys and packaged smoke tests; lint and docs build pass. [E2E run](https://github.com/WordPress/contributor-toolkit/actions/runs/34770983332), [unit run](https://github.com/WordPress/contributor-toolkit/actions/runs/34770983301). Pages deployment is intentionally skipped for a PR. </details> <details> <summary>Screenshots</summary> The banner and source controls are captured from the real Electron UI: ![Applied PR in its ticket](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/pr-checkout-ticket-continuity/docs/public/screenshots/pr-applied.png) ![Apply controls](https://raw.githubusercontent.com/WordPress/contributor-toolkit/juanmaguitar/pr-checkout-ticket-continuity/docs/public/screenshots/apply-patch-panel.png) </details> <details> <summary>Stack review update (2026-09-14)</summary> Rebased onto the reviewed lower stack as `c9e828f`. The CodeRabbit finding on `ticketSwitchLifecycleRef` remains fixed in the commit phase, and the same layout effect now publishes the corrected PR retry callback inherited from #461. Validation on the final stack: focused unit/integration suites 33/33; full Node suite 1,420 passed with 2 expected skips; targeted dirty-trunk journey 1/1; lint and `git diff --check` clean. **0 remaining [fix here] · 0 [follow-up].** </details>

…to the app (#465) ## Why A contributor finds the ticket they want on `core.trac.wordpress.org`, in a browser, and then has to get into the app by hand: leave the browser, find the app, open a site, read the ticket number back, type it in. The app cannot be reached from a browser at all — it registers no URL scheme, so there is no address any link could point at. See #464. This adds the address. Nothing on the web uses it yet: what ships here is the app's side, so that a link, a bookmarklet or a browser extension has something to hand a ticket to. ## What changes The app answers one shape of URL, `wpct://ticket/<id>`, and nothing else. **The parser is the boundary.** `src/deep-link.cjs` is pure, with no Electron import, in the shape `external-url.js` and `site-registry.js` already use: one host, one payload, every other address refused and logged through `describeRefused`. This is the only input to the app that comes from outside it — any page the contributor visits can navigate to a custom scheme, and on Windows and Linux the address lands in `argv` — so the refusals are most of what the module and its tests are about. The id then goes through the app's own `parseTicketRef` rather than a second definition of what a ticket is. **Nothing happens before the address parses, the window included.** Focusing first would hand every page a way to pull the app in front of whatever the contributor is doing, without passing the parser at all. **Three intake points, one queue.** `open-url` on macOS, which can fire before `whenReady`; `second-instance` argv on Windows and Linux; `process.argv` on a cold start. They feed one slot, because a `webContents.send` to a page that has not finished loading is dropped without a sound — the way this feature would most easily lose a ticket. The renderer says `deep-link:ready` when it has subscribed. A single-instance lock guards all of it: without one, a link clicked while the app runs starts a second copy. **Arriving at the app, the ticket is offered, not linked.** Linking parks the current branch and checks out another, and can carry uncommitted work. That is the app's most far-reaching ordinary write, and a click in a browser is not consent to it. So the link raises a question naming the ticket and the site, and one click answers it — through the same `saveTicket` as any other link, so the guards already there (a running install, a mid-switch site, a merge started in a terminal, the dirty-trunk question) apply unchanged. With no site open, the ticket waits and the app says where it can go. Deliberately not in this: any URL that creates a site, applies a patch, or does anything but name a ticket. One verb, and the contributor confirms it. ## How to test this **Platforms: macOS and Windows, both, and it needs an installed build** — a dev run does not exercise a registered scheme. On macOS the scheme has to be in the bundle's `Info.plist`, which cannot be written at runtime, so `npm start` cannot test this at all. Buildkite builds signed artifacts for this branch; check the build matches the current head commit. No journey covers this: Playwright cannot deliver an OS-level protocol activation. **Starting state:** the packaged app installed and launched once (which is what registers the scheme on Windows), then quit. At least one site, set up past the wizard. 1. Quit the app. In a terminal, anywhere: `open "wpct://ticket/62281"` on macOS, `start wpct://ticket/62281` in PowerShell on Windows. **The app launches, comes forward, and the ticket panel asks "Link ticket #62281 to \<your site\>?"** This is the cold-start path, the one most likely to drop the address. 2. Click **Not now**. **The question goes. The ticket field is untouched** — if you had typed something in it before step 1, it is still there. 3. With the app running and the site open, run the same command again. **The existing window comes forward and asks again. No second app icon appears, no second window.** 4. Click **Link ticket**. **The site switches to ticket #62281**, exactly as if you had typed the number and clicked Link ticket. 5. Run the same command once more, for the ticket now linked. **Nothing is asked** — you are already looking at it. 6. Run `open "wpct://ticket/../../etc/passwd"` and `open "wpct://evil/1"`. **The app does not come forward and says nothing about a ticket.** `~/Library/Logs/…/main.log` (macOS) records `deep-link refused`. 7. With two or more sites: trigger a link while site A is open, then click site B in the sidebar without answering. **B does not ask** — the question belongs to the site that was open when it arrived. 8. macOS only: with a site open, click **Review & submit changes** to open the patch window, then close the main window with ⌘W, leaving the patch window. Run the command. **The main window reopens and asks.** **What must not have happened:** - No ticket linked without you clicking **Link ticket** — check `git -C <site>/wordpress-develop branch --show-current` after step 1 and 2 and see it unchanged. - No uncommitted work moved or discarded by any step before step 4. - No second instance of the app after step 3 — two would be two processes writing one `electron-store`. - The ticket field never silently overwritten by a link. - Step 6 must not focus the window. A page that can raise the app on any string it likes is the point of that step. ## Risks and limitations **Both platforms are now verified by hand.** macOS, against `076ebb6` from a packaged `--dir` build: a cold start from `open "wpct://ticket/62281"` launched the app and raised the question for a site already linked to another ticket, with `(deep-link) delivering ticket 62281` in the log. `wpct://ticket/62281/../9`, `wpct://evil/1` and `wpct://ticket:80/62281` are refused in the log without bringing the window forward. Windows, driven through Edge rather than a terminal, which is the path a contributor actually takes: typing `wpct://ticket/62281` in the address bar produced Edge's "This site is trying to open WordPress Contributor Toolkit" prompt, and Open brought the running app forward with the question — for a site sitting on a different ticket (#56153), so the case the confirmation exists for. Confirming linked #62281 and the panel filled in its Trac facts and its linked pull request. That exercises the half of the registration electron-builder does not do: NSIS never reads `build.protocols`, so on Windows the scheme is claimed by `setAsDefaultProtocolClient` on first run. Windows cold start, with the app quit first: the link launched it from nothing and the question was there when the window painted. Driven with a ticket the site was *not* on (#60000, against a site sitting on #62281) precisely so the result could come out false — a dropped ticket and a delivered one look identical when the site already has that ticket, which is what the first attempt at this pass showed. That also exercises the queue end to end: the ticket was in hand before a renderer existed and waited for `deep-link:ready`. So all three intake points have now been driven by a person: `open-url` on macOS, `second-instance` argv on Windows with the app running, and `process.argv` on a Windows cold start. **Several copies of this app are one app to macOS.** They share a bundle identifier, so Launch Services resolves `wpct://` to that identifier and then picks a copy itself — not necessarily the one whose `Info.plist` claimed the scheme. The first attempt at the pass above opened a `dist` build from another worktree that carries no deep-link code, which answers by doing nothing: indistinguishable from this feature being broken until the log shows no `deep-link` line was ever written. Testing this means naming the build with `open -a "<path>.app"`, with every other copy quit. Recorded in TESTING.md, because a reviewer with an installed copy and a worktree build will hit it first thing. **Windows registers the scheme at runtime, not at install.** electron-builder's `protocols` key is consumed for the macOS bundle and the Linux desktop entry but not by the NSIS target (verified in `app-builder-lib`), so on Windows the registration is `setAsDefaultProtocolClient` on first run, writing `HKCU\Software\Classes`. A Windows user who installs but never opens the app has no handler. Step 1 on Windows is the test for this. **A reload loses a ticket already delivered but not yet answered.** The queue hands the ticket out once. The app offers no reload, so this costs a ticket only under devtools; holding it until an answer would mean a second channel to keep both sides in step. **The single-instance lock changes two local harnesses**: `npm run shots` (live tier) and the packaged smoke test both launch against the real user profile, so they now fail if the installed app is open. Noted in TESTING.md; the journeys are unaffected, each having its own throwaway profile. **Unverified, and cheap for a Windows reviewer to settle:** from source on Windows, the protocol registration passes `app.getAppPath()` into Electron's `args` array. Whether Electron quotes each element before writing `HKCU\Software\Classes` I could not confirm from here; if it does not, a dev checkout under a path with a space registers a broken command and reports success. Dev-mode only — packaged builds do not take this branch. **A refused address still costs a process on Windows and Linux**, which launch a whole Electron to lose the lock and quit. The lock is what bounds it; the app has no way to refuse the launch itself. ## Related Fixes #464 --- <details> <summary>Design decisions and alternatives considered</summary> **Why a confirmation rather than linking straight away.** The first shape of this was "link to the active site, no picker". `sites:set-ticket` is not a metadata write: it parks the current branch, checks out another, can carry uncommitted trunk work, and is refused mid-merge or on a legacy site. Doing that from a browser click, on whichever site happened to be selected, moves a contributor's work without them asking. The confirmation keeps the flow one click long and keeps the checkout theirs. **Why the ticket field is not prefilled.** It was, in the first pass. It is free text the contributor may be halfway through typing, every site row stays mounted behind `display: none`, and the question already names the ticket its button will link — so writing over what they typed bought nothing and could lose something. **Why the question is read off one value rather than held per row.** Same reason: rows are hidden, not unmounted. A row holding its own copy went on asking after another row had answered, and asked again when the contributor came back to it. **Why `wpct`.** Short, unclaimed, and not a WordPress.org-wide name this repository has any authority to reserve. **Why one slot and not a queue.** Two links clicked before the app is up are one contributor changing their mind. The last one is the answer. </details> <details> <summary>Review outcome (required — see AGENTS.md)</summary> **7 [fix here] · 3 [follow-up] — all 7 fixed, 2 of 3 follow-ups fixed, 1 kept deliberately.** Fixed: 1. **The packaged smoke test was already red.** `tests/e2e/packaged/smoke.spec.js` asserts the exact set of `contextBridge` keys, and the two new ones were missing. `npm run test:e2e` runs `--project=journeys` only, so a green local run could not see it. Fixed and verified by building an artifact and running layer 5 for real. 2. **The deep link leaked across site rows** and overwrote the ticket field. Rewritten to derive from one value, and to leave the field alone. 3. **On macOS with no main window but a patch or Trac window open, a link did nothing at all** — `activate` counts those as windows. `showWindowForDeepLink` opens one. 4. **The queue had no coverage at any layer** and one test's title claimed more than it asserted. The queue is now `createDeepLinkQueue` with five layer-1 tests; the misnamed test says what it checks. 5. **A comment claimed reload-safety the code does not have.** Corrected, and the limitation is recorded above. 6. **The window was focused before the address was judged.** Now only a parsed ticket brings the app forward. 7. **The unpackaged registration used `process.argv[1]`**, which is the script only when no switch was passed — `electron --inspect .` registered a handler pointing at a file called `--inspect` and reported success. Now `app.getAppPath()`, and only on Windows, since the other two platforms cannot be claimed from source at all. The comment said otherwise and would have sent the next person down a dead end on macOS. 8. *(follow-up, fixed)* The "already linked" decision was inline in `index.jsx`, where no test can reach it. Moved into `deep-link-notice.cjs`. 9. *(follow-up, kept)* One test covers a defensive branch — an active site with no label — that the app cannot currently produce. Kept: it is the guard that stops the app asking for consent to a site it cannot name. 10. *(follow-up, fixed)* The single-instance lock's effect on `npm run shots` and the packaged smoke test is now in TESTING.md. The parser itself was probed directly and held: `wpct://ticket./1`, `wpct://ticket\evil/1`, `%2e%2e` in the path, `wpct:/ticket/1`, `wpct:////ticket/1`, fragments, ports, userinfo, uppercase, ids past `Number.MAX_SAFE_INTEGER`, and `--x=wpct://…` in argv. No way past it; the only value that reaches the renderer is a bounded integer, which the renderer re-parses anyway. - **Review:** completed — self-review per `.github/instructions/code-review.instructions.md`, judgement pass run in a separate agent context that did not write the code; reviewed head `7ed988f`, base `origin/trunk` `0620fbd`; `npm run lint` clean, `npm test` 1337, `npm run test:electron` 1337, `npm run test:e2e` 23 journeys. - **CodeRabbit:** completed on `0cbfb23` — 4 findings (1 major stability, 1 major maintainability, 2 minor), **all 4 fixed** in `b094777`, each answered on its thread. Note the check itself reads `pass` with "Review skipped: automatic reviews are disabled": that is the check state, not the review. The review ran, on request, and left inline comments. 1. *The parser read its parts off `new URL()`.* Both shapes confirmed: `wpct://ticket/62281/../9` arrived as ticket 9 because `..` is resolved before anything saw the path, and `wpct://ticket/not-a-ticket?id=9` arrived as ticket 9 because the `?id=` fallback was read whenever the path was not a ticket. Neither is dangerous — a bounded ticket id is all that ever comes out — but both are addresses the app answers and does not document. The accept decision is now one anchored match on the address as it arrived, and the URL parse survives only to name the refusal in the log; `?id=` is gone. That also closed a shape neither pass had found: a control character inside the scheme, which the URL parser strips. 2. *The queue cleared the ticket before the send.* A window closing between the check and the send consumed it into a failed send. `deliver` now forgets the ticket only once the send returns, and the webContents is checked alongside the window. 3. *The already-linked case came back as `null`,* which the renderer read as "discard this ticket" — a decision about externally delivered state made in the one file no test can load. It is a `settled` state now, and `null` means only that no link has arrived. 4. *The IPC wiring test's queue stub had a no-op `markReady`,* so it passed whether or not the handler still called it. Recorded and asserted. - **Third pass** (self-review, fresh agent context again) on `b094777`: it verified all ten fixes from the two passes above as correct and complete, and found **2 more [fix here] · 1 [follow-up], all 3 fixed** in `076ebb6`. 1. *The main-process half of this feature was executed by nothing.* The IPC harness stubbed `isReady` as false, so `showWindowForDeepLink` returned on its first line in every test and `flushDeepLink` returned on the missing window — three of pass A's fixes and the effect of pass B's queue fix could each have been reverted with the suite still green. Five tests now drive the real parser and the real queue through main. Checked by mutation rather than assumed: restoring the clear-before-send fails five tests, removing the window creation fails nine. The one branch still unreachable from there, the per-platform registration inside `whenReady`, is a decision now rather than a call — `protocolRegistration`, with its three branches tested, including the empty app path that would otherwise register a handler pointing nowhere and report success. 2. *The `no-active-site` state could not happen.* `activeSite` falls back to the first site and nothing deselects it, so a null one means there are no sites at all. The state is gone, and so is the paragraph in the guide that promised behaviour the app does not have. 3. *(follow-up)* The queue reset on a new window but not on a navigation, so a ticket arriving during a reload would have been flushed into a loading page. It resets on `did-start-loading` too. - **Since review:** `7ed988f → 0cbfb23` — the self-review fixes; `0cbfb23 → e5bb14e`, a TESTING.md note only; `e5bb14e → b094777`, the CodeRabbit fixes; `b094777 → 076ebb6`, the third pass's fixes. Re-verified on `076ebb6`: `lint` clean, `npm test` 1349, `npm run test:e2e` 23 journeys, `npm run test:e2e:packaged` 15, and the macOS pass driven again by hand against a rebuilt artifact — `wpct://ticket/62281` delivers, and `wpct://ticket/62281/../9`, `wpct://evil/1` and `wpct://ticket:80/62281` are refused in the log without bringing the window forward. One flake to record rather than hide: `engine.spec.js`'s launch test failed once on a loaded machine and passed on both re-runs. Re-verified on the new head: `lint` clean, `npm test` 1343, `npm run test:e2e` 23 journeys, `npm run test:e2e:packaged` 15 (the layer the review found red). </details> <details> <summary>Implementation notes</summary> New: `src/deep-link.cjs` (parser, argv pick, intake, queue), `src/renderer/deep-link-notice.cjs` (the three states a ticket can land in), and a test file for each. `src/main.js`: the scheme registration, the single-instance lock, the three intake points, and `deep-link:ready`. `flushDeepLink` sends only to `mainWindow`, which is now tracked in a variable, because `getAllWindows()` cannot tell it from the patch and Trac windows. `package.json`: `build.protocols`. Verified against the packaged artifact — `CFBundleURLTypes` holds `["wpct"]` in the built `Info.plist`. `tests/unit/ipc-wiring.test.cjs`: the electron stub grew `requestSingleInstanceLock`, `setAsDefaultProtocolClient` and `isReady`, since the first runs at module scope and a missing one stops `main.js` loading at all. `tests/unit/user-data-override.test.cjs` needed the same for its own stub. </details> <details> <summary>Screenshots or recording</summary> The whole visible surface of this change is one question: ![The question a link raises above the Trac ticket panel: Link ticket #62281 to my-first-patch?, with Link ticket and Not now](https://raw.githubusercontent.com/WordPress/contributor-toolkit/1652a8f3cdd1b49ce936385874c0dba2eeacbe3e/docs/public/screenshots/deep-link-prompt.png) Captured by `npm run shots -- --only=deep-link-prompt`, fixture tier, so it carries a seeded site name rather than a real path — and it is in the guide now too, where the section describing this had words and no picture while every section around it has one. It is the one shot the harness could not reach by driving the UI, since the ticket arrives from the OS and not from a click, so `prepare` now receives the Electron app alongside the page and this shot makes the main process receive the address a browser would send it. The other two states are not photographed because there is nothing to see: with no site yet the same banner appears above the site list, and a link for the ticket the site is on already shows nothing at all. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01SNfwbKozBsY9ub5goLJyBm --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

## Why `v1.1.0-beta.1` shipped on 12 September. Trunk carries five commits since, and this is the **second beta for 1.1.0**, published so contributors can keep testing before the stable tag. The headline is checking out a pull request as a real branch, with the author's commits intact, in place of the unattributed diff the Apply panel produced (#459, #460, #461, #463, closing #290 and #458); it also gives the app an address a browser can hand a ticket to, `wpct://ticket/<id>` (#465, closing #464). The version reaches two places a contributor sees, and both must carry the real build: electron-builder embeds it in the artifact names (`wordpress-contributor-toolkit-1.1.0-beta.2-*`), and `src/logging.js` writes `app <version>` as the log's first line, so a problem report names the build it came from. That is why the bump merges before the tag. ## What changes Only the version. All three package-version fields in `package.json` and `package-lock.json` move together from `1.1.0-beta.1` to `1.1.0-beta.2`. No dependency versions change, and there is no application behaviour change in this PR. ## How to test this Platforms: any for the suite; macOS or Windows for the artifact check that follows the merge. In the repository root: 1. `npm run lint` is clean and `npm test` passes. 2. `node -e "console.log(require('./package.json').version)"` prints `1.1.0-beta.2`. 3. `python3 -c "import json;d=json.load(open('package-lock.json'));print(d['version'], d['packages']['']['version'])"` prints `1.1.0-beta.2 1.1.0-beta.2`. **What must not have happened:** no dependency version may change. `git diff trunk` shows exactly three changed lines, all of them the root package version. After merge, the release build must produce artifacts named `wordpress-contributor-toolkit-1.1.0-beta.2-*`, and the first line of a fresh app log must read `app 1.1.0-beta.2`. ## Risks and limitations No UI change. The risk is a mis-scoped edit in `package-lock.json`, where dependencies also carry `1.1.0-beta.1`; the replacement was confined to the first 2000 bytes of the file, which holds the two root entries, and both files were re-parsed as JSON afterwards. ## Related Same shape as #397 (v1.0.1) and #457 (v1.1.0-beta.1). --- <details> <summary>Review outcome (required — see AGENTS.md)</summary> Version-only change, three lines. Verified locally on this branch: `npm run lint` clean, `npm test` 1451 pass, 0 fail, 2 expected skips; `git diff --stat` shows two files, three insertions, three deletions; both JSON files parse. Nothing to fix, nothing deferred. </details> <details> <summary>Screenshots or recording</summary> Nothing on screen changes. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0121dia1RzfskutrzA8iSK1K Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Navigation Menu

Repository navigation

Comparing changes

Open a pull request

Commits on Sep 14, 2026

This comparison is taking too long to generate.

Uh oh!