From 6e3192f0d7fdd5bda4c8fd4a81af201ec7996531 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Sat, 24 Oct 2026 03:52:52 +0800 Subject: [PATCH] [node-maintenance-repair/3] Dispatch one authorized selected batch Bind the native Run selected fixes action to the signed Check identity, current maintainer permission and canonical PR head, base and exact selected versions. Keep each idle Check ID attached to its selection fingerprint so old confirmations cannot authorize another batch. Reserve one active request per installation, repository and PR with a D1 partial unique index. Dispatch only the fixed default-branch workflow with five validated inputs, retain its returned run identity, and preserve reservations after ambiguous outcomes without automatic retry. Add a harmless read-only installation fixture and operator reference. Completion handling belongs to slice 4; dependency repairs remain disabled. Live dispatch acceptance is pending. Validation: 140 Worker tests, native Workers/D1 concurrency and consent probes, 1294 repository tests, lint, format, typing and bundle checks passed. A fresh independent full-scope review is clean. --- docs/node-maintenance-operator.md | 60 +++- repair-app/pilot/README.md | 21 ++ repair-app/pilot/dependency-repair.yml | 84 +++++ repair-app/runtime-smoke.mjs | 120 +++++++ repair-app/schema.sql | 23 ++ repair-app/worker.mjs | 231 +++++++++++-- repair-app/worker.test.mjs | 438 ++++++++++++++++++++++++- repair-app/wrangler.jsonc | 7 +- 8 files changed, 933 insertions(+), 51 deletions(-) create mode 100644 repair-app/pilot/README.md create mode 100644 repair-app/pilot/dependency-repair.yml create mode 100644 repair-app/schema.sql diff --git a/docs/node-maintenance-operator.md b/docs/node-maintenance-operator.md index bcb8479..d6a2c03 100644 --- a/docs/node-maintenance-operator.md +++ b/docs/node-maintenance-operator.md @@ -6,8 +6,9 @@ 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. Check actions, dispatch, storage and dependency -repairs belong to later slices. +**Dependency repair** Check. A maintainer can request one harmless pilot run +through its native action. D1 retains the active request and returned Actions run +identity. Dependency repairs and completion handling belong to later slices. ## Check the selected batch @@ -27,9 +28,31 @@ Malformed or missing selections produce a Check that asks for attention. Editing selections or changing the PR head refreshes the Check from canonical GitHub data. The Check displays the exact selected versions, bound to the PR -number, head and base. It has no Run button in this slice. Checkbox edits do not -start a run. The token grants only Contents read, Pull requests read and Checks -write; the service never updates another App's Check. +number, head and base. With D1 configured, a nonempty valid batch shows +**Run selected fixes**. Checkbox edits do not start a run. PR-event tokens grant +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 + +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. + +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 +loses its action and retains the request identity across PR edits. A new head can +have its own selection Check, but the existing PR lock still blocks dispatch. + +Only `.github/workflows/dependency-repair.yml` on the current default branch is +dispatched, with the five C2 inputs. The API must return HTTP 200 with the exact +run ID and matching repository URLs. The known run link is saved in D1 and shown +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. ## Check the receiver locally @@ -43,21 +66,38 @@ npm run check ``` The unit suite reports coverage for the receiver, selection parser and Check -updates. `selection-fixtures.json` holds the shared C1 cases and expected +updates, action authorization, fixed dispatch and request-state failures. +`selection-fixtures.json` holds the shared C1 cases and expected fingerprints. The runtime test executes the receiver in local Cloudflare Workers with synthetic credentials and intercepted GitHub -requests. The final command bundles the preview with `--dry-run`; it does not +requests and an actual local D1 binding for concurrency and replay checks. +The final command bundles the preview with `--dry-run`; it does not deploy. The CI job runs the same checks without App or Cloudflare credentials. ## Stop deliveries or recover Disable the App webhook or uninstall the affected installation to stop new deliveries. Restore a previous approved Worker version when one exists. This -receiver slice has no request database, dispatched runs or repository writes. +service does not cancel an already dispatched Actions run; cancel it separately. +Restoring Worker code does not roll back the D1 database. Webhook processing shares an eight-second budget across input and GitHub reads. -Timed-out requests fail without an automatic retry. After recovery, redeliver -failed deliveries through the GitHub App's webhook delivery controls. +Timed-out requests fail without an automatic dispatch retry. Before a reservation +exists, a failed delivery can be redelivered after recovery. After reservation, +inspect the request record and Actions first: + +- `reserved`: a crash or Check update failure may have interrupted the attempt. +- `unknown`: the dispatch response was lost, invalid or unexpected. A run may exist. +- `dispatched`: the exact returned run ID and URLs are retained. + +Active locks never expire automatically. Replaying the same delivery never +repeats its dispatch POST. Do not delete an ambiguous row or start another attempt +until an operator proves no run remains active and reconciles any observed run +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. Keep App credentials in operator storage and Worker secrets. Keep verification receipts outside the repository, without credentials or webhook payloads. A diff --git a/repair-app/pilot/README.md b/repair-app/pilot/README.md new file mode 100644 index 0000000..bca9071 --- /dev/null +++ b/repair-app/pilot/README.md @@ -0,0 +1,21 @@ +# Harmless infrastructure fixture + +For the approved pilot, copy `dependency-repair.yml` to +`.github/workflows/dependency-repair.yml` on the repository's default branch. +Review that concrete change before merging it. This fixture is the service's +fixed dispatch target; there is no alternate dispatch endpoint or pilot mode. + +The five C2 inputs are validated and retained in the Actions job summary. +The workflow checks out no code, uses read-only permissions and changes no +repository files. A successful run proves input delivery, not dependency repair. + +For controlled completion probes, select the synthetic package `pilot-failure` +to fail after writing the receipt. Select `pilot-cancellation` to wait for two +minutes so an operator can cancel the run. Use any increasing stable same-minor +patch pair. These probes belong only to this installation fixture. + +Slice 3 proves action admission, replay rejection and one returned run ID. +Completion and releasing active requests belong to slice 4. Until that slice is +deployed, a finished pilot still retains its active request lock. Inspect its +known Actions run and D1 record; do not remove the lock or dispatch again merely +because the Check still says the workflow started. diff --git a/repair-app/pilot/dependency-repair.yml b/repair-app/pilot/dependency-repair.yml new file mode 100644 index 0000000..a3a83af --- /dev/null +++ b/repair-app/pilot/dependency-repair.yml @@ -0,0 +1,84 @@ +name: Dependency repair infrastructure pilot +on: + workflow_dispatch: + inputs: + request-id: + description: Verified request UUID + required: true + type: string + pull-request: + description: Managed PR number + required: true + type: string + expected-head: + description: Confirmed PR head SHA + required: true + type: string + expected-base: + description: Confirmed PR base SHA + required: true + type: string + selections: + description: Exact selected items as compact JSON + required: true + type: string +permissions: + contents: read +jobs: + validate-inputs: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Validate and summarize the five dispatch inputs + env: + REQUEST_ID: ${{ inputs['request-id'] }} + PULL_REQUEST: ${{ inputs['pull-request'] }} + EXPECTED_HEAD: ${{ inputs['expected-head'] }} + EXPECTED_BASE: ${{ inputs['expected-base'] }} + SELECTIONS: ${{ inputs.selections }} + shell: python + run: | + import json + import os + import re + import time + + def require(condition): + if not condition: + raise SystemExit('Invalid pilot input') + + require(re.fullmatch(r'[a-f0-9]{8}(?:-[a-f0-9]{4}){3}-[a-f0-9]{12}', os.environ['REQUEST_ID'])) + require(re.fullmatch(r'[1-9][0-9]*', os.environ['PULL_REQUEST'])) + for key in ['EXPECTED_HEAD', 'EXPECTED_BASE']: + require(re.fullmatch(r'[a-f0-9]{40}', os.environ[key])) + items = json.loads(os.environ['SELECTIONS']) + require(isinstance(items, list) and 1 <= len(items) <= 20) + names = [] + for item in items: + require(isinstance(item, dict) and list(item) == ['name', 'from', 'to']) + require(all(isinstance(value, str) for value in item.values())) + name = item['name'] + require(len(name) <= 214 and name not in ['node_modules', 'favicon.ico']) + require(re.fullmatch(r'(?:@[a-z0-9][a-z0-9._-]*/)?[a-z0-9][a-z0-9._-]*', name)) + for version in [item['from'], item['to']]: + require(re.fullmatch(r'(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)', version)) + before, after = item['from'].split('.'), item['to'].split('.') + require(before[:2] == after[:2]) + require((len(before[2]), before[2]) < (len(after[2]), after[2])) + names.append(name) + require(names == sorted(set(names))) + with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as summary: + summary.write('### Infrastructure pilot input receipt\n\n') + summary.write('No PR code was checked out. No dependency files or repository contents were changed.\n\n') + summary.write('```json\n' + json.dumps({ + 'request-id': os.environ['REQUEST_ID'], + 'pull-request': os.environ['PULL_REQUEST'], + 'expected-head': os.environ['EXPECTED_HEAD'], + 'expected-base': os.environ['EXPECTED_BASE'], + 'selections': items, + }, indent=2) + '\n```\n') + # Controlled infrastructure probes exist only in this harmless fixture. + if 'pilot-failure' in names: + raise SystemExit('Requested pilot failure') + if 'pilot-cancellation' in names: + time.sleep(120) diff --git a/repair-app/runtime-smoke.mjs b/repair-app/runtime-smoke.mjs index dd6b9e4..73b49be 100644 --- a/repair-app/runtime-smoke.mjs +++ b/repair-app/runtime-smoke.mjs @@ -73,3 +73,123 @@ try { } finally { await service.dispose(); } + +// A separate native Worker instance uses an actual local D1 binding. These +// constraints and concurrent admissions are not simulated with an object map. +const {readFileSync} = await import('node:fs'); +const {createHash, randomUUID} = await import('node:crypto'); +let nativeItems = [{name: 'launch-editor', from: '2.14.1', to: '2.14.2'}]; +const nativePR = {number: 12, state: 'open', user: {type: 'Bot'}, + body: '\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n', + head: {sha: 'a'.repeat(40), ref: 'maintenance/node-security', repo: {id: 7}}, + base: {sha: 'b'.repeat(40), ref: 'main', repo: {id: 7}}}; +let nativeFingerprint = createHash('sha256').update(JSON.stringify([12, nativePR.head.sha, nativePR.base.sha, nativeItems])).digest('hex'); +const nativeCheck = {id: 77, app: {id: 42}, name: 'Dependency repair', head_sha: nativePR.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 admissionService = new Miniflare(convertV4MiniflareOptions({ + modules: true, scriptPath: new URL('./worker.mjs', import.meta.url).pathname, + compatibilityDate: '2026-10-19', d1Databases: ['DB'], + bindings: {APP_ID: '42', APP_PRIVATE_KEY: keys.privateKey, WEBHOOK_SECRET: secret}, + outboundService: async request => { + const url = new URL(request.url), path = url.pathname; + const body = request.method === 'POST' || request.method === 'PATCH' ? await request.json() : null; + assert.equal(url.origin, 'https://api.github.com'); + assert.equal(request.headers.get('X-GitHub-Api-Version'), '2026-03-10'); + nativeCalls.push({method: request.method, path, body}); + if (path === '/app/installations/3/access_tokens') { + assert.deepEqual(body.repository_ids, [7]); + assert.equal(body.permissions.contents, 'read'); + assert.equal(body.permissions.checks, 'write'); + return Response.json({token: 'ghs_42.native-action-token'}); + } + assert.equal(request.headers.get('authorization'), 'Bearer ghs_42.native-action-token'); + 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/check-runs/77') { + if (request.method === 'PATCH') Object.assign(nativeCheck, body); + return Response.json(request.method === 'GET' ? documentedCheck(nativeCheck) : nativeCheck); + } + if (path.includes('/commits/') && path.endsWith('/check-runs')) return Response.json({total_count: 1, check_runs: [documentedCheck(nativeCheck)]}); + assert.equal(path, '/repos/fixture/repo/actions/workflows/dependency-repair.yml/dispatches'); + assert.equal(request.method, 'POST'); + assert.equal(body.ref, 'main'); + assert.deepEqual(Object.keys(body.inputs), ['request-id', 'pull-request', 'expected-head', 'expected-base', 'selections']); + assert.equal(body.inputs['pull-request'], '12');assert.equal(body.inputs['expected-head'], nativePR.head.sha); + assert.equal(body.inputs['expected-base'], nativePR.base.sha);assert.equal(body.inputs.selections, JSON.stringify(nativeItems)); + return Response.json({workflow_run_id: 900, run_url: 'https://api.github.com/repos/fixture/repo/actions/runs/900', + html_url: 'https://github.com/fixture/repo/actions/runs/900'}); + }, +})); +try { + const db = await admissionService.getD1Database('DB'); + await db.exec(readFileSync(new URL('./schema.sql', import.meta.url), 'utf8').replaceAll('\n', ' ')); + const insert = (id, installation = 3, repository = 7) => 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 (?, ?, ?, 'fixture/repo', 12, ?, ?, 'main', ?, 77, 'maintainer', 1, 'reserved')`) + .bind(id, installation, repository, nativePR.head.sha, nativePR.base.sha, nativeFingerprint).run(); + const admitted = await Promise.all(Array.from({length: 12}, () => insert(randomUUID()))); + assert.equal(admitted.reduce((sum, r) => sum + r.meta.changes, 0), 1); + const winner = await db.prepare('SELECT id FROM requests').first(); + assert.equal((await insert(winner.id)).meta.changes, 0); + assert.equal((await insert(randomUUID(), 4)).meta.changes, 1); + assert.equal((await insert(randomUUID(), 3, 8)).meta.changes, 1); + await db.prepare("UPDATE requests SET state = 'unknown' WHERE id = ?").bind(winner.id).run(); + assert.equal((await insert(randomUUID())).meta.changes, 0); + await db.exec('DELETE FROM requests'); + const click = (id, checkRun = nativeCheck) => { + const input = signed({action: 'requested_action', requested_action: {identifier: 'repair-selected'}, + installation: {id: 3}, repository: {id: 7, full_name: 'fixture/repo'}, + check_run: {...checkRun, app: {...checkRun.app}}, sender: {type: 'User', login: 'maintainer'}}, 'check_run'); + input.headers['x-github-delivery'] = id; + return admissionService.dispatchFetch('https://worker.test/webhook', input); + }; + const first = randomUUID(); + assert.deepEqual((await Promise.all([click(first), click(randomUUID())])).map(r => r.status), [200, 200]); + assert.equal((await click(first)).status, 200); + const rows = await db.prepare('SELECT * FROM requests').all(); + 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, []); + + // A queued A click must not admit the refreshed B selection or a changed base. + await db.exec('DELETE FROM requests'); + nativePR.body = nativePR.body.replace('2.14.2', '2.14.3'); + nativeItems = [{name: 'launch-editor', from: '2.14.1', to: '2.14.3'}]; + nativeFingerprint = createHash('sha256').update(JSON.stringify([12, nativePR.head.sha, nativePR.base.sha, nativeItems])).digest('hex'); + Object.assign(nativeCheck, {external_id: `node-repair:12:${nativeFingerprint}`, status: 'completed', conclusion: 'neutral', actions: []}); + const queuedA = {id: nativeCheck.id, app: {...nativeCheck.app}, name: nativeCheck.name, + head_sha: nativeCheck.head_sha, external_id: `node-repair:12:${createHash('sha256').update(JSON.stringify([12, nativePR.head.sha, 'b'.repeat(40), [{name: 'launch-editor', from: '2.14.1', to: '2.14.2'}]])).digest('hex')}`, + status: 'completed', conclusion: 'neutral'}; + const staleSelection = await click(randomUUID(), queuedA); + assert.equal(staleSelection.status, 200); + assert.equal((await db.prepare('SELECT * FROM requests').all()).results.length, 0); + assert.equal(nativeCalls.filter(c => c.path.endsWith('/dispatches')).length, 1); + + const queuedB = {...nativeCheck, app: {...nativeCheck.app}}; + nativePR.base.sha = 'c'.repeat(40); + nativeFingerprint = createHash('sha256').update(JSON.stringify([12, nativePR.head.sha, nativePR.base.sha, nativeItems])).digest('hex'); + Object.assign(nativeCheck, {external_id: `node-repair:12:${nativeFingerprint}`, status: 'completed', conclusion: 'neutral', actions: []}); + const staleBase = await click(randomUUID(), queuedB); + assert.equal(staleBase.status, 200); + assert.equal((await db.prepare('SELECT * FROM requests').all()).results.length, 0); + assert.equal(nativeCalls.filter(c => c.path.endsWith('/dispatches')).length, 1); + + const missingIdentity = {...queuedB}; + delete missingIdentity.head_sha; delete missingIdentity.external_id; + assert.equal((await click(randomUUID(), missingIdentity)).status, 200); + assert.equal((await db.prepare('SELECT * FROM requests').all()).results.length, 0); + + const fresh = await click(randomUUID(), {...nativeCheck, app: {...nativeCheck.app}}); + assert.equal(fresh.status, 200); + const freshRows = await db.prepare('SELECT * FROM requests').all(); + assert.equal(freshRows.results.length, 1); + 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.'); +} finally { + await admissionService.dispose(); +} diff --git a/repair-app/schema.sql b/repair-app/schema.sql new file mode 100644 index 0000000..66036b8 --- /dev/null +++ b/repair-app/schema.sql @@ -0,0 +1,23 @@ +CREATE TABLE IF NOT EXISTS requests ( + id TEXT PRIMARY KEY, + installation_id INTEGER NOT NULL CHECK (installation_id > 0), + repository_id INTEGER NOT NULL CHECK (repository_id > 0), + repository TEXT NOT NULL, + pr INTEGER NOT NULL CHECK (pr > 0), + head TEXT NOT NULL, + base TEXT NOT NULL, + dispatched_ref TEXT NOT NULL, + fingerprint TEXT NOT NULL, + check_id INTEGER NOT NULL CHECK (check_id > 0), + actor_login TEXT NOT NULL, + created_at INTEGER NOT NULL, + completed_at INTEGER, + state TEXT NOT NULL CHECK (state IN ('reserved', 'dispatched', 'unknown', 'success', 'failure', 'cancelled')), + run_id INTEGER CHECK (run_id > 0), + run_url TEXT, + html_url TEXT, + conclusion TEXT +); +CREATE UNIQUE INDEX IF NOT EXISTS one_active_request + ON requests (installation_id, repository_id, pr) + WHERE state IN ('reserved', 'dispatched', 'unknown'); diff --git a/repair-app/worker.mjs b/repair-app/worker.mjs index 00d88ee..4be3538 100644 --- a/repair-app/worker.mjs +++ b/repair-app/worker.mjs @@ -9,6 +9,11 @@ const PACKAGE = '(?:@[a-z0-9][a-z0-9._-]*/)?[a-z0-9][a-z0-9._-]*'; const LINE = new RegExp('^- \\[([ xX])\\] Allow `(' + PACKAGE + ')` `(' + VERSION + ') → (' + VERSION + ')` by updating its explicit pin\\.$'); const CHECK = 'Dependency repair'; const MAX_RECONCILIATIONS = 3; +const ACTION = {label: 'Run selected fixes', description: 'Run the selected batch in Actions', identifier: 'repair-selected'}; +const WORKFLOW = 'dependency-repair.yml'; +const ACTIVE = "state IN ('reserved', 'dispatched', 'unknown')"; +const MAX_INPUT_CHARS = 65_535; +const positiveID = value => Number.isSafeInteger(value) && value > 0; export function selections(body) { if (typeof body !== 'string' || body.split(START).length !== 2 || body.split(END).length !== 2) { @@ -38,6 +43,13 @@ export async function selectionFingerprint(pr, items) { .map(value => value.toString(16).padStart(2, '0')).join(''); } +export function workflowInputs(pr, items, requestID = '00000000-0000-0000-0000-000000000000') { + const inputs = {'request-id': requestID, 'pull-request': String(pr.number), + 'expected-head': pr.head.sha, 'expected-base': pr.base.sha, selections: JSON.stringify(items)}; + if (JSON.stringify(inputs).length > MAX_INPUT_CHARS) throw new Error('Workflow inputs exceed the payload limit'); + return inputs; +} + async function readBody(request, signal) { signal.throwIfAborted(); if (!request.body) return new Uint8Array(); @@ -134,7 +146,7 @@ export async function api(token, path, method = 'GET', body, signal = AbortSigna } catch { throw new Error('GitHub request failed'); } - if (!response.ok) { + if (!response.ok || (method === 'POST' && path.endsWith(`/workflows/${WORKFLOW}/dispatches`) && response.status !== 200)) { const error = new Error(`GitHub request failed (${response.status})`); error.status = response.status; throw error; @@ -153,66 +165,218 @@ export async function installationToken(env, installationID, repositoryID, signa return result.token; } -async function readRepository(payload, env, signal) { +async function readRepository(payload, env, signal, delivery) { const repository = payload.repository; if (typeof repository?.full_name !== 'string' || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repository.full_name)) { throw new Error('Invalid repository name'); } const number = payload.number; const hasPR = Number.isSafeInteger(number) && number > 0; - const permissions = hasPR ? {contents: 'read', pull_requests: 'read', checks: 'write'} : {contents: 'read'}; + const isAction = delivery !== undefined; + const permissions = hasPR || isAction ? {contents: 'read', pull_requests: 'read', checks: 'write', + ...(isAction ? {actions: 'write'} : {})} : {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()) { throw new Error('Repository identity changed'); } - if (hasPR) await refreshSelectionCheck(token, current, number, env.APP_ID, signal); + if (isAction) await requestRepair(token, current, payload, env, delivery, signal); + else if (hasPR) await refreshSelectionCheck(token, current, number, env.APP_ID, signal, env, payload.installation.id); +} + +function managedPR(pr, repository, number) { + return typeof repository.default_branch === 'string' && repository.default_branch && + pr?.number === number && pr.state === 'open' && pr.user?.type === 'Bot' && + pr.head?.repo?.id === repository.id && pr.base?.repo?.id === repository.id && + pr.head.ref === 'maintenance/node-security' && pr.base.ref === repository.default_branch && + /^[a-f0-9]{40}$/.test(pr.head.sha || '') && /^[a-f0-9]{40}$/.test(pr.base.sha || ''); +} + +async function activeRequest(env, installation, repository, number) { + return env.DB ? env.DB.prepare(`SELECT * FROM requests WHERE installation_id = ? AND repository_id = ? AND pr = ? AND ${ACTIVE}`) + .bind(installation, repository, number).first() : null; } -async function refreshSelectionCheck(token, repository, number, appID, signal) { +function ownedCheck(check, appID, head) { + return positiveID(check?.id) && String(check.app?.id) === String(appID) && check.name === CHECK && check.head_sha === head; +} + +function selectionCheck(check, appID) { + const match = /^node-repair:([1-9][0-9]*):([a-f0-9]{64})$/.exec(check?.external_id || ''); + if (!positiveID(check?.id) || String(check.app?.id) !== String(appID) || check.name !== CHECK || + !/^[a-f0-9]{40}$/.test(check.head_sha || '') || !match || + check.status !== 'completed' || check.conclusion !== 'neutral') return null; + return match; +} + +function sameCheckIdentity(left, right) { + return positiveID(left?.id) && positiveID(right?.id) && left.id === right.id && + String(left.app?.id) === String(right.app?.id) && left.name === right.name && + left.head_sha === right.head_sha && left.external_id === right.external_id; +} + +async function showRequest(token, repository, row, signal) { + 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.'}, + }, signal); +} + +async function preserveActiveCheck(token, repository, pr, env, installation, signal) { + const row = await activeRequest(env, installation, repository.id, pr.number); + if (!row || row.head !== pr.head.sha) return row; + await restoreRequestCheck(token, repository, row, env, signal); + return row; +} + +async function restoreRequestCheck(token, repository, row, env, signal) { + 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); + // 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; + 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 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(); + if (replay) { + if (['reserved', 'dispatched', 'unknown'].includes(replay.state)) { + await restoreRequestCheck(token, repository, replay, env, signal); + } + return; + } + const prefix = `/repos/${repository.full_name}`; + const check = await api(token, `${prefix}/check-runs/${clicked.id}`, 'GET', undefined, signal); + if (!sameCheckIdentity(check, clicked)) { + await refreshSelectionCheck(token, repository, Number(clickedMatch[1]), 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) || + current.status !== 'completed' || current.conclusion !== 'neutral' || + current.external_id !== `node-repair:${number}:${fingerprint}`) { + await refreshSelectionCheck(token, repository, number, env.APP_ID, signal, env, installation); + 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'}; + 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 (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`) + .bind(...Object.values(row)).run(); + if (inserted.meta?.changes !== 1) { + if (inserted.meta?.changes !== 0) throw new Error('Invalid admission result'); + const active = await activeRequest(env, installation, repository.id, number); + if (active?.check_id === current.id) await restoreRequestCheck(token, repository, active, env, signal); + else if (active) await api(token, `${prefix}/check-runs/${current.id}`, 'PATCH', { + actions: [], output: {title: 'Another request is active', summary: + `[Inspect the active Check](https://github.com/${repository.full_name}/runs/${active.check_id}). No new run was started.`}, + }, signal); + return; + } + // Failure here retains reserved state, so redelivery cannot repeat the POST. + await showRequest(token, repository, row, signal); + signal.throwIfAborted(); + try { + const run = await api(token, `${prefix}/actions/workflows/${WORKFLOW}/dispatches`, 'POST', { + ref: row.dispatched_ref, inputs, + }, signal); + if (!positiveID(run?.workflow_run_id) || run.run_url !== `https://api.github.com${prefix}/actions/runs/${run.workflow_run_id}` || + run.html_url !== `https://github.com/${repository.full_name}/actions/runs/${run.workflow_run_id}`) { + throw new Error('Invalid dispatch identity'); + } + 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); + } 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") + .bind(delivery, installation, repository.id).run(); + throw new Error('Dispatch requires inspection'); + } +} + +async function refreshSelectionCheck(token, repository, number, appID, signal, env = {}, installation) { const prefix = `/repos/${repository.full_name}`; for (let attempt = 0; attempt < MAX_RECONCILIATIONS; attempt++) { const pr = await api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal); - if (typeof repository.default_branch !== 'string' || !repository.default_branch || - pr?.number !== number || pr.state !== 'open' || pr.user?.type !== 'Bot' || - pr.head?.repo?.id !== repository.id || pr.base?.repo?.id !== repository.id || - pr.head.ref !== 'maintenance/node-security' || pr.base.ref !== repository.default_branch || - !/^[a-f0-9]{40}$/.test(pr.head.sha || '') || !/^[a-f0-9]{40}$/.test(pr.base.sha || '')) return; + if (!managedPR(pr, repository, number)) return; + const active = await preserveActiveCheck(token, repository, pr, env, installation, signal); + if (active?.head === pr.head.sha) return; let items = [], valid = true; - try { items = selections(pr.body); } catch { valid = false; } + 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}`; - const list = await api(token, `${prefix}/commits/${pr.head.sha}/check-runs?check_name=Dependency%20repair&app_id=${appID}&filter=all&per_page=100`, 'GET', undefined, signal); - if (!Array.isArray(list?.check_runs) || !Number.isSafeInteger(list.total_count) || - list.total_count < 0 || list.total_count > 100 || list.total_count !== list.check_runs.length) { - throw new Error('Incomplete Check list'); - } - const owned = list.check_runs.filter(check => String(check.app?.id) === String(appID) && - check.name === CHECK && check.head_sha === pr.head.sha && - new RegExp(`^node-repair:${number}:[a-f0-9]{64}$`).test(check.external_id || '')); - if (owned.some(check => !Number.isSafeInteger(check.id) || check.id <= 0 || check.status !== 'completed')) { - throw new Error('Repair Check is not idle'); + 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 = owned.reduce((newest, candidate) => !newest || candidate.id > newest.id ? candidate : newest, undefined); 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') + - '\n\nSelections are displayed for review. Running fixes is not enabled yet.' : + (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\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: [], + 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); + if (reserved?.head === pr.head.sha) return; const current = await api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal); - if (typeof repository.default_branch !== 'string' || !repository.default_branch || - current?.number !== number || current.state !== 'open' || current.user?.type !== 'Bot' || - current.head?.repo?.id !== repository.id || current.base?.repo?.id !== repository.id || - current.head.ref !== 'maintenance/node-security' || current.base.ref !== repository.default_branch || - !/^[a-f0-9]{40}$/.test(current.head.sha || '') || !/^[a-f0-9]{40}$/.test(current.base.sha || '')) return; + if (!managedPR(current, repository, number)) return; let currentItems = [], currentValid = true; - try { currentItems = selections(current.body); } catch { currentValid = false; } + try { currentItems = selections(current.body); workflowInputs(current, currentItems); } catch { currentValid = false; currentItems = []; } const currentExternal = `node-repair:${number}:${await selectionFingerprint(current, currentItems)}`; if (`${currentValid ? 'valid' : 'invalid'}:${currentExternal}` === state) return; } @@ -235,9 +399,12 @@ export default { if (!DELIVERY_ID.test(request.headers.get('x-github-delivery') || '')) throw new Error('Invalid delivery ID'); const payload = JSON.parse(new TextDecoder('utf-8', {fatal: true}).decode(body)); if (!payload || typeof payload !== 'object' || Array.isArray(payload)) throw new Error('Invalid payload'); - // PR edits refresh the selected batch only. Later slices own actions and dispatch. - if (request.headers.get('x-github-event') === 'pull_request' && PR_ACTIONS.includes(payload.action)) { + const event = request.headers.get('x-github-event'); + if (event === 'pull_request' && PR_ACTIONS.includes(payload.action)) { await readRepository(payload, env, controller.signal); + } 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')); } controller.signal.throwIfAborted(); return new Response('Accepted', {status: 200}); diff --git a/repair-app/worker.test.mjs b/repair-app/worker.test.mjs index 80175db..a51aec6 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} from './worker.mjs'; +import worker, {api, installationToken, selections, selectionFingerprint, verifySignature, workflowInputs} from './worker.mjs'; const keys = generateKeyPairSync('rsa', {modulusLength: 2048, privateKeyEncoding: {type: 'pkcs8', format: 'pem'}}); @@ -308,7 +308,10 @@ function checkMock({changePR, list = {total_count: 0, check_runs: []}, failPath} assert.equal(query.get('filter'), 'all'); assert.equal(query.get('check_name'), 'Dependency repair'); assert.equal(query.get('per_page'), '100'); - return json(list); + const page = typeof list === 'function' ? list(query) : list; + return json({...page, check_runs: page.check_runs.map(check => { + const response = {...check}; delete response.actions; return response; + })}); } if (path === '/repos/fixture/repo/check-runs' || path === '/repos/fixture/repo/check-runs/77' || path === '/repos/fixture/repo/check-runs/78') return json({id: 77}); @@ -317,7 +320,7 @@ function checkMock({changePR, list = {total_count: 0, check_runs: []}, failPath} return {calls, pr, repository}; } const ownCheck = (overrides = {}) => ({id: 77, app: {id: 42}, name: 'Dependency repair', - head_sha: 'a'.repeat(40), external_id: 'node-repair:12:' + '0'.repeat(64), status: 'completed', ...overrides}); + head_sha: 'a'.repeat(40), external_id: 'node-repair:12:33b17f82caff79dd2065972e92b5e5b83aa761be9c5799dcbbced028f0f60849', status: 'completed', ...overrides}); const listOf = (...checks) => ({total_count: checks.length, check_runs: checks}); const checkWrites = calls => calls.filter(call => call.path.includes('/check-runs') && call.method !== 'GET'); const managedPR = (body, headSha = 'a'.repeat(40), state = 'open') => ({number: 12, state, user: {type: 'Bot'}, body, @@ -367,6 +370,19 @@ test('selection edits and clearing the block replace the idle fingerprint and su assert.match(versions[3], /valid selection block/); }); +for (const [name, body] of [['changed', fixtures.cases[0].body.replace('2.14.2', '2.14.3')], + ['cleared', fixtures.cases[1].body]]) { + test(`a ${name} selection creates a new Check when the list response omits actions`, async () => { + const {calls, pr} = checkMock({changePR: current => { current.body = body; }, list: listOf(ownCheck())}); + assert.equal((await worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), env)).status, 200); + const writes = checkWrites(calls); + assert.equal(writes.length, 1); + assert.equal(writes[0].method, 'POST'); + assert.equal(writes[0].path, '/repos/fixture/repo/check-runs'); + assert.equal(writes[0].body.external_id, `node-repair:12:${await selectionFingerprint(pr, selections(pr.body))}`); + }); +} + test('head synchronization creates a Check on the canonical new head', async () => { const {calls, pr} = checkMock({changePR: pr => { pr.head.sha = 'c'.repeat(40); }}); assert.equal((await worker.fetch(signed({...payload, number: 12, action: 'synchronize', @@ -477,8 +493,8 @@ test('overlapping initial deliveries recover from duplicate idle Checks on a lat const writes = checkWrites(calls); assert.deepEqual(writes.slice(0, 2).map(write => write.method), ['POST', 'POST']); assert.equal(writes.length, 3); - assert.equal(writes[2].path, '/repos/fixture/repo/check-runs/78'); - assert.equal(writes[2].method, 'PATCH'); + assert.equal(writes[2].path, '/repos/fixture/repo/check-runs'); + assert.equal(writes[2].method, 'POST'); assert.match(writes[2].body.output.summary, /2\.14\.3/); assert.deepEqual(writes[2].body.actions, []); }); @@ -507,6 +523,10 @@ test('overlapping edits reconcile an existing Check to the current canonical sel writes.push(requestBody); return json({id: 77}); } + if (path === '/repos/fixture/repo/check-runs' && options.method === 'POST') { + writes.push(requestBody); + return json({id: 78}); + } throw new Error(`Unexpected API path: ${options.method} ${path}`); }); @@ -532,7 +552,7 @@ test('overlapping edits reconcile an existing Check to the current canonical sel test('repeated canonical changes exhaust bounded Check reconciliation', async () => { const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; const writes = []; - let prReads = 0; + let prReads = 0, currentExternal; fetch.mock.mockImplementation(async (url, options) => { const path = new URL(url).pathname; const requestBody = options.body === undefined ? undefined : JSON.parse(options.body); @@ -542,9 +562,12 @@ test('repeated canonical changes exhaust bounded Check reconciliation', async () if (path === '/repos/fixture/repo') return json(repository); if (path === '/repos/fixture/repo/pulls/12') { prReads++; - return json(managedPR(fixtures.cases[0].body.replace('2.14.2', `2.14.${prReads + 1}`))); + const current = managedPR(fixtures.cases[0].body.replace('2.14.2', `2.14.${prReads + 1}`)); + currentExternal = `node-repair:12:${await selectionFingerprint(current, selections(current.body))}`; + return json(current); } - if (path === `/repos/fixture/repo/commits/${'a'.repeat(40)}/check-runs`) return json(listOf(ownCheck())); + if (path === `/repos/fixture/repo/commits/${'a'.repeat(40)}/check-runs`) + return json(listOf(ownCheck({external_id: currentExternal}))); if (path === '/repos/fixture/repo/check-runs/77' && options.method === 'PATCH') { writes.push(requestBody); return json({id: 77}); @@ -628,3 +651,402 @@ test('canonical PR or Check-list API failures are sanitized and cannot cause wri assert.equal(checkWrites(calls).length, 0); } }); + +// Execute the service's SQL against SQLite constraints; the native suite also +// exercises these same queries through Cloudflare's real local D1 binding. +const {DatabaseSync} = await import('node:sqlite'); +function requestDatabase() { + const sql = new DatabaseSync(':memory:'); + sql.exec(readFileSync(new URL('./schema.sql', import.meta.url), 'utf8')); + return {sql, prepare(query) { + return {bind(...values) { + return {async first() { return sql.prepare(query).get(...values) ?? null; }, + async run() { return {meta: {changes: Number(sql.prepare(query).run(...values).changes)}}; }}; + }}; + }}; +} + +async function actionMock(t, {permission = 'write', alterPR, alterCheck, dispatch = 'ok', onCall} = {}) { + const db = requestDatabase();t.after(() => db.sql.close()); + const pr = managedPR(fixtures.cases[0].body); + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + const check = ownCheck({conclusion: 'neutral', external_id: `node-repair:12:${await selectionFingerprint(pr, selections(pr.body))}`}); + const clickedCheck = {...check, app: {...check.app}}; + alterPR?.(pr, repository);alterCheck?.(check); + const documentedCheck = value => { const response = {...value}; delete response.actions; return response; }; + const calls = [], environment = {...env, DB: db}; + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname, body = options.body && JSON.parse(options.body); + const call = {path, method: options.method, body, signal: options.signal};calls.push(call); + const intercepted = await onCall?.(call, {db, pr, check, repository}); + if (intercepted) return intercepted; + assert.equal(options.headers['X-GitHub-Api-Version'], '2026-03-10'); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/collaborators/maintainer/permission') return json({permission, user: {login: 'maintainer'}}); + if (path === '/repos/fixture/repo/pulls/12') return json(pr); + if (path.includes('/commits/') && path.endsWith('/check-runs')) return json(listOf(documentedCheck(check))); + if (path === '/repos/fixture/repo/check-runs/77' || path === '/repos/fixture/repo/check-runs/78') { + if (options.method === 'PATCH') Object.assign(check, body); + return json(options.method === 'GET' ? documentedCheck(check) : check); + } + if (path === '/repos/fixture/repo/check-runs' && options.method === 'POST') return json({...check, ...body, id: 78}); + if (path === '/repos/fixture/repo/actions/workflows/dependency-repair.yml/dispatches') { + if (dispatch === 'timeout') throw Error('private lost response'); + if (dispatch === '204') return new Response(null, {status: 204}); + if (dispatch === '201') return new Response(JSON.stringify({workflow_run_id: 900, + run_url: 'https://api.github.com/repos/fixture/repo/actions/runs/900', + html_url: 'https://github.com/fixture/repo/actions/runs/900'}), {status: 201}); + return json({workflow_run_id: 900, run_url: 'https://api.github.com/repos/fixture/repo/actions/runs/900', + html_url: `https://github.com/${dispatch === 'foreign' ? 'other/repo' : 'fixture/repo'}/actions/runs/900`}); + } + throw Error(`Unexpected request path ${path}`); + }); + const event = {...payload, action: 'requested_action', requested_action: {identifier: 'repair-selected'}, + check_run: clickedCheck, sender: {login: 'maintainer', type: 'User'}}; + const click = (id = delivery, changes = {}) => worker.fetch(signed({...event, ...changes}, 'check_run', {'x-github-delivery': id}), environment); + const rows = () => db.sql.prepare('SELECT * FROM requests').all(); + const dispatches = () => calls.filter(call => call.path.endsWith('/dispatches')); + return {calls, db, pr, repository, check, clickedCheck, environment, event, click, rows, dispatches}; +} + +test('an authorized current action reserves once and retains the documented run and exact C2 inputs', async t => { + const f = await actionMock(t); + assert.equal((await f.click()).status, 200); + assert.equal(f.dispatches().length, 1); + assert.deepEqual(f.dispatches()[0].body, {ref: 'main', inputs: {'request-id': delivery, 'pull-request': '12', + 'expected-head': 'a'.repeat(40), 'expected-base': 'b'.repeat(40), + selections: JSON.stringify([{name: 'launch-editor', from: '2.14.1', to: '2.14.2'}])}}); + assert.deepEqual(f.calls[0].body, {repository_ids: [7], permissions: {contents: 'read', pull_requests: 'read', checks: 'write', actions: 'write'}}); + const row = f.rows()[0]; + 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.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); + assert.equal(f.dispatches().length, 1);assert.equal(f.rows().length, 1); + assert.equal(f.calls.length - before, 4); // Token, repository and owned Check reconciliation; no new attempt. +}); + +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'); + const refreshedExternal = `node-repair:12:${await selectionFingerprint(f.pr, selections(f.pr.body))}`; + f.check.external_id = refreshedExternal; + assert.equal((await f.click()).status, 200); + assert.equal(f.rows().length, 0); + assert.equal(f.dispatches().length, 0); + assert.equal(f.check.external_id, refreshedExternal); +}); + +test('a queued click for the old base cannot dispatch after the canonical Check refreshes', async t => { + const f = await actionMock(t); + f.pr.base.sha = 'c'.repeat(40); + f.check.external_id = `node-repair:12:${await selectionFingerprint(f.pr, selections(f.pr.body))}`; + assert.equal((await f.click()).status, 200); + assert.equal(f.rows().length, 0); + assert.equal(f.dispatches().length, 0); +}); + +for (const [name, mutate] of [ + ['missing head and selection identity', clicked => { + delete clicked.head_sha; delete clicked.external_id; + }], + ['mismatched Check ID', clicked => { clicked.id = 78; }], + ['mismatched head identity', clicked => { clicked.head_sha = 'c'.repeat(40); }], + ['mismatched selection identity', clicked => { clicked.external_id = 'node-repair:12:' + '0'.repeat(64); }], +]) { + test(`a ${name} requires a fresh click and cannot reserve`, async t => { + const f = await actionMock(t); + const clicked = {...f.event.check_run, app: {...f.event.check_run.app}}; + mutate(clicked); + assert.equal((await f.click(delivery, {check_run: clicked})).status, 200); + assert.equal(f.rows().length, 0); + assert.equal(f.dispatches().length, 0); + }); +} + +test('a fresh click for the current refreshed Check dispatches only the current batch', async t => { + const f = await actionMock(t); + f.pr.body = fixtures.cases[0].body.replace('2.14.2', '2.14.3'); + f.check.external_id = `node-repair:12:${await selectionFingerprint(f.pr, selections(f.pr.body))}`; + const clicked = {...f.check, app: {...f.check.app}}; + const freshDelivery = 'a4e535a5-2a7d-491d-858a-3df51ce211df'; + assert.equal((await f.click(freshDelivery, {check_run: clicked})).status, 200); + assert.equal(f.rows().length, 1); + assert.equal(f.dispatches().length, 1); + assert.equal(JSON.parse(f.dispatches()[0].body.inputs.selections)[0].to, '2.14.3'); + assert.equal(f.rows()[0].fingerprint, await selectionFingerprint(f.pr, selections(f.pr.body))); +}); + +for (const permission of ['read', 'triage', 'none']) { + test(`${permission} permission cannot reserve or dispatch`, async t => { + const f = await actionMock(t, {permission});assert.equal((await f.click()).status, 200); + assert.equal(f.rows().length, 0);assert.equal(f.dispatches().length, 0); + }); +} +for (const permission of ['maintain', 'admin']) { + test(`${permission} permission may request the fixed workflow`, async t => { + const f = await actionMock(t, {permission});assert.equal((await f.click()).status, 200);assert.equal(f.dispatches().length, 1); + }); +} +for (const [name, alterPR] of [['head', pr => {pr.head.sha = 'c'.repeat(40);}], + ['base', pr => {pr.base.sha = 'c'.repeat(40);}], ['selections', pr => {pr.body = fixtures.cases[0].body.replaceAll('2.14.2', '2.14.3');}], + ['empty selections', pr => {pr.body = fixtures.cases[1].body;}], ['malformed selections', pr => {pr.body = 'invalid';}], + ['fork', pr => {pr.head.repo.id = 99;}], ['human author', pr => {pr.user.type = 'User';}], + ['wrong branch', pr => {pr.head.ref = 'other';}], ['closed', pr => {pr.state = 'closed';}]]) { + test(`a stale or unmanaged ${name} cannot dispatch`, async t => { + const f = await actionMock(t, {alterPR});assert.equal((await f.click()).status, 200); + assert.equal(f.dispatches().length, 0);assert.equal(f.rows().length, 0); + }); +} +for (const [name, alterCheck] of [['foreign App', c => {c.app.id = 99;}], ['wrong name', c => {c.name = 'other';}], + ['request identity', c => {c.external_id = `node-repair-request:${delivery}`;}], ['in progress', c => {c.status = 'in_progress';}], + ['wrong conclusion', c => {c.conclusion = 'success';}]]) { + test(`a ${name} Check cannot dispatch`, async t => { + const f = await actionMock(t, {alterCheck});await f.click();assert.equal(f.dispatches().length, 0);assert.equal(f.rows().length, 0); + }); +} + +test('the canonical Check is rechecked after permission and PR reads', async t => { + let reads = 0; + const f = await actionMock(t, {onCall(call, {check}) { + if (call.path.endsWith('/check-runs/77') && call.method === 'GET' && ++reads === 2) check.app.id = 999; + }}); + await f.click();assert.equal(f.dispatches().length, 0);assert.equal(f.rows().length, 0); +}); + +test('action routing rejects wrong identifiers and foreign event Apps without network calls', async t => { + const f = await actionMock(t); + for (const changed of [{requested_action: {identifier: 'other'}}, {check_run: {id: 77, app: {id: 99}}}]) { + assert.equal((await f.click(delivery, changed)).status, 200); + } + assert.equal(f.calls.length, 0); + assert.equal((await f.click(delivery, {sender: {login: '../escape', type: 'User'}})).status, 200); + assert.equal(f.rows().length, 0); +}); + +for (const dispatch of ['timeout', '204', '201', 'foreign']) { + test(`ambiguous ${dispatch} dispatch retains its active lock and does not retry`, async t => { + const f = await actionMock(t, {dispatch});assert.equal((await f.click()).status, 500); + assert.equal(f.rows()[0].state, 'unknown');assert.equal(f.rows()[0].run_id, null); + assert.equal((await f.click()).status, 200);assert.equal(f.dispatches().length, 1); + const next = '94e535a5-2a7d-491d-858a-3df51ce211de'; + // A new delivery UUID cannot release the active request either. + assert.equal((await f.click(next)).status, 200);assert.equal(f.rows().length, 1);assert.equal(f.dispatches().length, 1); + }); +} + +test('a failed in-progress Check update retains reserved state without dispatch', async t => { + let failed = false; + const f = await actionMock(t, {onCall(call) { + if (!failed && call.path.endsWith('/check-runs/77') && call.method === 'PATCH') { + failed = true; + return new Response('private', {status: 502}); + } + }}); + 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.output.title, 'Request reserved'); +}); + +test('a fresh click restores a same-Check reservation after the first Check write failed', async t => { + let failed = false; + const f = await actionMock(t, {onCall(call) { + if (!failed && call.path.endsWith('/check-runs/77') && call.method === 'PATCH') { + failed = true; + return new Response('private', {status: 502}); + } + }}); + assert.equal((await f.click()).status, 500); + assert.equal((await f.click('94e535a5-2a7d-491d-858a-3df51ce211de')).status, 200); + assert.equal(f.rows().length, 1);assert.equal(f.rows()[0].state, 'reserved');assert.equal(f.dispatches().length, 0); + assert.equal(f.check.external_id, `node-repair-request:${delivery}`);assert.deepEqual(f.check.actions, []); +}); + +test('replay cannot overwrite a foreign Check or regress a completed request', async t => { + const f = await actionMock(t);assert.equal((await f.click()).status, 200); + f.check.app.id = 999; + const writes = checkWrites(f.calls).length; + assert.equal((await f.click()).status, 500);assert.equal(checkWrites(f.calls).length, writes); + f.db.sql.prepare("UPDATE requests SET state = 'success' WHERE id = ?").run(delivery); + assert.equal((await f.click()).status, 200);assert.equal(checkWrites(f.calls).length, writes); + assert.equal(f.dispatches().length, 1); +}); + +test('the exact five-input payload accepts 65535 characters and rejects one more', () => { + const pr = managedPR(fixtures.cases[0].body); + const items = [{name: 'launch-editor', from: '1.0.1', to: '1.0.2'}]; + const remaining = 65_535 - JSON.stringify(workflowInputs(pr, items, delivery)).length; + items[0].to += '2'.repeat(remaining); + assert.equal(JSON.stringify(workflowInputs(pr, items, delivery)).length, 65_535); + items[0].to += '2'; + assert.throws(() => workflowInputs(pr, items, delivery), /payload limit/); +}); + +test('oversized selected versions expose no action and reserve no request', async t => { + const f = await actionMock(t); + f.pr.body = '\n- [x] Allow `launch-editor` `1.0.1 → 1.0.' + '2'.repeat(65_535) + '` by updating its explicit pin.\n'; + f.check.external_id = `node-repair:12:${await selectionFingerprint(f.pr, selections(f.pr.body))}`; + f.clickedCheck.external_id = f.check.external_id; + assert.equal((await f.click()).status, 200);assert.equal(f.rows().length, 0);assert.equal(f.dispatches().length, 0); + const writes = checkWrites(f.calls); + assert.equal(writes.at(-1).body.output.title, 'Repair selections need attention'); + assert.deepEqual(writes.at(-1).body.actions, []); +}); + +test('failure after retaining the known run cannot erase its identity or dispatch again', async t => { + let writes = 0; + const f = await actionMock(t, {onCall(call) { + if (call.path.endsWith('/check-runs/77') && call.method === 'PATCH' && ++writes === 2) return new Response('private', {status: 502}); + }}); + assert.equal((await f.click()).status, 500);assert.equal(f.rows()[0].state, 'dispatched');assert.equal(f.rows()[0].run_id, 900); + 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 => { + let arrivals = 0, release; + 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 result = await Promise.all([f.click(), f.click('94e535a5-2a7d-491d-858a-3df51ce211de')]); + assert.deepEqual(result.map(r => r.status), [200, 200]);assert.equal(f.rows().length, 1);assert.equal(f.dispatches().length, 1); +}); + +test('D1-enabled PR edits show an action only for an eligible selected batch', async t => { + const f = await actionMock(t); + const refresh = () => worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), f.environment); + assert.equal((await refresh()).status, 200);assert.equal(f.check.actions[0].identifier, 'repair-selected'); + f.pr.body = fixtures.cases[1].body;assert.equal((await refresh()).status, 200); + const writes = checkWrites(f.calls); + assert.equal(writes.at(-1).method, 'POST');assert.deepEqual(writes.at(-1).body.actions, []); + assert.equal(f.dispatches().length, 0); +}); + +test('PR edits preserve a reserved/dispatched Check request identity and do not dispatch', async t => { + 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.deepEqual(f.check.actions, []);assert.equal(f.dispatches().length, 1); +}); + +test('deadline after reservation prevents dispatch and preserves the ambiguous reservation', async t => { + shortenWebhookDeadline(t); + const f = await actionMock(t, {async onCall(call) { + if (call.path.endsWith('/check-runs/77') && call.method === 'PATCH') await new Promise(resolve => setTimeout(resolve, 200)); + }}); + assert.equal((await f.click()).status, 500);assert.equal(f.dispatches().length, 0);assert.equal(f.rows()[0].state, 'reserved'); +}); + +test('an idle refresh that races admission restores the reserved Check identity', async t => { + let raced = false; + const f = await actionMock(t, {onCall(call, {db}) { + if (!raced && call.path.endsWith('/check-runs/77') && call.method === 'PATCH' && call.body.external_id.startsWith('node-repair:')) { + raced = true; + db.sql.prepare(`INSERT INTO requests (id, installation_id, repository_id, repository, pr, head, base, + dispatched_ref, fingerprint, check_id, actor_login, created_at, state) + VALUES (?, 3, 7, 'fixture/repo', 12, ?, ?, 'main', ?, 77, 'maintainer', 1, 'reserved')`) + .run(delivery, 'a'.repeat(40), 'b'.repeat(40), '0'.repeat(64)); + } + }}); + 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); +}); + +test('a different active Check blocks a fresh idle click and links the active request', async t => { + const f = await actionMock(t); + f.db.sql.prepare(`INSERT INTO requests (id, installation_id, repository_id, repository, pr, head, base, + dispatched_ref, fingerprint, check_id, actor_login, created_at, state) + VALUES (?, 3, 7, 'fixture/repo', 12, ?, ?, 'main', ?, 88, 'maintainer', 1, 'unknown')`) + .run('94e535a5-2a7d-491d-858a-3df51ce211de', 'a'.repeat(40), 'b'.repeat(40), '0'.repeat(64)); + assert.equal((await f.click()).status, 200);assert.equal(f.rows().length, 1);assert.equal(f.dispatches().length, 0); + assert.equal(f.check.output.title, 'Another request is active');assert.match(f.check.output.summary, /runs\/88/); + assert.deepEqual(f.check.actions, []); +}); + +test('an active request cannot authorize overwriting a replaced or foreign Check', async t => { + const f = await actionMock(t);assert.equal((await f.click()).status, 200); + f.check.app.id = 999; + const writes = checkWrites(f.calls).length; + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), f.environment)).status, 500); + assert.equal(checkWrites(f.calls).length, writes);assert.equal(f.rows()[0].state, 'dispatched'); +}); + +for (const route of ['PR edit', 'delivery replay']) { + for (const pause of ['GET', 'PATCH']) { + test(`a ${route} paused on Check ${pause} cannot erase a newly retained run link`, async t => { + const dispatchReached = Promise.withResolvers(), finishDispatch = Promise.withResolvers(); + const restoreReached = Promise.withResolvers(), finishRestore = Promise.withResolvers(); + let dispatchPending = false, held = false; + const f = await actionMock(t, {async onCall(call) { + if (call.path.endsWith('/dispatches')) { + dispatchPending = true; dispatchReached.resolve(); await finishDispatch.promise; + } else if (dispatchPending && !held && call.path.endsWith('/check-runs/77') && call.method === pause) { + held = true; restoreReached.resolve(); await finishRestore.promise; + } + }}); + const click = f.click(); + await dispatchReached.promise; + const restore = route === 'PR edit' ? + worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), f.environment) : f.click(); + await restoreReached.promise; + finishDispatch.resolve(); + assert.equal((await click).status, 200); + assert.equal(f.rows()[0].state, 'dispatched'); + assert.match(f.check.output.summary, /actions\/runs\/900/); + finishRestore.resolve(); + assert.equal((await restore).status, 200); + assert.equal(f.rows()[0].run_id, 900); + assert.equal(f.check.output.title, 'Workflow started'); + assert.match(f.check.output.summary, /actions\/runs\/900/); + assert.equal(f.check.external_id, `node-repair-request:${delivery}`); + assert.deepEqual(f.check.actions, []); + assert.equal(f.dispatches().length, 1); + }); + } +} + +for (const [total, reuse] of [[101, false], [201, true], [1000, true]]) { + test(`selection history of ${total} Checks ${reuse ? 'reuses the matching later-page Check' : 'allows a fresh selection'}`, async t => { + const checks = Array.from({length: total}, (_, i) => ownCheck({id: i + 100, + external_id: 'node-repair:12:' + '0'.repeat(64)})); + if (reuse) checks[total - 1] = ownCheck({id: 78}); + const {calls} = checkMock({list: query => { + const start = (Number(query.get('page')) - 1) * 100; + return {total_count: total, check_runs: checks.slice(start, start + 100)}; + }}); + const db = requestDatabase(); t.after(() => db.sql.close()); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), {...env, DB: db})).status, 200); + const writes = checkWrites(calls); + assert.equal(writes.length, 1); + assert.equal(writes[0].method, reuse ? 'PATCH' : 'POST'); + assert.equal(writes[0].path, '/repos/fixture/repo/check-runs' + (reuse ? '/78' : '')); + assert.equal(writes[0].body.actions[0].identifier, 'repair-selected'); + const pages = calls.filter(c => c.path.includes('/commits/')).map(c => new URL(c.url).searchParams.get('page')); + assert.deepEqual(pages, Array.from({length: Math.ceil(total / 100)}, (_, i) => String(i + 1))); + }); +} + +for (const failure of ['incomplete page', 'changed total', 'failed page']) { + test(`a ${failure} in later Check history prevents every Check write`, async () => { + const {calls} = checkMock({list: query => { + if (query.get('page') === '1') return {total_count: 101, check_runs: Array.from({length: 100}, (_, i) => ownCheck({id: i + 100}))}; + if (failure === 'failed page') throw Error('private upstream failure'); + return {total_count: failure === 'changed total' ? 102 : 101, + check_runs: failure === 'incomplete page' ? [] : [ownCheck()]}; + }}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 500); + assert.equal(checkWrites(calls).length, 0); + }); +} diff --git a/repair-app/wrangler.jsonc b/repair-app/wrangler.jsonc index 76199fe..a1e6b5f 100644 --- a/repair-app/wrangler.jsonc +++ b/repair-app/wrangler.jsonc @@ -6,7 +6,12 @@ "env": { "preview": { "name": "replace-with-approved-preview-worker-name", - "workers_dev": true + "workers_dev": true, + "d1_databases": [{ + "binding": "DB", + "database_name": "replace-with-approved-database-name", + "database_id": "00000000-0000-0000-0000-000000000000" + }] } } }