[Feature] Answer wpct:// ticket links so a browser can hand a ticket to the app - #465

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.

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).

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.

The whole visible surface of this change is one question:

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.