List and apply a ticket's Trac attachments

[juanmaguitar]

Part of #110. Refs #109, #11. Stacked on #136 (base is that branch), which stacks on #135 → #123. Review order: #123 → #135 → #136 → this. The whole flow merges once the stack is complete.

Why

PRs are only half of "the work already on a ticket". On many tickets — good-first-bugs especially — the patch a contributor wants to try is a .diff attached to the ticket, not a PR (verified live: ticket #37578 has three .diff attachments and one PR; before this, the app showed only the PR). This adds the attachment half, under the PR list, loaded on demand.

What this does

A "Show Trac attachments" button under the linked-PR list. Because Trac serves the attachment list only to a real browser (everything else hits the proof-of-work interstitial), clicking it opens the real ticket in an embedded window where the contributor clears the challenge once, the app scrapes the #attachments block, and the window closes — it is a means, not the UI. The attachments appear as a native in-app list (filename · author · date · size), each .diff/.patch with Apply…; non-patches (e.g. a .txt) are shown but marked "not a patch". Applying downloads the file through that same challenge-passing session and hands it to the existing preview → apply engine, so an attachment, a PR and a chosen file are one path from the preview onward.

On demand, not on link: opening a Trac window can surface the challenge, so it happens when the contributor asks, not for every ticket. The persistent session means the challenge is passed once, not per open.

Verified end to end against live Trac (#37578)

A deterministic Electron harness exercised the real trac-view.js:

• the window passes the challenge unattended;
• the parser reads the real markup — 4 attachments with authors and absolute timestamps, the .txt correctly marked not-a-patch;
• the raw-attachment download is authorised by the session cookie — a real wp-admin/includes/dashboard.php diff (1701 bytes) comes back. This was the one load-bearing runtime assumption, now confirmed.

Also confirmed in the running app: the panel renders, the attachment list populates from the live scrape, and the PR apply→preview wiring works.

Security (the app's first remote, untrusted content — AGENTS.md)

• The window: contextIsolation, no nodeIntegration, sandbox, a dedicated persist:trac partition, and no preload — the page cannot reach the app or Node; only the #attachments HTML crosses back, read from the main process via executeJavaScript.
• Navigation pinned to core.trac.wordpress.org against both will-navigate and will-redirect (the latter catches 3xx / <meta refresh>).
• The parser never emits an off-host URL, and fetchAttachment re-checks the host before sending the session cookie — so a poisoned ticket page cannot get an attacker link in front of the user or leak the cookie off-host.
• The downloaded diff is untrusted → flows through Apply a patch from a file to the checkout, then rebuild #135's apply engine, which defends against path traversal.

Testing

• test/trac-attachments.test.cjs — the pure parser: dedup, encoded names, off-host and cross-ticket rejection, absolute-date extraction, missing-metadata rows, empty input. The fixture matches the live markup (confirmed by the harness above).
• npm test / npm run test:electron: 229/229 both runtimes. npm run lint: clean.
• src/trac-view.js's window/net glue is untested, consistent with the other net client (github-prs.js) — no repo precedent for mocking BrowserWindow; the logic lives in the covered pure parser.

Self-review (per AGENTS.md)

Ran the review with the judgement pass on fresh context. It returned 2 [fix here] (both 🔵), both fixed before this PR:

• Navigation lock covered only will-navigate → added will-redirect (3xx / meta-refresh could otherwise move the pinned window off-origin).
• The link regex accepted an absolute off-host href (Apply was safe, but the filename rendered as an openExternal link) → the parser now rejects any non-Trac-host URL, with a test.

Reviewer verified clean: window config, setWindowOpenHandler, host re-check before the cookie fetch, poll-loop lifecycle with isDestroyed guards + destroy() in finally, additive electron-store (no migration), no new dependency.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds on-demand Trac attachment discovery and integrates patch attachments with the existing preview/apply workflow.

Changes:

• Adds a secured embedded Trac scraper and session-backed attachment downloads.
• Parses and displays attachment metadata with patch applicability controls.
• Adds IPC wiring and parser/wiring tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/trac-view.js	Implements the Trac window and downloads.
src/trac-attachments.cjs	Parses attachment markup.
src/renderer/index.jsx	Adds attachment listing and apply UI.
src/preload.js	Exposes attachment IPC methods.
src/main.js	Registers attachment IPC handlers.
src/github-prs.js	Adds session-cookie request options.
test/trac-attachments.test.cjs	Tests attachment parsing.
test/ipc-wiring.test.cjs	Tests attachment IPC delegation.

Suppressed comments (1)

src/renderer/index.jsx:2401

• [Architecture · 🔵 low · fix here] openAndScrape returns closed when the contributor closes the Trac window and includes details for error, but this branch neither handles closed nor shows those details. Closing the window therefore leaves a blank panel, while real failures become undiagnosable. Render cancellation explicitly and surface the returned error fallback.

              {tracAttachments && (tracAttachments.status === 'challenge-timeout' || tracAttachments.status === 'error') ? (
                <div style={{ marginTop: 10, padding: '8px 10px', background: '#fcf9e8', border: '1px solid #dba617', borderRadius: 6, fontSize: 12, color: '#6e5406' }}>
                  {tracAttachments.status === 'challenge-timeout'
                    ? 'Trac’s human-check did not complete in time. Try again, and click “I am human” if it appears.'
                    : 'Could not read the attachments from Trac.'}

[juanmaguitar]

Partially addressed in f0f13d3, and deliberately left partial.

The security-critical part of this module was extracted into a pure helper, secureTracUrl(url) in src/trac-attachments.cjs, which is exported and directly unit-tested — including the cases that matter here: core.trac.wordpress.org.evil.com, userinfo (https://core.trac.wordpress.org@evil.com), same-host http:, and non-URL garbage. pinToTrac, parseOne and fetchAttachment all delegate to it, so the origin decision is no longer untested code.

What remains untested is the BrowserWindow lifecycle and network glue around it. Standing up an injected-Electron harness for that is a disproportionate amount of scaffolding for this PR, so it is going to a follow-up issue rather than blocking here. Leaving this thread open until that issue is filed.