From a262a35724b3246353f07b5de96ec4731681dd4b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:16:19 +0200 Subject: [PATCH 1/2] ci(pr): classify changed files from the merge ref when the files API 422s The `changes` job decides whether a PR touches product code, and ci-ok requires it. It asks GitHub for the PR's file list. An earlier fix moved it off the .diff endpoint, which rejects PRs over 20k lines, onto the paginated pulls/files endpoint -- but that endpoint renders the diff server-side as well, and for a PR that regenerates a vendored parser it answers HTTP 422: Sorry, this diff is taking too long to generate. Observed on #2246 (a 42 MB sql/parser.c): `changes` red, therefore ci-ok red, for a reason that has nothing to do with the contribution. The compare endpoint fails the same way, so there is no API route to the file list for such a PR. Every grammar refresh would hit this. When the API call fails, the job now takes the list from git instead. The PR merge ref's first parent is the base, so a name-only diff across that one merge commit is exactly the PR's changed files. The fetch is depth 2 with --filter=blob:none into a bare repository under RUNNER_TEMP: commits and trees only, no blobs, nothing checked out and nothing from the PR executed. No new action, no new permission, and the API path and the classification regex are untouched, so ordinary PRs behave as before. A ::notice:: line records when the fallback was used. If the fetch fails too, the step exits non-zero and writes no output -- the gate fails closed, as it did before. Proof, by executing the workflow's own run block under `bash -eo pipefail` with a stub gh: API path, docs-only list rc 0 product=false fallback, real #2246 rc 0 product=true, the same five files `git diff --numstat` reports for the PR; 0.85 s, 188 KB fetched fallback, nonexistent merge ref rc 128 no output written Contract tests that read pr.yml -- security_gate_fail_closed, smoke_fixture_contract, windows_bundle_contract, venue_parity_contract -- pass. Not addressed here, noted for a follow-up: the classification pipes the list into `grep -q` under pipefail, which can report a match as a failure once the list outgrows the pipe buffer. Signed-off-by: Martin Vogel --- .github/workflows/pr.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0cfc796be8..6e56914238 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -64,7 +64,23 @@ jobs: run: | # The full .diff endpoint rejects large-but-valid PRs at 20k lines. # The paginated files endpoint remains filename-only for this gate. - FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename') + if FILES=$(gh api --paginate "repos/$REPO/pulls/$PR/files?per_page=100" --jq '.[].filename'); then + echo "file list: pulls/files API" + else + # The files endpoint renders the diff server-side too, and answers + # 422 "this diff is taking too long to generate" for a PR that + # regenerates a vendored parser (#2246: a 42 MB sql/parser.c) -- + # so does the compare endpoint. The PR merge ref's first parent is + # the base, so a name-only diff across it is the same file list. + # depth 2 + blob:none fetches commits and trees only (well under + # 1 MB, under a second); nothing from the PR is checked out or run. + echo "::notice::pulls/files API failed; file list taken from refs/pull/$PR/merge" + CHANGES_GIT="$RUNNER_TEMP/changes.git" + git init -q --bare "$CHANGES_GIT" + git --git-dir="$CHANGES_GIT" fetch -q --depth=2 --filter=blob:none \ + "https://github.com/$REPO" "refs/pull/$PR/merge" + FILES=$(git --git-dir="$CHANGES_GIT" diff --name-only --no-renames FETCH_HEAD^1 FETCH_HEAD) + fi printf '%s\n' "$FILES" if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then echo "product=true" >> "$GITHUB_OUTPUT" From 65d60e72f5b86f5982b15f0d88826ba79a085aa2 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 19:34:37 +0200 Subject: [PATCH 2/2] ci(pr): the changes classifier no longer fails open on a large file list Found while proving the previous commit, in the same step, and the worse of the two defects because it fails OPEN. The classification was if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|...)'; then under the runner's `bash -eo pipefail`. grep -q exits on its first match; once the list is larger than the pipe buffer printf is still writing, dies of SIGPIPE, and pipefail turns the MATCH into a failed pipeline. The else branch then writes product=false, and pr-smoke and memwaste are skipped -- on exactly the PRs that change the most files. The previous commit's message flagged this as a follow-up; it is fixed here instead. The list now reaches grep through a here-string, so there is no pipe to break. The regex is unchanged. Proof, by executing the workflow's own run block under `bash -eo pipefail` with a stub gh, ten runs each: 849 KB list, first line src/a.c before: product=false 10/10 after: product=true 10/10 849 KB list, docs only product=false before and after small docs-only list product=false 422 fallback on the real #2246 product=true fallback, nonexistent merge ref rc 128, no output written It takes roughly 1,500 changed files to cross the buffer, which is why ordinary PRs never showed it. Signed-off-by: Martin Vogel --- .github/workflows/pr.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 6e56914238..8cb464b190 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -82,7 +82,11 @@ jobs: FILES=$(git --git-dir="$CHANGES_GIT" diff --name-only --no-renames FETCH_HEAD^1 FETCH_HEAD) fi printf '%s\n' "$FILES" - if printf '%s\n' "$FILES" | grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)'; then + # A here-string, not a pipe: under this shell's pipefail, grep -q + # exits on its first match, printf dies of SIGPIPE once the list + # outgrows the pipe buffer, and the MATCH is then reported as a + # failure -- product=false, smoke skipped, on the largest PRs. + if grep -qE '^(src/|internal/|install\.(sh|ps1)|scripts/build\.sh|scripts/smoke-test\.sh|scripts/smoke-local\.sh|scripts/smoke-fixture-server\.py|scripts/gen-third-party-notices\.sh|scripts/env\.sh|test-infrastructure/vm/(vm-smoke\.sh|windows-user-path-guard\.ps1)|Makefile\.cbm)' <<<"$FILES"; then echo "product=true" >> "$GITHUB_OUTPUT" else echo "product=false" >> "$GITHUB_OUTPUT"