diff --git a/.github/workflows/companion-check.yaml b/.github/workflows/companion-check.yaml new file mode 100644 index 000000000..e621c33e7 --- /dev/null +++ b/.github/workflows/companion-check.yaml @@ -0,0 +1,255 @@ +# Gates PR merges on companion PRs in other repos. +# +# Add one or more markers to a PR body: +# Depends-on: HarperFast/harper#2147 +# Depends-on: harper#2147 (same-org shorthand) +# Depends-on: https://github.com/HarperFast/harper-pro/pull/512 +# +# A marker line may only contain refs in those forms (comma/space separated); +# anything else on the line fails the check rather than being ignored. +# +# The `companion-check` commit status stays pending until every referenced PR +# is merged, and fails if one closes without merging (or if a marker is +# present but empty/unparseable — the gate fails closed). With +# `companion-check` configured as a required status check, approving a PR and +# arming auto-merge queues it to merge automatically once its companions land. +# +# PRs without a marker get an immediate `success` status, so the required +# check never blocks ordinary PRs. The cron sweep reconciles every open PR, +# so it also backfills statuses after this workflow first lands and heals +# any status a dropped webhook left behind. +# +# Referencing a private repo requires a `COMPANION_CHECK_TOKEN` secret with +# pull-request read access to that repo; public repos work with GITHUB_TOKEN. +# The secret is only used for same-org refs on non-fork PRs. +# +# Maintenance rule for `pull_request_target`: never add a checkout step and +# never interpolate PR-controlled text into `run:` or expressions — the PR +# body must only be handled as data inside github-script. +# +# Logic is covered by scripts/companion-check.test.mjs (plain node, no deps). + +name: Companion Check + +on: + pull_request_target: + types: [opened, edited, reopened, synchronize] + schedule: + - cron: '*/15 * * * *' + workflow_dispatch: + +permissions: + statuses: write + pull-requests: read + contents: read + +concurrency: + group: companion-check-${{ github.event.pull_request.number || 'sweep' }} + cancel-in-progress: ${{ github.event_name == 'pull_request_target' }} + +jobs: + evaluate: + runs-on: ubuntu-latest + steps: + - name: Evaluate companion dependencies + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + env: + COMPANION_CHECK_TOKEN: ${{ secrets.COMPANION_CHECK_TOKEN }} + with: + script: | + const STATUS_CONTEXT = 'companion-check'; + const MAX_DEPS = 10; + const { owner, repo } = context.repo; + const crossToken = process.env.COMPANION_CHECK_TOKEN || ''; + const isSweep = context.eventName !== 'pull_request_target'; + const apiUrl = context.apiUrl || 'https://api.github.com'; + const depCache = new Map(); + + function parseDeps(body) { + const deps = []; + let unparseable = false; + const lineRe = /^[^\S\r\n]*depends[- ]on:[ \t]*(.*)$/gim; + const refRe = + /https:\/\/github\.com\/([\w.-]+)\/([\w.-]+)\/pull\/(\d+)|([\w.-]+)\/([\w.-]+)#(\d+)|([\w.-]+)#(\d+)/g; + const badSegment = (s) => s.includes('..') || s.startsWith('.'); + for (const [, refs] of (body || '').matchAll(lineRe)) { + let matched = 0; + for (const m of refs.matchAll(refRe)) { + matched++; + const dep = m[3] + ? { owner: m[1], repo: m[2], number: +m[3] } + : m[6] + ? { owner: m[4], repo: m[5], number: +m[6] } + : { owner, repo: m[7], number: +m[8] }; + if (badSegment(dep.owner) || badSegment(dep.repo)) unparseable = true; + else deps.push(dep); + } + // Anything on the line that is not a recognized ref fails closed. + if (!matched || refs.replace(refRe, '').replace(/[\s,;]+/g, '')) unparseable = true; + } + const seen = new Set(); + const unique = deps.filter((dep) => { + const key = `${dep.owner}/${dep.repo}#${dep.number}`.toLowerCase(); + if (seen.has(key)) return false; + seen.add(key); + return true; + }); + return { deps: unique, unparseable }; + } + + const refOf = (dep) => `${dep.owner}/${dep.repo}#${dep.number}`; + const stateOf = (data) => + data.merged ? 'merged' : data.state === 'open' ? 'open' : 'closed'; + + async function depState(dep, allowSecret) { + const cacheKey = `${allowSecret}:${refOf(dep).toLowerCase()}`; + if (depCache.has(cacheKey)) return depCache.get(cacheKey); + const state = await depStateUncached(dep, allowSecret); + depCache.set(cacheKey, state); + return state; + } + + async function depStateUncached(dep, allowSecret) { + try { + const { data } = await github.rest.pulls.get({ + owner: dep.owner, + repo: dep.repo, + pull_number: dep.number, + }); + return stateOf(data); + } catch (e) { + if (e.status !== 404) return `unreadable (${e.status || e.message})`; + // 404 falls through: definitive miss unless the secret can see more. + } + // Oracle guard: the secret never serves foreign-org refs or fork PRs. + if (crossToken && allowSecret && dep.owner.toLowerCase() === owner.toLowerCase()) { + try { + const res = await fetch( + `${apiUrl}/repos/${encodeURIComponent(dep.owner)}/${encodeURIComponent(dep.repo)}/pulls/${dep.number}`, + { + headers: { + authorization: `Bearer ${crossToken}`, + accept: 'application/vnd.github+json', + 'user-agent': 'companion-check', + }, + } + ); + if (res.ok) return stateOf(await res.json()); + return res.status === 404 ? 'missing' : `unreadable (${res.status})`; + } catch (e) { + return `unreadable (${e.message})`; + } + } + return 'missing'; + } + + async function evaluate({ deps, unparseable }, allowSecret) { + if (unparseable) + return { + state: 'failure', + description: 'Empty or unparseable Depends-on marker (use owner/repo#N or a PR URL)', + }; + if (!deps.length) + return { state: 'success', description: 'No companion dependencies' }; + if (deps.length > MAX_DEPS) + return { + state: 'failure', + description: `More than ${MAX_DEPS} companion PRs referenced`, + }; + const states = []; + for (const dep of deps) + states.push({ ref: refOf(dep), state: await depState(dep, allowSecret) }); + const closed = states.find((s) => s.state === 'closed'); + if (closed) + return { state: 'failure', description: `${closed.ref} closed without merging` }; + const missing = states.find((s) => s.state === 'missing'); + if (missing) + return { + state: 'failure', + description: `${missing.ref} not found or inaccessible (typo? private repo needs COMPANION_CHECK_TOKEN?)`, + }; + const unreadable = states.find((s) => s.state.startsWith('unreadable')); + if (unreadable) + return { + state: 'pending', + description: `Cannot read ${unreadable.ref} ${unreadable.state.slice(11)}`, + }; + const open = states.filter((s) => s.state === 'open'); + if (open.length) + return { + state: 'pending', + description: `Waiting on ${open.map((s) => s.ref).join(', ')}`, + }; + return { + state: 'success', + description: `All companion PRs merged (${states.map((s) => s.ref).join(', ')})`, + }; + } + + const runUrl = `${context.serverUrl}/${owner}/${repo}/actions/runs/${context.runId}`; + + async function postStatus(sha, state, description) { + await github.rest.repos.createCommitStatus({ + owner, + repo, + sha, + context: STATUS_CONTEXT, + state, + description: description.slice(0, 140), + target_url: runUrl, + }); + } + + async function postIfChanged(pr, desired) { + const { data: statuses } = await github.rest.repos.listCommitStatusesForRef({ + owner, + repo, + ref: pr.head.sha, + per_page: 100, + }); + const description = desired.description.slice(0, 140); + const current = statuses.find((s) => s.context === STATUS_CONTEXT); + if (current && current.state === desired.state && current.description === description) + return; + if (isSweep) { + // Sweep data may predate a racing PR event; the fresher run wins. + const { data: fresh } = await github.rest.pulls.get({ + owner, + repo, + pull_number: pr.number, + }); + if (fresh.head.sha !== pr.head.sha || fresh.body !== pr.body) return; + } + await postStatus(pr.head.sha, desired.state, description); + core.info(`${pr.number} -> ${desired.state}: ${description}`); + } + + const prs = + context.eventName === 'pull_request_target' + ? [context.payload.pull_request] + : await github.paginate(github.rest.pulls.list, { + owner, + repo, + state: 'open', + per_page: 100, + }); + let failures = 0; + for (const pr of prs) { + let parsed; + try { + parsed = parseDeps(pr.body); + const allowSecret = + !!pr.head.repo && pr.head.repo.full_name === `${owner}/${repo}`; + await postIfChanged(pr, await evaluate(parsed, allowSecret)); + } catch (e) { + failures++; + core.warning(`PR #${pr.number}: ${e.message}`); + // A failed refresh must not leave a stale success behind. + if (parsed && (parsed.deps.length || parsed.unparseable)) { + try { + await postStatus(pr.head.sha, 'pending', 'companion-check errored; next run will retry'); + } catch {} + } + } + } + if (failures) core.setFailed(`${failures} PR(s) could not be updated`); diff --git a/.github/workflows/validate.yaml b/.github/workflows/validate.yaml index 7d70926f1..ecf599ece 100644 --- a/.github/workflows/validate.yaml +++ b/.github/workflows/validate.yaml @@ -19,6 +19,7 @@ jobs: - TypeScript Check - Lint Check - Format Check + - Workflow Tests include: - check: TypeScript Check command: typecheck @@ -26,6 +27,8 @@ jobs: command: lint - check: Format Check command: format:check + - check: Workflow Tests + command: test:workflows steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 diff --git a/package.json b/package.json index 160206f72..764829735 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,8 @@ "format:check": "npm run format -- --check", "format:write": "npm run format -- --write", "lint": "echo 0;", - "preview:pr": "node scripts/preview-pr.mjs" + "preview:pr": "node scripts/preview-pr.mjs", + "test:workflows": "node scripts/companion-check.test.mjs" }, "devDependencies": { "@docusaurus/core": "3.9.2", diff --git a/scripts/companion-check.test.mjs b/scripts/companion-check.test.mjs new file mode 100644 index 000000000..7c6cc3e20 --- /dev/null +++ b/scripts/companion-check.test.mjs @@ -0,0 +1,207 @@ +// Test harness for the companion-check workflow's embedded github-script. +// Usage: node scripts/companion-check.test.mjs [path-to-companion-check.yaml] +import { readFileSync } from 'node:fs'; +import assert from 'node:assert'; + +const yaml = readFileSync( + process.argv[2] || new URL('../.github/workflows/companion-check.yaml', import.meta.url), + 'utf8' +); +const scriptLines = []; +let inScript = false; +for (const line of yaml.split('\n')) { + if (!inScript) { + if (/^\s*script: \|/.test(line)) inScript = true; + } else if (line.trim() === '' || line.startsWith(' ')) { + scriptLines.push(line.slice(12)); + } else break; +} +const script = scriptLines.join('\n'); +assert(script.includes('parseDeps'), 'script extraction failed'); + +const OWN = 'HarperFast/documentation'; + +// Dep PR database: "owner/repo#n" -> PR data | 'private' | 'flaky500' +const prDb = { + 'HarperFast/harper#2147': { merged: false, state: 'open' }, + 'HarperFast/harper#2000': { merged: true, state: 'closed' }, + 'HarperFast/harper#1999': { merged: false, state: 'closed' }, + 'HarperFast/harper#1500': { merged: true, state: 'closed' }, + 'HarperFast/harper-pro#512': 'private', + 'HarperFast/harper#666': 'flaky500', + 'HarperFast/documentation#600': { merged: true, state: 'closed' }, +}; + +function makeEnv(eventName, openPrs, payloadPr) { + const posted = []; + const getCalls = []; + const github = { + rest: { + pulls: { + get: async ({ owner, repo, pull_number }) => { + const key = `${owner}/${repo}#${pull_number}`; + getCalls.push(key); + const fresh = openPrs.find((p) => key === `${OWN}#${p.number}`); + if (fresh) return { data: fresh }; // sweep re-read of our own PR + const rec = prDb[key]; + if (rec === 'flaky500') throw Object.assign(new Error('boom'), { status: 500 }); + if (!rec || rec === 'private') throw Object.assign(new Error('nf'), { status: 404 }); + return { data: rec }; + }, + list: 'LIST', + }, + repos: { + listCommitStatusesForRef: async ({ ref }) => ({ + data: + ref === 'ddd' + ? [ + { + context: 'companion-check', + state: 'failure', + description: 'HarperFast/harper#1999 closed without merging', + }, + ] + : ref === 'heal' + ? [{ context: 'companion-check', state: 'pending', description: 'Waiting on HarperFast/harper#2147' }] + : [], + }), + createCommitStatus: async (s) => posted.push(s), + }, + }, + paginate: async (fn) => (assert.equal(fn, 'LIST'), openPrs), + }; + const core = { info: () => {}, warning: () => {}, setFailed: (m) => (core.failed = m) }; + const context = { + repo: { owner: 'HarperFast', repo: 'documentation' }, + eventName, + payload: { pull_request: payloadPr }, + serverUrl: 'https://github.com', + runId: 1, + }; + return { github, core, context, posted, getCalls }; +} + +async function run(env) { + await new Function('github', 'context', 'core', `return (async()=>{${script}})()`)(env.github, env.context, env.core); + return Object.fromEntries(env.posted.map((s) => [s.sha, s])); +} + +const ownHead = { repo: { full_name: OWN } }; +const pr = (number, body, sha, head = {}) => ({ number, body, head: { sha, ...ownHead, ...head } }); + +process.env.COMPANION_CHECK_TOKEN = ''; +globalThis.fetch = async () => ({ ok: false, status: 403 }); + +// --- schedule sweep --- +const sweepPrs = [ + pr(623, 'Docs.\n\nDepends-on: HarperFast/harper#2147\n', 'aaa'), + pr(700, 'plain docs PR, no deps', 'bbb'), + pr(701, 'Depends-on: https://github.com/HarperFast/harper/pull/2000, documentation#600', 'ccc'), + pr(702, 'depends-on: HarperFast/harper#1999', 'ddd'), + pr(703, 'Depends-on: HarperFast/harper-pro#512', 'eee'), + pr(704, 'Depends-on: harper#1500', 'fff'), // repo#N shorthand + pr(705, 'Depends-on: the harper PR', 'ggg'), // unparseable -> failure + pr(706, 'Depends-on: HarperFast/harper#666', 'hhh'), // 500 -> unreadable, no crash + pr(707, `Depends-on: ${Array.from({ length: 12 }, (_, i) => `HarperFast/harper#${i + 1}`).join(', ')}`, 'iii'), + pr(708, 'Depends-on: harper#1500, HarperFast/harper#1500', 'jjj'), // dedupe -> 1 dep + pr(709, 'Depends-on: HarperFast/harper#2147 (the pagination PR)', 'res1'), // residue -> failure + pr(710, 'Depends-on:', 'res2'), // empty marker -> failure + pr(711, 'Depends-on: ../../evil/x#1', 'res3'), // traversal segments -> failure + pr(712, 'no marker, stale pending from removed marker', 'heal'), // heals to success + pr(713, 'Depends-on: harper#1500', 'memo'), // same dep as 704: memoized +]; +const sweep = makeEnv('schedule', sweepPrs); +const bySha = await run(sweep); + +assert.equal(bySha.aaa.state, 'pending'); +assert.match(bySha.aaa.description, /Waiting on HarperFast\/harper#2147/); +assert.equal(bySha.bbb.state, 'success', 'sweep backfills no-marker PRs'); +assert.equal(bySha.ccc.state, 'success'); +assert.match(bySha.ccc.description, /harper#2000.*documentation#600/); +assert.equal(bySha.ddd, undefined, 'unchanged status must not repost'); +assert.equal(bySha.eee.state, 'failure', 'definitive 404 without secret blocks as not-found'); +assert.match(bySha.eee.description, /not found or inaccessible/); +assert.equal(bySha.fff.state, 'success', 'repo#N shorthand resolves in own org'); +assert.equal(bySha.ggg.state, 'failure', 'unparseable marker fails closed'); +assert.match(bySha.ggg.description, /unparseable/i); +assert.equal(bySha.hhh.state, 'pending', 'HTTP 500 must degrade, not crash'); +assert.match(bySha.hhh.description, /^Cannot read HarperFast\/harper#666 \(500\)$/); +assert.equal(bySha.iii.state, 'failure', 'dep cap enforced'); +assert.equal(bySha.jjj.state, 'success', 'duplicates deduped'); +assert.match(bySha.jjj.description, /^All companion PRs merged \(HarperFast\/harper#1500\)$/); +assert.equal(bySha.res1.state, 'failure', 'non-ref residue on marker line fails closed'); +assert.equal(bySha.res2.state, 'failure', 'empty marker fails closed'); +assert.equal(bySha.res3.state, 'failure', 'path-traversal segments fail closed'); +assert.equal(bySha.heal.state, 'success', 'sweep heals stale pending after marker removal'); +assert.equal(bySha.memo.state, 'success'); +assert.equal( + sweep.getCalls.filter((k) => k === 'HarperFast/harper#1500').length, + 1, + 'dep lookups memoized within a run' +); +assert.equal(sweep.core.failed, undefined, 'no PR-level failures expected'); +assert.ok(sweep.posted.every((s) => s.context === 'companion-check' && s.description.length <= 140)); + +// --- pull_request_target: no-marker PR gets success, no sweep re-read --- +const evt = makeEnv('pull_request_target', [], pr(700, 'plain', 'bbb')); +const evtBySha = await run(evt); +assert.equal(evtBySha.bbb.state, 'success'); +assert.equal(evtBySha.bbb.description, 'No companion dependencies'); + +// --- sweep race guard: body changed between list and post -> skip --- +const stale = pr(720, 'Depends-on: HarperFast/harper#2147', 'kkk'); +const race = makeEnv('schedule', [stale]); +const origGet = race.github.rest.pulls.get; +race.github.rest.pulls.get = async (args) => + args.repo === 'documentation' ? { data: pr(720, 'edited body', 'kkk') } : origGet(args); +const raceBySha = await run(race); +assert.equal(raceBySha.kkk, undefined, 'sweep must not post over a changed PR'); + +// --- error mid-PR must downgrade a would-be-stale status to pending --- +const errEnv = makeEnv('pull_request_target', [], pr(721, 'Depends-on: HarperFast/harper#2147', 'err1')); +errEnv.github.rest.repos.listCommitStatusesForRef = async () => { + throw new Error('api down'); +}; +const errBySha = await run(errEnv); +assert.equal(errBySha.err1.state, 'pending', 'failed refresh posts pending, not silence'); +assert.match(errBySha.err1.description, /errored/); +assert.match(errEnv.core.failed, /1 PR/); + +// --- secret guards --- +process.env.COMPANION_CHECK_TOKEN = 'tok'; +let fetched = []; +globalThis.fetch = async (url) => (fetched.push(url), { ok: false, status: 404 }); + +// foreign org: secret withheld +const foreign = makeEnv('pull_request_target', [], pr(730, 'Depends-on: HarperFast/../evil#1', 'lll')); +await run(foreign); // traversal caught in parse; now a clean foreign ref: +const foreign2 = makeEnv('pull_request_target', [], { + number: 731, + body: 'Depends-on: https://github.com/evilorg/private/pull/1', + head: { sha: 'mmm', repo: { full_name: OWN } }, +}); +const foreignBySha = await run(foreign2); +assert.equal(fetched.length, 0, 'secret must not be sent for foreign orgs'); +assert.equal(foreignBySha.mmm.state, 'failure'); + +// fork PR: secret withheld even for same-org refs +fetched = []; +const fork = makeEnv('pull_request_target', [], { + number: 732, + body: 'Depends-on: HarperFast/harper-pro#512', + head: { sha: 'nnn', repo: { full_name: 'outsider/documentation' } }, +}); +const forkBySha = await run(fork); +assert.equal(fetched.length, 0, 'secret must not be sent for fork PRs'); +assert.equal(forkBySha.nnn.state, 'failure', 'blocks without leaking; message explains'); + +// non-fork same-org: secret used; token-confirmed 404 -> failure +fetched = []; +const typo = makeEnv('pull_request_target', [], pr(733, 'Depends-on: HarperFast/ghost#1', 'ooo')); +const typoBySha = await run(typo); +assert.equal(fetched.length, 1, 'secret used for same-org non-fork refs'); +assert.equal(typoBySha.ooo.state, 'failure', 'token-confirmed 404 fails, not pending'); +assert.match(typoBySha.ooo.description, /not found/); +process.env.COMPANION_CHECK_TOKEN = ''; + +console.log('PASS: all companion-check scenarios correct');