From f42a76b1eaf007fd6fd8af954ba7dab39b69d1c6 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Mon, 27 Jul 2026 14:21:37 +0700 Subject: [PATCH] fix(ci): stop the codegen gate from skipping itself on a large diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git diff --name-only | grep -qE` matched, grep exited, git died on SIGPIPE, and `pipefail` turned the whole pipeline into a failure — so a push that edited the schema alongside enough other files printed `changed=false` and skipped the freshness check entirely. Read the list into a variable and match it in the shell instead. The two unresolvable-base cases could not see any of this: they asserted `changed=true`, which the diff-failed fallback also prints, so deleting the guard they were named for left them green. The script now names the branch it took on stderr and they assert that. Co-Authored-By: Claude Opus 5 --- .github/scripts/codegen-inputs-changed.sh | 27 +++++++++++++++++--- frontend/e2e/ci-codegen-gate.unit.test.ts | 31 +++++++++++++++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/.github/scripts/codegen-inputs-changed.sh b/.github/scripts/codegen-inputs-changed.sh index 6b05aa15..0efa1041 100755 --- a/.github/scripts/codegen-inputs-changed.sh +++ b/.github/scripts/codegen-inputs-changed.sh @@ -21,16 +21,37 @@ else fi # When the range can't be resolved (new branch, force-push, tag), check anyway. +# stdout is consumed verbatim as a workflow output, so diagnostics go to stderr. if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ] \ || ! git cat-file -e "$base^{commit}" 2>/dev/null; then + echo "reason=unresolvable-base" >&2 echo "changed=true" exit 0 fi # The generated file is in the list too: a push that edits only it would otherwise skip # the check, and the drift surfaces later on someone else's unrelated codegen push. -if git diff --name-only "$base" "$HEAD_SHA" | grep -qE '^(backend/pkg/graph/schema\.graphqls|frontend/graphql-schema\.graphql|frontend/graphql-codegen\.ts|frontend/pnpm-lock\.yaml|frontend/src/graphql/types\.ts)$'; then +INPUTS=( + backend/pkg/graph/schema.graphqls + frontend/graphql-schema.graphql + frontend/graphql-codegen.ts + frontend/pnpm-lock.yaml + frontend/src/graphql/types.ts +) + +if ! files=$(git diff --name-only "$base" "$HEAD_SHA"); then + echo "reason=diff-failed" >&2 echo "changed=true" -else - echo "changed=false" + exit 0 fi + +for input in "${INPUTS[@]}"; do + case $'\n'"$files"$'\n' in + *$'\n'"$input"$'\n'*) + echo "changed=true" + exit 0 + ;; + esac +done + +echo "changed=false" diff --git a/frontend/e2e/ci-codegen-gate.unit.test.ts b/frontend/e2e/ci-codegen-gate.unit.test.ts index d8f96128..b892d069 100644 --- a/frontend/e2e/ci-codegen-gate.unit.test.ts +++ b/frontend/e2e/ci-codegen-gate.unit.test.ts @@ -1,4 +1,4 @@ -import { execFileSync } from 'node:child_process'; +import { execFileSync, spawnSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -23,6 +23,9 @@ const commit = (path: string, body: string, message: string) => { const run = (event: string, before: string, baseSha: string, headSha: string) => execFileSync(SCRIPT, [event, before, baseSha, headSha], { cwd: repo, encoding: 'utf8' }).trim(); +const runReason = (event: string, before: string, baseSha: string, headSha: string) => + spawnSync(SCRIPT, [event, before, baseSha, headSha], { cwd: repo, encoding: 'utf8' }).stderr.trim(); + // The PR arrives as the merge commit GitHub builds, not as the branch head — the range the gate // picks has to span the whole PR, not the newest push. beforeAll(() => { @@ -45,8 +48,16 @@ describe('codegen freshness gate — range selection', () => { expect(run('pull_request', '', sha.base, sha.merge)).toBe('changed=true'); }); + // The answer alone cannot tell this branch from the diff-failed fallback below, which prints + // the same `changed=true`; only the reason distinguishes them. it('checks a force-push, where the previous head no longer resolves', () => { - expect(run('pull_request', '0'.repeat(40), sha.base, sha.merge)).toBe('changed=true'); + expect(run('push', 'b'.repeat(40), '', sha.schema)).toBe('changed=true'); + expect(runReason('push', 'b'.repeat(40), '', sha.schema)).toContain('reason=unresolvable-base'); + }); + + it('checks the first push of a branch, where there is no previous head at all', () => { + expect(run('push', '0'.repeat(40), '', sha.schema)).toBe('changed=true'); + expect(runReason('push', '0'.repeat(40), '', sha.schema)).toContain('reason=unresolvable-base'); }); it('keeps checking a push by its own range', () => { @@ -56,4 +67,20 @@ describe('codegen freshness gate — range selection', () => { it('still skips a PR that touches no codegen input', () => { expect(run('pull_request', '', sha.schema, sha.unrelated)).toBe('changed=false'); }); + + it('checks a codegen input whose diff is larger than one pipe buffer', () => { + const pad = 'x'.repeat(180); + mkdirSync(join(repo, 'frontend', 'filler'), { recursive: true }); + + for (let i = 0; i < 1000; i += 1) { + writeFileSync(join(repo, 'frontend', 'filler', `${i}-${pad}.txt`), ''); + } + + const bulk = commit('backend/pkg/graph/schema.graphqls', 'type Query { a: Int, b: Int }\n', 'bulk'); + const names = git('diff', '--name-only', sha.unrelated, bulk); + + expect(names.split('\n')[0]).toBe('backend/pkg/graph/schema.graphqls'); + expect(names.length).toBeGreaterThan(64 * 1024); + expect(run('push', sha.unrelated, '', bulk)).toBe('changed=true'); + }); });