diff --git a/src/github-prs.js b/src/github-prs.js new file mode 100644 index 0000000..c042445 --- /dev/null +++ b/src/github-prs.js @@ -0,0 +1,149 @@ +'use strict'; + +/** + * Finding the pull requests linked to a Trac ticket, and fetching one's diff + * (issue #11 / #109). + * + * All GitHub access goes through the documented REST API, not the web `.diff` + * route: `github.com/…/pull/N.diff` and `patch-diff.githubusercontent.com` + * both return 422 to unauthenticated clients now (verified 2026-08-06), so the + * only reliable unauthenticated path is `repos/…/pulls/N` with the diff media + * type. The cost is the shared 60-requests-per-hour limit, which is why the + * caller caches and why classifyHttpFailure separates a spent limit from an + * empty ticket. + * + * Requests use Electron's `net` rather than a new HTTP dependency: it rides the + * Chromium network stack, so it honours the same proxy and TLS configuration + * the rest of the app already relies on, and adds nothing to install. + */ + +const { parseLinkedPrs, classifyHttpFailure } = require('./patch-sources.cjs'); + +const REPO = 'WordPress/wordpress-develop'; +// GitHub rejects API requests with no User-Agent; an identifying one is also +// the honest thing to send. +const USER_AGENT = 'WordPress-Contributor-Toolkit (+https://github.com/WordPress/experimental-wp-dev-env)'; +const REQUEST_TIMEOUT_MS = 15000; + +/** + * A single GET over Electron net. Never rejects on an HTTP status — the status + * is data the caller classifies — only on a transport failure or timeout. + * Modelled on the never-reject readiness probe in main.js. + * + * The `deps` seam (net client and timers) exists only so the response, + * transport-error, timeout, and settle-once paths can be exercised without the + * network or a real 15s wait; production callers pass nothing and get Electron's + * `net` and the global timers. + * + * @param {string} url + * @param {Object} [headers] + * @param {Object} [deps] + * @return {Promise<{status: number, headers: Object, body: string}>} + */ +function httpGet(url, headers = {}, deps = {}) { + // Required lazily, not at module load: requiring `electron` outside Electron + // resolves the binary and can spawn its installer on a cold checkout, and the + // standalone tests inject their own client and must never reach it. + const netImpl = deps.net || require('electron').net; + const setTimeoutImpl = deps.setTimeout || setTimeout; + const clearTimeoutImpl = deps.clearTimeout || clearTimeout; + return new Promise((resolve, reject) => { + let settled = false; + const finish = (fn, arg) => { if (!settled) { settled = true; fn(arg); } }; + + const request = netImpl.request({ method: 'GET', url }); + request.setHeader('User-Agent', USER_AGENT); + for (const [key, value] of Object.entries(headers)) request.setHeader(key, value); + + const timer = setTimeoutImpl(() => { + try { request.abort(); } catch {} + finish(reject, new Error(`Timed out after ${REQUEST_TIMEOUT_MS}ms`)); + }, REQUEST_TIMEOUT_MS); + + request.on('response', (response) => { + const chunks = []; + response.on('data', (chunk) => chunks.push(chunk)); + response.on('end', () => { + clearTimeoutImpl(timer); + const lowerHeaders = {}; + for (const [key, value] of Object.entries(response.headers || {})) { + lowerHeaders[key.toLowerCase()] = Array.isArray(value) ? value[0] : value; + } + finish(resolve, { status: response.statusCode, headers: lowerHeaders, body: Buffer.concat(chunks).toString('utf8') }); + }); + response.on('error', (e) => { clearTimeoutImpl(timer); finish(reject, e); }); + }); + request.on('error', (e) => { clearTimeoutImpl(timer); finish(reject, e); }); + request.end(); + }); +} + +/** + * The pull requests that cite a ticket, newest first. + * + * @param {number|string} ticketId + * @param {Object} [deps] + * @return {Promise<{status: 'ok'|'rate-limited'|'error'|'offline', items: Array, error?: string}>} + */ +async function fetchLinkedPrs(ticketId, deps = {}) { + const get = deps.httpGet || httpGet; + const id = String(ticketId).replace(/[^0-9]/g, ''); + if (!id) return { status: 'error', items: [], error: 'No ticket number' }; + + // 100 is GitHub's per-page maximum. One request covers any realistic ticket; + // paginating would multiply requests against the shared unauthenticated quota + // this whole feature is careful with, so instead a result that does not fit in + // one page is treated as incomplete below. + const query = encodeURIComponent(`repo:${REPO} is:pr ${id}`); + const url = `https://api.github.com/search/issues?q=${query}&per_page=100`; + + let res; + try { + res = await get(url, { Accept: 'application/vnd.github+json' }); + } catch (e) { + // A transport failure is offline, not empty: the contributor may simply + // have no network, which the panel should say rather than "no patches". + return { status: 'offline', items: [], error: String(e && e.message ? e.message : e) }; + } + + if (res.status !== 200) { + return { status: classifyHttpFailure(res.status, res.headers), items: [], error: `GitHub returned ${res.status}` }; + } + + let json; + try { json = JSON.parse(res.body); } catch { return { status: 'error', items: [], error: 'Unreadable response from GitHub' }; } + + // A truncated search must not be cached as the complete list: the linked PR + // could be one we did not receive, and "no patches" shown as final is the + // exact failure this feature guards against. Fall back to the cache instead. + const returned = Array.isArray(json.items) ? json.items.length : 0; + if (json.incomplete_results === true || (typeof json.total_count === 'number' && json.total_count > returned)) { + return { status: 'error', items: [], error: 'Too many results to list reliably' }; + } + + return { status: 'ok', items: parseLinkedPrs(json, id) }; +} + +/** + * The unified diff for one pull request. + * + * @param {number} number + * @return {Promise<{ok: true, text: string}|{ok: false, status: string, error: string}>} + */ +async function fetchPrDiff(number) { + const n = String(number).replace(/[^0-9]/g, ''); + if (!n) return { ok: false, status: 'error', error: 'No pull request number' }; + + let res; + try { + res = await httpGet(`https://api.github.com/repos/${REPO}/pulls/${n}`, { Accept: 'application/vnd.github.v3.diff' }); + } catch (e) { + return { ok: false, status: 'offline', error: String(e && e.message ? e.message : e) }; + } + if (res.status !== 200) { + return { ok: false, status: classifyHttpFailure(res.status, res.headers), error: `GitHub returned ${res.status}` }; + } + return { ok: true, text: res.body }; +} + +module.exports = { fetchLinkedPrs, fetchPrDiff, httpGet }; diff --git a/src/main.js b/src/main.js index c7e0455..5926618 100644 --- a/src/main.js +++ b/src/main.js @@ -31,6 +31,7 @@ const { normalizeEol } = require('./git-update.cjs'); const { ensureAutocrlf, readTrunkInfo, collectDirtyFiles, discardChanges, updateToLatestTrunk } = require('./trunk-update'); const { applyPatchToDir } = require('./patch-apply'); const { parsePatchFiles, planApply } = require('./patch-plan.cjs'); +const { fetchLinkedPrs, fetchPrDiff } = require('./github-prs'); const { openExternalUrl, ALLOWED_URL_SCHEMES } = require('./external-url'); const { deleteRegisteredSite } = require('./site-registry'); const { getStore } = require('./settings-store'); @@ -478,6 +479,47 @@ ipcMain.handle('git:update-trunk', async (event, sitePath) => { return { updateId }; }); +// --- Discovering the patches on a ticket (#109/#11) --- linked PRs come from +// GitHub; the network code is in src/github-prs.js, these handlers add the +// cache and IPC. A last-known-good copy per ticket, in electron-store, is what +// lets a rate-limited or offline lookup still show the work that exists. +const patchCacheKey = (ticketId) => `ticketPatches:${ticketId}`; + +ipcMain.handle('git:list-ticket-patches', async (_e, sitePath) => { + try { + const s = await getStore(); + const meta = (s.get('siteMeta') || {})[sitePath] || {}; + const ticketId = meta.tracTicket; + if (!ticketId) return { ok: true, ticket: null, prs: { status: 'no-ticket', items: [] } }; + + const result = await fetchLinkedPrs(ticketId); + if (result.status === 'ok') { + s.set(patchCacheKey(ticketId), { checkedAt: new Date().toISOString(), items: result.items }); + return { ok: true, ticket: ticketId, prs: { status: 'ok', items: result.items } }; + } + + // Could not read GitHub. Fall back to whatever was last seen for this + // ticket, labelled with when — a stale-but-shown list beats a short one + // presented as complete. + const cached = s.get(patchCacheKey(ticketId)) || null; + return { + ok: true, + ticket: ticketId, + prs: { status: result.status, items: cached ? cached.items : [], cachedAt: cached ? cached.checkedAt : null, error: result.error } + }; + } catch (e) { + return { ok: false, error: String(e) }; + } +}); + +ipcMain.handle('git:fetch-pr-diff', async (_e, number) => { + try { + return await fetchPrDiff(number); + } catch (e) { + return { ok: false, status: 'error', error: String(e) }; + } +}); + // --- Applying someone else's patch (#11) --- the diff mechanics live in // src/patch-apply.js; these handlers add IPC plumbing and electron-store writes. diff --git a/src/patch-sources.cjs b/src/patch-sources.cjs new file mode 100644 index 0000000..06a5818 --- /dev/null +++ b/src/patch-sources.cjs @@ -0,0 +1,99 @@ +'use strict'; + +/** + * Turning a GitHub search response into the pull requests that actually belong + * to a Trac ticket (issue #109 / #11). + * + * On the busiest tickets the real work is a wordpress-develop PR, not a Trac + * attachment — the attachment list is empty precisely where activity is + * highest. Core's Trac↔GitHub convention is that a PR cites its ticket in the + * body ("Trac ticket: https://core.trac.wordpress.org/ticket/NNNNN"), so the + * search is: ask GitHub broadly for PRs mentioning the number, then verify + * narrowly, here, that each one cites this ticket's URL. GitHub's search + * tokeniser matches the bare number in comments and unrelated text, so the + * verification is what makes the list trustworthy rather than merely plausible. + * + * Kept pure and dependency-free so the verification and the failure + * classification — the parts that decide whether the UI shows work that exists + * — are unit tested without a network: the main process requires it, and so + * does `node --test` (same convention as git-update.cjs / patch-plan.cjs). + */ + +const TICKET_HOST = 'core.trac.wordpress.org'; + +/** + * True when a PR body cites this exact ticket. The negative lookahead stops + * `/ticket/6582` from matching inside `/ticket/65820`, and the host is required + * so a bare "#65822" in prose does not count. + * + * @param {string} body + * @param {number|string} ticketId + * @return {boolean} + */ +function bodyCitesTicket(body, ticketId) { + if (typeof body !== 'string') return false; + const id = String(ticketId).replace(/[^0-9]/g, ''); + if (!id) return false; + const re = new RegExp(`${TICKET_HOST.replace(/\./g, '\\.')}/ticket/${id}(?![0-9])`); + return re.test(body); +} + +/** + * Reduces a GitHub `search/issues` response to the PRs that cite the ticket. + * Returns newest-first — for a moving target like a PR the freshest is the one + * a contributor most likely wants. + * + * @param {Object} searchJson + * @param {number|string} ticketId + * @return {Array<{number: number, title: string, state: string, updatedAt: string, url: string}>} + */ +function parseLinkedPrs(searchJson, ticketId) { + const items = searchJson && Array.isArray(searchJson.items) ? searchJson.items : []; + const seen = new Set(); + const prs = []; + for (const item of items) { + // `search/issues` returns issues and PRs together; only PRs carry + // `pull_request`. + if (!item || !item.pull_request) continue; + if (!bodyCitesTicket(item.body, ticketId)) continue; + if (seen.has(item.number)) continue; + seen.add(item.number); + prs.push({ + number: item.number, + title: typeof item.title === 'string' ? item.title : '', + state: item.state === 'closed' ? 'closed' : 'open', + updatedAt: item.updated_at || item.created_at || '', + url: item.html_url || `https://github.com/WordPress/wordpress-develop/pull/${item.number}` + }); + } + prs.sort((a, b) => (b.updatedAt || '').localeCompare(a.updatedAt || '')); + return prs; +} + +/** + * Classifies a non-2xx GitHub response so the UI can tell "nothing on this + * ticket" apart from "we could not read it". A rate-limited answer is not an + * empty ticket: on a shared Contributor-Day IP the unauthenticated 60/hour is + * spent quickly, and a short list shown as complete is the exact failure this + * feature exists to prevent. + * + * @param {number} status + * @param {Object} [headers] Lower-cased header map. + * @return {'rate-limited'|'error'} + */ +function classifyHttpFailure(status, headers = {}) { + const remaining = headers['x-ratelimit-remaining']; + if (status === 429) return 'rate-limited'; + if ((status === 403 || status === 401) && String(remaining) === '0') return 'rate-limited'; + // GitHub's secondary (abuse) limit is a 403 with a Retry-After header while + // the primary quota is not yet spent — the burst case on a shared IP. + if (status === 403 && headers['retry-after'] !== undefined && headers['retry-after'] !== null) return 'rate-limited'; + return 'error'; +} + +module.exports = { + TICKET_HOST, + bodyCitesTicket, + parseLinkedPrs, + classifyHttpFailure +}; diff --git a/src/preload.js b/src/preload.js index 2c23dc7..fb64d6e 100644 --- a/src/preload.js +++ b/src/preload.js @@ -81,6 +81,10 @@ contextBridge.exposeInMainWorld('api', { choosePatchFile: () => ipcRenderer.invoke('dialog:choose-patch-file') , previewPatch: (sitePath, patchText) => ipcRenderer.invoke('git:preview-patch', sitePath, patchText) +, + listTicketPatches: (sitePath) => ipcRenderer.invoke('git:list-ticket-patches', sitePath) +, + fetchPrDiff: (number) => ipcRenderer.invoke('git:fetch-pr-diff', number) , applyPatch: async (sitePath, options, onLog, onDone) => { const { applyId } = await ipcRenderer.invoke('git:apply-patch', sitePath, options); diff --git a/src/renderer/index.jsx b/src/renderer/index.jsx index f701219..21963a1 100644 --- a/src/renderer/index.jsx +++ b/src/renderer/index.jsx @@ -43,6 +43,12 @@ const CREATE_SITE_NAME_INPUT_ID = 'create-site-name-input'; const CREATE_SITE_LOCATION_INPUT_ID = 'create-site-location-input'; const CREATE_SITE_LOCATION_HELP_ID = 'create-site-location-help'; const CREATE_SITE_TICKET_INPUT_ID = 'create-site-ticket-input'; +// Why the ticket's PR list could not be read, worded for the contributor. +const TICKET_PATCH_STATUS_MESSAGE = { + 'rate-limited': 'GitHub is rate-limiting this connection.', + offline: 'Could not reach GitHub.', + error: 'Could not read the pull requests from GitHub.' +}; const TRAC_TICKET_LISTS_URL = 'https://core.trac.wordpress.org/tickets/good-first-bugs'; const CREATE_SITE_MODAL_STYLE_ID = 'create-site-modal-theme'; @@ -685,6 +691,7 @@ function App() { onRename={onRename} isPending={pendingSites.includes(s)} setupLogs={setupLogsBySite[s] || ''} + isActive={activeSite === s} /> )) @@ -773,7 +780,7 @@ function App() { ); } -function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSiteMetaPatch, onForget, onDelete, onRename, isPending = false, setupLogs = '' }) { +function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSiteMetaPatch, onForget, onDelete, onRename, isPending = false, setupLogs = '', isActive = false }) { // Kept in a ref so loadStatus's dependency list stays [sitePath] — a // recreated callback prop must not retrigger the status-loading effect. const metaPatchRef = useRef(onSiteMetaPatch); @@ -807,6 +814,10 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit const [ticketInput, setTicketInput] = useState(''); const [ticketError, setTicketError] = useState(''); const [ticketSaving, setTicketSaving] = useState(false); + // Patches on the linked ticket (#11): { status, items, cachedAt } or null. + const [ticketPatches, setTicketPatches] = useState(null); + const [ticketPatchesLoading, setTicketPatchesLoading] = useState(false); + const [fetchingPr, setFetchingPr] = useState(null); // Trunk update path (#94) const [trunkDate, setTrunkDate] = useState(null); const [updateIncomplete, setUpdateIncomplete] = useState(false); @@ -1617,6 +1628,69 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit } }; + // Loads the PRs linked to the ticket. Manual, not on a timer: each call is a + // request against a shared, unauthenticated GitHub limit, so it runs when the + // contributor asks — on link, and on an explicit refresh. + const loadTicketPatches = useCallback(async () => { + setTicketPatchesLoading(true); + try { + const res = await window.api.listTicketPatches(sitePath); + setTicketPatches(res && res.ok ? res.prs : { status: 'error', items: [] }); + } catch { + setTicketPatches({ status: 'error', items: [] }); + } finally { + setTicketPatchesLoading(false); + } + }, [sitePath]); + + // Load the ticket's PRs only for the active site. Every SiteRow stays mounted + // (the parent hides inactive ones), so fetching on mount would spend the + // shared, unauthenticated GitHub quota once per linked site on every launch. + // The ref keeps re-activating a site from re-fetching the same ticket; a + // relink (ticket change) and the Refresh button still fetch. Unlinking clears + // the list. Placed after loadTicketPatches is defined: an effect that named it + // earlier in the body would read the const before its declaration ran. + const loadedTicketRef = useRef(null); + useEffect(() => { + if (!tracTicket) { + setTicketPatches(null); + loadedTicketRef.current = null; + return; + } + if (!isActive || loadedTicketRef.current === tracTicket) return; + // Marked loaded before the fetch resolves, on purpose: a failed initial + // fetch is not retried on every re-activation (which could keep spending a + // rate-limited quota) — the Refresh button is the retry. + loadedTicketRef.current = tracTicket; + loadTicketPatches(); + }, [tracTicket, isActive, loadTicketPatches]); + + // Fetches a PR's diff and drops into the same preview the file picker uses, + // so applying a PR and applying a downloaded patch are one path from here on. + const previewPr = async (pr) => { + setApplyError(''); + setFetchingPr(pr.number); + try { + const diff = await window.api.fetchPrDiff(pr.number); + if (!diff || !diff.ok) { + setApplyError(diff?.status === 'rate-limited' + ? 'GitHub is rate-limiting this connection right now. Open the PR and download its .diff, then use “Choose a patch file”.' + : `Could not fetch the diff for PR #${pr.number}: ${diff?.error || 'unknown error'}`); + return; + } + const preview = await window.api.previewPatch(sitePath, diff.text); + if (!preview || !preview.ok) { + setApplyError(preview?.error || 'Could not read that diff.'); + return; + } + setApplyPreview({ ...preview, label: `PR #${pr.number}`, text: diff.text }); + } catch (e) { + setApplyError(String(e)); + } finally { + setFetchingPr(null); + } + }; + const runApply = ({ reverse = false } = {}) => { const state = terminalStateRef.current; if (state.running) { @@ -2195,13 +2269,69 @@ function SiteRow({ sitePath, initialized, createdAt, label, onInitialized, onSit
Trac ticket
{tracTicket ? ( -
- - #{tracTicket} - - - -
+ <> +
+ + #{tracTicket} + + + +
+ +
+
+
Linked pull requests
+ +
+
+ See the work that already exists on this ticket before adding your own. Trac attachments are not listed yet — open the ticket for those. +
+ + {ticketPatchesLoading && !ticketPatches ? ( +
Checking GitHub…
+ ) : null} + + {ticketPatches && ticketPatches.status === 'ok' && ticketPatches.items.length === 0 ? ( +
No pull requests cite this ticket yet.
+ ) : null} + + {ticketPatches && ticketPatches.status !== 'ok' && ticketPatches.status !== 'no-ticket' ? ( +
+ {TICKET_PATCH_STATUS_MESSAGE[ticketPatches.status] || TICKET_PATCH_STATUS_MESSAGE.error} + {ticketPatches.items && ticketPatches.items.length && ticketPatches.cachedAt + ? ` Showing what was last seen ${new Date(ticketPatches.cachedAt).toLocaleString()}.` + : ' No cached list to fall back on.'} +
+ ) : null} + + {ticketPatches && ticketPatches.items && ticketPatches.items.length ? ( +
+ {ticketPatches.items.map((pr) => ( +
+
+
+ + {' '}{pr.title} +
+
+ {pr.state === 'closed' ? 'closed' : 'open'}{pr.updatedAt ? ` · updated ${new Date(pr.updatedAt).toLocaleDateString()}` : ''} +
+
+ +
+ ))} +
+ ) : null} +
+ ) : ( <>
diff --git a/test/github-prs.test.cjs b/test/github-prs.test.cjs new file mode 100644 index 0000000..440752b --- /dev/null +++ b/test/github-prs.test.cjs @@ -0,0 +1,122 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); +const { EventEmitter } = require('node:events'); +const { httpGet, fetchLinkedPrs } = require('../src/github-prs'); + +// A stand-in for Electron's `net`: request() hands back an EventEmitter whose +// end() lets the test drive the response (or an error) the way the real client +// would. Nothing here touches the network. +function fakeNet(onEnd) { + return { + request() { + const req = new EventEmitter(); + req.setHeader = () => {}; + req.abort = () => { req.aborted = true; }; + req.end = () => onEnd(req); + return req; + } + }; +} + +function respond(req, { status = 200, headers = {}, body = '' }) { + const res = new EventEmitter(); + res.statusCode = status; + res.headers = headers; + req.emit('response', res); + if (body) res.emit('data', Buffer.from(body)); + res.emit('end'); +} + +// --- httpGet transport paths (Copilot #136 #4) --------------------------- + +test('httpGet resolves with status, lower-cased headers, and body on success', async () => { + const res = await httpGet('https://x', {}, { + net: fakeNet((req) => respond(req, { status: 200, headers: { 'X-RateLimit-Remaining': '59' }, body: 'hello' })) + }); + assert.strictEqual(res.status, 200); + assert.strictEqual(res.body, 'hello'); + assert.strictEqual(res.headers['x-ratelimit-remaining'], '59'); +}); + +test('httpGet rejects on a transport error, never resolving', async () => { + await assert.rejects( + httpGet('https://x', {}, { net: fakeNet((req) => req.emit('error', new Error('boom'))) }), + /boom/ + ); +}); + +test('httpGet rejects and aborts the request on timeout', async () => { + let fire; + let aborted = false; + const p = httpGet('https://x', {}, { + net: { request() { + const req = new EventEmitter(); + req.setHeader = () => {}; + req.abort = () => { aborted = true; }; + req.end = () => {}; // never responds + return req; + } }, + setTimeout: (cb) => { fire = cb; return 1; }, + clearTimeout: () => {} + }); + fire(); // simulate the timeout elapsing + await assert.rejects(p, /Timed out/); + assert.strictEqual(aborted, true); +}); + +test('httpGet settles once: a timeout after a successful response is a no-op', async () => { + let fire; + const p = httpGet('https://x', {}, { + net: fakeNet((req) => respond(req, { status: 200, body: 'ok' })), + setTimeout: (cb) => { fire = cb; return 1; }, + clearTimeout: () => {} + }); + const res = await p; + assert.strictEqual(res.body, 'ok'); + // The promise is already resolved; firing the timer must not throw or change it. + assert.doesNotThrow(() => fire()); +}); + +// --- fetchLinkedPrs completeness (Copilot #136 #3) ----------------------- + +const CITE = (id) => `Trac: https://core.trac.wordpress.org/ticket/${id}`; + +test('fetchLinkedPrs returns ok with the citing PRs when the result is complete', async () => { + const body = JSON.stringify({ + total_count: 1, + incomplete_results: false, + items: [{ number: 42, pull_request: { url: 'x' }, title: 'Fix', state: 'open', updated_at: '2026-01-01T00:00:00Z', html_url: 'u', body: CITE(123) }] + }); + const res = await fetchLinkedPrs('123', { httpGet: async () => ({ status: 200, headers: {}, body }) }); + assert.strictEqual(res.status, 'ok'); + assert.strictEqual(res.items.length, 1); + assert.strictEqual(res.items[0].number, 42); +}); + +test('fetchLinkedPrs refuses to cache a truncated result, so the cache is used instead', async () => { + // total_count exceeds what one page returned: the linked PR could be one we + // did not receive, so this must not be reported (or cached) as complete. + const body = JSON.stringify({ total_count: 150, incomplete_results: true, items: [{ number: 1, pull_request: {}, body: CITE(123) }] }); + const res = await fetchLinkedPrs('123', { httpGet: async () => ({ status: 200, headers: {}, body }) }); + assert.strictEqual(res.status, 'error'); + assert.deepStrictEqual(res.items, []); +}); + +test('fetchLinkedPrs treats total_count beyond the page as incomplete even without the flag', async () => { + const body = JSON.stringify({ total_count: 101, incomplete_results: false, items: [{ number: 1, pull_request: {}, body: CITE(123) }] }); + const res = await fetchLinkedPrs('123', { httpGet: async () => ({ status: 200, headers: {}, body }) }); + assert.strictEqual(res.status, 'error'); +}); + +test('fetchLinkedPrs maps a rate-limited status through classifyHttpFailure', async () => { + const res = await fetchLinkedPrs('123', { httpGet: async () => ({ status: 403, headers: { 'x-ratelimit-remaining': '0' }, body: '' }) }); + assert.strictEqual(res.status, 'rate-limited'); +}); + +test('fetchLinkedPrs reports offline on a transport failure rather than empty', async () => { + const res = await fetchLinkedPrs('123', { httpGet: async () => { throw new Error('no network'); } }); + assert.strictEqual(res.status, 'offline'); + assert.deepStrictEqual(res.items, []); +}); diff --git a/test/ipc-wiring.test.cjs b/test/ipc-wiring.test.cjs index 79edea9..b896e10 100644 --- a/test/ipc-wiring.test.cjs +++ b/test/ipc-wiring.test.cjs @@ -1102,6 +1102,61 @@ test('git:apply-patch reports applied-but-untracked when the undo also fails', a assert.match(done.error, /could not be undone/); }); +// --- linked-PR discovery (#109 / #11) ------------------------------------ + +test('git:fetch-pr-diff asks github-prs for the diff', async () => { + const fetchPrDiff = spy(async () => ({ ok: true, text: 'DIFF' })); + const main = loadMain({ stubs: { ...silentLogging(), './github-prs': { fetchPrDiff, fetchLinkedPrs: async () => ({}) } } }); + + const result = await main.invoke('git:fetch-pr-diff', 7319); + + assert.deepEqual(fetchPrDiff.calls, [[7319]]); + assert.deepEqual(result, { ok: true, text: 'DIFF' }); +}); + +// git:list-ticket-patches reads the stored ticket, then delegates to github-prs +// and caches the result — reachable through the same fakeSettingsStore seam. +test('git:list-ticket-patches fetches the linked PRs for the stored ticket', async () => { + const fetchLinkedPrs = spy(async () => ({ status: 'ok', items: [{ number: 7, title: 'x' }] })); + const settings = fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': { tracTicket: 62281 } } }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './github-prs': { fetchLinkedPrs } } }); + + const result = await main.invoke('git:list-ticket-patches', '/sites/wp'); + + assert.deepEqual(fetchLinkedPrs.calls, [[62281]]); + assert.equal(result.ok, true); + assert.equal(result.ticket, 62281); + assert.equal(result.prs.status, 'ok'); + assert.deepEqual(result.prs.items, [{ number: 7, title: 'x' }]); +}); + +test('git:list-ticket-patches falls back to the cached list when GitHub cannot be read', async () => { + let call = 0; + const fetchLinkedPrs = spy(async () => (++call === 1 + ? { status: 'ok', items: [{ number: 7 }] } + : { status: 'rate-limited', items: [], error: 'limit' })); + const settings = fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': { tracTicket: 62281 } } }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './github-prs': { fetchLinkedPrs } } }); + + await main.invoke('git:list-ticket-patches', '/sites/wp'); // populates the cache + const result = await main.invoke('git:list-ticket-patches', '/sites/wp'); + + assert.equal(result.prs.status, 'rate-limited'); + assert.deepEqual(result.prs.items, [{ number: 7 }], 'the last-known-good list is shown, not empty'); + assert.ok(result.prs.cachedAt, 'stamped with when it was last seen'); +}); + +test('git:list-ticket-patches returns no-ticket without calling github-prs when none is linked', async () => { + const fetchLinkedPrs = spy(async () => ({ status: 'ok', items: [] })); + const settings = fakeSettingsStore({ sites: ['/sites/wp'], siteMeta: { '/sites/wp': {} } }); + const main = loadMain({ stubs: { ...silentLogging(), ...settings.stubs, './github-prs': { fetchLinkedPrs } } }); + + const result = await main.invoke('git:list-ticket-patches', '/sites/wp'); + + assert.equal(result.prs.status, 'no-ticket'); + assert.deepEqual(fetchLinkedPrs.calls, []); +}); + // --- the harness's own guard --------------------------------------------- // Requiring the real `electron` package is not a harmless fallback: its @@ -1152,7 +1207,9 @@ const WIRED = new Set([ 'playground-web:stop', 'sites:set-ticket', 'git:preview-patch', - 'git:apply-patch' + 'git:apply-patch', + 'git:fetch-pr-diff', + 'git:list-ticket-patches' ]); // Channels with no module to reach: they read or write electron-store, drive a diff --git a/test/patch-sources.test.cjs b/test/patch-sources.test.cjs new file mode 100644 index 0000000..2426b07 --- /dev/null +++ b/test/patch-sources.test.cjs @@ -0,0 +1,81 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert'); +const { bodyCitesTicket, parseLinkedPrs, classifyHttpFailure } = require('../src/patch-sources.cjs'); + +// Shaped like a real search/issues item, trimmed to the fields the parser reads. +function item(number, overrides = {}) { + return { + number, + title: `PR ${number}`, + state: 'open', + updated_at: '2026-08-01T00:00:00Z', + html_url: `https://github.com/WordPress/wordpress-develop/pull/${number}`, + pull_request: { url: 'x' }, + body: 'Trac ticket: https://core.trac.wordpress.org/ticket/62281', + ...overrides + }; +} + +test('bodyCitesTicket: the full ticket URL counts, a bare number does not (issue #11)', () => { + assert.strictEqual(bodyCitesTicket('see https://core.trac.wordpress.org/ticket/62281 for context', 62281), true); + assert.strictEqual(bodyCitesTicket('this is about #62281 somewhere', 62281), false); + assert.strictEqual(bodyCitesTicket('', 62281), false); + assert.strictEqual(bodyCitesTicket(null, 62281), false); +}); + +// The precision bug the URL check exists to prevent: GitHub's tokeniser can +// surface a PR for a longer number that starts with the same digits. +test('bodyCitesTicket: a longer ticket number is not a match (issue #11)', () => { + assert.strictEqual(bodyCitesTicket('https://core.trac.wordpress.org/ticket/658200', 65820), false); + assert.strictEqual(bodyCitesTicket('https://core.trac.wordpress.org/ticket/65820', 65820), true); + // A trailing slash or anchor still matches. + assert.strictEqual(bodyCitesTicket('https://core.trac.wordpress.org/ticket/65820#comment:3', 65820), true); +}); + +test('parseLinkedPrs: keeps only PRs whose body cites the ticket (issue #11)', () => { + const json = { items: [ + item(101), + item(102, { body: 'unrelated work, mentions 62281 in passing only' }), + item(103, { pull_request: undefined, body: 'Trac ticket: https://core.trac.wordpress.org/ticket/62281' }) + ] }; + const prs = parseLinkedPrs(json, 62281); + assert.deepStrictEqual(prs.map((p) => p.number), [101], 'only the verified PR survives; the issue and the passing mention drop'); +}); + +test('parseLinkedPrs: newest first, and duplicates collapse (issue #11)', () => { + const json = { items: [ + item(1, { updated_at: '2026-01-01T00:00:00Z' }), + item(2, { updated_at: '2026-08-06T00:00:00Z' }), + item(2, { updated_at: '2026-08-06T00:00:00Z' }) + ] }; + const prs = parseLinkedPrs(json, 62281); + assert.deepStrictEqual(prs.map((p) => p.number), [2, 1]); +}); + +test('parseLinkedPrs: a closed PR is marked closed, not dropped (issue #11)', () => { + const prs = parseLinkedPrs({ items: [item(7, { state: 'closed' })] }, 62281); + assert.strictEqual(prs[0].state, 'closed'); +}); + +test('parseLinkedPrs: an empty or malformed response yields an empty list, not a throw (issue #11)', () => { + assert.deepStrictEqual(parseLinkedPrs({ items: [] }, 62281), []); + assert.deepStrictEqual(parseLinkedPrs({}, 62281), []); + assert.deepStrictEqual(parseLinkedPrs(null, 62281), []); +}); + +// The distinction the panel depends on: an exhausted rate limit must never read +// as "no patches on this ticket". +test('classifyHttpFailure: a spent rate limit is told apart from a plain error (issue #11)', () => { + assert.strictEqual(classifyHttpFailure(403, { 'x-ratelimit-remaining': '0' }), 'rate-limited'); + assert.strictEqual(classifyHttpFailure(429, {}), 'rate-limited'); + assert.strictEqual(classifyHttpFailure(401, { 'x-ratelimit-remaining': '0' }), 'rate-limited'); + // GitHub's secondary/abuse limit: a 403 with Retry-After while the primary + // quota is not yet spent — the burst case on a shared IP. + assert.strictEqual(classifyHttpFailure(403, { 'x-ratelimit-remaining': '57', 'retry-after': '60' }), 'rate-limited'); + // A 403 that is not about any rate limit is a real error. + assert.strictEqual(classifyHttpFailure(403, { 'x-ratelimit-remaining': '57' }), 'error'); + assert.strictEqual(classifyHttpFailure(500, {}), 'error'); + assert.strictEqual(classifyHttpFailure(404, {}), 'error'); +});