From 8e9afc85b130999892598fa4e9e830f55cdbd810 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:43:48 -0500 Subject: [PATCH 01/43] Replace CODEOWNERS with author-merge GitHub Action CODEOWNERS requires all listed users to have repo write access, which doesn't work for external GAP authors. Replace it with a GitHub Action that reads metadata.yml to determine who can merge PRs scoped to a single GAP directory. Co-Authored-By: Claude Opus 4.7 --- .github/workflows/author-merge.yml | 95 +++++++++++++++++++++++++++ .github/workflows/sync-codeowners.yml | 40 ----------- CODEOWNERS | 3 - CONTRIBUTING.md | 23 +++++-- package.json | 3 +- scripts/sync-codeowners.js | 34 ---------- 6 files changed, 113 insertions(+), 85 deletions(-) create mode 100644 .github/workflows/author-merge.yml delete mode 100644 .github/workflows/sync-codeowners.yml delete mode 100644 CODEOWNERS delete mode 100755 scripts/sync-codeowners.js diff --git a/.github/workflows/author-merge.yml b/.github/workflows/author-merge.yml new file mode 100644 index 0000000..8e43833 --- /dev/null +++ b/.github/workflows/author-merge.yml @@ -0,0 +1,95 @@ +name: Author Merge + +on: + pull_request_review: + types: [submitted] + issue_comment: + types: [created] + +permissions: + contents: write + pull-requests: write + +jobs: + author-merge: + runs-on: ubuntu-latest + if: > + (github.event_name == 'pull_request_review' && github.event.review.state == 'approved') || + (github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.body == '/merge') + steps: + - uses: actions/checkout@v4 + + - name: Get PR details + id: pr + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ "${{ github.event_name }}" = "pull_request_review" ]; then + PR_NUMBER="${{ github.event.pull_request.number }}" + ACTOR="${{ github.event.review.user.login }}" + else + PR_NUMBER="${{ github.event.issue.number }}" + ACTOR="${{ github.event.comment.user.login }}" + fi + echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT" + echo "actor=$ACTOR" >> "$GITHUB_OUTPUT" + + - name: Fetch PR changed files + id: files + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + FILES=$(gh pr diff "${{ steps.pr.outputs.pr_number }}" --name-only) + echo "files<> "$GITHUB_OUTPUT" + echo "$FILES" >> "$GITHUB_OUTPUT" + echo "EOF" >> "$GITHUB_OUTPUT" + + - name: Determine affected GAP directory + id: gap + run: | + DIRS=$(echo "${{ steps.files.outputs.files }}" | grep -oP '^gaps/GAP-\d+' | sort -u) + DIR_COUNT=$(echo "$DIRS" | wc -l) + + if [ "$DIR_COUNT" -ne 1 ]; then + echo "::error::PR touches files in multiple GAP directories (or none). Author merge only works for single-GAP PRs." + exit 1 + fi + + # Ensure all files are within this directory + GAP_DIR="$DIRS" + NON_GAP_FILES=$(echo "${{ steps.files.outputs.files }}" | grep -v "^${GAP_DIR}/" || true) + if [ -n "$NON_GAP_FILES" ]; then + echo "::error::PR modifies files outside of $GAP_DIR — author merge not permitted." + exit 1 + fi + + echo "gap_dir=$GAP_DIR" >> "$GITHUB_OUTPUT" + + - name: Check actor is an author or sponsor + run: | + METADATA="${{ steps.gap.outputs.gap_dir }}/metadata.yml" + if [ ! -f "$METADATA" ]; then + echo "::error::No metadata.yml found in ${{ steps.gap.outputs.gap_dir }}" + exit 1 + fi + + ACTOR="${{ steps.pr.outputs.actor }}" + + # Extract GitHub usernames from authors list and sponsor field + AUTHORIZED=$(grep -oP '(?<=githubUsername:\s"@)\w[-\w]*' "$METADATA" || true) + SPONSOR=$(grep -oP '(?<=sponsor:\s"@)\w[-\w]*' "$METADATA" || true) + + ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORIZED" "$SPONSOR" | sort -u) + + if echo "$ALL_AUTHORIZED" | grep -qx "$ACTOR"; then + echo "✓ $ACTOR is authorized to merge changes to ${{ steps.gap.outputs.gap_dir }}" + else + echo "::error::$ACTOR is not listed as an author or sponsor in $METADATA" + exit 1 + fi + + - name: Merge PR + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + gh pr merge "${{ steps.pr.outputs.pr_number }}" --squash --auto diff --git a/.github/workflows/sync-codeowners.yml b/.github/workflows/sync-codeowners.yml deleted file mode 100644 index 48833b9..0000000 --- a/.github/workflows/sync-codeowners.yml +++ /dev/null @@ -1,40 +0,0 @@ -name: Sync CODEOWNERS - -on: - push: - branches: [main] - paths: - - "gaps/*/metadata.yml" - -jobs: - sync: - name: Sync CODEOWNERS from metadata - runs-on: ubuntu-latest - - permissions: - contents: write - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Setup Node.js - uses: actions/setup-node@v4 - with: - node-version: "24" - cache: "npm" - - - name: Install dependencies - run: npm ci - - - name: Sync CODEOWNERS - run: node scripts/sync-codeowners.js - - - name: Commit if changed - run: | - git diff --quiet CODEOWNERS && exit 0 - git config user.name "github-actions[bot]" - git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - git add CODEOWNERS - git commit -m "Sync CODEOWNERS from GAP metadata" - git push diff --git a/CODEOWNERS b/CODEOWNERS deleted file mode 100644 index 908e748..0000000 --- a/CODEOWNERS +++ /dev/null @@ -1,3 +0,0 @@ -/gaps/GAP-10/ @magicmark @rebello95 -/gaps/GAP-13/ @benjie -/gaps/GAP-7/ @magicmark diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c223c4..ebab92a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,9 +11,8 @@ that address issues outside the core GraphQL specifications. - **Sponsor** — an _editor_ assigned to a GAP who is responsible for approving the initial contents. A _sponsor_ may also be an _author_. - **Author** — a person (or people) who have made significant contributions to a - GAP, listed in the `authors` field of `metadata.yml`. _Authors_ are given - commit access via `CODEOWNERS` to merge their own and others' submissions to - the GAP. + GAP, listed in the `authors` field of `metadata.yml`. _Authors_ can merge PRs + that only touch their GAP directory (see [Merging](#merging) below). ## GAP Numbering @@ -62,9 +61,6 @@ gauge public interest, but doing so is not necessary. Once approved by the _authors_ and _sponsor_, the PR should be merged by the _sponsor_. -`CODEOWNERS` will automatically be updated allowing _authors_ to merge future -contributions to their GAP. - > [!IMPORTANT] > GAP numbers never change. If a proposal needs significant changes, create a > new GAP and deprecate the old one. @@ -147,6 +143,21 @@ Rules: This optional file can be created/edited by the TSC or editors to outline the status of a published release, including a top-of-document notice or errata. +## Merging + +PRs that only modify files within a single `gaps/GAP-N/` directory can be merged +by any _author_ or _sponsor_ listed in that directory's `metadata.yml`. To +trigger a merge, either: + +- Submit a PR review with "Approve", or +- Comment `/merge` on the PR. + +A GitHub Action will verify that the actor is authorized and that the PR is +scoped to a single GAP directory before merging. + +PRs that touch files outside a single GAP directory (e.g. repository-wide +changes) must be merged by an _editor_ or TSC member. + ## Commit access Commit access is granted to this repo to _editors_ diff --git a/package.json b/package.json index a2621eb..8f6bf9f 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,7 @@ "suggest:format": "echo \"\nTo resolve this, run: $(tput bold)npm run format$(tput sgr0)\" && exit 1", "test:format": "prettier --check . || npm run suggest:format", "test:spelling": "cspell \"spec/**/*.md\" README.md LICENSE.md", - "test:structure": "find ./gaps -maxdepth 1 -type d -name 'GAP-*' | xargs -I{} ./scripts/validate-structure.js {}", - "sync:codeowners": "node scripts/sync-codeowners.js" + "test:structure": "find ./gaps -maxdepth 1 -type d -name 'GAP-*' | xargs -I{} ./scripts/validate-structure.js {}" }, "devDependencies": { "@mlarah/spec-md": "^3.1.0", diff --git a/scripts/sync-codeowners.js b/scripts/sync-codeowners.js deleted file mode 100755 index 614b40f..0000000 --- a/scripts/sync-codeowners.js +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env node - -import { readdir, readFile, writeFile } from "node:fs/promises"; -import { join, dirname } from "node:path"; -import { fileURLToPath } from "node:url"; -import { parse as parseYaml } from "yaml"; - -const __dirname = dirname(fileURLToPath(import.meta.url)); -const rootDir = join(__dirname, ".."); -const gapsDir = join(rootDir, "gaps"); - -async function getGapDirs() { - const entries = await readdir(gapsDir, { withFileTypes: true }); - return entries.filter((d) => d.isDirectory() && /^GAP-[1-9]\d*$/.test(d.name)); -} - -async function main() { - const dirs = await getGapDirs(); - dirs.sort(); - - const lines = await Promise.all( - dirs.map(async (dir) => { - const metadataPath = join(gapsDir, dir.name, "metadata.yml"); - const metadata = parseYaml(await readFile(metadataPath, "utf8")); - const owners = metadata.authors.map((a) => a.githubUsername.replace(/^@/, "")); - const ownerList = owners.map((o) => `@${o}`).join(" "); - return `/gaps/${dir.name}/ ${ownerList}`; - }), - ); - - await writeFile(join(rootDir, "CODEOWNERS"), lines.join("\n") + "\n"); -} - -main(); From aece13a431aca78a4611a11c115f997ed18f4bf2 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:48:19 -0500 Subject: [PATCH 02/43] Simplify to /merge comment trigger only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove auto-merge on PR approval — merging should always be an explicit action via /merge comment. Co-Authored-By: Claude Opus 4.7 --- .github/workflows/author-merge.yml | 18 +++--------------- CONTRIBUTING.md | 9 +++------ 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/.github/workflows/author-merge.yml b/.github/workflows/author-merge.yml index 8e43833..3ebeb14 100644 --- a/.github/workflows/author-merge.yml +++ b/.github/workflows/author-merge.yml @@ -1,8 +1,6 @@ name: Author Merge on: - pull_request_review: - types: [submitted] issue_comment: types: [created] @@ -14,25 +12,15 @@ jobs: author-merge: runs-on: ubuntu-latest if: > - (github.event_name == 'pull_request_review' && github.event.review.state == 'approved') || - (github.event_name == 'issue_comment' && github.event.issue.pull_request && github.event.comment.body == '/merge') + github.event.issue.pull_request && github.event.comment.body == '/merge' steps: - uses: actions/checkout@v4 - name: Get PR details id: pr - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - if [ "${{ github.event_name }}" = "pull_request_review" ]; then - PR_NUMBER="${{ github.event.pull_request.number }}" - ACTOR="${{ github.event.review.user.login }}" - else - PR_NUMBER="${{ github.event.issue.number }}" - ACTOR="${{ github.event.comment.user.login }}" - fi - echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT" - echo "actor=$ACTOR" >> "$GITHUB_OUTPUT" + echo "pr_number=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT" + echo "actor=${{ github.event.comment.user.login }}" >> "$GITHUB_OUTPUT" - name: Fetch PR changed files id: files diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ebab92a..99ee884 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,13 +146,10 @@ status of a published release, including a top-of-document notice or errata. ## Merging PRs that only modify files within a single `gaps/GAP-N/` directory can be merged -by any _author_ or _sponsor_ listed in that directory's `metadata.yml`. To -trigger a merge, either: +by any _author_ or _sponsor_ listed in that directory's `metadata.yml` by +commenting `/merge` on the PR. -- Submit a PR review with "Approve", or -- Comment `/merge` on the PR. - -A GitHub Action will verify that the actor is authorized and that the PR is +A GitHub Action will verify that the commenter is authorized and that the PR is scoped to a single GAP directory before merging. PRs that touch files outside a single GAP directory (e.g. repository-wide From 4ac87baaae8ac38fe60377d82af3a85127e37190 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:49:52 -0500 Subject: [PATCH 03/43] Bump actions/checkout to v6 Co-Authored-By: Claude Opus 4.7 --- .github/workflows/author-merge.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/author-merge.yml b/.github/workflows/author-merge.yml index 3ebeb14..2676938 100644 --- a/.github/workflows/author-merge.yml +++ b/.github/workflows/author-merge.yml @@ -14,7 +14,7 @@ jobs: if: > github.event.issue.pull_request && github.event.comment.body == '/merge' steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - name: Get PR details id: pr From 2d3af10a139f1014b0046113f584f2f53d0230b0 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:50:56 -0500 Subject: [PATCH 04/43] Rename to merge.yml and clean up - Rename author-merge.yml -> merge.yml - Rename job to "merge" - Use contains() for slightly more forgiving /merge detection - Remove redundant comments Co-Authored-By: Claude Opus 4.7 --- .github/workflows/{author-merge.yml => merge.yml} | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) rename .github/workflows/{author-merge.yml => merge.yml} (91%) diff --git a/.github/workflows/author-merge.yml b/.github/workflows/merge.yml similarity index 91% rename from .github/workflows/author-merge.yml rename to .github/workflows/merge.yml index 2676938..9c25187 100644 --- a/.github/workflows/author-merge.yml +++ b/.github/workflows/merge.yml @@ -1,4 +1,4 @@ -name: Author Merge +name: Merge on: issue_comment: @@ -9,10 +9,11 @@ permissions: pull-requests: write jobs: - author-merge: + merge: runs-on: ubuntu-latest if: > - github.event.issue.pull_request && github.event.comment.body == '/merge' + github.event.issue.pull_request && + contains(github.event.comment.body, '/merge') steps: - uses: actions/checkout@v6 @@ -43,7 +44,6 @@ jobs: exit 1 fi - # Ensure all files are within this directory GAP_DIR="$DIRS" NON_GAP_FILES=$(echo "${{ steps.files.outputs.files }}" | grep -v "^${GAP_DIR}/" || true) if [ -n "$NON_GAP_FILES" ]; then @@ -63,10 +63,8 @@ jobs: ACTOR="${{ steps.pr.outputs.actor }}" - # Extract GitHub usernames from authors list and sponsor field AUTHORIZED=$(grep -oP '(?<=githubUsername:\s"@)\w[-\w]*' "$METADATA" || true) SPONSOR=$(grep -oP '(?<=sponsor:\s"@)\w[-\w]*' "$METADATA" || true) - ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORIZED" "$SPONSOR" | sort -u) if echo "$ALL_AUTHORIZED" | grep -qx "$ACTOR"; then From 561ab6aed472db989b378e22352414ecd3582f46 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:53:27 -0500 Subject: [PATCH 05/43] Use yq instead of grep -oP for parsing metadata.yml Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 9c25187..c2a5c62 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -63,9 +63,9 @@ jobs: ACTOR="${{ steps.pr.outputs.actor }}" - AUTHORIZED=$(grep -oP '(?<=githubUsername:\s"@)\w[-\w]*' "$METADATA" || true) - SPONSOR=$(grep -oP '(?<=sponsor:\s"@)\w[-\w]*' "$METADATA" || true) - ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORIZED" "$SPONSOR" | sort -u) + AUTHORS=$(yq '.authors[].githubUsername | sub("^@", "")' "$METADATA") + SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") + ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) if echo "$ALL_AUTHORIZED" | grep -qx "$ACTOR"; then echo "✓ $ACTOR is authorized to merge changes to ${{ steps.gap.outputs.gap_dir }}" From 4bf612cad9e636fcc6022e9c1b63eafafb3a64ff Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:55:00 -0500 Subject: [PATCH 06/43] Harden merge workflow - Fix shell injection: route all ${{ }} through env vars - Fix empty-grep bug: explicit check for zero GAP dirs - Fix regex matching: use grep -qFx for username comparison - Use portable grep -oE instead of Perl -oP - Add concurrency guard to prevent merge races - Remove redundant "Get PR details" step Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 44 ++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index c2a5c62..2e2261c 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -8,6 +8,10 @@ permissions: contents: write pull-requests: write +concurrency: + group: merge-${{ github.event.issue.number }} + cancel-in-progress: true + jobs: merge: runs-on: ubuntu-latest @@ -17,35 +21,37 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Get PR details - id: pr - run: | - echo "pr_number=${{ github.event.issue.number }}" >> "$GITHUB_OUTPUT" - echo "actor=${{ github.event.comment.user.login }}" >> "$GITHUB_OUTPUT" - - name: Fetch PR changed files id: files env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.issue.number }} run: | - FILES=$(gh pr diff "${{ steps.pr.outputs.pr_number }}" --name-only) + FILES=$(gh pr diff "$PR_NUMBER" --name-only) echo "files<> "$GITHUB_OUTPUT" echo "$FILES" >> "$GITHUB_OUTPUT" echo "EOF" >> "$GITHUB_OUTPUT" - name: Determine affected GAP directory id: gap + env: + CHANGED_FILES: ${{ steps.files.outputs.files }} run: | - DIRS=$(echo "${{ steps.files.outputs.files }}" | grep -oP '^gaps/GAP-\d+' | sort -u) - DIR_COUNT=$(echo "$DIRS" | wc -l) + DIRS=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u) + if [ -z "$DIRS" ]; then + echo "::error::PR does not touch any GAP directory." + exit 1 + fi + + DIR_COUNT=$(echo "$DIRS" | wc -l) if [ "$DIR_COUNT" -ne 1 ]; then - echo "::error::PR touches files in multiple GAP directories (or none). Author merge only works for single-GAP PRs." + echo "::error::PR touches multiple GAP directories. Author merge only works for single-GAP PRs." exit 1 fi GAP_DIR="$DIRS" - NON_GAP_FILES=$(echo "${{ steps.files.outputs.files }}" | grep -v "^${GAP_DIR}/" || true) + NON_GAP_FILES=$(echo "$CHANGED_FILES" | grep -v "^${GAP_DIR}/" || true) if [ -n "$NON_GAP_FILES" ]; then echo "::error::PR modifies files outside of $GAP_DIR — author merge not permitted." exit 1 @@ -54,21 +60,22 @@ jobs: echo "gap_dir=$GAP_DIR" >> "$GITHUB_OUTPUT" - name: Check actor is an author or sponsor + env: + GAP_DIR: ${{ steps.gap.outputs.gap_dir }} + ACTOR: ${{ github.event.comment.user.login }} run: | - METADATA="${{ steps.gap.outputs.gap_dir }}/metadata.yml" + METADATA="${GAP_DIR}/metadata.yml" if [ ! -f "$METADATA" ]; then - echo "::error::No metadata.yml found in ${{ steps.gap.outputs.gap_dir }}" + echo "::error::No metadata.yml found in $GAP_DIR" exit 1 fi - ACTOR="${{ steps.pr.outputs.actor }}" - AUTHORS=$(yq '.authors[].githubUsername | sub("^@", "")' "$METADATA") SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) - if echo "$ALL_AUTHORIZED" | grep -qx "$ACTOR"; then - echo "✓ $ACTOR is authorized to merge changes to ${{ steps.gap.outputs.gap_dir }}" + if echo "$ALL_AUTHORIZED" | grep -qFx "$ACTOR"; then + echo "✓ $ACTOR is authorized to merge changes to $GAP_DIR" else echo "::error::$ACTOR is not listed as an author or sponsor in $METADATA" exit 1 @@ -77,5 +84,6 @@ jobs: - name: Merge PR env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.issue.number }} run: | - gh pr merge "${{ steps.pr.outputs.pr_number }}" --squash --auto + gh pr merge "$PR_NUMBER" --squash --auto From 7ea531b8826138a1f8fc848cd3144d86f5382521 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:55:28 -0500 Subject: [PATCH 07/43] Clarify sponsor merges the initial GAP PR Co-Authored-By: Claude Opus 4.7 --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 99ee884..7a885ca 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,7 +9,7 @@ that address issues outside the core GraphQL specifications. ([@graphql/gaps-editors](https://github.com/orgs/graphql/teams/gaps-editors)), approved by the TSC to administer the GAP program. - **Sponsor** — an _editor_ assigned to a GAP who is responsible for approving - the initial contents. A _sponsor_ may also be an _author_. + and merging the initial contents. A _sponsor_ may also be an _author_. - **Author** — a person (or people) who have made significant contributions to a GAP, listed in the `authors` field of `metadata.yml`. _Authors_ can merge PRs that only touch their GAP directory (see [Merging](#merging) below). From 58c8995a31b75151bf29ee382666b9ca78f869f4 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:56:21 -0500 Subject: [PATCH 08/43] Use exact match for /merge comment trigger Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 2e2261c..bb263ad 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -17,7 +17,7 @@ jobs: runs-on: ubuntu-latest if: > github.event.issue.pull_request && - contains(github.event.comment.body, '/merge') + github.event.comment.body == '/merge' steps: - uses: actions/checkout@v6 From b8f0081e6046b2623946f8c7839e9f5eaff19d8d Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:56:56 -0500 Subject: [PATCH 09/43] =?UTF-8?q?Remove=20concurrency=20guard=20=E2=80=94?= =?UTF-8?q?=20merge=20is=20idempotent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index bb263ad..f00ccb8 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -8,10 +8,6 @@ permissions: contents: write pull-requests: write -concurrency: - group: merge-${{ github.event.issue.number }} - cancel-in-progress: true - jobs: merge: runs-on: ubuntu-latest From d3857bc8e6b68aaab853011823a8fecbf7c67144 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 12:59:53 -0500 Subject: [PATCH 10/43] Simplify auth check to early-exit on failure Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index f00ccb8..93a2ced 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -70,9 +70,7 @@ jobs: SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) - if echo "$ALL_AUTHORIZED" | grep -qFx "$ACTOR"; then - echo "✓ $ACTOR is authorized to merge changes to $GAP_DIR" - else + if ! echo "$ALL_AUTHORIZED" | grep -qFx "$ACTOR"; then echo "::error::$ACTOR is not listed as an author or sponsor in $METADATA" exit 1 fi From d47fe1cf09cccde92e442d3f20f9f41a60785a94 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 13:11:18 -0500 Subject: [PATCH 11/43] Update CONTRIBUTING.md --- CONTRIBUTING.md | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7a885ca..64e0992 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -122,6 +122,12 @@ The _sponsor_ of a GAP is responsible for ensuring changes to the GAP are approved by the _authors_ before merging, though this task may also be performed by the TSC. The _authors_ are responsible for guiding contribution to the GAP. +### Merging + +PRs that only modify files within a single `gaps/GAP-N/` directory can be merged +by any _author_ listed in that directory's `metadata.yml` by commenting `/merge` +on the PR. + ### Versioning To release a version of a GAP, copy the current `DRAFT.md` into a `versions` @@ -143,18 +149,6 @@ Rules: This optional file can be created/edited by the TSC or editors to outline the status of a published release, including a top-of-document notice or errata. -## Merging - -PRs that only modify files within a single `gaps/GAP-N/` directory can be merged -by any _author_ or _sponsor_ listed in that directory's `metadata.yml` by -commenting `/merge` on the PR. - -A GitHub Action will verify that the commenter is authorized and that the PR is -scoped to a single GAP directory before merging. - -PRs that touch files outside a single GAP directory (e.g. repository-wide -changes) must be merged by an _editor_ or TSC member. - ## Commit access Commit access is granted to this repo to _editors_ From 1278fb90546d6c5fdd475dbea17ea822a05267b9 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 13:12:59 -0500 Subject: [PATCH 12/43] Merge file-fetch and directory-check into one step Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 93a2ced..d40a513 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -17,22 +17,13 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Fetch PR changed files - id: files - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.issue.number }} - run: | - FILES=$(gh pr diff "$PR_NUMBER" --name-only) - echo "files<> "$GITHUB_OUTPUT" - echo "$FILES" >> "$GITHUB_OUTPUT" - echo "EOF" >> "$GITHUB_OUTPUT" - - name: Determine affected GAP directory id: gap env: - CHANGED_FILES: ${{ steps.files.outputs.files }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.issue.number }} run: | + CHANGED_FILES=$(gh pr diff "$PR_NUMBER" --name-only) DIRS=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u) if [ -z "$DIRS" ]; then From 96e855a50d2c6edad7eb3198859016733385f851 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 13:14:50 -0500 Subject: [PATCH 13/43] Remove redundant metadata.yml existence check Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index d40a513..b7d2afe 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -52,11 +52,6 @@ jobs: ACTOR: ${{ github.event.comment.user.login }} run: | METADATA="${GAP_DIR}/metadata.yml" - if [ ! -f "$METADATA" ]; then - echo "::error::No metadata.yml found in $GAP_DIR" - exit 1 - fi - AUTHORS=$(yq '.authors[].githubUsername | sub("^@", "")' "$METADATA") SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) From 0a58804cd20a1532360ecccfeb2692a1d9de6e12 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 13:19:13 -0500 Subject: [PATCH 14/43] Rewrite verification step with github-script Bash step resolves metadata (authors/sponsor as JSON), then a github-script step does the authorization and path-scoping checks using the GitHub API. Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 72 ++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index b7d2afe..3e31e67 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -17,49 +17,55 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Determine affected GAP directory - id: gap + - name: Resolve authors from metadata + id: metadata env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.issue.number }} run: | CHANGED_FILES=$(gh pr diff "$PR_NUMBER" --name-only) - DIRS=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u) - - if [ -z "$DIRS" ]; then - echo "::error::PR does not touch any GAP directory." - exit 1 - fi - - DIR_COUNT=$(echo "$DIRS" | wc -l) - if [ "$DIR_COUNT" -ne 1 ]; then - echo "::error::PR touches multiple GAP directories. Author merge only works for single-GAP PRs." - exit 1 - fi - - GAP_DIR="$DIRS" - NON_GAP_FILES=$(echo "$CHANGED_FILES" | grep -v "^${GAP_DIR}/" || true) - if [ -n "$NON_GAP_FILES" ]; then - echo "::error::PR modifies files outside of $GAP_DIR — author merge not permitted." - exit 1 - fi - + GAP_DIR=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u | head -1) echo "gap_dir=$GAP_DIR" >> "$GITHUB_OUTPUT" + echo "authors=$(yq -o=json '.authors[].githubUsername | sub("^@", "")' "$GAP_DIR/metadata.yml" | jq -sc '.')" >> "$GITHUB_OUTPUT" + echo "sponsor=$(yq '.sponsor | sub("^@", "")' "$GAP_DIR/metadata.yml")" >> "$GITHUB_OUTPUT" - - name: Check actor is an author or sponsor + - name: Verify actor is authorized + uses: actions/github-script@v7 env: - GAP_DIR: ${{ steps.gap.outputs.gap_dir }} ACTOR: ${{ github.event.comment.user.login }} - run: | - METADATA="${GAP_DIR}/metadata.yml" - AUTHORS=$(yq '.authors[].githubUsername | sub("^@", "")' "$METADATA") - SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") - ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) + PR_NUMBER: ${{ github.event.issue.number }} + GAP_DIR: ${{ steps.metadata.outputs.gap_dir }} + AUTHORS: ${{ steps.metadata.outputs.authors }} + SPONSOR: ${{ steps.metadata.outputs.sponsor }} + with: + script: | + const { ACTOR, PR_NUMBER, GAP_DIR, AUTHORS, SPONSOR } = process.env; + const authorized = new Set([...JSON.parse(AUTHORS), SPONSOR]); + + if (!authorized.has(ACTOR)) { + core.setFailed(`${ACTOR} is not listed as an author or sponsor in ${GAP_DIR}/metadata.yml`); + return; + } + + const { data: files } = await github.rest.pulls.listFiles({ + ...context.repo, + pull_number: Number(PR_NUMBER), + }); + const paths = files.map(f => f.filename); + const gaps = [...new Set(paths.map(p => p.match(/^gaps\/GAP-\d+/)?.[0]).filter(Boolean))]; - if ! echo "$ALL_AUTHORIZED" | grep -qFx "$ACTOR"; then - echo "::error::$ACTOR is not listed as an author or sponsor in $METADATA" - exit 1 - fi + if (gaps.length === 0) { + core.setFailed('PR does not touch any GAP directory.'); + return; + } + if (gaps.length !== 1) { + core.setFailed('PR touches multiple GAP directories.'); + return; + } + if (paths.some(p => !p.startsWith(`${gaps[0]}/`))) { + core.setFailed(`PR modifies files outside of ${gaps[0]}.`); + return; + } - name: Merge PR env: From d973b2b0bef8291cab1ecc09e77247de8b34ccc4 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 13:20:20 -0500 Subject: [PATCH 15/43] Revert "Rewrite verification step with github-script" This reverts commit 0a58804cd20a1532360ecccfeb2692a1d9de6e12. --- .github/workflows/merge.yml | 72 +++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 3e31e67..b7d2afe 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -17,55 +17,49 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Resolve authors from metadata - id: metadata + - name: Determine affected GAP directory + id: gap env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.issue.number }} run: | CHANGED_FILES=$(gh pr diff "$PR_NUMBER" --name-only) - GAP_DIR=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u | head -1) + DIRS=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u) + + if [ -z "$DIRS" ]; then + echo "::error::PR does not touch any GAP directory." + exit 1 + fi + + DIR_COUNT=$(echo "$DIRS" | wc -l) + if [ "$DIR_COUNT" -ne 1 ]; then + echo "::error::PR touches multiple GAP directories. Author merge only works for single-GAP PRs." + exit 1 + fi + + GAP_DIR="$DIRS" + NON_GAP_FILES=$(echo "$CHANGED_FILES" | grep -v "^${GAP_DIR}/" || true) + if [ -n "$NON_GAP_FILES" ]; then + echo "::error::PR modifies files outside of $GAP_DIR — author merge not permitted." + exit 1 + fi + echo "gap_dir=$GAP_DIR" >> "$GITHUB_OUTPUT" - echo "authors=$(yq -o=json '.authors[].githubUsername | sub("^@", "")' "$GAP_DIR/metadata.yml" | jq -sc '.')" >> "$GITHUB_OUTPUT" - echo "sponsor=$(yq '.sponsor | sub("^@", "")' "$GAP_DIR/metadata.yml")" >> "$GITHUB_OUTPUT" - - name: Verify actor is authorized - uses: actions/github-script@v7 + - name: Check actor is an author or sponsor env: + GAP_DIR: ${{ steps.gap.outputs.gap_dir }} ACTOR: ${{ github.event.comment.user.login }} - PR_NUMBER: ${{ github.event.issue.number }} - GAP_DIR: ${{ steps.metadata.outputs.gap_dir }} - AUTHORS: ${{ steps.metadata.outputs.authors }} - SPONSOR: ${{ steps.metadata.outputs.sponsor }} - with: - script: | - const { ACTOR, PR_NUMBER, GAP_DIR, AUTHORS, SPONSOR } = process.env; - const authorized = new Set([...JSON.parse(AUTHORS), SPONSOR]); - - if (!authorized.has(ACTOR)) { - core.setFailed(`${ACTOR} is not listed as an author or sponsor in ${GAP_DIR}/metadata.yml`); - return; - } - - const { data: files } = await github.rest.pulls.listFiles({ - ...context.repo, - pull_number: Number(PR_NUMBER), - }); - const paths = files.map(f => f.filename); - const gaps = [...new Set(paths.map(p => p.match(/^gaps\/GAP-\d+/)?.[0]).filter(Boolean))]; + run: | + METADATA="${GAP_DIR}/metadata.yml" + AUTHORS=$(yq '.authors[].githubUsername | sub("^@", "")' "$METADATA") + SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") + ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) - if (gaps.length === 0) { - core.setFailed('PR does not touch any GAP directory.'); - return; - } - if (gaps.length !== 1) { - core.setFailed('PR touches multiple GAP directories.'); - return; - } - if (paths.some(p => !p.startsWith(`${gaps[0]}/`))) { - core.setFailed(`PR modifies files outside of ${gaps[0]}.`); - return; - } + if ! echo "$ALL_AUTHORIZED" | grep -qFx "$ACTOR"; then + echo "::error::$ACTOR is not listed as an author or sponsor in $METADATA" + exit 1 + fi - name: Merge PR env: From 32b5d6050fa2b42fda17b231ffe1c7e4e9a72750 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 13:39:33 -0500 Subject: [PATCH 16/43] Rewrite merge workflow with github-script Use actions/github-script for verification logic instead of shell. Checks changed files via the GitHub API and reads metadata.yml with js-yaml to authorize the commenter. Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 80 +++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 44 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index b7d2afe..edfed3d 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -16,50 +16,42 @@ jobs: github.event.comment.body == '/merge' steps: - uses: actions/checkout@v6 - - - name: Determine affected GAP directory - id: gap - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - PR_NUMBER: ${{ github.event.issue.number }} - run: | - CHANGED_FILES=$(gh pr diff "$PR_NUMBER" --name-only) - DIRS=$(echo "$CHANGED_FILES" | grep -oE '^gaps/GAP-[0-9]+' | sort -u) - - if [ -z "$DIRS" ]; then - echo "::error::PR does not touch any GAP directory." - exit 1 - fi - - DIR_COUNT=$(echo "$DIRS" | wc -l) - if [ "$DIR_COUNT" -ne 1 ]; then - echo "::error::PR touches multiple GAP directories. Author merge only works for single-GAP PRs." - exit 1 - fi - - GAP_DIR="$DIRS" - NON_GAP_FILES=$(echo "$CHANGED_FILES" | grep -v "^${GAP_DIR}/" || true) - if [ -n "$NON_GAP_FILES" ]; then - echo "::error::PR modifies files outside of $GAP_DIR — author merge not permitted." - exit 1 - fi - - echo "gap_dir=$GAP_DIR" >> "$GITHUB_OUTPUT" - - - name: Check actor is an author or sponsor - env: - GAP_DIR: ${{ steps.gap.outputs.gap_dir }} - ACTOR: ${{ github.event.comment.user.login }} - run: | - METADATA="${GAP_DIR}/metadata.yml" - AUTHORS=$(yq '.authors[].githubUsername | sub("^@", "")' "$METADATA") - SPONSOR=$(yq '.sponsor | sub("^@", "")' "$METADATA") - ALL_AUTHORIZED=$(printf '%s\n%s' "$AUTHORS" "$SPONSOR" | sort -u) - - if ! echo "$ALL_AUTHORIZED" | grep -qFx "$ACTOR"; then - echo "::error::$ACTOR is not listed as an author or sponsor in $METADATA" - exit 1 - fi + - run: npm install js-yaml + - name: Verify actor is authorized + uses: actions/github-script@v9 + with: + script: | + const fs = require('fs'); + const yaml = require('js-yaml'); + + const actor = context.payload.comment.user.login; + const prNumber = context.issue.number; + + const { data: files } = await github.rest.pulls.listFiles({ + ...context.repo, + pull_number: prNumber, + }); + + const gapDirs = files + .map(f => f.filename) + .map(p => p.split('/') + .slice(0, 2).join('/')); + + const gapsChanged = [...new Set(gapDirs)]; + + if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { + throw new Error('You can only run /merge for PRs that touch exactly one GAP directory and nothing else.'); + } + + const metadata = yaml.load(fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, 'utf8')); + const authorizedMergers = new Set([ + ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), + metadata.sponsor.replace(/^@/, ''), + ]); + + if (!authorizedMergers.has(actor)) { + throw new Error(`${actor} is not authorized to merge ${gapsChanged[0]}.`); + } - name: Merge PR env: From 72794b7bc7cbf56f2242bbfdffd71bac96e9abbf Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 16:00:48 -0500 Subject: [PATCH 17/43] Extract verify-merge logic to dedicated script file Addresses review feedback: move JS out of YAML into scripts/verify-merge.js. Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 34 +++--------------------------- scripts/verify-merge.js | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 31 deletions(-) create mode 100644 scripts/verify-merge.js diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index edfed3d..18188da 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -17,41 +17,13 @@ jobs: steps: - uses: actions/checkout@v6 - run: npm install js-yaml + - name: Verify actor is authorized uses: actions/github-script@v9 with: script: | - const fs = require('fs'); - const yaml = require('js-yaml'); - - const actor = context.payload.comment.user.login; - const prNumber = context.issue.number; - - const { data: files } = await github.rest.pulls.listFiles({ - ...context.repo, - pull_number: prNumber, - }); - - const gapDirs = files - .map(f => f.filename) - .map(p => p.split('/') - .slice(0, 2).join('/')); - - const gapsChanged = [...new Set(gapDirs)]; - - if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { - throw new Error('You can only run /merge for PRs that touch exactly one GAP directory and nothing else.'); - } - - const metadata = yaml.load(fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, 'utf8')); - const authorizedMergers = new Set([ - ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), - metadata.sponsor.replace(/^@/, ''), - ]); - - if (!authorizedMergers.has(actor)) { - throw new Error(`${actor} is not authorized to merge ${gapsChanged[0]}.`); - } + const verifyMerge = require('./scripts/verify-merge.js'); + await verifyMerge({ github, context }); - name: Merge PR env: diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js new file mode 100644 index 0000000..6a76b49 --- /dev/null +++ b/scripts/verify-merge.js @@ -0,0 +1,41 @@ +// @ts-check +const fs = require("fs"); +const yaml = require("js-yaml"); + +/** @param {{ github: any, context: any }} opts */ +module.exports = async ({ github, context }) => { + const actor = context.payload.comment.user.login; + const prNumber = context.issue.number; + + const { data: files } = await github.rest.pulls.listFiles({ + ...context.repo, + pull_number: prNumber, + }); + + const gapDirs = files + .map((f) => f.filename) + .map((p) => p.split("/").slice(0, 2).join("/")); + + const gapsChanged = [...new Set(gapDirs)]; + + if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { + throw new Error( + "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", + ); + } + + const metadata = yaml.load( + fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, "utf8"), + ); + + const authorizedMergers = new Set([ + ...metadata.authors.map((author) => + author.githubUsername.replace(/^@/, ""), + ), + metadata.sponsor.replace(/^@/, ""), + ]); + + if (!authorizedMergers.has(actor)) { + throw new Error(`${actor} is not authorized to merge ${gapsChanged[0]}.`); + } +}; From ba69f3ea7bd7f19cd1d76437afc8e73b977417de Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 16:02:01 -0500 Subject: [PATCH 18/43] Remove unused ts-check and jsdoc annotation Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 6a76b49..5c7ff28 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -1,8 +1,6 @@ -// @ts-check const fs = require("fs"); const yaml = require("js-yaml"); -/** @param {{ github: any, context: any }} opts */ module.exports = async ({ github, context }) => { const actor = context.payload.comment.user.login; const prNumber = context.issue.number; From c0f5b4221f48f48456022255f3d5a6586e18cf78 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 16:03:21 -0500 Subject: [PATCH 19/43] Update verify-merge.js --- scripts/verify-merge.js | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 5c7ff28..8b61e28 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -12,25 +12,19 @@ module.exports = async ({ github, context }) => { const gapDirs = files .map((f) => f.filename) - .map((p) => p.split("/").slice(0, 2).join("/")); + .map((p) => p.split("/") + .slice(0, 2).join("/")); const gapsChanged = [...new Set(gapDirs)]; if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { - throw new Error( - "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", - ); + throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else."); } - const metadata = yaml.load( - fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, "utf8"), - ); - + const metadata = yaml.load(fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, "utf8")); const authorizedMergers = new Set([ - ...metadata.authors.map((author) => - author.githubUsername.replace(/^@/, ""), - ), - metadata.sponsor.replace(/^@/, ""), + ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), + metadata.sponsor.replace(/^@/, ''), ]); if (!authorizedMergers.has(actor)) { From 0fcd6d835110dddc04a446b1cb4084df2b8c67c0 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Fri, 15 May 2026 16:04:51 -0500 Subject: [PATCH 20/43] Fix gap directory mapping in verify-merge.js --- scripts/verify-merge.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 8b61e28..5ad8e3e 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -12,8 +12,7 @@ module.exports = async ({ github, context }) => { const gapDirs = files .map((f) => f.filename) - .map((p) => p.split("/") - .slice(0, 2).join("/")); + .map((p) => p.split("/").slice(0, 2).join("/")); const gapsChanged = [...new Set(gapDirs)]; From 607cad118a4f8ae95a940b9e56381edf96344d92 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:07:06 -0700 Subject: [PATCH 21/43] Harden /merge workflow against race conditions and path traversal - Remove --auto flag from gh pr merge to prevent race condition where PR contents could change between authorization check and merge - Add path normalization check to reject traversal attempts - Add explicit check that all files are within the authorized GAP dir - Check PR mergeable state before attempting merge - Filter out comments from users with no repo association to reduce Actions minute burn from spam - Switch from ad-hoc js-yaml install to npm ci with lockfile (uses the yaml package already in devDependencies) - Convert to ESM to match the rest of the project - Parallelize API calls for faster execution - Add per_page: 100 to listFiles to handle larger PRs - Use async readFile instead of blocking readFileSync Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 18 +++++++--- scripts/verify-merge.js | 70 ++++++++++++++++++++++++++++--------- 2 files changed, 68 insertions(+), 20 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index 18188da..d320075 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -13,16 +13,26 @@ jobs: runs-on: ubuntu-latest if: > github.event.issue.pull_request && - github.event.comment.body == '/merge' + github.event.comment.body == '/merge' && + github.event.comment.author_association != 'NONE' steps: - uses: actions/checkout@v6 - - run: npm install js-yaml + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: "24" + cache: "npm" + + - run: npm ci - name: Verify actor is authorized uses: actions/github-script@v9 with: script: | - const verifyMerge = require('./scripts/verify-merge.js'); + const { default: verifyMerge } = await import( + `${process.env.GITHUB_WORKSPACE}/scripts/verify-merge.js` + ); await verifyMerge({ github, context }); - name: Merge PR @@ -30,4 +40,4 @@ jobs: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ github.event.issue.number }} run: | - gh pr merge "$PR_NUMBER" --squash --auto + gh pr merge "$PR_NUMBER" --squash diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 5ad8e3e..a46d75a 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -1,32 +1,70 @@ -const fs = require("fs"); -const yaml = require("js-yaml"); +import { readFile } from "fs/promises"; +import path from "path"; +import { parse as parseYaml } from "yaml"; -module.exports = async ({ github, context }) => { +export default async ({ github, context }) => { const actor = context.payload.comment.user.login; const prNumber = context.issue.number; - const { data: files } = await github.rest.pulls.listFiles({ - ...context.repo, - pull_number: prNumber, - }); + const [{ data: pr }, { data: files }] = await Promise.all([ + github.rest.pulls.get({ + ...context.repo, + pull_number: prNumber, + }), + github.rest.pulls.listFiles({ + ...context.repo, + pull_number: prNumber, + per_page: 100, + }), + ]); + + if (pr.mergeable === false) { + throw new Error( + "PR is not in a mergeable state. Resolve conflicts and try again.", + ); + } + + const gapDirSet = new Set(); - const gapDirs = files - .map((f) => f.filename) - .map((p) => p.split("/").slice(0, 2).join("/")); + for (const f of files) { + const normalized = path.normalize(f.filename); + if (normalized !== f.filename || normalized.startsWith("..")) { + throw new Error( + `File path "${f.filename}" contains path traversal or is not normalized.`, + ); + } + gapDirSet.add(f.filename.split("/").slice(0, 2).join("/")); + } - const gapsChanged = [...new Set(gapDirs)]; + const gapsChanged = [...gapDirSet]; if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { - throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else."); + throw new Error( + "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", + ); + } + + const gapDir = gapsChanged[0]; + + for (const f of files) { + if (!f.filename.startsWith(`${gapDir}/`)) { + throw new Error( + `File "${f.filename}" is outside the expected GAP directory (${gapDir}).`, + ); + } } - const metadata = yaml.load(fs.readFileSync(`${gapsChanged[0]}/metadata.yml`, "utf8")); + const metadata = parseYaml( + await readFile(`${gapDir}/metadata.yml`, "utf8"), + ); const authorizedMergers = new Set([ - ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), - metadata.sponsor.replace(/^@/, ''), + ...metadata.authors.map((author) => + author.githubUsername.replace(/^@/, ""), + ), + metadata.sponsor.replace(/^@/, ""), ]); if (!authorizedMergers.has(actor)) { - throw new Error(`${actor} is not authorized to merge ${gapsChanged[0]}.`); + throw new Error(`${actor} is not authorized to merge ${gapDir}.`); } }; From 6217809a7a859d1e803ef530dc67af503cf1a0e2 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:28:44 -0700 Subject: [PATCH 22/43] Reject PRs with 100+ files to guard against pagination truncation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit listFiles returns at most 100 per page. If we get exactly 100 back, we may be missing files — reject early rather than silently validating an incomplete file list. Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index a46d75a..4fe42a9 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -18,6 +18,12 @@ export default async ({ github, context }) => { }), ]); + if (files.length >= 100) { + throw new Error( + "PR touches too many files (100+). /merge only supports PRs with fewer than 100 changed files.", + ); + } + if (pr.mergeable === false) { throw new Error( "PR is not in a mergeable state. Resolve conflicts and try again.", From d9f5e774011fd36861f5b8ec6cc2d0f1fdcdcb8d Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:29:53 -0700 Subject: [PATCH 23/43] Simplify import path to use relative ./ Co-Authored-By: Claude Opus 4.7 --- .github/workflows/merge.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/merge.yml b/.github/workflows/merge.yml index d320075..dfc056e 100644 --- a/.github/workflows/merge.yml +++ b/.github/workflows/merge.yml @@ -30,9 +30,7 @@ jobs: uses: actions/github-script@v9 with: script: | - const { default: verifyMerge } = await import( - `${process.env.GITHUB_WORKSPACE}/scripts/verify-merge.js` - ); + const { default: verifyMerge } = await import('./scripts/verify-merge.js'); await verifyMerge({ github, context }); - name: Merge PR From f2610be4230641d6f0af2909c54bbbe552eaa232 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:31:13 -0700 Subject: [PATCH 24/43] Simplify error message for too many files Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 4fe42a9..4f9ba47 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -19,9 +19,7 @@ export default async ({ github, context }) => { ]); if (files.length >= 100) { - throw new Error( - "PR touches too many files (100+). /merge only supports PRs with fewer than 100 changed files.", - ); + throw new Error("PR touches too many files!"); } if (pr.mergeable === false) { From 99b191b030e588057fa12568d375436ab992e938 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:32:32 -0700 Subject: [PATCH 25/43] Preserve original formatting for unchanged logic Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 4f9ba47..f3cac3f 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -43,9 +43,7 @@ export default async ({ github, context }) => { const gapsChanged = [...gapDirSet]; if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { - throw new Error( - "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", - ); + throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else."); } const gapDir = gapsChanged[0]; @@ -62,10 +60,8 @@ export default async ({ github, context }) => { await readFile(`${gapDir}/metadata.yml`, "utf8"), ); const authorizedMergers = new Set([ - ...metadata.authors.map((author) => - author.githubUsername.replace(/^@/, ""), - ), - metadata.sponsor.replace(/^@/, ""), + ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), + metadata.sponsor.replace(/^@/, ''), ]); if (!authorizedMergers.has(actor)) { From 6922e08cef18f3c9d2965079dadde17958c1e2be Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:33:23 -0700 Subject: [PATCH 26/43] Keep metadata read on one line Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index f3cac3f..0576ea4 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -56,9 +56,7 @@ export default async ({ github, context }) => { } } - const metadata = parseYaml( - await readFile(`${gapDir}/metadata.yml`, "utf8"), - ); + const metadata = parseYaml(await readFile(`${gapDir}/metadata.yml`, "utf8")); const authorizedMergers = new Set([ ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), metadata.sponsor.replace(/^@/, ''), From e604b8e9d91a21fa52212e596c49b48afc298643 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:36:49 -0700 Subject: [PATCH 27/43] Rename gapDirSet -> gapDirs Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 0576ea4..7052c69 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -28,7 +28,7 @@ export default async ({ github, context }) => { ); } - const gapDirSet = new Set(); + const gapDirs = new Set(); for (const f of files) { const normalized = path.normalize(f.filename); @@ -37,10 +37,10 @@ export default async ({ github, context }) => { `File path "${f.filename}" contains path traversal or is not normalized.`, ); } - gapDirSet.add(f.filename.split("/").slice(0, 2).join("/")); + gapDirs.add(f.filename.split("/").slice(0, 2).join("/")); } - const gapsChanged = [...gapDirSet]; + const gapsChanged = [...gapDirs]; if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else."); From 141026a16c6acdf3bb6f0c0d95b74bead5202e5e Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:38:53 -0700 Subject: [PATCH 28/43] Add clarifying comment for gapDirs extraction Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 7052c69..d14ac25 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -37,6 +37,7 @@ export default async ({ github, context }) => { `File path "${f.filename}" contains path traversal or is not normalized.`, ); } + // 'gaps/GAP-10/proposal.md' -> 'gaps/GAP-10' gapDirs.add(f.filename.split("/").slice(0, 2).join("/")); } From 5055253ea1e4b1a67db5b0318fd1a5910b07d2c1 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:39:17 -0700 Subject: [PATCH 29/43] Fix comment example Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index d14ac25..70a6f0b 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -37,7 +37,7 @@ export default async ({ github, context }) => { `File path "${f.filename}" contains path traversal or is not normalized.`, ); } - // 'gaps/GAP-10/proposal.md' -> 'gaps/GAP-10' + // e.g. 'gaps/GAP-10/DRAFT.md' -> 'gaps/GAP-10' gapDirs.add(f.filename.split("/").slice(0, 2).join("/")); } From e07a65ed480cf52738d0bc6b62330751587e42d9 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:40:25 -0700 Subject: [PATCH 30/43] Use nested path example in comment to better illustrate slice behavior Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 70a6f0b..4a2781a 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -37,7 +37,7 @@ export default async ({ github, context }) => { `File path "${f.filename}" contains path traversal or is not normalized.`, ); } - // e.g. 'gaps/GAP-10/DRAFT.md' -> 'gaps/GAP-10' + // e.g. 'gaps/GAP-10/versions/2026-01.md' -> 'gaps/GAP-10' gapDirs.add(f.filename.split("/").slice(0, 2).join("/")); } From f1db80c32fa59c8e73c1aa529e352a68c9f7c2f3 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:43:28 -0700 Subject: [PATCH 31/43] Refactor error handling and improve code readability --- scripts/verify-merge.js | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 4a2781a..bd7f0f8 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -6,16 +6,12 @@ export default async ({ github, context }) => { const actor = context.payload.comment.user.login; const prNumber = context.issue.number; - const [{ data: pr }, { data: files }] = await Promise.all([ - github.rest.pulls.get({ - ...context.repo, - pull_number: prNumber, - }), - github.rest.pulls.listFiles({ - ...context.repo, - pull_number: prNumber, - per_page: 100, - }), + const [ + { data: pr }, + { data: files }, + ] = await Promise.all([ + github.rest.pulls.get({ ...context.repo, pull_number: prNumber }), + github.rest.pulls.listFiles({ ...context.repo, pull_number: prNumber, per_page: 100 }), ]); if (files.length >= 100) { @@ -23,9 +19,7 @@ export default async ({ github, context }) => { } if (pr.mergeable === false) { - throw new Error( - "PR is not in a mergeable state. Resolve conflicts and try again.", - ); + throw new Error("PR is not in a mergeable state. Resolve conflicts and try again."); } const gapDirs = new Set(); @@ -33,10 +27,9 @@ export default async ({ github, context }) => { for (const f of files) { const normalized = path.normalize(f.filename); if (normalized !== f.filename || normalized.startsWith("..")) { - throw new Error( - `File path "${f.filename}" contains path traversal or is not normalized.`, - ); + throw new Error(`File path "${f.filename}" contains path traversal or is not normalized.`); } + // e.g. 'gaps/GAP-10/versions/2026-01.md' -> 'gaps/GAP-10' gapDirs.add(f.filename.split("/").slice(0, 2).join("/")); } @@ -51,9 +44,7 @@ export default async ({ github, context }) => { for (const f of files) { if (!f.filename.startsWith(`${gapDir}/`)) { - throw new Error( - `File "${f.filename}" is outside the expected GAP directory (${gapDir}).`, - ); + throw new Error(`File "${f.filename}" is outside the expected GAP directory (${gapDir}).`); } } From 6218a0f6b33ae193c30f5120db8ced8e95277e1e Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:49:12 -0700 Subject: [PATCH 32/43] Add test suite for verify-merge.js Uses Node's built-in test runner with memfs to mock the filesystem. Run with: node --experimental-test-module-mocks --test scripts/verify-merge.test.js Co-Authored-By: Claude Opus 4.7 --- package-lock.json | 502 +++++++++++++++++++++++++++++++++++ package.json | 1 + scripts/verify-merge.test.js | 166 ++++++++++++ 3 files changed, 669 insertions(+) create mode 100644 scripts/verify-merge.test.js diff --git a/package-lock.json b/package-lock.json index e69676e..e037258 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "handlebars": "^4.7.9", "lodash.memoize": "^4.1.2", "lodash.merge": "^4.6.2", + "memfs": "^4.57.2", "nodemon": "2.0.20", "p-limit": "^7.3.0", "prettier": "^3.8.1", @@ -354,6 +355,415 @@ "dev": true, "license": "MIT" }, + "node_modules/@jsonjoy.com/base64": { + "version": "1.1.2", + "integrity": "sha512-q6XAnWQDIMA3+FTiOYajoYqySkO+JSat0ytXGSuRdq9uXE7o92gzuQwQM14xaCRlBLGq3v5miDGC4vkVTn54xA==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/buffers": { + "version": "17.67.0", + "integrity": "sha512-tfExRpYxBvi32vPs9ZHaTjSP4fHAfzSmcahOfNxtvGHcyJel+aibkPlGeBB+7AoC6hL7lXIE++8okecBxx7lcw==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/codegen": { + "version": "1.0.0", + "integrity": "sha512-E8Oy+08cmCf0EK/NMxpaJZmOxPqM+6iSe2S4nlSBrPZOORoDJILxtbSUEDKQyTamm/BVAhIGllOBNU79/dwf0g==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-core": { + "version": "4.57.2", + "integrity": "sha512-SVjwklkpIV5wrynpYtuYnfYH1QF4/nDuLBX7VXdb+3miglcAgBVZb/5y0cOsehRV/9Vb+3UqhkMq3/NR3ztdkQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-node-builtins": "4.57.2", + "@jsonjoy.com/fs-node-utils": "4.57.2", + "thingies": "^2.5.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-fsa": { + "version": "4.57.2", + "integrity": "sha512-fhO8+iR2I+OCw668ISDJdn1aArc9zx033sWejIyzQ8RBeXa9bDSaUeA3ix0poYOfrj1KdOzytmYNv2/uLDfV6g==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-core": "4.57.2", + "@jsonjoy.com/fs-node-builtins": "4.57.2", + "@jsonjoy.com/fs-node-utils": "4.57.2", + "thingies": "^2.5.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-node": { + "version": "4.57.2", + "integrity": "sha512-nX2AdL6cOFwLdju9G4/nbRnYevmCJbh7N7hvR3gGm97Cs60uEjyd0rpR+YBS7cTg175zzl22pGKXR5USaQMvKg==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-core": "4.57.2", + "@jsonjoy.com/fs-node-builtins": "4.57.2", + "@jsonjoy.com/fs-node-utils": "4.57.2", + "@jsonjoy.com/fs-print": "4.57.2", + "@jsonjoy.com/fs-snapshot": "4.57.2", + "glob-to-regex.js": "^1.0.0", + "thingies": "^2.5.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-node-builtins": { + "version": "4.57.2", + "integrity": "sha512-xhiegylRmhw43Ki2HO1ZBL7DQ5ja/qpRsL29VtQ2xuUHiuDGbgf2uD4p9Qd8hJI5P6RCtGYD50IXHXVq/Ocjcg==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-node-to-fsa": { + "version": "4.57.2", + "integrity": "sha512-18LmWTSONhoAPW+IWRuf8w/+zRolPFGPeGwMxlAhhfY11EKzX+5XHDBPAw67dBF5dxDErHJbl40U+3IXSDRXSQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-fsa": "4.57.2", + "@jsonjoy.com/fs-node-builtins": "4.57.2", + "@jsonjoy.com/fs-node-utils": "4.57.2" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-node-utils": { + "version": "4.57.2", + "integrity": "sha512-rsPSJgekz43IlNbLyAM/Ab+ouYLWGp5DDBfYBNNEqDaSpsbXfthBn29Q4muFA9L0F+Z3mKo+CWlgSCXrf+mOyQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-node-builtins": "4.57.2" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-print": { + "version": "4.57.2", + "integrity": "sha512-wK9NSow48i4DbDl9F1CQE5TqnyZOJ04elU3WFG5aJ76p+YxO/ulyBBQvKsessPxdo381Bc2pcEoyPujMOhcRqQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-node-utils": "4.57.2", + "tree-dump": "^1.1.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-snapshot": { + "version": "4.57.2", + "integrity": "sha512-GdduDZuoP5V/QCgJkx9+BZ6SC0EZ/smXAdTS7PfMqgMTGXLlt/bH/FqMYaqB9JmLf05sJPtO0XRbAwwkEEPbVw==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/buffers": "^17.65.0", + "@jsonjoy.com/fs-node-utils": "4.57.2", + "@jsonjoy.com/json-pack": "^17.65.0", + "@jsonjoy.com/util": "^17.65.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-snapshot/node_modules/@jsonjoy.com/base64": { + "version": "17.67.0", + "integrity": "sha512-5SEsJGsm15aP8TQGkDfJvz9axgPwAEm98S5DxOuYe8e1EbfajcDmgeXXzccEjh+mLnjqEKrkBdjHWS5vFNwDdw==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-snapshot/node_modules/@jsonjoy.com/codegen": { + "version": "17.67.0", + "integrity": "sha512-idnkUplROpdBOV0HMcwhsCUS5TRUi9poagdGs70A6S4ux9+/aPuKbh8+UYRTLYQHtXvAdNfQWXDqZEx5k4Dj2Q==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-snapshot/node_modules/@jsonjoy.com/json-pack": { + "version": "17.67.0", + "integrity": "sha512-t0ejURcGaZsn1ClbJ/3kFqSOjlryd92eQY465IYrezsXmPcfHPE/av4twRSxf6WE+TkZgLY+71vCZbiIiFKA/w==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/base64": "17.67.0", + "@jsonjoy.com/buffers": "17.67.0", + "@jsonjoy.com/codegen": "17.67.0", + "@jsonjoy.com/json-pointer": "17.67.0", + "@jsonjoy.com/util": "17.67.0", + "hyperdyperid": "^1.2.0", + "thingies": "^2.5.0", + "tree-dump": "^1.1.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-snapshot/node_modules/@jsonjoy.com/json-pointer": { + "version": "17.67.0", + "integrity": "sha512-+iqOFInH+QZGmSuaybBUNdh7yvNrXvqR+h3wjXm0N/3JK1EyyFAeGJvqnmQL61d1ARLlk/wJdFKSL+LHJ1eaUA==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/util": "17.67.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/fs-snapshot/node_modules/@jsonjoy.com/util": { + "version": "17.67.0", + "integrity": "sha512-6+8xBaz1rLSohlGh68D1pdw3AwDi9xydm8QNlAFkvnavCJYSze+pxoW2VKP8p308jtlMRLs5NTHfPlZLd4w7ew==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/buffers": "17.67.0", + "@jsonjoy.com/codegen": "17.67.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/json-pack": { + "version": "1.21.0", + "integrity": "sha512-+AKG+R2cfZMShzrF2uQw34v3zbeDYUqnQ+jg7ORic3BGtfw9p/+N6RJbq/kkV8JmYZaINknaEQ2m0/f693ZPpg==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/base64": "^1.1.2", + "@jsonjoy.com/buffers": "^1.2.0", + "@jsonjoy.com/codegen": "^1.0.0", + "@jsonjoy.com/json-pointer": "^1.0.2", + "@jsonjoy.com/util": "^1.9.0", + "hyperdyperid": "^1.2.0", + "thingies": "^2.5.0", + "tree-dump": "^1.1.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/json-pack/node_modules/@jsonjoy.com/buffers": { + "version": "1.2.1", + "integrity": "sha512-12cdlDwX4RUM3QxmUbVJWqZ/mrK6dFQH4Zxq6+r1YXKXYBNgZXndx2qbCJwh3+WWkCSn67IjnlG3XYTvmvYtgA==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/json-pointer": { + "version": "1.0.2", + "integrity": "sha512-Fsn6wM2zlDzY1U+v4Nc8bo3bVqgfNTGcn6dMgs6FjrEnt4ZCe60o6ByKRjOGlI2gow0aE/Q41QOigdTqkyK5fg==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/codegen": "^1.0.0", + "@jsonjoy.com/util": "^1.9.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/util": { + "version": "1.9.0", + "integrity": "sha512-pLuQo+VPRnN8hfPqUTLTHk126wuYdXVxE6aDmjSeV4NCAgyxWbiOIeNJVtID3h1Vzpoi9m4jXezf73I6LgabgQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/buffers": "^1.0.0", + "@jsonjoy.com/codegen": "^1.0.0" + }, + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/@jsonjoy.com/util/node_modules/@jsonjoy.com/buffers": { + "version": "1.2.1", + "integrity": "sha512-12cdlDwX4RUM3QxmUbVJWqZ/mrK6dFQH4Zxq6+r1YXKXYBNgZXndx2qbCJwh3+WWkCSn67IjnlG3XYTvmvYtgA==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, "node_modules/@mlarah/spec-md": { "version": "3.1.0", "integrity": "sha1-zkfFwC4q9JRVYeeXu1MCmvh1Qps=", @@ -922,6 +1332,22 @@ "node": ">= 6" } }, + "node_modules/glob-to-regex.js": { + "version": "1.2.0", + "integrity": "sha512-QMwlOQKU/IzqMUOAZWubUOT8Qft+Y0KQWnX9nK3ch0CJg0tTp4TvGZsTfudYKv2NzoQSyPcnA6TYeIQ3jGichQ==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, "node_modules/global-dirs": { "version": "0.1.1", "integrity": "sha512-NknMLn7F2J7aflwFOlGdNIuCDpN3VGoSoB+aap3KABFWbHVn1TCgFC+np23J8W2BiZbjfEw3BFBycSMv1AFblg==", @@ -970,6 +1396,15 @@ "node": ">=8" } }, + "node_modules/hyperdyperid": { + "version": "1.2.0", + "integrity": "sha512-Y93lCzHYgGWdrJ66yIktxiaGULYc6oGiABxhcO5AufBeOyoIdZF7bIfLaOrbM0iGIOXQQgxxRrFEnb+Y6w1n4A==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10.18" + } + }, "node_modules/ignore-by-default": { "version": "1.0.1", "integrity": "sha512-Ius2VYcGNk7T90CppJqcIkS5ooHUZyIQK+ClZfMfMNFEF9VSE73Fq+906u/CWu92x4gzZMWOwfFYckPObzdEbA==", @@ -1186,6 +1621,35 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/memfs": { + "version": "4.57.2", + "integrity": "sha512-2nWzSsJzrukurSDna4Z0WywuScK4Id3tSKejgu74u8KCdW4uNrseKRSIDg75C6Yw5ZRqBe0F0EtMNlTbUq8bAQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "@jsonjoy.com/fs-core": "4.57.2", + "@jsonjoy.com/fs-fsa": "4.57.2", + "@jsonjoy.com/fs-node": "4.57.2", + "@jsonjoy.com/fs-node-builtins": "4.57.2", + "@jsonjoy.com/fs-node-to-fsa": "4.57.2", + "@jsonjoy.com/fs-node-utils": "4.57.2", + "@jsonjoy.com/fs-print": "4.57.2", + "@jsonjoy.com/fs-snapshot": "4.57.2", + "@jsonjoy.com/json-pack": "^1.11.0", + "@jsonjoy.com/util": "^1.9.0", + "glob-to-regex.js": "^1.0.1", + "thingies": "^2.5.0", + "tree-dump": "^1.0.3", + "tslib": "^2.0.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, "node_modules/micromatch": { "version": "4.0.8", "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", @@ -1581,6 +2045,22 @@ "node": ">=8" } }, + "node_modules/thingies": { + "version": "2.6.0", + "integrity": "sha512-rMHRjmlFLM1R96UYPvpmnc3LYtdFrT33JIB7L9hetGue1qAPfn1N2LJeEjxUSidu1Iku+haLZXDuEXUHNGO/lg==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=10.18" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "^2" + } + }, "node_modules/to-regex-range": { "version": "5.0.1", "integrity": "sha512-65P7iz6X5yEr1cwcgvQxbbIw7Uk3gOy5dIdtZ4rDveLqhrdJP+Li/Hx6tyK0NEb+2GCyneCMJiGqrADCSNk8sQ==", @@ -1602,6 +2082,28 @@ "nodetouch": "bin/nodetouch.js" } }, + "node_modules/tree-dump": { + "version": "1.1.0", + "integrity": "sha512-rMuvhU4MCDbcbnleZTFezWsaZXRFemSqAM+7jPnzUl1fo9w3YEKOxAeui0fz3OI4EU4hf23iyA7uQRVko+UaBA==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=10.0" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/streamich" + }, + "peerDependencies": { + "tslib": "2" + } + }, + "node_modules/tslib": { + "version": "2.8.1", + "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", + "dev": true, + "license": "0BSD" + }, "node_modules/typedarray-to-buffer": { "version": "3.1.5", "integrity": "sha512-zdu8XMNEDepKKR+XYOXAVPtWui0ly0NtohUscw+UmaHiAWT8hrV1rr//H6V+0DvJ3OQ19S979M0laLfX8rm82Q==", diff --git a/package.json b/package.json index 8f6bf9f..19e1794 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "handlebars": "^4.7.9", "lodash.memoize": "^4.1.2", "lodash.merge": "^4.6.2", + "memfs": "^4.57.2", "nodemon": "2.0.20", "p-limit": "^7.3.0", "prettier": "^3.8.1", diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js new file mode 100644 index 0000000..84620a9 --- /dev/null +++ b/scripts/verify-merge.test.js @@ -0,0 +1,166 @@ +import { describe, it, mock, beforeEach } from "node:test"; +import assert from "node:assert"; +import { vol } from "memfs"; + +mock.module("fs/promises", { exports: vol.promises }); + +const { default: verifyMerge } = await import("./verify-merge.js"); + +function makeContext({ actor = "magicmark", prNumber = 1 } = {}) { + return { + payload: { comment: { user: { login: actor } } }, + issue: { number: prNumber }, + repo: { owner: "graphql", repo: "gaps" }, + }; +} + +function makeGithub({ mergeable = true, files = [] } = {}) { + return { + rest: { + pulls: { + get: mock.fn(async () => ({ data: { mergeable } })), + listFiles: mock.fn(async () => ({ data: files })), + }, + }, + }; +} + +const METADATA = ` +authors: + - name: "Mark Larah" + email: "markl@yelp.com" + githubUsername: "@magicmark" + - name: "Michael Rebello" + email: "me@michaelrebello.com" + githubUsername: "@rebello95" +sponsor: "@magicmark" +`; + +beforeEach(() => { + vol.reset(); +}); + +describe("verify-merge", () => { + it("allows an authorized author to merge", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + files: [{ filename: "gaps/GAP-10/DRAFT.md" }], + }); + + await assert.doesNotReject( + verifyMerge({ github, context: makeContext({ actor: "magicmark" }) }), + ); + }); + + it("allows the sponsor to merge", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + files: [{ filename: "gaps/GAP-10/DRAFT.md" }], + }); + + await assert.doesNotReject( + verifyMerge({ github, context: makeContext({ actor: "magicmark" }) }), + ); + }); + + it("allows a co-author to merge", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + files: [{ filename: "gaps/GAP-10/DRAFT.md" }], + }); + + await assert.doesNotReject( + verifyMerge({ github, context: makeContext({ actor: "rebello95" }) }), + ); + }); + + it("rejects an unauthorized user", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + files: [{ filename: "gaps/GAP-10/DRAFT.md" }], + }); + + await assert.rejects( + verifyMerge({ github, context: makeContext({ actor: "evil-hacker" }) }), + { message: "evil-hacker is not authorized to merge gaps/GAP-10." }, + ); + }); + + it("rejects PRs touching multiple GAP directories", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + files: [ + { filename: "gaps/GAP-10/DRAFT.md" }, + { filename: "gaps/GAP-7/DRAFT.md" }, + ], + }); + + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, + ); + }); + + it("rejects PRs touching files outside gaps/", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + files: [ + { filename: "gaps/GAP-10/DRAFT.md" }, + { filename: ".github/workflows/evil.yml" }, + ], + }); + + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, + ); + }); + + it("rejects when PR is not mergeable", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + mergeable: false, + files: [{ filename: "gaps/GAP-10/DRAFT.md" }], + }); + + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "PR is not in a mergeable state. Resolve conflicts and try again." }, + ); + }); + + it("allows merge when mergeable is null (still computing)", async () => { + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ + mergeable: null, + files: [{ filename: "gaps/GAP-10/DRAFT.md" }], + }); + + await assert.doesNotReject( + verifyMerge({ github, context: makeContext() }), + ); + }); + + it("rejects PRs with 100+ files", async () => { + const files = Array.from({ length: 100 }, (_, i) => ({ + filename: `gaps/GAP-10/file${i}.md`, + })); + const github = makeGithub({ files }); + + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "PR touches too many files!" }, + ); + }); + + it("rejects path traversal attempts", async () => { + const github = makeGithub({ + files: [{ filename: "gaps/GAP-10/../../../etc/passwd" }], + }); + + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + /path traversal/, + ); + }); +}); From e1fc9d0cd14bc51c26f32c9955946a1e1527eded Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 17:53:13 -0700 Subject: [PATCH 33/43] Use alice/bob/charlie/eve in tests instead of real usernames Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index 84620a9..c451404 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -6,7 +6,7 @@ mock.module("fs/promises", { exports: vol.promises }); const { default: verifyMerge } = await import("./verify-merge.js"); -function makeContext({ actor = "magicmark", prNumber = 1 } = {}) { +function makeContext({ actor = "alice", prNumber = 1 } = {}) { return { payload: { comment: { user: { login: actor } } }, issue: { number: prNumber }, @@ -27,13 +27,13 @@ function makeGithub({ mergeable = true, files = [] } = {}) { const METADATA = ` authors: - - name: "Mark Larah" - email: "markl@yelp.com" - githubUsername: "@magicmark" - - name: "Michael Rebello" - email: "me@michaelrebello.com" - githubUsername: "@rebello95" -sponsor: "@magicmark" + - name: "Alice" + email: "alice@example.com" + githubUsername: "@alice" + - name: "Bob" + email: "bob@example.com" + githubUsername: "@bob" +sponsor: "@charlie" `; beforeEach(() => { @@ -41,36 +41,36 @@ beforeEach(() => { }); describe("verify-merge", () => { - it("allows an authorized author to merge", async () => { + it("allows an author to merge", async () => { vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); await assert.doesNotReject( - verifyMerge({ github, context: makeContext({ actor: "magicmark" }) }), + verifyMerge({ github, context: makeContext({ actor: "alice" }) }), ); }); - it("allows the sponsor to merge", async () => { + it("allows a co-author to merge", async () => { vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); await assert.doesNotReject( - verifyMerge({ github, context: makeContext({ actor: "magicmark" }) }), + verifyMerge({ github, context: makeContext({ actor: "bob" }) }), ); }); - it("allows a co-author to merge", async () => { + it("allows the sponsor to merge", async () => { vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); await assert.doesNotReject( - verifyMerge({ github, context: makeContext({ actor: "rebello95" }) }), + verifyMerge({ github, context: makeContext({ actor: "charlie" }) }), ); }); @@ -81,8 +81,8 @@ describe("verify-merge", () => { }); await assert.rejects( - verifyMerge({ github, context: makeContext({ actor: "evil-hacker" }) }), - { message: "evil-hacker is not authorized to merge gaps/GAP-10." }, + verifyMerge({ github, context: makeContext({ actor: "eve" }) }), + { message: "eve is not authorized to merge gaps/GAP-10." }, ); }); From c5df1a96b89b3429b96b1e8a215af198b505a4a5 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:26:56 -0700 Subject: [PATCH 34/43] Reject when mergeable is null (still computing) Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 4 ++++ scripts/verify-merge.test.js | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index bd7f0f8..74154fb 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -18,6 +18,10 @@ export default async ({ github, context }) => { throw new Error("PR touches too many files!"); } + if (pr.mergeable === null) { + throw new Error("GitHub is still computing mergeability. Try again in a moment."); + } + if (pr.mergeable === false) { throw new Error("PR is not in a mergeable state. Resolve conflicts and try again."); } diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index c451404..f249814 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -129,15 +129,16 @@ describe("verify-merge", () => { ); }); - it("allows merge when mergeable is null (still computing)", async () => { + it("rejects when mergeable is null (still computing)", async () => { vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); const github = makeGithub({ mergeable: null, files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); - await assert.doesNotReject( + await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: "GitHub is still computing mergeability. Try again in a moment." }, ); }); From 21c34c9eb7889ca540f64e362d31cce7c0271179 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:27:59 -0700 Subject: [PATCH 35/43] Remove message assertions from tests Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index f249814..9786299 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -82,7 +82,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext({ actor: "eve" }) }), - { message: "eve is not authorized to merge gaps/GAP-10." }, ); }); @@ -97,7 +96,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, ); }); @@ -112,7 +110,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, ); }); @@ -125,7 +122,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "PR is not in a mergeable state. Resolve conflicts and try again." }, ); }); @@ -138,7 +134,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "GitHub is still computing mergeability. Try again in a moment." }, ); }); @@ -150,7 +145,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "PR touches too many files!" }, ); }); @@ -161,7 +155,6 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - /path traversal/, ); }); }); From ef46c32f2c4f368d25c6c95771b0031821f3da1b Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:51:13 -0700 Subject: [PATCH 36/43] Move vol.fromJSON into beforeEach Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index 9786299..d3f9b5a 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -38,11 +38,11 @@ sponsor: "@charlie" beforeEach(() => { vol.reset(); + vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); }); describe("verify-merge", () => { it("allows an author to merge", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -53,7 +53,7 @@ describe("verify-merge", () => { }); it("allows a co-author to merge", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -64,7 +64,7 @@ describe("verify-merge", () => { }); it("allows the sponsor to merge", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -75,7 +75,7 @@ describe("verify-merge", () => { }); it("rejects an unauthorized user", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -86,7 +86,7 @@ describe("verify-merge", () => { }); it("rejects PRs touching multiple GAP directories", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ files: [ { filename: "gaps/GAP-10/DRAFT.md" }, @@ -100,7 +100,7 @@ describe("verify-merge", () => { }); it("rejects PRs touching files outside gaps/", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ files: [ { filename: "gaps/GAP-10/DRAFT.md" }, @@ -114,7 +114,7 @@ describe("verify-merge", () => { }); it("rejects when PR is not mergeable", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ mergeable: false, files: [{ filename: "gaps/GAP-10/DRAFT.md" }], @@ -126,7 +126,7 @@ describe("verify-merge", () => { }); it("rejects when mergeable is null (still computing)", async () => { - vol.fromJSON({ "gaps/GAP-10/metadata.yml": METADATA }); + const github = makeGithub({ mergeable: null, files: [{ filename: "gaps/GAP-10/DRAFT.md" }], From 59de5f9d7c1a6fe9b52732e0d9d89b987082ae73 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:53:35 -0700 Subject: [PATCH 37/43] Remove stray blank lines in tests Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index d3f9b5a..16470cc 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -53,7 +53,6 @@ describe("verify-merge", () => { }); it("allows a co-author to merge", async () => { - const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -64,7 +63,6 @@ describe("verify-merge", () => { }); it("allows the sponsor to merge", async () => { - const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -75,7 +73,6 @@ describe("verify-merge", () => { }); it("rejects an unauthorized user", async () => { - const github = makeGithub({ files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); @@ -86,7 +83,6 @@ describe("verify-merge", () => { }); it("rejects PRs touching multiple GAP directories", async () => { - const github = makeGithub({ files: [ { filename: "gaps/GAP-10/DRAFT.md" }, @@ -100,7 +96,6 @@ describe("verify-merge", () => { }); it("rejects PRs touching files outside gaps/", async () => { - const github = makeGithub({ files: [ { filename: "gaps/GAP-10/DRAFT.md" }, @@ -114,7 +109,6 @@ describe("verify-merge", () => { }); it("rejects when PR is not mergeable", async () => { - const github = makeGithub({ mergeable: false, files: [{ filename: "gaps/GAP-10/DRAFT.md" }], @@ -126,7 +120,6 @@ describe("verify-merge", () => { }); it("rejects when mergeable is null (still computing)", async () => { - const github = makeGithub({ mergeable: null, files: [{ filename: "gaps/GAP-10/DRAFT.md" }], From 6802346af9779957280ad1c322696586620c6909 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:55:21 -0700 Subject: [PATCH 38/43] Add message assertions back to rejects checks Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index 16470cc..9f0b5c4 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -79,6 +79,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext({ actor: "eve" }) }), + { message: "eve is not authorized to merge gaps/GAP-10." }, ); }); @@ -92,6 +93,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, ); }); @@ -105,6 +107,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, ); }); @@ -116,6 +119,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: "PR is not in a mergeable state. Resolve conflicts and try again." }, ); }); @@ -127,6 +131,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: "GitHub is still computing mergeability. Try again in a moment." }, ); }); @@ -138,6 +143,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: "PR touches too many files!" }, ); }); @@ -148,6 +154,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), + { message: 'File path "gaps/GAP-10/../../../etc/passwd" contains path traversal or is not normalized.' }, ); }); }); From 1ef445ad5cbbbd747f0ead1668a9672f3a27ec92 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:57:50 -0700 Subject: [PATCH 39/43] Run prettier on verify-merge files Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.js | 37 +++++++++++++++++++---------- scripts/verify-merge.test.js | 46 +++++++++++++++++------------------- 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 74154fb..23f36bd 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -6,12 +6,13 @@ export default async ({ github, context }) => { const actor = context.payload.comment.user.login; const prNumber = context.issue.number; - const [ - { data: pr }, - { data: files }, - ] = await Promise.all([ + const [{ data: pr }, { data: files }] = await Promise.all([ github.rest.pulls.get({ ...context.repo, pull_number: prNumber }), - github.rest.pulls.listFiles({ ...context.repo, pull_number: prNumber, per_page: 100 }), + github.rest.pulls.listFiles({ + ...context.repo, + pull_number: prNumber, + per_page: 100, + }), ]); if (files.length >= 100) { @@ -19,11 +20,15 @@ export default async ({ github, context }) => { } if (pr.mergeable === null) { - throw new Error("GitHub is still computing mergeability. Try again in a moment."); + throw new Error( + "GitHub is still computing mergeability. Try again in a moment.", + ); } if (pr.mergeable === false) { - throw new Error("PR is not in a mergeable state. Resolve conflicts and try again."); + throw new Error( + "PR is not in a mergeable state. Resolve conflicts and try again.", + ); } const gapDirs = new Set(); @@ -31,7 +36,9 @@ export default async ({ github, context }) => { for (const f of files) { const normalized = path.normalize(f.filename); if (normalized !== f.filename || normalized.startsWith("..")) { - throw new Error(`File path "${f.filename}" contains path traversal or is not normalized.`); + throw new Error( + `File path "${f.filename}" contains path traversal or is not normalized.`, + ); } // e.g. 'gaps/GAP-10/versions/2026-01.md' -> 'gaps/GAP-10' @@ -41,21 +48,27 @@ export default async ({ github, context }) => { const gapsChanged = [...gapDirs]; if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { - throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else."); + throw new Error( + "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", + ); } const gapDir = gapsChanged[0]; for (const f of files) { if (!f.filename.startsWith(`${gapDir}/`)) { - throw new Error(`File "${f.filename}" is outside the expected GAP directory (${gapDir}).`); + throw new Error( + `File "${f.filename}" is outside the expected GAP directory (${gapDir}).`, + ); } } const metadata = parseYaml(await readFile(`${gapDir}/metadata.yml`, "utf8")); const authorizedMergers = new Set([ - ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), - metadata.sponsor.replace(/^@/, ''), + ...metadata.authors.map((author) => + author.githubUsername.replace(/^@/, ""), + ), + metadata.sponsor.replace(/^@/, ""), ]); if (!authorizedMergers.has(actor)) { diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index 9f0b5c4..ed2a869 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -91,10 +91,10 @@ describe("verify-merge", () => { ], }); - await assert.rejects( - verifyMerge({ github, context: makeContext() }), - { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, - ); + await assert.rejects(verifyMerge({ github, context: makeContext() }), { + message: + "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", + }); }); it("rejects PRs touching files outside gaps/", async () => { @@ -105,10 +105,10 @@ describe("verify-merge", () => { ], }); - await assert.rejects( - verifyMerge({ github, context: makeContext() }), - { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, - ); + await assert.rejects(verifyMerge({ github, context: makeContext() }), { + message: + "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", + }); }); it("rejects when PR is not mergeable", async () => { @@ -117,10 +117,10 @@ describe("verify-merge", () => { files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); - await assert.rejects( - verifyMerge({ github, context: makeContext() }), - { message: "PR is not in a mergeable state. Resolve conflicts and try again." }, - ); + await assert.rejects(verifyMerge({ github, context: makeContext() }), { + message: + "PR is not in a mergeable state. Resolve conflicts and try again.", + }); }); it("rejects when mergeable is null (still computing)", async () => { @@ -129,10 +129,9 @@ describe("verify-merge", () => { files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); - await assert.rejects( - verifyMerge({ github, context: makeContext() }), - { message: "GitHub is still computing mergeability. Try again in a moment." }, - ); + await assert.rejects(verifyMerge({ github, context: makeContext() }), { + message: "GitHub is still computing mergeability. Try again in a moment.", + }); }); it("rejects PRs with 100+ files", async () => { @@ -141,10 +140,9 @@ describe("verify-merge", () => { })); const github = makeGithub({ files }); - await assert.rejects( - verifyMerge({ github, context: makeContext() }), - { message: "PR touches too many files!" }, - ); + await assert.rejects(verifyMerge({ github, context: makeContext() }), { + message: "PR touches too many files!", + }); }); it("rejects path traversal attempts", async () => { @@ -152,9 +150,9 @@ describe("verify-merge", () => { files: [{ filename: "gaps/GAP-10/../../../etc/passwd" }], }); - await assert.rejects( - verifyMerge({ github, context: makeContext() }), - { message: 'File path "gaps/GAP-10/../../../etc/passwd" contains path traversal or is not normalized.' }, - ); + await assert.rejects(verifyMerge({ github, context: makeContext() }), { + message: + 'File path "gaps/GAP-10/../../../etc/passwd" contains path traversal or is not normalized.', + }); }); }); From 24b5ba85d142e7a33cfbdbaa6afcc327c93065b5 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:58:54 -0700 Subject: [PATCH 40/43] Revert "Run prettier on verify-merge files" This reverts commit 1ef445ad5cbbbd747f0ead1668a9672f3a27ec92. --- scripts/verify-merge.js | 37 ++++++++++------------------- scripts/verify-merge.test.js | 46 +++++++++++++++++++----------------- 2 files changed, 36 insertions(+), 47 deletions(-) diff --git a/scripts/verify-merge.js b/scripts/verify-merge.js index 23f36bd..74154fb 100644 --- a/scripts/verify-merge.js +++ b/scripts/verify-merge.js @@ -6,13 +6,12 @@ export default async ({ github, context }) => { const actor = context.payload.comment.user.login; const prNumber = context.issue.number; - const [{ data: pr }, { data: files }] = await Promise.all([ + const [ + { data: pr }, + { data: files }, + ] = await Promise.all([ github.rest.pulls.get({ ...context.repo, pull_number: prNumber }), - github.rest.pulls.listFiles({ - ...context.repo, - pull_number: prNumber, - per_page: 100, - }), + github.rest.pulls.listFiles({ ...context.repo, pull_number: prNumber, per_page: 100 }), ]); if (files.length >= 100) { @@ -20,15 +19,11 @@ export default async ({ github, context }) => { } if (pr.mergeable === null) { - throw new Error( - "GitHub is still computing mergeability. Try again in a moment.", - ); + throw new Error("GitHub is still computing mergeability. Try again in a moment."); } if (pr.mergeable === false) { - throw new Error( - "PR is not in a mergeable state. Resolve conflicts and try again.", - ); + throw new Error("PR is not in a mergeable state. Resolve conflicts and try again."); } const gapDirs = new Set(); @@ -36,9 +31,7 @@ export default async ({ github, context }) => { for (const f of files) { const normalized = path.normalize(f.filename); if (normalized !== f.filename || normalized.startsWith("..")) { - throw new Error( - `File path "${f.filename}" contains path traversal or is not normalized.`, - ); + throw new Error(`File path "${f.filename}" contains path traversal or is not normalized.`); } // e.g. 'gaps/GAP-10/versions/2026-01.md' -> 'gaps/GAP-10' @@ -48,27 +41,21 @@ export default async ({ github, context }) => { const gapsChanged = [...gapDirs]; if (gapsChanged.length !== 1 || !gapsChanged[0].match(/^gaps\/GAP-\d+$/)) { - throw new Error( - "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", - ); + throw new Error("You can only run /merge for PRs that touch exactly one GAP directory and nothing else."); } const gapDir = gapsChanged[0]; for (const f of files) { if (!f.filename.startsWith(`${gapDir}/`)) { - throw new Error( - `File "${f.filename}" is outside the expected GAP directory (${gapDir}).`, - ); + throw new Error(`File "${f.filename}" is outside the expected GAP directory (${gapDir}).`); } } const metadata = parseYaml(await readFile(`${gapDir}/metadata.yml`, "utf8")); const authorizedMergers = new Set([ - ...metadata.authors.map((author) => - author.githubUsername.replace(/^@/, ""), - ), - metadata.sponsor.replace(/^@/, ""), + ...metadata.authors.map(author => author.githubUsername.replace(/^@/, '')), + metadata.sponsor.replace(/^@/, ''), ]); if (!authorizedMergers.has(actor)) { diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index ed2a869..9f0b5c4 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -91,10 +91,10 @@ describe("verify-merge", () => { ], }); - await assert.rejects(verifyMerge({ github, context: makeContext() }), { - message: - "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", - }); + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, + ); }); it("rejects PRs touching files outside gaps/", async () => { @@ -105,10 +105,10 @@ describe("verify-merge", () => { ], }); - await assert.rejects(verifyMerge({ github, context: makeContext() }), { - message: - "You can only run /merge for PRs that touch exactly one GAP directory and nothing else.", - }); + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, + ); }); it("rejects when PR is not mergeable", async () => { @@ -117,10 +117,10 @@ describe("verify-merge", () => { files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); - await assert.rejects(verifyMerge({ github, context: makeContext() }), { - message: - "PR is not in a mergeable state. Resolve conflicts and try again.", - }); + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "PR is not in a mergeable state. Resolve conflicts and try again." }, + ); }); it("rejects when mergeable is null (still computing)", async () => { @@ -129,9 +129,10 @@ describe("verify-merge", () => { files: [{ filename: "gaps/GAP-10/DRAFT.md" }], }); - await assert.rejects(verifyMerge({ github, context: makeContext() }), { - message: "GitHub is still computing mergeability. Try again in a moment.", - }); + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "GitHub is still computing mergeability. Try again in a moment." }, + ); }); it("rejects PRs with 100+ files", async () => { @@ -140,9 +141,10 @@ describe("verify-merge", () => { })); const github = makeGithub({ files }); - await assert.rejects(verifyMerge({ github, context: makeContext() }), { - message: "PR touches too many files!", - }); + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: "PR touches too many files!" }, + ); }); it("rejects path traversal attempts", async () => { @@ -150,9 +152,9 @@ describe("verify-merge", () => { files: [{ filename: "gaps/GAP-10/../../../etc/passwd" }], }); - await assert.rejects(verifyMerge({ github, context: makeContext() }), { - message: - 'File path "gaps/GAP-10/../../../etc/passwd" contains path traversal or is not normalized.', - }); + await assert.rejects( + verifyMerge({ github, context: makeContext() }), + { message: 'File path "gaps/GAP-10/../../../etc/passwd" contains path traversal or is not normalized.' }, + ); }); }); From 5c4239e132d447628efd49552577bfb7e85e9e96 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Tue, 19 May 2026 23:59:57 -0700 Subject: [PATCH 41/43] Use regex for path traversal error assertion Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index 9f0b5c4..d4df9a2 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -154,7 +154,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: 'File path "gaps/GAP-10/../../../etc/passwd" contains path traversal or is not normalized.' }, + /contains path traversal or is not normalized/, ); }); }); From 38bd8b18451115fc7823030db47076f254b527fa Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Wed, 20 May 2026 00:00:34 -0700 Subject: [PATCH 42/43] Use regexes for error assertions Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index d4df9a2..4b9bec4 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -119,7 +119,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "PR is not in a mergeable state. Resolve conflicts and try again." }, + /not in a mergeable state/, ); }); @@ -131,7 +131,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "GitHub is still computing mergeability. Try again in a moment." }, + /still computing mergeability/, ); }); @@ -143,7 +143,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "PR touches too many files!" }, + /touches too many files/, ); }); From 682822d0ec8b748a84b97e2bd6e2345071d4fe64 Mon Sep 17 00:00:00 2001 From: Mark Larah Date: Wed, 20 May 2026 00:01:03 -0700 Subject: [PATCH 43/43] Use regexes for remaining error assertions Co-Authored-By: Claude Opus 4.7 --- scripts/verify-merge.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/verify-merge.test.js b/scripts/verify-merge.test.js index 4b9bec4..63dcff3 100644 --- a/scripts/verify-merge.test.js +++ b/scripts/verify-merge.test.js @@ -79,7 +79,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext({ actor: "eve" }) }), - { message: "eve is not authorized to merge gaps/GAP-10." }, + /not authorized to merge/, ); }); @@ -93,7 +93,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, + /touch exactly one GAP/, ); }); @@ -107,7 +107,7 @@ describe("verify-merge", () => { await assert.rejects( verifyMerge({ github, context: makeContext() }), - { message: "You can only run /merge for PRs that touch exactly one GAP directory and nothing else." }, + /touch exactly one GAP/, ); });