fix(ci): attribute pull requests against the tested target parent (#9644)

Author: waleedlatif1

.github/CI.md

@@ -2,9 +2,9 @@
 
 The `changes` job runs Turborepo 2.9.14's `affectedTasks` query before installing application dependencies or provisioning services. `turbo.json` records package dependency propagation; `apps/sim/turbo.json` partitions the app into shared runtime, server, workspace UI, and acceptance tasks. `.github/ci-suites.json` turns those tasks into job matrices. The uploaded `ci-plan` artifact names changed files, selected tasks, attribution reasons, and any fallback.
 
-A PR compares its merge base with the checked-out merge commit. Main/staging pushes execute all checks; the parent `ci` workflow skips checks on `dev` pushes, while PRs targeting `dev` still run checks. A manual `checks` run executes all checks unless supplied a full base commit SHA to reproduce a scoped plan. Missing history, an empty diff, unknown files outside the workspace graph, root script/CI changes, or a planner error run every check. Missing selection outputs fail the aggregate gate. The gate also rejects an omitted dependency, any failed/cancelled job, and any skip that its explicit selection flag did not authorize.
+A PR compares the checked-out GitHub merge commit with its first parent: the actual target revision tested by that merge. The PR event's base SHA can become stale when the target advances. Scoping requires a matching GitHub merge ref and SHA, exactly two merge parents, and a usable target parent; otherwise every check runs. This works with the existing depth-two checkout without fetching older ancestry. Main/staging pushes execute all checks; the parent `ci` workflow skips checks on `dev` pushes, while PRs targeting `dev` still run checks. A manual `checks` run executes all checks unless supplied a full base commit SHA to reproduce a scoped plan. Missing history, an empty diff, unknown files outside the workspace graph, root script/CI changes, or a planner error run every check. Missing selection outputs fail the aggregate gate. The gate also rejects an omitted dependency, any failed/cancelled job, and any skip that its explicit selection flag did not authorize.
 
-Dedicated docs verification uses the same planner, comparing pushes with their previous commit and PRs with their merge base. Site acceptance follows `docs#build` and its transitive package dependencies. The app's docs-consumer task depends on server runtime and that docs build: backend-only changes run the platform consumer against the docs app, while site changes also run corpus and browser checks. Independent SDK and unit-only edits skip those production jobs. The required `docs HTTP` gate verifies explicit site/platform selections before accepting any skip.
+Dedicated docs verification uses the same planner, comparing pushes with their previous commit and PRs with the tested merge's first parent. Site acceptance follows `docs#build` and its transitive package dependencies. The app's docs-consumer task depends on server runtime and that docs build: backend-only changes run the platform consumer against the docs app, while site changes also run corpus and browser checks. Independent SDK and unit-only edits skip those production jobs. The required `docs HTTP` gate verifies explicit site/platform selections before accepting any skip.
 
 Docs production builds run once and publish an artifact to isolated HTTP, corpus, Chromium, and WebKit jobs. Source checks keep their own checkout because generator verification mutates inputs. Content caches use separate lint/source/build namespaces; generation validates every restore, repairs missing declarations, and removes obsolete modules. The Next build cache keeps its supported incremental behavior. Content and Next caches have size limits, and Next cache files expire after seven days; successful content eviction happens after all consumers finish.
 

scripts/ci-changes.test.ts

@@ -2,13 +2,15 @@ import { spawnSync } from 'node:child_process'
 import { copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { dirname, join, resolve } from 'node:path'
-import { fileURLToPath } from 'node:url'
+import { fileURLToPath, pathToFileURL } from 'node:url'
 import { afterEach, describe, expect, it } from 'vitest'
 
 const root = fileURLToPath(new URL('../', import.meta.url))
 const script = resolve(root, '.github/scripts/ci-changes.sh')
 const fixtures: string[] = []
 interface Plan {
+  base: string
+  files: string[]
   flags: Record<string, boolean>
   fallback?: string
   http: { group: string }[]
@@ -102,13 +104,17 @@ function fixture() {
   git(repo, 'commit', '--quiet', '-m', 'base')
   return { repo, base: git(repo, 'rev-parse', 'HEAD') }
 }
-function classify(repo: string, base: string): Plan {
+function classify(repo: string, base: string, environment: Record<string, string> = {}): Plan {
   const report = join(repo, 'plan.json')
   const result = spawnSync('bash', [script, base], {
     cwd: repo,
     encoding: 'utf8',
     env: {
       ...process.env,
+      GITHUB_EVENT_NAME: 'workflow_dispatch',
+      GITHUB_REF: 'refs/heads/fixture',
+      GITHUB_SHA: '',
+      ...environment,
       PATH: `${resolve(root, 'node_modules/.bin')}:${process.env.PATH}`,
       CI_PLAN_PATH: report,
     },
@@ -119,6 +125,39 @@ function classify(repo: string, base: string): Plan {
     expect(result.stdout).toContain(`${flag}=${value}\n`)
   return plan
 }
+function pullRequestFixture(files: string[], depth = 2) {
+  const { repo: source, base } = fixture()
+  git(source, 'checkout', '--quiet', '-b', 'contribution')
+  for (const file of files) {
+    write(source, file, `${file}\n`)
+    git(source, 'add', '-A')
+    git(source, 'commit', '--quiet', '-m', `contribute ${file}`)
+  }
+  git(source, 'checkout', '--quiet', '-b', 'target', base)
+  write(source, 'apps/sim/lib/upstream.ts', 'export const upstream = true\n')
+  git(source, 'add', '-A')
+  git(source, 'commit', '--quiet', '-m', 'advance target')
+  const target = git(source, 'rev-parse', 'HEAD')
+  git(source, 'merge', '--quiet', '--no-ff', '-m', 'synthetic PR merge', 'contribution')
+  const head = git(source, 'rev-parse', 'HEAD')
+  const repo = mkdtempSync(join(tmpdir(), 'ci-pr-merge-'))
+  fixtures.push(repo)
+  git(repo, 'init', '--quiet')
+  git(repo, 'remote', 'add', 'origin', pathToFileURL(source).href)
+  git(repo, 'fetch', '--quiet', `--depth=${depth}`, 'origin', head)
+  git(repo, 'checkout', '--quiet', '--detach', 'FETCH_HEAD')
+  expect(git(repo, 'rev-parse', '--is-shallow-repository')).toBe('true')
+  return {
+    repo,
+    base,
+    target,
+    environment: {
+      GITHUB_EVENT_NAME: 'pull_request',
+      GITHUB_REF: 'refs/pull/42/merge',
+      GITHUB_SHA: head,
+    },
+  }
+}
 function classifyFiles(files: string[]): Plan {
   const { repo, base } = fixture()
   for (const file of files) write(repo, file, `${file}\n`)
@@ -133,6 +172,71 @@ afterEach(() => {
   for (const repo of fixtures.splice(0)) rmSync(repo, { recursive: true, force: true })
 })
 describe('CI task attribution through real Turborepo', () => {
+  it('scopes a shallow multi-commit PR against its tested target after the event base goes stale', () => {
+    const files = ['packages/ts-sdk/src/index.ts', 'packages/ts-sdk/src/second.ts']
+    const { repo, base, target, environment } = pullRequestFixture(files)
+    const plan = classify(repo, base, environment)
+    expect(plan.fallback).toBeUndefined()
+    expect(plan.base).toBe(target)
+    expect(plan.files).toEqual(files)
+    expect(groups(plan)).toEqual([])
+    expect(plan.testPackages).toEqual(['simstudio-ts-sdk'])
+    expect(plan.typecheckPackages).toEqual(['simstudio-ts-sdk'])
+    expect(plan.flags).toMatchObject({
+      test: true,
+      test_app: false,
+      integration: false,
+      build: false,
+      desktop_live: false,
+      e2e_build: false,
+      docs: false,
+    })
+  })
+  it('keeps an earlier server change when the final PR commit only changes the SDK', () => {
+    const files = ['apps/sim/lib/server.ts', 'packages/ts-sdk/src/second.ts']
+    const { repo, base, target, environment } = pullRequestFixture(files)
+    const plan = classify(repo, base, environment)
+    expect(plan.fallback).toBeUndefined()
+    expect(plan.base).toBe(target)
+    expect(plan.files).toEqual(files)
+    expect(plan.testPackages).toContain('simstudio-ts-sdk')
+    expect(plan.flags).toMatchObject({
+      test_app: true,
+      integration: true,
+      build: true,
+      desktop_live: true,
+      e2e_build: true,
+    })
+  })
+  it.each<[string, Record<string, string>]>([
+    ['a branch ref', { GITHUB_REF: 'refs/heads/staging' }],
+    ['a missing ref', { GITHUB_REF: '' }],
+    ['a different triggering SHA', { GITHUB_SHA: 'f'.repeat(40) }],
+    ['a missing triggering SHA', { GITHUB_SHA: '' }],
+  ])('runs everything for a PR checkout with %s', (_label, invalid) => {
+    const { repo, target, environment } = pullRequestFixture(['packages/ts-sdk/src/index.ts'])
+    const plan = classify(repo, target, { ...environment, ...invalid })
+    expect(plan.fallback).toBeDefined()
+    expect(Object.values(plan.flags).every(Boolean)).toBe(true)
+  })
+  it('runs everything if a PR event checks out a non-merge commit', () => {
+    const { repo, base } = fixture()
+    write(repo, 'packages/ts-sdk/src/index.ts', 'export const value = 2\n')
+    git(repo, 'commit', '--quiet', '-am', 'SDK change')
+    const plan = classify(repo, base, {
+      GITHUB_EVENT_NAME: 'pull_request',
+      GITHUB_REF: 'refs/pull/42/merge',
+      GITHUB_SHA: git(repo, 'rev-parse', 'HEAD'),
+    })
+    expect(plan.fallback).toBeDefined()
+    expect(Object.values(plan.flags).every(Boolean)).toBe(true)
+  })
+  it('runs everything if a shallow PR checkout does not contain its merge parents', () => {
+    const { repo, base, environment } = pullRequestFixture(['packages/ts-sdk/src/index.ts'], 1)
+    const plan = classify(repo, base, environment)
+    expect(plan.fallback).toBeDefined()
+    expect(Object.values(plan.flags).every(Boolean)).toBe(true)
+  })
   it('runs SDK tests without provisioning the app, browsers, or database', () => {
     const plan = classifyFiles(['packages/ts-sdk/src/index.ts'])
     expect(plan.fallback).toBeUndefined()

scripts/ci-plan.ts

@@ -44,12 +44,22 @@ try {
     throw new Error('No comparison base; push and unscoped dispatch run the full suite')
   if (!/^[0-9a-f]{40}$/.test(sourceBase))
     throw new Error('Comparison base must be a full commit SHA')
-  try {
-    command('git', ['cat-file', '-e', `${sourceBase}^{commit}`])
-  } catch {
-    command('git', ['fetch', '--quiet', '--depth=2', 'origin', sourceBase])
+  if (process.env.GITHUB_EVENT_NAME === 'pull_request') {
+    if (!/^refs\/pull\/[1-9]\d*\/merge$/.test(process.env.GITHUB_REF ?? ''))
+      throw new Error('PR attribution requires the GitHub merge ref')
+    if (process.env.GITHUB_SHA !== command('git', ['rev-parse', 'HEAD']))
+      throw new Error('PR attribution requires the checked-out GitHub merge SHA')
+    const parents = command('git', ['show', '--no-patch', '--format=%P', 'HEAD']).split(' ')
+    if (parents.length !== 2) throw new Error('PR attribution requires a two-parent merge')
+    base = parents[0]
+  } else {
+    try {
+      command('git', ['cat-file', '-e', `${sourceBase}^{commit}`])
+    } catch {
+      command('git', ['fetch', '--quiet', '--depth=2', 'origin', sourceBase])
+    }
+    base = command('git', ['merge-base', sourceBase, 'HEAD'])
   }
-  base = command('git', ['merge-base', sourceBase, 'HEAD'])
   files = command('git', ['diff', '--no-renames', '--name-only', '-z', base, 'HEAD'])
     .split('\0')
     .filter(Boolean)