From 475355edb87693bb970d28d6674c295df476684f Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Sat, 24 Oct 2026 15:13:10 +0800 Subject: [PATCH] Support selected repair commands in PR conversations Accept a maintainer's /repair selected comment for the exact checked versions and report revision in a managed maintenance PR. Validate the canonical comment, current permission, PR and App-owned Check before atomic D1 admission. Retain one App-owned status reply and the exact dispatched workflow identity; replayed deliveries cannot dispatch a second workflow. Overlap independent canonical reads so the first command can update its Check and conversation reply within the existing eight-second webhook deadline. The live Hydra pilot dispatched successfully but timed out before these updates; an authorized redelivery recovered status without another workflow. Keep that initial failure visible until a corrected live pilot proves the first delivery. Validation: 186 Worker tests, native Workers/D1 concurrency and comment probes, and the pinned Wrangler dry-run bundle pass. The latency regression fails on the original source at eight seconds and passes at about 6.6 seconds with 550 ms per GitHub call. Permission and PR-revision race regressions also pass. The harmless pilot changes no dependency files. Terminal workflow completion and production report integration remain separate delivery steps. --- docs/node-maintenance-operator.md | 71 ++++- docs/node-maintenance.md | 14 + .../migrations/0001_comment_requests.sql | 8 + repair-app/runtime-smoke.mjs | 38 ++- repair-app/schema.sql | 8 +- repair-app/worker.mjs | 205 ++++++++++---- repair-app/worker.test.mjs | 257 +++++++++++++++++- 7 files changed, 537 insertions(+), 64 deletions(-) create mode 100644 repair-app/migrations/0001_comment_requests.sql diff --git a/docs/node-maintenance-operator.md b/docs/node-maintenance-operator.md index d6a2c03..957fdc5 100644 --- a/docs/node-maintenance-operator.md +++ b/docs/node-maintenance-operator.md @@ -6,8 +6,10 @@ service. Repository maintainers install the shared App; they do not host it. The receiver verifies signed webhook deliveries and reads canonical repository metadata with a token restricted to one repository. For managed PRs it also reads the canonical PR and displays the selected package versions in the App's -**Dependency repair** Check. A maintainer can request one harmless pilot run -through its native action. D1 retains the active request and returned Actions run +**Dependency repair** Check. A maintainer can select fixes in the PR description +and post `/repair selected` in the Conversation tab. The App replies there with +the request status and exact Actions link. The native Check action remains an +optional secondary entry point. D1 retains the active request and returned Actions run identity. Dependency repairs and completion handling belong to later slices. ## Check the selected batch @@ -34,13 +36,43 @@ only Contents read, Pull requests read and Checks write. Action-event tokens also grant Actions write, restricted to the same repository. The service never updates another App's Check. -## Request the selected batch +## Request the selected batch from the PR conversation + +The report includes the head and base revisions in one hidden line outside the +selection block (replace these illustrative hashes with the exact PR revisions): + +```markdown + +``` + +Tick the allowed fixes in the PR description, then post a new comment containing +only `/repair selected`. Use GitHub's normal **Comment** button to submit it. +Neither ticking a checkbox nor editing an existing comment starts an attempt. +There is no need to visit Checks. Request status appears in an App-owned reply; +supporting Check details remain available separately. + +The receiver accepts only a newly created human comment on a managed PR. It +re-fetches the comment and verifies its author, exact body, PR association and +unchanged creation/update timestamps. It checks current maintainer permission, +the selections captured in the signed comment event, the report's head/base +binding, and the current canonical PR/Check fingerprint. A stale report or +selection starts no run and produces a reply asking for a fresh command. +Unrelated comments and edits are ignored. Missing or edited/deleted commands +and unmanaged PRs do not authorize an attempt. + +For this unreleased integration, reports/pilot descriptions must include the +revision line. Existing reports without it cannot authorize comment dispatch. +The production report renderer is still a later slice; it is not deployed. + +### Existing Check action Review the exact versions in the Check summary, then click **Run selected fixes**. The service reads current write, maintain or admin permission and verifies the canonical PR and Check again. A stale selection or head refreshes the Check and requires another click; it starts no run. +### Shared admission and recovery + The request UUID is the signed GitHub delivery ID. A single D1 insert reserves one active request per installation, repository and PR before dispatch. Replayed deliveries and competing clicks cannot admit another active request. The Check @@ -54,6 +86,37 @@ in the Check. The fixture in `repair-app/pilot/` validates those inputs without checking out code or changing dependency files. A successful pilot does not mean a dependency repair was published. +Comment requests additionally retain the source comment ID, reply ID and exact +selections. A unique source-comment index prevents the same comment from +authorizing a second attempt, even under a different delivery UUID. Only the +winning reservation creates a status reply. Its ID is saved before dispatch; +a lost reply POST or failed save retains the lock and starts no run. Replays +update only the recorded reply owned by this App on this PR. They never retry +an ambiguous reply POST. PR refreshes use read-only PR tokens and do not edit +conversation replies. + +The control Check stays completed/neutral while its text changes. Real GitHub +did not reopen the previously completed Check when asked for `in_progress`. +The Actions link, and later canonical completion handling, supply run state. + +## Enable comment delivery after review + +This change is local and unreleased. Before testing the reviewed revision live: + +1. Add **Issues: read** and change **Pull requests** to **read/write** on the App. + Subscribe to **Issue comment** events. Existing installations must accept the + new permissions. Keep Contents read, Checks write and Actions write. +2. For an existing slice-3 database, apply + `repair-app/migrations/0001_comment_requests.sql` exactly once. It preserves + all existing request rows and active locks. New databases use `schema.sql` + instead; do not apply both initial schema and migration to one database. +3. Deploy the reviewed Worker. Put the correct revision marker and selections + in the pilot description, then verify a real command, exact inputs, reply + link, replay and stale-command rejection against the harmless fixture. + +No live permission, database, deployment or repository change is performed by +these instructions. The local/native tests cannot replace that acceptance. + ## Check the receiver locally Use Node 24 and run these commands from `repair-app/`: @@ -97,7 +160,7 @@ with the exact request inputs, repository, workflow and actor. Completion updates and releasing finished requests arrive in slice 4. Until then, even a finished harmless pilot keeps its active lock. Do not describe its -in-progress Check as proof that Actions is still running. +neutral Check as proof that Actions is still running. Keep App credentials in operator storage and Worker secrets. Keep verification receipts outside the repository, without credentials or webhook payloads. A diff --git a/docs/node-maintenance.md b/docs/node-maintenance.md index 34b1632..79f33bc 100644 --- a/docs/node-maintenance.md +++ b/docs/node-maintenance.md @@ -281,6 +281,20 @@ It also checks security and still obeys the cooldown and upgrade scope. It does not enable automatic upgrades. Nothing merges automatically. See [reports and PR behavior](../node/maintenance.md#reports-and-prs) for details. +## Interactive repairs (unreleased) + +The repair integration under development keeps selection and submission in the +PR's **Conversation** tab. Tick the fixes you allow in the PR description, then +post a new comment containing only `/repair selected` using **Comment**. The App +replies with the request status and the exact Actions link. Checkbox edits alone +start no work. The Checks tab is optional for this flow. + +This requires an installed repair App, a current report that supports comment +requests, and the fixed repair workflow. The current pilot only validates inputs; +it changes no dependencies. Completion reporting and the production repair +renderer are still later delivery slices. Nothing merges automatically. +See the [service reference](node-maintenance-operator.md) for current limits. + ## Toolchain selection The basic setup follows project declarations. Explicit or inherited constraints diff --git a/repair-app/migrations/0001_comment_requests.sql b/repair-app/migrations/0001_comment_requests.sql new file mode 100644 index 0000000..f08aab5 --- /dev/null +++ b/repair-app/migrations/0001_comment_requests.sql @@ -0,0 +1,8 @@ +-- Apply once to an existing slice-3 database before deploying comment support. +-- New databases use schema.sql, which already includes these columns/index. +ALTER TABLE requests ADD COLUMN comment_id INTEGER CHECK (comment_id > 0); +ALTER TABLE requests ADD COLUMN status_comment_id INTEGER CHECK (status_comment_id > 0); +ALTER TABLE requests ADD COLUMN selections_json TEXT; +CREATE UNIQUE INDEX one_comment_request + ON requests (installation_id, repository_id, comment_id) + WHERE comment_id IS NOT NULL; diff --git a/repair-app/runtime-smoke.mjs b/repair-app/runtime-smoke.mjs index 73b49be..9afcab6 100644 --- a/repair-app/runtime-smoke.mjs +++ b/repair-app/runtime-smoke.mjs @@ -88,6 +88,10 @@ const nativeCheck = {id: 77, app: {id: 42}, name: 'Dependency repair', head_sha: external_id: `node-repair:12:${nativeFingerprint}`, status: 'completed', conclusion: 'neutral'}; const documentedCheck = value => { const response = {...value}; delete response.actions; return response; }; const nativeCalls = []; +const commandComment = {id: 501, body: '/repair selected', user: {id: 9, type: 'User', login: 'maintainer'}, + issue_url: 'https://api.github.com/repos/fixture/repo/issues/12', + created_at: '2026-10-24T05:00:00Z', updated_at: '2026-10-24T05:00:00Z'}; +let statusComment; const admissionService = new Miniflare(convertV4MiniflareOptions({ modules: true, scriptPath: new URL('./worker.mjs', import.meta.url).pathname, compatibilityDate: '2026-10-19', d1Databases: ['DB'], @@ -108,6 +112,15 @@ const admissionService = new Miniflare(convertV4MiniflareOptions({ if (path === '/repos/fixture/repo') return Response.json({id: 7, full_name: 'fixture/repo', default_branch: 'main'}); if (path === '/repos/fixture/repo/pulls/12') return Response.json(nativePR); if (path === '/repos/fixture/repo/collaborators/maintainer/permission') return Response.json({permission: 'maintain', user: {login: 'maintainer'}}); + if (path === '/repos/fixture/repo/issues/comments/501') return Response.json(commandComment); + if (path === '/repos/fixture/repo/issues/12/comments' && request.method === 'POST') { + statusComment = {id: 600, ...body, performed_via_github_app: {id: 42}, issue_url: commandComment.issue_url}; + return Response.json(statusComment); + } + if (path === '/repos/fixture/repo/issues/comments/600') { + if (request.method === 'PATCH') Object.assign(statusComment, body); + return Response.json(statusComment); + } if (path === '/repos/fixture/repo/check-runs/77') { if (request.method === 'PATCH') Object.assign(nativeCheck, body); return Response.json(request.method === 'GET' ? documentedCheck(nativeCheck) : nativeCheck); @@ -153,7 +166,7 @@ try { assert.equal(rows.results.length, 1);assert.equal(rows.results[0].state, 'dispatched');assert.equal(rows.results[0].run_id, 900); assert.equal(nativeCalls.filter(c => c.path.endsWith('/dispatches')).length, 1); assert.equal(nativeCheck.external_id, `node-repair-request:${rows.results[0].id}`); - assert.equal(nativeCheck.status, 'in_progress');assert.deepEqual(nativeCheck.actions, []); + assert.equal(nativeCheck.status, 'completed');assert.equal(nativeCheck.conclusion, 'neutral');assert.deepEqual(nativeCheck.actions, []); // A queued A click must not admit the refreshed B selection or a changed base. await db.exec('DELETE FROM requests'); @@ -190,6 +203,29 @@ try { assert.equal(nativeCalls.filter(c => c.path.endsWith('/dispatches')).length, 2); assert.equal(JSON.parse(nativeCalls.findLast(c => c.path.endsWith('/dispatches')).body.inputs.selections)[0].to, '2.14.3'); console.log('Native D1 passed: concurrent atomic admission, UUID replay, scoped active locks, ambiguous-lock retention, complete clicked Check identity binding across stale selection/base changes, missing identity rejection, and one fresh exact dispatch.'); + + await db.exec('DELETE FROM requests'); + Object.assign(nativeCheck, {external_id: `node-repair:12:${nativeFingerprint}`, status: 'completed', conclusion: 'neutral'}); + nativePR.body += `\n`; + const commentEvent = {action: 'created', installation: {id: 3}, repository: {id: 7, full_name: 'fixture/repo'}, + issue: {number: 12, body: nativePR.body, pull_request: {url: 'https://api.github.com/repos/fixture/repo/pulls/12'}}, + comment: commandComment, sender: commandComment.user}; + const submit = id => { + const input = signed(commentEvent, 'issue_comment');input.headers['x-github-delivery'] = id; + return admissionService.dispatchFetch('https://worker.test/webhook', input); + }; + const commandID = randomUUID(); + assert.deepEqual((await Promise.all([submit(commandID), submit(randomUUID())])).map(r => r.status), [200, 200]); + assert.equal((await submit(commandID)).status, 200); + const commentRows = (await db.prepare('SELECT * FROM requests').all()).results; + assert.equal(commentRows.length, 1);assert.equal(commentRows[0].comment_id, 501); + assert.equal(commentRows[0].status_comment_id, 600);assert.equal(commentRows[0].run_id, 900); + assert.match(statusComment.body, /Workflow started/);assert.match(statusComment.body, /launch-editor/); + assert.match(statusComment.body, /actions\/runs\/900/); + assert.equal(nativeCalls.filter(c => c.path.endsWith('/dispatches')).length, 3); + assert.equal(nativeCalls.filter(c => c.path.endsWith('/12/comments') && c.method === 'POST').length, 1); + assert.ok(nativeCalls.every(c => c.body?.status !== 'in_progress')); + console.log('Native conversation command passed: real local D1 deduplicates concurrent/replayed comments, retains one reply, and links the exact dispatched run without requiring Check navigation.'); } finally { await admissionService.dispose(); } diff --git a/repair-app/schema.sql b/repair-app/schema.sql index 66036b8..a65bd4f 100644 --- a/repair-app/schema.sql +++ b/repair-app/schema.sql @@ -16,8 +16,14 @@ CREATE TABLE IF NOT EXISTS requests ( run_id INTEGER CHECK (run_id > 0), run_url TEXT, html_url TEXT, - conclusion TEXT + conclusion TEXT, + comment_id INTEGER CHECK (comment_id > 0), + status_comment_id INTEGER CHECK (status_comment_id > 0), + selections_json TEXT ); CREATE UNIQUE INDEX IF NOT EXISTS one_active_request ON requests (installation_id, repository_id, pr) WHERE state IN ('reserved', 'dispatched', 'unknown'); +CREATE UNIQUE INDEX IF NOT EXISTS one_comment_request + ON requests (installation_id, repository_id, comment_id) + WHERE comment_id IS NOT NULL; diff --git a/repair-app/worker.mjs b/repair-app/worker.mjs index 4be3538..600522d 100644 --- a/repair-app/worker.mjs +++ b/repair-app/worker.mjs @@ -13,8 +13,17 @@ const ACTION = {label: 'Run selected fixes', description: 'Run the selected batc const WORKFLOW = 'dependency-repair.yml'; const ACTIVE = "state IN ('reserved', 'dispatched', 'unknown')"; const MAX_INPUT_CHARS = 65_535; +const COMMAND = '/repair selected'; const positiveID = value => Number.isSafeInteger(value) && value > 0; +function repairComment(payload) { + return payload.action === 'created' && positiveID(payload.issue?.number) && + positiveID(payload.comment?.id) && typeof payload.issue.pull_request?.url === 'string' && + typeof payload.comment.body === 'string' && payload.comment.body.trim() === COMMAND && + payload.sender?.type === 'User' && payload.comment.user?.type === 'User' && + positiveID(payload.sender.id) && payload.sender.id === payload.comment.user.id; +} + export function selections(body) { if (typeof body !== 'string' || body.split(START).length !== 2 || body.split(END).length !== 2) { throw new Error('Expected one selection block'); @@ -173,8 +182,9 @@ async function readRepository(payload, env, signal, delivery) { const number = payload.number; const hasPR = Number.isSafeInteger(number) && number > 0; const isAction = delivery !== undefined; + const isComment = isAction && repairComment(payload); const permissions = hasPR || isAction ? {contents: 'read', pull_requests: 'read', checks: 'write', - ...(isAction ? {actions: 'write'} : {})} : {contents: 'read'}; + ...(isAction ? {actions: 'write'} : {}), ...(isComment ? {pull_requests: 'write', issues: 'read'} : {})} : {contents: 'read'}; const token = await installationToken(env, payload.installation?.id, repository.id, signal, permissions); const current = await api(token, `/repos/${repository.full_name}`, 'GET', undefined, signal); if (current?.id !== repository.id || current.full_name?.toLowerCase() !== repository.full_name.toLowerCase()) { @@ -215,13 +225,40 @@ function sameCheckIdentity(left, right) { left.head_sha === right.head_sha && left.external_id === right.external_id; } -async function showRequest(token, repository, row, signal) { +export function requestStatus(row, items = []) { + if (row.selections_json) items = JSON.parse(row.selections_json); + const title = row.state === 'unknown' ? 'Request requires inspection' : row.run_id ? 'Workflow started' : 'Request reserved'; + const summary = row.run_id ? `[View workflow run](${row.html_url}). This pilot performs no dependency repair.` : + 'A request is reserved. Inspect Actions and the request record before recovery; do not retry dispatch automatically.'; + return {title, summary, body: `\n### Dependency repair: ${title}\n\n` + + (items.length ? items.map(item => `- \`${item.name}\` \`${item.from} → ${item.to}\``).join('\n') + '\n\n' : '') + + `${summary}\n\nSelections remain in the PR description. No merge is authorized by this request.`}; +} + +async function showRequest(token, repository, row, signal, env, items = [], createComment = false) { + const {title, summary, body} = requestStatus(row, items); await api(token, `/repos/${repository.full_name}/check-runs/${row.check_id}`, 'PATCH', { - external_id: `node-repair-request:${row.id}`, status: 'in_progress', actions: [], - output: {title: row.state === 'unknown' ? 'Request requires inspection' : row.run_id ? 'Workflow started' : 'Request reserved', - summary: row.run_id ? `[View workflow run](${row.html_url}). This pilot performs no dependency repair.` : - 'A request is reserved. Inspect Actions and the request record before recovery; do not retry dispatch automatically.'}, + external_id: `node-repair-request:${row.id}`, status: 'completed', conclusion: 'neutral', actions: [], + output: {title, summary}, }, signal); + if (!row.comment_id || !env) return; + const prefix = `/repos/${repository.full_name}/issues`; + if (row.status_comment_id) { + const comment = await api(token, `${prefix}/comments/${row.status_comment_id}`, 'GET', undefined, signal); + if (String(comment.performed_via_github_app?.id) !== String(env.APP_ID) || + comment.issue_url !== `https://api.github.com${prefix}/${row.pr}` || + !comment.body?.startsWith(`\n`)) throw new Error('Status comment identity changed'); + await api(token, `${prefix}/comments/${row.status_comment_id}`, 'PATCH', {body}, signal); + } else if (createComment) { + // Only the winning admission creates a reply. An ambiguous POST is never retried. + const comment = await api(token, `${prefix}/${row.pr}/comments`, 'POST', {body}, signal); + if (!positiveID(comment?.id) || String(comment.performed_via_github_app?.id) !== String(env.APP_ID) || + comment.issue_url !== `https://api.github.com${prefix}/${row.pr}`) throw new Error('Invalid status comment identity'); + const saved = await env.DB.prepare('UPDATE requests SET status_comment_id = ? WHERE id = ? AND installation_id = ? AND repository_id = ? AND status_comment_id IS NULL') + .bind(comment.id, row.id, row.installation_id, repository.id).run(); + if (saved.meta?.changes !== 1) throw new Error('Status comment identity was not retained'); + row.status_comment_id = comment.id; + } } async function preserveActiveCheck(token, repository, pr, env, installation, signal) { @@ -231,68 +268,95 @@ async function preserveActiveCheck(token, repository, pr, env, installation, sig return row; } -async function restoreRequestCheck(token, repository, row, env, signal) { +async function restoreRequestCheck(token, repository, row, env, signal, conversation = false) { for (let attempt = 0; attempt < MAX_RECONCILIATIONS; attempt++) { const check = await api(token, `/repos/${repository.full_name}/check-runs/${row.check_id}`, 'GET', undefined, signal); if (!ownedCheck(check, env.APP_ID, row.head) || !(check.external_id === `node-repair-request:${row.id}` || new RegExp(`^node-repair:${row.pr}:[a-f0-9]{64}$`).test(check.external_id || ''))) { throw new Error('Active Check identity changed'); } - await showRequest(token, repository, row, signal); + await showRequest(token, repository, row, signal, conversation ? env : undefined); // Dispatch may advance while either Check call is pending. Repair any stale write. const current = await env.DB.prepare('SELECT * FROM requests WHERE id = ? AND installation_id = ? AND repository_id = ?') .bind(row.id, row.installation_id, repository.id).first(); if (!current || !['reserved', 'dispatched', 'unknown'].includes(current.state)) throw new Error('Active request state changed'); - if (current.state === row.state && current.run_id === row.run_id && current.html_url === row.html_url) return; + if (current.state === row.state && current.run_id === row.run_id && current.html_url === row.html_url && + current.status_comment_id === row.status_comment_id) return; row = current; } throw new Error('Active Check state did not settle'); } async function requestRepair(token, repository, payload, env, delivery, signal) { - const clicked = payload.check_run; - const clickedMatch = selectionCheck(clicked, env.APP_ID); - if (!env.DB || !clickedMatch) return; + const isComment = repairComment(payload); + let clicked = payload.check_run, clickedMatch = selectionCheck(clicked, env.APP_ID); + if (!env.DB || (!isComment && !clickedMatch)) return; const installation = payload.installation.id; // A retained delivery is never another dispatch attempt, even after a crash. - const replay = await env.DB.prepare('SELECT * FROM requests WHERE id = ? AND installation_id = ? AND repository_id = ?') - .bind(delivery, installation, repository.id).first(); + const replay = await env.DB.prepare('SELECT * FROM requests WHERE installation_id = ? AND repository_id = ? AND (id = ? OR comment_id = ?)') + .bind(installation, repository.id, delivery, isComment ? payload.comment.id : null).first(); if (replay) { if (['reserved', 'dispatched', 'unknown'].includes(replay.state)) { - await restoreRequestCheck(token, repository, replay, env, signal); + await restoreRequestCheck(token, repository, replay, env, signal, isComment); } return; } const prefix = `/repos/${repository.full_name}`; - const check = await api(token, `${prefix}/check-runs/${clicked.id}`, 'GET', undefined, signal); + const actor = payload.sender?.login; + if (payload.sender?.type !== 'User' || typeof actor !== 'string' || !/^[a-z0-9](?:[a-z0-9-]{0,37}[a-z0-9])?$/i.test(actor)) return; + // These reads are independent. Both must finish before any reply or admission. + const [permission, candidate] = await Promise.all([ + api(token, `${prefix}/collaborators/${actor}/permission`, 'GET', undefined, signal), + isComment ? commentSelection(token, repository, payload, env, signal) : undefined, + ]); + if (!['write', 'maintain', 'admin'].includes(permission?.permission) || permission.user?.login?.toLowerCase() !== actor.toLowerCase()) return; + const submitted = candidate; + if (isComment) { + if (!submitted) return; + if (submitted.active) { + await restoreRequestCheck(token, repository, submitted.active, env, signal, true); + return; + } + if (submitted.rejection) { + await api(token, `${prefix}/issues/${payload.issue.number}/comments`, 'POST', {body: submitted.rejection}, signal); + return; + } + clicked = submitted.check; + clickedMatch = selectionCheck(clicked, env.APP_ID); + if (!clickedMatch) return; + } + const number = Number(clickedMatch[1]); + const [check, pr] = await Promise.all([ + api(token, `${prefix}/check-runs/${clicked.id}`, 'GET', undefined, signal), + api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal), + ]); if (!sameCheckIdentity(check, clicked)) { - await refreshSelectionCheck(token, repository, Number(clickedMatch[1]), env.APP_ID, signal, env, installation); + await refreshSelectionCheck(token, repository, number, env.APP_ID, signal, env, installation); return; } - const match = clickedMatch; - const number = Number(match[1]), actor = payload.sender?.login; - if (payload.sender?.type !== 'User' || typeof actor !== 'string' || !/^[a-z0-9](?:[a-z0-9-]{0,37}[a-z0-9])?$/i.test(actor)) return; - const permission = await api(token, `${prefix}/collaborators/${actor}/permission`, 'GET', undefined, signal); - if (!['write', 'maintain', 'admin'].includes(permission?.permission) || permission.user?.login?.toLowerCase() !== actor.toLowerCase()) return; - const pr = await api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal); if (!managedPR(pr, repository, number)) return; let items, inputs; try { items = selections(pr.body); inputs = workflowInputs(pr, items, delivery); } catch { items = []; } const fingerprint = await selectionFingerprint(pr, items); const current = await api(token, `${prefix}/check-runs/${check.id}`, 'GET', undefined, signal); - if (!items.length || !sameCheckIdentity(current, clicked) || !ownedCheck(current, env.APP_ID, pr.head.sha) || + if (!items.length || (submitted && fingerprint !== submitted.fingerprint) || + !sameCheckIdentity(current, clicked) || !ownedCheck(current, env.APP_ID, pr.head.sha) || current.status !== 'completed' || current.conclusion !== 'neutral' || current.external_id !== `node-repair:${number}:${fingerprint}`) { await refreshSelectionCheck(token, repository, number, env.APP_ID, signal, env, installation); + if (isComment) await api(token, `${prefix}/issues/${number}/comments`, 'POST', { + body: 'No run started: the report or PR revision changed. Review the refreshed selections and post `/repair selected` again.', + }, signal); return; } signal.throwIfAborted(); const row = {id: delivery, installation_id: installation, repository_id: repository.id, repository: repository.full_name, pr: number, head: pr.head.sha, base: pr.base.sha, dispatched_ref: repository.default_branch, fingerprint, - check_id: current.id, actor_login: actor, created_at: Date.now(), state: 'reserved'}; + check_id: current.id, actor_login: actor, created_at: Date.now(), state: 'reserved', + comment_id: isComment ? payload.comment.id : null, selections_json: JSON.stringify(items)}; const inserted = await env.DB.prepare(`INSERT OR IGNORE INTO requests - (id, installation_id, repository_id, repository, pr, head, base, dispatched_ref, fingerprint, check_id, actor_login, created_at, state) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`) + (id, installation_id, repository_id, repository, pr, head, base, dispatched_ref, fingerprint, check_id, actor_login, created_at, state, comment_id, selections_json) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`) .bind(...Object.values(row)).run(); if (inserted.meta?.changes !== 1) { if (inserted.meta?.changes !== 0) throw new Error('Invalid admission result'); @@ -305,7 +369,7 @@ async function requestRepair(token, repository, payload, env, delivery, signal) return; } // Failure here retains reserved state, so redelivery cannot repeat the POST. - await showRequest(token, repository, row, signal); + await showRequest(token, repository, row, signal, env, items, isComment); signal.throwIfAborted(); try { const run = await api(token, `${prefix}/actions/workflows/${WORKFLOW}/dispatches`, 'POST', { @@ -318,7 +382,7 @@ async function requestRepair(token, repository, payload, env, delivery, signal) const saved = await env.DB.prepare("UPDATE requests SET state = 'dispatched', run_id = ?, run_url = ?, html_url = ? WHERE id = ? AND installation_id = ? AND repository_id = ? AND state = 'reserved'") .bind(run.workflow_run_id, run.run_url, run.html_url, delivery, installation, repository.id).run(); if (saved.meta?.changes !== 1) throw new Error('Dispatch identity was not retained'); - await showRequest(token, repository, {...row, state: 'dispatched', run_id: run.workflow_run_id, html_url: run.html_url}, signal); + await showRequest(token, repository, {...row, state: 'dispatched', run_id: run.workflow_run_id, html_url: run.html_url}, signal, env, items); } catch { // The POST may have reached GitHub. Never release this lock or repeat it. await env.DB.prepare("UPDATE requests SET state = 'unknown' WHERE id = ? AND installation_id = ? AND repository_id = ? AND state = 'reserved' AND run_id IS NULL") @@ -327,6 +391,62 @@ async function requestRepair(token, repository, payload, env, delivery, signal) } } +async function commentSelection(token, repository, payload, env, signal) { + const prefix = `/repos/${repository.full_name}`, number = payload.issue.number; + if (payload.issue.pull_request.url !== `https://api.github.com${prefix}/pulls/${number}`) return; + const [comment, pr] = await Promise.all([ + api(token, `${prefix}/issues/comments/${payload.comment.id}`, 'GET', undefined, signal) + .catch(error => { if (error.status === 404) return null; throw error; }), + api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal), + ]); + if (!comment || comment.id !== payload.comment.id || comment.issue_url !== `https://api.github.com${prefix}/issues/${number}` || + comment.user?.id !== payload.sender.id || comment.user?.type !== 'User' || + comment.user.login !== payload.sender.login || comment.body !== payload.comment.body || + comment.created_at !== payload.comment.created_at || comment.updated_at !== comment.created_at || + !Number.isFinite(Date.parse(comment.created_at))) return; + if (!managedPR(pr, repository, number)) return; + const rejection = {rejection: 'No run started: the report or selections changed, or no valid fixes are selected. Wait for a current report, review its checkboxes, and post `/repair selected` again.'}; + const active = await activeRequest(env, payload.installation.id, repository.id, number); + if (active) return active.comment_id ? {active} : { + rejection: `No new run started: another repair request is active. ${requestStatus(active).summary}`, + }; + let items; + try { + items = selections(pr.body); + if (!items.length || JSON.stringify(items) !== JSON.stringify(selections(payload.issue.body))) return rejection; + // issue_comment includes the description snapshot, but has no PR head/base. + // The report binds those revisions explicitly so a queued command cannot + // authorize a later head, even when its checked package versions are equal. + const lines = payload.issue.body.split(/\r?\n/).filter(line => line.includes('`) return rejection; + workflowInputs(pr, items); + } catch { return rejection; } + const fingerprint = await selectionFingerprint(pr, items); + const check = await findSelectionCheck(token, repository, pr, env.APP_ID, `node-repair:${number}:${fingerprint}`, signal); + if (!check) return rejection; + return {check, fingerprint}; +} + +async function findSelectionCheck(token, repository, pr, appID, external, signal) { + let check, total, read = 0; + for (let page = 1; ; page++) { + const list = await api(token, `/repos/${repository.full_name}/commits/${pr.head.sha}/check-runs?check_name=Dependency%20repair&app_id=${appID}&filter=all&per_page=100&page=${page}`, 'GET', undefined, signal); + if (!Array.isArray(list?.check_runs) || !Number.isSafeInteger(list.total_count) || list.total_count < 0 || + (total !== undefined && list.total_count !== total) || list.check_runs.length !== Math.min(100, list.total_count - read)) { + throw new Error('Incomplete Check list'); + } + total = list.total_count; + for (const candidate of list.check_runs) { + if (String(candidate.app?.id) !== String(appID) || candidate.name !== CHECK || + candidate.head_sha !== pr.head.sha || candidate.external_id !== external) continue; + if (!positiveID(candidate.id) || candidate.status !== 'completed') throw new Error('Repair Check is not idle'); + if (!check || candidate.id > check.id) check = candidate; + } + read += list.check_runs.length; + if (read === total) return check; + } +} + async function refreshSelectionCheck(token, repository, number, appID, signal, env = {}, installation) { const prefix = `/repos/${repository.full_name}`; for (let attempt = 0; attempt < MAX_RECONCILIATIONS; attempt++) { @@ -338,37 +458,20 @@ async function refreshSelectionCheck(token, repository, number, appID, signal, e try { items = selections(pr.body); workflowInputs(pr, items); } catch { valid = false; items = []; } const external = `node-repair:${number}:${await selectionFingerprint(pr, items)}`; const state = `${valid ? 'valid' : 'invalid'}:${external}`; - let check, total, read = 0; - // Fold complete pages without retaining history; all calls share the webhook deadline. - for (let page = 1; ; page++) { - const list = await api(token, `${prefix}/commits/${pr.head.sha}/check-runs?check_name=Dependency%20repair&app_id=${appID}&filter=all&per_page=100&page=${page}`, 'GET', undefined, signal); - if (!Array.isArray(list?.check_runs) || !Number.isSafeInteger(list.total_count) || list.total_count < 0 || - (total !== undefined && list.total_count !== total) || - list.check_runs.length !== Math.min(100, list.total_count - read)) { - throw new Error('Incomplete Check list'); - } - total = list.total_count; - for (const candidate of list.check_runs) { - if (String(candidate.app?.id) !== String(appID) || candidate.name !== CHECK || - candidate.head_sha !== pr.head.sha || candidate.external_id !== external) continue; - if (!positiveID(candidate.id) || candidate.status !== 'completed') throw new Error('Repair Check is not idle'); - if (!check || candidate.id > check.id) check = candidate; - } - read += list.check_runs.length; - if (read === total) break; - } + const check = await findSelectionCheck(token, repository, pr, appID, external, signal); const output = { title: !valid ? 'Repair selections need attention' : items.length ? `${items.length} selected fix${items.length === 1 ? '' : 'es'}` : 'No fixes selected', summary: !valid ? 'Use one valid selection block in the PR description. No run was started.' : items.length ? items.map(item => `- \`${item.name}\` \`${item.from} → ${item.to}\``).join('\n') + (active ? '\n\nAnother request is active. Inspect it before requesting a new attempt.' : env.DB ? - '\n\nReview these exact versions, then click Run selected fixes. The pilot only validates inputs.' : + '\n\nPost `/repair selected` in the PR conversation to request these fixes. The native Run selected fixes action is also available here. The pilot only validates inputs.' : '\n\nSelections are displayed for review. Running fixes is not enabled yet.') : 'Select the allowed fixes in the PR description. Editing checkboxes does not start a run.', }; await api(token, `${prefix}/check-runs${check ? `/${check.id}` : ''}`, check ? 'PATCH' : 'POST', { ...(check ? {} : {name: CHECK, head_sha: pr.head.sha}), external_id: external, - status: 'completed', conclusion: 'neutral', output, actions: valid && items.length && env.DB && !active ? [ACTION] : [], + status: 'completed', conclusion: 'neutral', output, + actions: valid && items.length && env.DB && !active ? [ACTION] : [], }, signal); // Repair a concurrent idle write that raced an action's reservation/Check update. const reserved = await preserveActiveCheck(token, repository, pr, env, installation, signal); @@ -405,6 +508,8 @@ export default { } else if (event === 'check_run' && payload.action === 'requested_action' && payload.requested_action?.identifier === ACTION.identifier && String(payload.check_run?.app?.id) === String(env.APP_ID)) { await readRepository(payload, env, controller.signal, request.headers.get('x-github-delivery')); + } else if (event === 'issue_comment' && repairComment(payload)) { + await readRepository(payload, env, controller.signal, request.headers.get('x-github-delivery')); } controller.signal.throwIfAborted(); return new Response('Accepted', {status: 200}); diff --git a/repair-app/worker.test.mjs b/repair-app/worker.test.mjs index a51aec6..481e0c7 100644 --- a/repair-app/worker.test.mjs +++ b/repair-app/worker.test.mjs @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import {createHmac, createPrivateKey, generateKeyPairSync, verify} from 'node:crypto'; import {beforeEach, test} from 'node:test'; import {readFileSync} from 'node:fs'; -import worker, {api, installationToken, selections, selectionFingerprint, verifySignature, workflowInputs} from './worker.mjs'; +import worker, {api, installationToken, selections, selectionFingerprint, verifySignature, workflowInputs, requestStatus} from './worker.mjs'; const keys = generateKeyPairSync('rsa', {modulusLength: 2048, privateKeyEncoding: {type: 'pkcs8', format: 'pem'}}); @@ -723,7 +723,7 @@ test('an authorized current action reserves once and retains the documented run assert.equal(row.state, 'dispatched');assert.equal(row.run_id, 900);assert.equal(row.check_id, 77); assert.equal(row.installation_id, 3);assert.equal(row.repository_id, 7);assert.equal(row.dispatched_ref, 'main'); assert.equal(row.completed_at, null);assert.equal(row.conclusion, null); - assert.equal(f.check.external_id, `node-repair-request:${delivery}`);assert.equal(f.check.status, 'in_progress'); + assert.equal(f.check.external_id, `node-repair-request:${delivery}`);assert.equal(f.check.status, 'completed');assert.equal(f.check.conclusion, 'neutral'); assert.deepEqual(f.check.actions, []);assert.match(f.check.output.summary, /actions\/runs\/900/); const before = f.calls.length; assert.equal((await f.click()).status, 200); @@ -731,6 +731,243 @@ test('an authorized current action reserves once and retains the documented run assert.equal(f.calls.length - before, 4); // Token, repository and owned Check reconciliation; no new attempt. }); +async function commentMock(t, options = {}) { + const comments = new Map(); + const command = {id: 501, body: '/repair selected', user: {id: 9, type: 'User', login: 'maintainer'}, + issue_url: 'https://api.github.com/repos/fixture/repo/issues/12', + created_at: '2026-10-24T05:00:00Z', updated_at: '2026-10-24T05:00:00Z'}; + const f = await actionMock(t, {...options, async onCall(call, context) { + const intercepted = await options.onCall?.(call, {...context, comments, command}); + if (intercepted) return intercepted; + if (call.path === '/repos/fixture/repo/issues/comments/501' || + call.path === `/repos/fixture/repo/issues/comments/${command.id}`) return json(command); + if (call.path === '/repos/fixture/repo/issues/12/comments' && call.method === 'POST') { + const reply = {id: 600, body: call.body.body, performed_via_github_app: {id: 42}, + issue_url: command.issue_url}; + comments.set(reply.id, reply);return json(reply); + } + if (call.path === '/repos/fixture/repo/issues/comments/600') { + const reply = comments.get(600); + if (call.method === 'PATCH') Object.assign(reply, call.body); + return json(reply); + } + }}); + f.pr.body += `\n`; + const event = {...payload, action: 'created', issue: {number: 12, body: f.pr.body, + pull_request: {url: 'https://api.github.com/repos/fixture/repo/pulls/12'}}, + comment: {...command, user: {...command.user}}, sender: {...command.user}}; + const submit = (id = delivery, changes = {}) => worker.fetch(signed({...event, ...changes}, 'issue_comment', + {'x-github-delivery': id}), f.environment); + return {...f, command, comments, commentEvent: event, submit}; +} + +test('a conversation command submits description selections and updates one App-owned reply', async t => { + const f = await commentMock(t); + assert.equal((await f.submit()).status, 200); + assert.equal(f.dispatches().length, 1);assert.equal(f.rows().length, 1); + assert.equal(f.rows()[0].comment_id, 501);assert.equal(f.rows()[0].status_comment_id, 600); + assert.deepEqual(f.calls[0].body.permissions, {contents: 'read', pull_requests: 'write', checks: 'write', actions: 'write', issues: 'read'}); + assert.match(f.comments.get(600).body, /Workflow started/); + assert.match(f.comments.get(600).body, /launch-editor.*2\.14\.1 → 2\.14\.2/); + assert.match(f.comments.get(600).body, /actions\/runs\/900/); + const id = 'a4e535a5-2a7d-491d-858a-3df51ce211df'; + assert.equal((await f.submit()).status, 200);assert.equal((await f.submit(id)).status, 200); + assert.equal(f.calls.filter(c => c.path.endsWith('/12/comments') && c.method === 'POST').length, 1); + assert.equal(f.dispatches().length, 1);assert.match(f.comments.get(600).body, /launch-editor/); + f.db.sql.prepare("UPDATE requests SET state = 'success' WHERE id = ?").run(delivery); + assert.equal((await f.submit(id)).status, 200);assert.equal(f.dispatches().length, 1); +}); + +test('a conversation command finishes its status updates within the webhook deadline at realistic API latency', {timeout: 12_000}, async t => { + const f = await commentMock(t, {async onCall(call) { + await new Promise((resolve, reject) => { + const abort = () => { clearTimeout(timer); reject(new Error('fixture API deadline')); }; + const timer = setTimeout(() => { call.signal.removeEventListener('abort', abort); resolve(); }, 550); + if (call.signal.aborted) abort(); + else call.signal.addEventListener('abort', abort, {once: true}); + }); + }}); + assert.equal((await f.submit()).status, 200); + assert.equal(f.rows().length, 1);assert.equal(f.dispatches().length, 1); + assert.equal(f.rows()[0].state, 'dispatched'); + assert.match(f.check.output.summary, /actions\/runs\/900/); + assert.match(f.comments.get(600).body, /Workflow started/); + assert.match(f.comments.get(600).body, /actions\/runs\/900/); +}); + +for (const [name, mutate] of [ + ['quoted command', e => {e.comment.body = '> /repair selected';}], + ['extra command text', e => {e.comment.body = '/repair selected extra';}], + ['edited event', e => {e.action = 'edited';}], + ['deleted event', e => {e.action = 'deleted';}], + ['ordinary issue', e => {delete e.issue.pull_request;}], + ['bot comment', e => {e.sender.type = e.comment.user.type = 'Bot';}], + ['mismatched author', e => {e.comment.user.id = 10;}], +]) { + test(`${name} is not a repair submission`, async t => { + const f = await commentMock(t), event = structuredClone(f.commentEvent);mutate(event); + assert.equal((await f.submit(delivery, event)).status, 200); + assert.equal(f.calls.length, 0);assert.equal(f.rows().length, 0); + }); +} + +for (const [name, mutate] of [ + ['changed selections', f => {f.pr.body = f.pr.body.replace('2.14.2', '2.14.3');}], + ['changed head with refreshed Check', f => {f.pr.head.sha = 'c'.repeat(40);f.check.head_sha = f.pr.head.sha;}], + ['changed base', f => {f.pr.base.sha = 'c'.repeat(40);}], + ['missing revision binding', f => {f.commentEvent.issue.body = fixtures.cases[0].body;}], + ['multiple revision bindings', f => {f.commentEvent.issue.body += '\n' + f.commentEvent.issue.body.split('\n').at(-1);}], + ['mismatched canonical comment ID', f => {f.command.id = 999;}], + ['edited canonical comment', f => {f.command.updated_at = '2026-10-24T05:01:00Z';}], + ['canonical body changed', f => {f.command.body += ' extra';}], + ['foreign comment owner', f => {f.command.user.id = 10;}], + ['foreign issue URL', f => {f.command.issue_url = 'https://api.github.com/repos/other/repo/issues/12';}], +]) { + test(`a ${name} cannot authorize a different repair`, async t => { + const f = await commentMock(t);mutate(f); + f.check.external_id = `node-repair:12:${await selectionFingerprint(f.pr, selections(f.pr.body))}`; + assert.equal((await f.submit()).status, 200); + assert.equal(f.rows().length, 0);assert.equal(f.dispatches().length, 0); + if (['changed selections', 'changed head with refreshed Check', 'changed base', 'missing revision binding', 'multiple revision bindings'].includes(name)) { + assert.match(f.comments.get(600).body, /No run started/); + } else assert.equal(f.comments.size, 0); + }); +} + +test('comment authorization still requires current maintainer permission', async t => { + const f = await commentMock(t, {permission: 'read'}); + assert.equal((await f.submit()).status, 200);assert.equal(f.rows().length, 0); + assert.equal(f.dispatches().length, 0);assert.equal(f.comments.size, 0); +}); + +test('parallel selection reads cannot write before the permission result is known', {timeout: 2_000}, async t => { + let releasePermission, selectionRead; + const permissionPending = new Promise(resolve => { releasePermission = resolve; }); + const selectionPending = new Promise(resolve => { selectionRead = resolve; }); + const f = await commentMock(t, {permission: 'read', async onCall(call) { + if (call.path.endsWith('/permission')) await permissionPending; + if (call.path.includes('/commits/') && call.path.endsWith('/check-runs')) selectionRead(); + }}); + const pending = f.submit(); + try { + await selectionPending; + assert.equal(f.rows().length, 0);assert.equal(f.comments.size, 0); + assert.equal(f.calls.filter(c => c.method !== 'GET' && !c.path.endsWith('/access_tokens')).length, 0); + } finally { releasePermission(); } + assert.equal((await pending).status, 200); + assert.equal(f.rows().length, 0);assert.equal(f.comments.size, 0);assert.equal(f.dispatches().length, 0); +}); + +test('a head change after parallel comment validation is rejected before reservation', async t => { + let prReads = 0; + const f = await commentMock(t, {onCall(call, {pr}) { + if (call.path.endsWith('/pulls/12') && ++prReads === 2) pr.head.sha = 'c'.repeat(40); + }}); + assert.equal((await f.submit()).status, 200); + assert.equal(f.rows().length, 0);assert.equal(f.dispatches().length, 0); + assert.match(f.comments.get(600).body, /No run started: the report or PR revision changed/); +}); + +test('a lost reply POST retains the reservation and neither reply nor dispatch is retried', async t => { + const f = await commentMock(t, {onCall(call) { + if (call.path.endsWith('/12/comments') && call.method === 'POST') throw Error('lost reply response'); + }}); + assert.equal((await f.submit()).status, 500);assert.equal(f.rows()[0].state, 'reserved'); + assert.equal((await f.submit()).status, 200);assert.equal(f.dispatches().length, 0); + assert.equal(f.calls.filter(c => c.path.endsWith('/12/comments') && c.method === 'POST').length, 1); +}); + +test('a deleted source comment cannot reserve or dispatch', async t => { + const f = await commentMock(t, {onCall(call) { + if (call.path.endsWith('/comments/501')) return new Response(null, {status: 404}); + }}); + assert.equal((await f.submit()).status, 200);assert.equal(f.rows().length, 0); + assert.equal(f.dispatches().length, 0);assert.equal(f.comments.size, 0); +}); + +test('an invalid reply owner retains the reservation without dispatch', async t => { + const f = await commentMock(t, {onCall(call) { + if (call.path.endsWith('/12/comments') && call.method === 'POST') return json({id: 600, + performed_via_github_app: {id: 99}, issue_url: 'https://api.github.com/repos/fixture/repo/issues/12'}); + }}); + assert.equal((await f.submit()).status, 500);assert.equal(f.rows()[0].state, 'reserved'); + assert.equal(f.rows()[0].status_comment_id, null);assert.equal(f.dispatches().length, 0); +}); + +test('a delayed reply refresh cannot erase the newly retained run link', {timeout: 2000}, async t => { + const dispatchReached = Promise.withResolvers(), finishDispatch = Promise.withResolvers(); + const replyReached = Promise.withResolvers(), finishReply = Promise.withResolvers(); + let dispatchPending = false, held = false; + const f = await commentMock(t, {async onCall(call) { + if (call.path.endsWith('/dispatches')) { + dispatchPending = true;dispatchReached.resolve();await finishDispatch.promise; + } else if (dispatchPending && !held && call.path.endsWith('/comments/600') && call.method === 'PATCH') { + held = true;replyReached.resolve();await finishReply.promise; + } + }}); + const submit = f.submit();await dispatchReached.promise; + const replay = f.submit();await replyReached.promise; + finishDispatch.resolve();assert.equal((await submit).status, 200); + assert.match(f.comments.get(600).body, /actions\/runs\/900/); + finishReply.resolve();assert.equal((await replay).status, 200); + assert.match(f.comments.get(600).body, /Workflow started/); + assert.match(f.comments.get(600).body, /actions\/runs\/900/); + assert.equal(f.dispatches().length, 1); +}); + +test('comment and native action submissions share the same active lock', async t => { + const f = await commentMock(t); + assert.deepEqual((await Promise.all([f.submit(), f.click('a4e535a5-2a7d-491d-858a-3df51ce211df')])).map(r => r.status), [200, 200]); + assert.equal(f.dispatches().length, 1);assert.equal(f.rows().length, 1); +}); + +test('a replaced status comment is not overwritten on delivery replay', async t => { + const f = await commentMock(t);assert.equal((await f.submit()).status, 200); + f.comments.get(600).performed_via_github_app.id = 99; + const writes = f.calls.filter(c => c.path.endsWith('/comments/600') && c.method === 'PATCH').length; + assert.equal((await f.submit()).status, 500); + assert.equal(f.calls.filter(c => c.path.endsWith('/comments/600') && c.method === 'PATCH').length, writes); + assert.equal(f.dispatches().length, 1); +}); + +test('a second comment updates the active conversation reply without another dispatch', async t => { + const f = await commentMock(t);assert.equal((await f.submit()).status, 200); + f.command.id = f.commentEvent.comment.id = 502; + assert.equal((await f.submit('a4e535a5-2a7d-491d-858a-3df51ce211df')).status, 200); + assert.equal(f.dispatches().length, 1);assert.equal(f.comments.size, 1); +}); + +test('an existing native request is explained in the conversation without a new dispatch', async t => { + const f = await commentMock(t);assert.equal((await f.click()).status, 200); + assert.equal((await f.submit('a4e535a5-2a7d-491d-858a-3df51ce211df')).status, 200); + assert.equal(f.dispatches().length, 1);assert.equal(f.rows().length, 1); + assert.match(f.comments.get(600).body, /another repair request is active/); + assert.match(f.comments.get(600).body, /actions\/runs\/900/); +}); + +test('the source-comment index blocks a second UUID after the first request terminates', async t => { + const f = await commentMock(t);assert.equal((await f.submit()).status, 200); + f.db.sql.prepare("UPDATE requests SET state = 'success' WHERE id = ?").run(delivery); + const inserted = f.db.sql.prepare(`INSERT OR IGNORE INTO requests + (id, installation_id, repository_id, repository, pr, head, base, dispatched_ref, fingerprint, check_id, actor_login, created_at, state, comment_id) + SELECT 'new-uuid', installation_id, repository_id, repository, pr, head, base, dispatched_ref, fingerprint, check_id, actor_login, created_at, 'reserved', comment_id FROM requests`).run(); + assert.equal(inserted.changes, 0);assert.equal(f.rows().length, 1); +}); + +test('existing slice-3 rows survive the one-time comment migration', () => { + const db = new DatabaseSync(':memory:'); + try { + const oldSchema = readFileSync(new URL('./schema.sql', import.meta.url), 'utf8') + .replace(/,\n comment_id[\s\S]*?selections_json TEXT/, '').replace(/CREATE UNIQUE INDEX IF NOT EXISTS one_comment_request[\s\S]*/, ''); + db.exec(oldSchema); + db.exec("INSERT INTO requests (id, installation_id, repository_id, repository, pr, head, base, dispatched_ref, fingerprint, check_id, actor_login, created_at, state) VALUES ('old', 3, 7, 'fixture/repo', 12, 'head', 'base', 'main', 'fingerprint', 77, 'maintainer', 1, 'unknown')"); + db.exec(readFileSync(new URL('./migrations/0001_comment_requests.sql', import.meta.url), 'utf8')); + const row = db.prepare('SELECT * FROM requests').get(); + assert.equal(row.state, 'unknown');assert.equal(row.comment_id, null); + assert.equal(row.status_comment_id, null);assert.equal(row.selections_json, null); + } finally { db.close(); } +}); + test('a queued click for selection A cannot dispatch after the canonical Check refreshes to selection B', async t => { const f = await actionMock(t); f.pr.body = fixtures.cases[0].body.replace('2.14.2', '2.14.3'); @@ -851,7 +1088,7 @@ test('a failed in-progress Check update retains reserved state without dispatch' assert.equal((await f.click()).status, 500);assert.equal(f.rows()[0].state, 'reserved');assert.equal(f.dispatches().length, 0); assert.equal((await f.click()).status, 200);assert.equal(f.dispatches().length, 0); assert.equal(f.check.external_id, `node-repair-request:${delivery}`); - assert.equal(f.check.status, 'in_progress');assert.deepEqual(f.check.actions, []); + assert.equal(f.check.status, 'completed');assert.equal(f.check.conclusion, 'neutral');assert.deepEqual(f.check.actions, []); assert.equal(f.check.output.title, 'Request reserved'); }); @@ -909,13 +1146,17 @@ test('failure after retaining the known run cannot erase its identity or dispatc assert.equal((await f.click()).status, 200);assert.equal(f.dispatches().length, 1); }); -test('concurrent current clicks admit one request through the actual SQLite unique index', async t => { +test('concurrent current clicks admit one request through the actual SQLite unique index', {timeout: 2000}, async t => { let arrivals = 0, release; + const reads = new Map(); const ready = new Promise(resolve => {release = resolve;}); const f = await actionMock(t, {async onCall(call) { if (call.path.endsWith('/check-runs/77') && call.method === 'GET') { - if (++arrivals === 4) release(); - if (arrivals > 2) await ready; + const count = (reads.get(call.signal) || 0) + 1;reads.set(call.signal, count); + if (count === 2) { + if (++arrivals === 2) release(); + await ready; + } } }}); const result = await Promise.all([f.click(), f.click('94e535a5-2a7d-491d-858a-3df51ce211de')]); @@ -936,7 +1177,7 @@ test('PR edits preserve a reserved/dispatched Check request identity and do not const f = await actionMock(t);assert.equal((await f.click()).status, 200); f.pr.body = fixtures.cases[1].body; assert.equal((await worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), f.environment)).status, 200); - assert.equal(f.check.external_id, `node-repair-request:${delivery}`);assert.equal(f.check.status, 'in_progress'); + assert.equal(f.check.external_id, `node-repair-request:${delivery}`);assert.equal(f.check.status, 'completed');assert.equal(f.check.conclusion, 'neutral'); assert.deepEqual(f.check.actions, []);assert.equal(f.dispatches().length, 1); }); @@ -961,7 +1202,7 @@ test('an idle refresh that races admission restores the reserved Check identity' }}); assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), f.environment)).status, 200); assert.equal(raced, true);assert.equal(f.check.external_id, `node-repair-request:${delivery}`); - assert.equal(f.check.status, 'in_progress');assert.deepEqual(f.check.actions, []);assert.equal(f.dispatches().length, 0); + assert.equal(f.check.status, 'completed');assert.equal(f.check.conclusion, 'neutral');assert.deepEqual(f.check.actions, []);assert.equal(f.dispatches().length, 0); }); test('a different active Check blocks a fresh idle click and links the active request', async t => {