diff --git a/.claude/skills/catalog-pairing/SKILL.md b/.claude/skills/catalog-pairing/SKILL.md index e9994155d93..25ddef050f7 100644 --- a/.claude/skills/catalog-pairing/SKILL.md +++ b/.claude/skills/catalog-pairing/SKILL.md @@ -1,6 +1,6 @@ --- name: catalog-pairing -description: How a boxel pull request and a boxel-catalog pull request that depend on each other are paired — a `Merges before:` / `Merges after:` line in each description naming the other, declared from both sides — and which merges first. Covers when a change needs a pair (a boxel change the catalog's cards must follow, such as renaming or removing a host tool, command, base export or type the catalog imports; or a catalog definition boxel's tests need first), the declaration syntax and the rules a declaration must meet, merge order in each direction and in the three-PR flow, what boxel's Lint Catalog does with the pairing (it lints the paired catalog pull request's head and refuses to let catalog main be left broken behind a pull request that isn't ready), and how to read its failures. Use before opening or editing any pull request in cardstack/boxel-catalog; before opening a boxel pull request that changes something boxel-catalog's cards import or that pins or tests a catalog change; whenever Lint Catalog fails; and whenever a check says `pairing:`, "doesn't name … back", "waiting on cardstack/boxel-catalog#… to merge", "is not approved yet", "pair the two", or "Merges before" / "Merges after". +description: How a boxel pull request and a boxel-catalog pull request that depend on each other are paired — a `Merges before:` / `Merges after:` line in each description naming the other, declared from both sides — and which merges first. Covers when a change needs a pair (a boxel change the catalog's cards must follow, such as renaming or removing a host tool, command, base export or type the catalog imports; or a catalog definition boxel's tests need first), the declaration syntax and the rules a declaration must meet, merge order in each direction and in the three-PR flow, what boxel's Lint Catalog does with the pairing (it lints the paired catalog pull request's head and refuses to let catalog main be left broken behind a pull request that isn't ready), and how to read its failures. Use before opening or editing any pull request in cardstack/boxel-catalog; before opening a boxel pull request that changes something boxel-catalog's cards import or that pins or tests a catalog change; whenever Lint Catalog fails; and whenever a check says `pairing:`, "doesn't name … back", "waiting on cardstack/boxel-catalog#… to merge", "is not approved yet", "pair the two", "Merges before" / "Merges after", "which predates it", or "can't contain it yet". --- # Pairing boxel and boxel-catalog pull requests @@ -53,6 +53,7 @@ Edit a description with `gh pr edit --repo --body-file ` **boxel's Lint Catalog** (`.github/workflows/lint-catalog.yaml`, `packages/catalog/scripts/pairing.ts` and `lint-sweep.ts`): 1. It reads this pull request's declaration and checks it from both sides. A problem fails the job immediately, naming the line to add or fix and the pull request it belongs on. + The catalog revision this pull request pins in `packages/catalog/test-subset.json` must contain its `Merges after` pair's merge commit (for a pair stacked in its own repository, the merge commit its stack reached `main` through), since production deploys the catalog at the revision boxel pins. While that pair is open, the job's last step fails with `… hasn't merged, so … can't contain it yet`, after the lint steps have judged the pair's head. Nothing re-runs the job when the pair merges, so the push that re-pins with `pnpm --dir packages/catalog catalog:test-subset --bump` is what runs it again. A pin that predates the merge commit fails with `cardstack/boxel-catalog#N merged as , but … pins , which predates it`. 2. It lints the paired catalog pull request's head against the change, in place of catalog `main`. That is the `Merges after` pair if it's unmerged, since it lands first, otherwise the `Merges before` pair. Errors there are for the catalog pull request to fix. 3. It also lints catalog `main` against the change, against the base branch too, so only errors this change adds count. If the change adds errors to catalog `main`: - **`Merges after` pair unmerged**: fails with `waiting on cardstack/boxel-catalog#N to merge`. Re-run after it merges. This comes first, because that pull request has to land before this one. @@ -67,5 +68,5 @@ Edit a description with `gh pr edit --repo --body-file ` ## Merging - **Boxel first:** merge the boxel pull request only once its catalog pull request is approved and neither one is stacked, then merge the catalog one right after. Until the catalog one merges, catalog `main` fails against boxel `main`, and boxel-catalog CI is red for everyone. -- **Catalog first:** merge the catalog pull request, re-pin the boxel pull request onto catalog `main` (`pnpm --dir packages/catalog catalog:test-subset --bump`), re-run its checks, then merge it. +- **Catalog first:** merge the catalog pull request, re-pin the boxel pull request onto catalog `main` (`pnpm --dir packages/catalog catalog:test-subset --bump`), re-run its checks, then merge it. Lint Catalog enforces the re-pin: it fails the boxel pull request that declares `Merges after:` the catalog pull request until that has merged and the pin contains its merge commit, since production deploys the catalog at the revision boxel pins. - **Each environment gets the catalog on its own schedule.** Staging syncs catalog `main` on every catalog merge. Production gets only what boxel-catalog's "Deploy to production" deploys: the revision boxel pins, before and after Manual Deploy [boxel] to production releases, or a catalog `main` revision someone deploys by hand. That deploy refuses while a catalog pull request in it says `Merges after:` a boxel pull request production doesn't run. So a catalog change that needs boxel code declares its pair even when Lint Catalog wouldn't require one. The `catalog-deploy` skill covers the deploy and its refusals. diff --git a/.claude/skills/catalog-test-subset/SKILL.md b/.claude/skills/catalog-test-subset/SKILL.md index 8772f8a27b2..fc2329d41f2 100644 --- a/.claude/skills/catalog-test-subset/SKILL.md +++ b/.claude/skills/catalog-test-subset/SKILL.md @@ -81,7 +81,7 @@ Never commit a pin that exists only in a local catalog checkout (the check fails **For merge.** Every subset file at the pin must match catalog `main` byte for byte. `--check-pin` enforces this on every boxel PR that changes the manifest. -- `waiting on cardstack/boxel-catalog# to merge`: the pin is a commit in an open catalog PR. Merge that PR first, then re-run the check. A pin from a catalog PR passes once the PR merges, whether as a merge commit, a squash or a rebase, so the pin doesn't have to move. +- `waiting on cardstack/boxel-catalog# to merge`: the pin is a commit in an open catalog PR. Merge that PR first, then re-run the check. A pin from a catalog PR passes this check once the PR merges, whether as a merge commit, a squash or a rebase. A boxel PR that declares `Merges after:` that catalog PR still has to re-pin with `--bump` (below). - "main has changed … since": catalog `main` has a newer version of a subset file. Re-pin with `--bump`, run the sync, and re-run the consumers. If the message names a boxel PR that already pins a matching revision, merging boxel `main` after it lands works too. ## Changing a subset definition, or adding one @@ -97,7 +97,7 @@ The order matters, because the deployed catalog is boxel-catalog `main`, not the 3. **Push the boxel branch, with its manifest entry, before you push the boxel-catalog branch or open its PR.** boxel-catalog's `Boxel Test Subset` workflow decides which files to test from the manifest on the same-named boxel branch, looked up at the moment it runs. If that branch isn't on GitHub yet, the workflow falls back to boxel `main`. That manifest doesn't list a file you are adding, so the check passes without running a single test. A changed file fails loudly against boxel `main` instead of passing, but the tests it ran still weren't yours. If the catalog workflow ran before the boxel branch existed, re-run it once the branch is pushed. Then open the boxel-catalog PR, set the manifest's `revision` to its head sha (a commit on a pushed branch fetches fine), and push the boxel branch again. Re-pin whenever you push to the catalog PR. Pin before you open or merge the boxel PR. 4. **Validate the catalog PR against boxel.** boxel-catalog's `Boxel Test Subset` workflow runs the manifest's `tests` consumers whenever a PR touches a subset path. It runs them against the boxel branch of the same name when one exists, otherwise against boxel `main`, so matching branch names are how a catalog change and the boxel change that depends on it are tested together before either merges. A manual run takes any `boxel_ref`. The catalog workflow only decides whether to run and which boxel ref to use. The tests run in boxel's reusable `.github/workflows/catalog-test-subset-consumers.yml`, which the catalog calls at boxel `main`, so a CI setup change belongs in boxel, not in the catalog. The workflow's steps, and those of the `test-web-assets.yaml` it calls, come from boxel `main`. What the steps run comes from the chosen ref: its mise tasks, composite actions, manifest and tests, and the source the host dist is built from. So a paired boxel branch's edit to one of those steps reaches catalog runs only once it merges; the branch's own boxel PR run is what tests it. And if the catalog run fails on a task or argument the paired branch doesn't have, the branch is behind `main`: merge `main` into it. While the pin is a catalog PR commit, boxel's `Catalog Test Subset` check fails with `waiting on cardstack/boxel-catalog# to merge`. That is expected until the catalog merges. -5. **Merge the catalog PR first.** Then re-run the boxel PR's `Catalog Test Subset` check. It passes once every subset file at the pin matches catalog `main`, whether the catalog PR merged as a merge commit, a squash or a rebase, so the pin doesn't have to move. If it fails with "main has changed … since" (the catalog PR got commits after you pinned, or another catalog PR changed a subset file), re-pin with `pnpm --dir packages/catalog catalog:test-subset --bump`, run the sync, and re-run the consumers. Then merge the boxel PR. +5. **Merge the catalog PR first.** Then re-pin the boxel PR onto catalog `main` with `pnpm --dir packages/catalog catalog:test-subset --bump`, run the sync, re-run the consumers, and push. Production deploys the catalog at the revision the deployed boxel pins, so the pin has to contain the catalog PR's merge commit, not only its head: Lint Catalog fails a boxel PR that declares `Merges after:` a catalog PR until that has merged and the pin contains its merge commit. If catalog `main` moves again before the boxel PR merges and `Catalog Test Subset` fails with "main has changed … since", bump again. Then merge the boxel PR. A catalog change to a subset file reaches deployments as soon as the catalog merges, but it reaches boxel's tests only when the pin is bumped. So it must stay compatible with boxel `main`. Bump the pin in the same boxel PR as any platform or test change that depends on the new definition. A catalog change that merges with no boxel PR paired with it still needs a follow-up boxel PR that only runs `--bump`, so boxel keeps testing what deployments serve. Until that lands, the next boxel PR that touches the manifest fails `--check-pin` and has to bump. diff --git a/.github/workflows/lint-catalog.yaml b/.github/workflows/lint-catalog.yaml index 68b61e8c495..d033b7ec899 100644 --- a/.github/workflows/lint-catalog.yaml +++ b/.github/workflows/lint-catalog.yaml @@ -51,6 +51,9 @@ jobs: # The catalog change a pull request depends on, or that follows it, is # the one its description pairs it with (scripts/pairing.ts). Lint that # pull request's head in place of main, and catalog main as well below. + # Production deploys the catalog at the revision boxel pins, so a catalog + # pull request this change merges after must be in the pin once it has + # merged. - name: Read the catalog pairing id: pairing if: github.event_name == 'pull_request' @@ -65,6 +68,7 @@ jobs: --number="$PR_NUMBER" \ --base="$PR_BASE" \ --counterpart=cardstack/boxel-catalog \ + --pin=packages/catalog/test-subset.json \ --out="$RUNNER_TEMP/catalog-pairing.json" # An open pair this change merges after is what it builds on, and # lands first; otherwise an open pair it merges before is the @@ -193,3 +197,12 @@ jobs: args+=(--baseline="$RUNNER_TEMP/catalog-main-lint-base.json") fi node "$RUNNER_TEMP/lint-sweep.ts" "${args[@]}" + + # A catalog pull request this change merges after has to be in the pin + # before this change merges, and nothing re-runs this job when that pull + # request merges. So while it is open this fails, after the lint above + # has judged its head, and the push that re-pins once it merges is the + # run that compares the pin with its merge commit. + - name: Wait for the re-pin + if: ${{ !cancelled() && steps.pairing.outcome == 'success' }} + run: node packages/catalog/scripts/pairing.ts --pin-verdict="$RUNNER_TEMP/catalog-pairing.json" diff --git a/packages/catalog/scripts/pairing.test.ts b/packages/catalog/scripts/pairing.test.ts index c5909388a09..290cf881886 100644 --- a/packages/catalog/scripts/pairing.test.ts +++ b/packages/catalog/scripts/pairing.test.ts @@ -5,7 +5,12 @@ import { createServer, type Server } from 'node:http'; import type { AddressInfo } from 'node:net'; import { after, before, beforeEach, test } from 'node:test'; -import { mainVerdict, resolvePairing, type Resolution } from './pairing.ts'; +import { + mainVerdict, + pinVerdict, + resolvePairing, + type Resolution, +} from './pairing.ts'; const BOXEL = 'cardstack/boxel'; const CATALOG = 'cardstack/boxel-catalog'; @@ -24,7 +29,17 @@ interface StubPull { body?: string; } +// The commit a merged pull request merged as. +function mergeCommit(n: number) { + return `m${n}`.padEnd(40, '0'); +} + +const PIN = 'a'.repeat(40); + let pulls: StubPull[] = []; +// GitHub's compare status for `...`; a pair it doesn't list is +// two commits GitHub doesn't know. +let comparisons = new Map(); let server: Server; // A pull request as the list endpoint returns it: `merged_at`, and no @@ -36,6 +51,7 @@ function listedPull(pull: StubPull) { html_url: `https://github.com/${pull.repository}/pull/${pull.number}`, state: pull.state ?? 'open', merged_at: pull.merged ? (pull.mergedAt ?? '2026-10-01T12:00:00Z') : null, + merge_commit_sha: pull.merged ? mergeCommit(pull.number) : null, draft: false, body: pull.body ?? '', base: { ref: pull.base ?? 'main' }, @@ -71,6 +87,17 @@ before(async () => { ); return; } + let compared = /^\/repos\/[^/]+\/[^/]+\/compare\/(.+)$/.exec(url.pathname); + if (compared) { + let status = comparisons.get(compared[1]); + if (!status) { + res.statusCode = 404; + res.end('{}'); + return; + } + res.end(JSON.stringify({ status })); + return; + } let one = /^\/repos\/([^/]+\/[^/]+)\/pulls\/(\d+)$/.exec(url.pathname); if (one) { let pull = pulls.find( @@ -115,6 +142,7 @@ after(() => { beforeEach(() => { pulls = []; + comparisons = new Map(); }); // A boxel pull request and the catalog pull request it pairs with, each @@ -138,7 +166,7 @@ function pair(boxel: Partial, catalog: Partial = {}) { ); } -function resolveBoxel() { +function resolveBoxel(pinnedRevision?: string) { let boxel = pulls.find((p) => p.repository === BOXEL && p.number === 6454)!; return resolvePairing( BOXEL, @@ -146,6 +174,7 @@ function resolveBoxel() { boxel.base ?? 'main', CATALOG, boxel.body ?? '', + pinnedRevision, ); } @@ -386,6 +415,124 @@ test('a counterpart whose merged parent merged into a grandparent that is still ); }); +test('a pin that contains the merge commit of the pull request this change merges after resolves', async () => { + pair({}, { state: 'closed', merged: true }); + comparisons.set(`${mergeCommit(791)}...${PIN}`, 'ahead'); + let { resolution, problems } = await resolveBoxel(PIN); + assert.deepEqual(problems, []); + assert.equal(resolution.pairs[0]?.merged, true); +}); + +test('a pin at the merge commit of the pull request this change merges after resolves', async () => { + pair({}, { state: 'closed', merged: true }); + comparisons.set(`${mergeCommit(791)}...${mergeCommit(791)}`, 'identical'); + let { problems } = await resolveBoxel(mergeCommit(791)); + assert.deepEqual(problems, []); +}); + +test('a pin that predates the merge of the pull request this change merges after fails, saying to bump it', async () => { + pair({}, { state: 'closed', merged: true }); + comparisons.set(`${mergeCommit(791)}...${PIN}`, 'behind'); + let { resolution, problems } = await resolveBoxel(PIN); + assert.deepEqual(resolution.pairs, []); + assert.equal(problems.length, 1); + assert.equal( + problems[0], + `cardstack/boxel-catalog#791 merged as ${mergeCommit(791).slice(0, 12)}, ` + + `but cardstack/boxel#6454 pins ${PIN.slice(0, 12)}, which predates it. ` + + 'Run `pnpm --dir packages/catalog catalog:test-subset --bump`.', + ); +}); + +test("a pin at the head of the pull request this change merges after, which main doesn't contain after a squash, fails", async () => { + pair({}, { state: 'closed', merged: true }); + comparisons.set(`${mergeCommit(791)}...${PIN}`, 'diverged'); + let { problems } = await resolveBoxel(PIN); + assert.equal(problems.length, 1); + assert.match(problems[0], /which predates it\. Run `pnpm/); +}); + +test('a pin GitHub cannot compare with the merge commit fails, saying to re-run', async () => { + pair({}, { state: 'closed', merged: true }); + let { problems } = await resolveBoxel(PIN); + assert.equal(problems.length, 1); + assert.match( + problems[0], + /Could not read whether cardstack\/boxel#6454's pin aaaaaaaaaaaa contains cardstack\/boxel-catalog#791/, + ); +}); + +test('the pin must contain the commit a stacked counterpart reached main through: its last parent', async () => { + pair({}, { base: 'catalog-parent', state: 'closed', merged: true }); + pulls.push({ + repository: CATALOG, + number: 780, + head: 'catalog-parent', + state: 'closed', + merged: true, + }); + comparisons.set(`${mergeCommit(791)}...${PIN}`, 'diverged'); + comparisons.set(`${mergeCommit(780)}...${PIN}`, 'ahead'); + let { problems } = await resolveBoxel(PIN); + assert.deepEqual(problems, []); +}); + +test('an open pull request this change merges after resolves, and the pin verdict waits for it to merge and a re-pin', async () => { + pair({}); + let { resolution, problems } = await resolveBoxel(PIN); + assert.deepEqual(problems, []); + assert.equal( + pinVerdict(resolution), + `cardstack/boxel#6454 merges after cardstack/boxel-catalog#791, which ` + + `hasn't merged, so cardstack/boxel#6454's pin ${PIN.slice(0, 12)} ` + + `can't contain it yet. Production deploys the catalog at the revision ` + + `boxel pins. Once cardstack/boxel-catalog#791 merges, run ` + + '`pnpm --dir packages/catalog catalog:test-subset --bump` and push: ' + + 'that push re-runs this check.', + ); +}); + +test('the pin verdict passes once the pull request this change merges after is merged and in the pin', async () => { + pair({}, { state: 'closed', merged: true }); + comparisons.set(`${mergeCommit(791)}...${PIN}`, 'ahead'); + let { resolution } = await resolveBoxel(PIN); + assert.equal(pinVerdict(resolution), undefined); +}); + +test('the pin verdict does not wait on an open pull request this change merges before', async () => { + pair( + { body: 'Merges before: cardstack/boxel-catalog#791' }, + { body: 'Merges after: cardstack/boxel#6454' }, + ); + let { resolution } = await resolveBoxel(PIN); + assert.equal(pinVerdict(resolution), undefined); +}); + +test('without a pin, the pin verdict does not wait on an open pull request this change merges after', async () => { + pair({}); + let { resolution } = await resolveBoxel(); + assert.equal(pinVerdict(resolution), undefined); +}); + +test('a merged pull request this change merges before is not held to the pin', async () => { + pair( + { body: 'Merges before: cardstack/boxel-catalog#791' }, + { + body: 'Merges after: cardstack/boxel#6454', + state: 'closed', + merged: true, + }, + ); + let { problems } = await resolveBoxel(PIN); + assert.deepEqual(problems, []); +}); + +test('without a pin, a merged pull request this change merges after resolves', async () => { + pair({}, { state: 'closed', merged: true }); + let { problems } = await resolveBoxel(); + assert.deepEqual(problems, []); +}); + test('a description that declares no pair says nothing about its base', async () => { let { problems, notices } = await resolvePairing( BOXEL, diff --git a/packages/catalog/scripts/pairing.ts b/packages/catalog/scripts/pairing.ts index ab2fda2d942..53bdd75637d 100644 --- a/packages/catalog/scripts/pairing.ts +++ b/packages/catalog/scripts/pairing.ts @@ -18,7 +18,8 @@ // Run with the pull request's description in $PR_BODY: // // node scripts/pairing.ts --repository= --number= \ -// --base= --counterpart= --out= +// --base= --counterpart= --out= \ +// [--pin=] // // Writes the resolution to as JSON, with one entry in `pairs` per // declared key: the counterpart's number, head commit, whether it has merged, @@ -28,6 +29,19 @@ // stacked, come from a branch of its own repository, and name this pull // request back. // +// With --pin, the counterpart repository's revision that this pull request +// pins (the `revision` of the catalog test subset manifest) must contain every +// merged counterpart this pull request merges after. Production deploys the +// catalog at the revision the deployed boxel pins, so a pull request that +// merges after a catalog change ships without it until the pin moves past that +// change's merge commit. While that counterpart is still open, the pin can't +// contain it yet, so the resolution records the pin and +// +// node scripts/pairing.ts --pin-verdict= +// +// fails until the counterpart merges and a push re-pins past it: nothing else +// re-runs the check when a pull request in the other repository merges. +// // A paired pull request targets main, or is stacked on an open pull request // of its own repository: it targets that pull request's branch, and is // retargeted to main once that pull request merges. Either side of a pair may @@ -41,7 +55,7 @@ // the reason, and whatever consumes the resolution decides whether that // matters. -import { writeFileSync } from 'node:fs'; +import { readFileSync, writeFileSync } from 'node:fs'; import { resolve } from 'node:path'; export type PairingKey = 'merges-before' | 'merges-after'; @@ -70,16 +84,19 @@ export interface Resolution { pairs: Pair[]; // The open pull request this one is stacked on, when it is. stackedOn?: string; + // The counterpart revision this pull request pins, when it was checked. + pinnedRevision?: string; } // Where a pull request's base leads. `main` is main itself. `stacked` is the // branch of an open pull request, `parent`, reached directly or through the // branches of parents that have merged since (`mergedVia`, the first of them). -// `landed` is a chain of merged parents that ends on main. +// `landed` is a chain of merged parents that ends on main, and `mergeCommit` +// is the commit the last of them merged into main as. export type Landing = | { kind: 'main' } | { kind: 'stacked'; parent: string; branch: string; mergedVia?: string } - | { kind: 'landed'; mergedVia: string } + | { kind: 'landed'; mergedVia: string; mergeCommit: string | null } | { kind: 'problem'; problem: string }; // A pull request as the list endpoint returns it, which has `merged_at` but @@ -88,6 +105,7 @@ interface ListedPull { number: number; state: string; merged_at: string | null; + merge_commit_sha: string | null; base: { ref: string }; head: { repo: { full_name: string } | null }; } @@ -198,6 +216,7 @@ interface RestPull { state: string; merged: boolean; merged_at: string | null; + merge_commit_sha: string | null; draft?: boolean; body: string | null; base: { ref: string }; @@ -244,10 +263,13 @@ export async function landing( let [owner] = repository.split('/'); let branch = base; let mergedVia: string | undefined; + let mergeCommit: string | null = null; let since = mergedAt; for (let depth = 0; depth < MAX_STACK_DEPTH; depth++) { if (branch === 'main') { - return mergedVia ? { kind: 'landed', mergedVia } : { kind: 'main' }; + return mergedVia + ? { kind: 'landed', mergedVia, mergeCommit } + : { kind: 'main' }; } let listed: ListedPull[] | undefined; try { @@ -304,6 +326,7 @@ export async function landing( }; } mergedVia ??= `${repository}#${merged.number}`; + mergeCommit = merged.merge_commit_sha; branch = merged.base.ref; since = merged.merged_at!; } @@ -421,14 +444,68 @@ async function fetchApproval(repository: string, n: number) { return states.includes('APPROVED') && !states.includes('CHANGES_REQUESTED'); } +// Whether `revision` of `repository` contains `commit`: GitHub's compare of +// the commit (base) against the revision (head) is ahead or identical. +async function contains(repository: string, commit: string, revision: string) { + let comparison = await getJson<{ status: string }>( + `repos/${repository}/compare/${commit}...${revision}?per_page=1`, + ); + if (!comparison) { + throw new Error( + `GitHub can't compare ${commit.slice(0, 12)} with ${revision.slice(0, 12)}`, + ); + } + return comparison.status === 'ahead' || comparison.status === 'identical'; +} + +// What a pull request that merges after `there`, which reached main as +// `mergeCommit`, is told when the revision it pins doesn't contain that, or +// nothing when it does. +async function pinProblem( + here: string, + there: string, + repository: string, + mergeCommit: string | null, + pinnedRevision: string, +) { + if (!mergeCommit) { + return ( + `${there} has merged, but GitHub reports no merge commit for it, so ` + + `whether ${here}'s pin contains it can't be read. Re-run this check.` + ); + } + let pinned: boolean; + try { + pinned = await contains(repository, mergeCommit, pinnedRevision); + } catch (error) { + return ( + `Could not read whether ${here}'s pin ${pinnedRevision.slice(0, 12)} ` + + `contains ${there}, which merged as ${mergeCommit.slice(0, 12)} ` + + `(${error instanceof Error ? error.message : String(error)}). Re-run ` + + `this check.` + ); + } + if (pinned) { + return undefined; + } + return ( + `${there} merged as ${mergeCommit.slice(0, 12)}, but ${here} pins ` + + `${pinnedRevision.slice(0, 12)}, which predates it. Run ` + + `\`pnpm --dir packages/catalog catalog:test-subset --bump\`.` + ); +} + // Checks each declaration against its counterpart and returns the pairs, or -// the problems, each one saying what to change on which pull request. +// the problems, each one saying what to change on which pull request. With +// `pinnedRevision`, the counterpart revision this pull request pins, a merged +// counterpart it merges after must be in that revision. export async function resolvePairing( repository: string, n: number, base: string, counterpartRepository: string, body: string, + pinnedRevision?: string, ): Promise<{ resolution: Resolution; problems: string[]; notices: string[] }> { let here = `${repository}#${n}`; let notices: string[] = []; @@ -582,6 +659,21 @@ export async function resolvePairing( ); continue; } + if (pull.merged && key === 'merges-after' && pinnedRevision) { + let problem = await pinProblem( + here, + there, + counterpartRepository, + counterpartLanding.kind === 'landed' + ? counterpartLanding.mergeCommit + : pull.merge_commit_sha, + pinnedRevision, + ); + if (problem) { + problems.push(problem); + continue; + } + } let approved: boolean | null = false; let approvalError: string | undefined; if (!pull.merged) { @@ -611,12 +703,37 @@ export async function resolvePairing( number: n, pairs, ...(stackedOn ? { stackedOn } : {}), + ...(pinnedRevision ? { pinnedRevision } : {}), }, problems, notices, }; } +// Why a pull request with a pin can't pass yet, or nothing when it can. A +// counterpart it merges after that is still open isn't in the pin, and the +// push that re-pins after it merges is what runs the check again. +export function pinVerdict(resolution: Resolution) { + if (!resolution.pinnedRevision) { + return undefined; + } + let open = resolution.pairs.find( + (p) => p.key === 'merges-after' && !p.merged, + ); + if (!open) { + return undefined; + } + let here = `${resolution.repository}#${resolution.number}`; + let there = `${open.repository}#${open.number}`; + return ( + `${here} merges after ${there}, which hasn't merged, so ${here}'s pin ` + + `${resolution.pinnedRevision.slice(0, 12)} can't contain it yet. ` + + `Production deploys the catalog at the revision boxel pins. Once ` + + `${there} merges, run \`pnpm --dir packages/catalog catalog:test-subset ` + + `--bump\` and push: that push re-runs this check.` + ); +} + // Whether catalog main may fail with the errors the change adds to it. A // catalog pull request this change merges after has to land first, so while it // is open the verdict is to wait for it. Otherwise they may only while a @@ -751,11 +868,23 @@ async function main() { let args = process.argv.slice(2); let option = (name: string) => args.find((a) => a.startsWith(`--${name}=`))?.slice(name.length + 3); + let verdictFile = option('pin-verdict'); + if (verdictFile) { + let message = pinVerdict( + JSON.parse(readFileSync(resolve(verdictFile), 'utf8')) as Resolution, + ); + if (message) { + console.log(`::error title=pairing::${annotation(message)}`); + process.exit(1); + } + process.exit(0); + } let repository = option('repository'); let n = Number(option('number')); let base = option('base'); let counterpart = option('counterpart'); let out = option('out'); + let pin = option('pin'); if ( !repository || !Number.isInteger(n) || @@ -765,16 +894,28 @@ async function main() { !out ) { console.error( - 'pairing: pass --repository= --number= --base= --counterpart= --out=, with the description in $PR_BODY', + 'pairing: pass --repository= --number= --base= --counterpart= --out= [--pin=], with the description in $PR_BODY', ); process.exit(1); } + let pinnedRevision: string | undefined; + if (pin) { + let manifest = JSON.parse(readFileSync(resolve(pin), 'utf8')) as { + revision?: unknown; + }; + if (typeof manifest.revision !== 'string' || !manifest.revision) { + console.error(`pairing: ${pin} has no revision`); + process.exit(1); + } + pinnedRevision = manifest.revision; + } let { resolution, problems, notices } = await resolvePairing( repository, n, base, counterpart, process.env.PR_BODY ?? '', + pinnedRevision, ); let here = `${repository}#${n}`; for (let pair of resolution.pairs) {