From b58a40dd728dbec5fd8c0d2a3cd4fbe466bfac5a Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sat, 25 Jul 2026 22:46:19 +0700 Subject: [PATCH] fix(ci): pick the codegen-freshness range from the PR base, not the last push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On pull_request, `github.event.before` is the PR's previous head, so the second push to a PR diffs a range that no longer contains the commit that edited the schema: the detector emits changed=false, the freshness step is skipped by its `if:`, and lint-and-test goes green on a stale src/graphql/types.ts. Every later push that does not itself touch one of the five inputs repeats it, forks included — which is the case the trigger was added for. Nothing else covers the gap: tsc reads the committed file as truth, eslint ignores it, and prettier passes on codegen's own output. Diff from pull_request.base.sha on PR events and keep `before` for push. The range selection moves out of the YAML into .github/scripts so it can be driven over real git fixtures: on the three-commit fixture the old range yields changed=false and the new one changed=true. Co-Authored-By: Claude Opus 4.8 --- .github/scripts/codegen-inputs-changed.sh | 36 ++++++++++++++ .github/workflows/ci.yml | 19 ++------ frontend/e2e/ci-codegen-gate.unit.test.ts | 59 +++++++++++++++++++++++ 3 files changed, 100 insertions(+), 14 deletions(-) create mode 100755 .github/scripts/codegen-inputs-changed.sh create mode 100644 frontend/e2e/ci-codegen-gate.unit.test.ts diff --git a/.github/scripts/codegen-inputs-changed.sh b/.github/scripts/codegen-inputs-changed.sh new file mode 100755 index 00000000..6b05aa15 --- /dev/null +++ b/.github/scripts/codegen-inputs-changed.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# Decide whether a run has to re-check that frontend/src/graphql/types.ts is fresh. +# Echoes `changed=true|false`. Split out of ci.yml so the range selection is testable +# (frontend/e2e/ci-codegen-gate.unit.test.ts drives it over real git fixtures). +# +# Usage: codegen-inputs-changed.sh +set -uo pipefail + +EVENT="${1:-}" +BEFORE="${2:-}" +BASE_SHA="${3:-}" +HEAD_SHA="${4:-}" + +# On pull_request, `before` is the PR's PREVIOUS HEAD, so a follow-up push diffs a range +# that no longer contains the commit that edited the schema — the gate would skip itself +# on every push after the first. The PR's base is the range that always spans the change. +if [ "$EVENT" = "pull_request" ]; then + base="$BASE_SHA" +else + base="$BEFORE" +fi + +# When the range can't be resolved (new branch, force-push, tag), check anyway. +if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ] \ + || ! git cat-file -e "$base^{commit}" 2>/dev/null; then + 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 + echo "changed=true" +else + echo "changed=false" +fi diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c2e158ee..558d4c44 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,23 +80,14 @@ jobs: # Only when a codegen input moved. The lockfile is included on purpose: a codegen # version bump can change the output, and skipping it there would let types.ts go # stale and fail the next schema push for something its author did not do. - # When the range can't be resolved (new branch, force-push, tag), check anyway. - name: Frontend - Detect GraphQL codegen input changes id: codegen-inputs run: | - base='${{ github.event.before }}' - if [ -z "$base" ] || [ "$base" = "0000000000000000000000000000000000000000" ] \ - || ! git cat-file -e "$base^{commit}" 2>/dev/null; then - echo "changed=true" >> "$GITHUB_OUTPUT" - 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" "${{ github.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 - echo "changed=true" >> "$GITHUB_OUTPUT" - else - echo "changed=false" >> "$GITHUB_OUTPUT" - fi + .github/scripts/codegen-inputs-changed.sh \ + '${{ github.event_name }}' \ + '${{ github.event.before }}' \ + '${{ github.event.pull_request.base.sha }}' \ + '${{ github.sha }}' >> "$GITHUB_OUTPUT" # The app ships the operations compiled into src/graphql/types.ts; regenerate and # diff so it can't drift from graphql-schema.graphql (the codegen input the stand's diff --git a/frontend/e2e/ci-codegen-gate.unit.test.ts b/frontend/e2e/ci-codegen-gate.unit.test.ts new file mode 100644 index 00000000..ebcd2504 --- /dev/null +++ b/frontend/e2e/ci-codegen-gate.unit.test.ts @@ -0,0 +1,59 @@ +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +const SCRIPT = join(__dirname, '..', '..', '.github', 'scripts', 'codegen-inputs-changed.sh'); + +let repo = ''; +const sha: Record = {}; + +const git = (...args: string[]) => execFileSync('git', ['-C', repo, ...args], { encoding: 'utf8' }).trim(); + +const commit = (path: string, body: string, message: string) => { + mkdirSync(join(repo, path.slice(0, path.lastIndexOf('/'))), { recursive: true }); + writeFileSync(join(repo, path), body); + git('add', '-A'); + git('-c', 'user.email=e2e@example.com', '-c', 'user.name=e2e', 'commit', '-m', message); + + return git('rev-parse', 'HEAD'); +}; + +const run = (event: string, before: string, baseSha: string, headSha: string) => + execFileSync(SCRIPT, [event, before, baseSha, headSha], { cwd: repo, encoding: 'utf8' }).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(() => { + repo = mkdtempSync(join(tmpdir(), 'codegen-gate-')); + git('init', '-q', '-b', 'main'); + sha.base = commit('README.md', 'base\n', 'base'); + sha.schema = commit('backend/pkg/graph/schema.graphqls', 'type Query { a: Int }\n', 'edit the schema'); + sha.unrelated = commit('README.md', 'base\nmore\n', 'unrelated follow-up'); + sha.merge = git('commit-tree', `${sha.unrelated}^{tree}`, '-p', sha.base, '-p', sha.unrelated, '-m', 'merge'); +}); + +afterAll(() => rmSync(repo, { force: true, recursive: true })); + +describe('codegen freshness gate — range selection', () => { + it('checks a follow-up push whose newest commit did not touch a codegen input', () => { + expect(run('pull_request', sha.schema, sha.base, sha.merge)).toBe('changed=true'); + }); + + it('checks a freshly opened PR, where there is no previous head', () => { + expect(run('pull_request', '', sha.base, sha.merge)).toBe('changed=true'); + }); + + 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'); + }); + + it('keeps checking a push by its own range', () => { + expect(run('push', sha.base, '', sha.schema)).toBe('changed=true'); + }); + + it('still skips a PR that touches no codegen input', () => { + expect(run('pull_request', '', sha.schema, sha.unrelated)).toBe('changed=false'); + }); +});