fix(evals): screen build-image keys the CI image on the merge commit it tests (#3962)

* test(evals): build-image must key the screen's image on its merge commit (fails on main)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(evals): screen build-image keys the CI image on the merge commit it tests

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes (superseded screens build no image)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

.github/workflows/evals.yml

@@ -76,7 +76,13 @@ jobs:
     # (EV6, fork port wave 2). Skip the build for dependabot; the evals job's
     # explicit actor guard mirrors it because no eval test selects on a lockfile-only
     # diff — a maintainer's next push rebuilds the image with real perms.
-    if: github.actor != 'dependabot[bot]'
+    # A dispatched screen's slices test inputs.screen_merge_sha, so the image is
+    # keyed and built from that commit's bun.lock, not the dispatch ref's (screen
+    # 41662815506 ran Playwright 1.64.1 against main's Chromium). It waits for the
+    # route job, which refuses a screen of a fork or closed PR, and skips a
+    # superseded screen, whose slices plan nothing.
+    needs: [route]
+    if: github.actor != 'dependabot[bot]' && needs.route.outputs.superseded != 'true'
     runs-on: ubicloud-standard-8
     timeout-minutes: 15
     permissions:
@@ -87,6 +93,9 @@ jobs:
       runtime-id: ${{ steps.runtime.outputs.id }}
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
+        with:
+          ref: ${{ inputs.screen_merge_sha || github.sha }}
+          persist-credentials: false
 
       - id: meta
         # Key on Dockerfile + lockfile only. package.json is deliberately NOT

changelog.d/gsta-260-screen-image-merge-lock.md

@@ -0,0 +1,2 @@
+### For contributors
+- **A dispatched screen now builds its CI image from the commit it tests.** evals.yml's `build-image` job used to check out the dispatch ref (main) and key the image on main's `bun.lock`, while the slices checked out the PR's merge commit. A PR that changed the lockfile was then tested against main's installed browsers. Screen run 41662815506 for #3947 failed eight cases with `Executable doesn't exist … chromium_headless_shell-1248` because the image had only revision 1234. `build-image` now checks out `screen_merge_sha` (or `github.sha` for every other run) before it hashes and builds, without persisting the token. It also waits for the `route` job: a refused dispatch (a fork or closed PR, for one) and a superseded screen build no image.

test/evals-train-workflow.test.ts

@@ -224,7 +224,20 @@ describe('under a train block (the gate edits, each only on its mode)', () => {
     expect(sim.steps('slices-report')).not.toContain('Write the train record');
     expect(sim.step('slices-comment', 'Post PR comment')!.env).toMatchObject({ PR_NUMBER: '42', SCREEN_POLICY: 'narrow', SCREEN_MERGE_SHA: SHA.merge });
     const superseded = simulate(evals, { github: dispatch(), inputs, outputs: { route: { ...route, superseded: 'true' } } });
-    expect(superseded.jobs).toEqual(['build-image', 'route']);
+    expect(superseded.jobs).toEqual(['route']);
+  });
+
+  test('build-image keys and builds the CI image from the commit the slices test, only after the route accepts the run', () => {
+    const inputs = { ...INPUTS, evals_all: false, nonce: 'pr.screen.narrow-x', expected_sha: SHA.merge, base_ref: 'main', base_sha: SHA.base, screen_pr: '42', screen_merge_sha: SHA.merge, screen_policy: 'narrow' };
+    const route = { ...ROUTE, ...train, mode: 'screen', screen_head: SHA.screenHead, screen_base: SHA.screenBase, screen_branch: 'feature' };
+    const screen = simulate(evals, { github: dispatch(), inputs, outputs: { route } });
+    expect(screen.checkout('build-image')?.ref).toBe(SHA.merge);
+    expect(screen.checkout('build-image')?.ref).toBe(screen.checkout('eval-slices')?.ref);
+    expect(screen.checkout('build-image')?.['persist-credentials']).toBe(false);
+    const steps = evals.jobs['build-image']!.steps;
+    expect(steps.findIndex(st => st.uses?.startsWith('actions/checkout@'))).toBeLessThan(steps.findIndex(st => st.id === 'meta'));
+    expect(simulate(evals, { github: dispatch(), inputs, outputs: { route }, results: { route: 'failure' } }).jobs).not.toContain('build-image');
+    for (const github of [pr(), dispatch()]) expect(simulate(evals, { github, inputs: github.event_name === 'pull_request' ? {} : INPUTS, outputs: { route: { ...ROUTE, mode: github.event_name === 'pull_request' ? 'pr' : 'dispatch' } } }).checkout('build-image')?.ref).toBe(SHA.main);
   });
 
   test('a train gate runs the full gate, writes its record from the report and publishes the check; it never reuses or binds', () => {
@@ -262,9 +275,9 @@ describe('under a train block (the gate edits, each only on its mode)', () => {
     expect(job.steps.find(st => st.name === 'Verify the prefix checkout')!.run).toContain('echo "short=${PREFIX_SHA:0:12}"');
   });
 
-  test('a refused dispatch starts nothing paid', () => {
+  test('a refused dispatch starts nothing paid and builds no image', () => {
     const sim = simulate(evals, { github: dispatch('train/t1'), inputs: { ...INPUTS, nonce: 'gate.bisect-x' }, outputs: { route: { ...ROUTE, mode: '' } }, results: { route: 'failure' } });
-    expect(sim.jobs).toEqual(['build-image', 'route']);
+    expect(sim.jobs).toEqual(['route']);
   });
 });