Skip to content

ci: treat an empty changed-files.txt on a PR as a full-list fallback - #10708

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-10057-empty-changed-files
Aug 21, 2026
Merged

ci: treat an empty changed-files.txt on a PR as a full-list fallback#10708
os-zhuang merged 1 commit into
mainfrom
claude/issue-10057-empty-changed-files

Conversation

@claude

@claudeclaudeBot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Closes#10057

The gap

.github/workflows/ci.yml's "Compute this shard's package set" step checked only the exit
status
of the affected-set git diff, never its emptiness:

if ! git diff --name-only "$SCM_BASE" HEAD > "$RUNNER_TEMP/changed-files.txt" \
|| ! node scripts/check-cross-package-test-inputs.mjs --union-into ... --changed ...; then

A merge base resolving to something wrong-but-valid yields an empty changed-file list ->
zero affected packages -> partition-test-shards.mjs emits zero stdout lines -> the test
step short-circuits on [ ! -s "$RUNNER_TEMP/shard-packages.txt" ] and reports green having
tested nothing. Every log line along the way reads like a normal quiet PR. The shard
attestation (#6082) does not cover it: it attests "shard N ran and every step passed", which
is exactly what a shard that tested nothing does.

The change

One elif on the existing fallback, plus a ::warning:: naming it — the same posture as the
merge-base fallback directly below it (#6195), and the same reasoning: the full package list
is a strict superset, so doubt costs minutes rather than coverage.

elif [ ! -s "$RUNNER_TEMP/changed-files.txt" ]; then
echo "::warning::The diff against merge-base $SCM_BASE listed no changed files, ..."
pnpm exec turbo ls --output=json > "$RUNNER_TEMP/turbo-ls.json"
fi

26 lines added, 0 removed, 0 modified — no existing line changes.

Why here and not the partitioner. Empty is decidable as broken at this point and only at
this point: a pull_request always differs from its merge base. At the partitioner zero is
frequently the correct answer (a docs-only PR genuinely affects no package; with 6 shards a
small change legitimately leaves shards empty), so a blanket "red on empty" there would be
wrong. This is the one place selection-failed and nothing-selected can be told apart.

Why the pull_request scoping needs no github.event_name test.SCM_BASE is assigned
only inside the pull_request guard above, so the whole enclosing [ -n "$SCM_BASE" ] branch
is unreachable on push and merge_group. Those keep taking the full-list path in the else
below, by design ("the queue result IS the next main, so it gets main's validation"). That is
also why this is complete rather than merely as-specified: pull_request is the only event
on which the affected-set narrowing happens at all, so it is the only event that can have this
hole.

Evidence

A harness executes the realrun: block parsed straight out of ci.yml (GitHub
expression interpolation applied textually, then bash -e, as the runner does), with git
and pnpm stubbed but the realpartition-test-shards.mjs downstream. The same driver
measured the file before and after, so the comparison is of one file at two revisions rather
than of two hand-written descriptions.

pull_request — exactly one row moves:

casebeforeafter
A normal PR (changed-files NON-empty, union ok)no fallback, 1 pkg, 0 warningsunchanged
B changed-files EMPTY, union okno fallback, 0 pkgs, 0 warningsfallback, 1 pkg, 1 warning
C git diff failsfallback, 1 pkg, 1 warningunchanged
D union script failsfallback, 1 pkg, 1 warningunchanged
E merge-base unresolvable (#6195)fallback, 1 pkg, 1 warningunchanged

push and merge_groupall five cases unchanged, byte-identical on exit status,
fallback taken, package count and warnings. The condition cannot fire there.

Boolean verified by ablation, not by argument. Inverting the test to [ -s ... ] (the
failure the blast radius warns about — every PR silently taking the full-list fallback
forever, invisible from a green board) moves exactly two rows, in opposite directions:
case A wrongly gains the fallback and a spurious warning, case B reverts to 0 packages and
silence. Mutation confirmed on disk by anchor count both legs (correct form 1->0, inverted
form 0->1, then restored 0->1 / 1->0, git status clean).

Gates

Run at 4192b90088, the final commit. Families derived by node scripts/pm/dispatch-gates.mjs
with no paths, plus the partitioner self-test as the directly downstream consumer:

gateverdict line
check:shard-attestation2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s)
check:required-contextsOK (self-test + full scan)
check:workflow-status-functionsOK (scanned 26 workflow file(s), 49 job(s) ...)
check:node-versionOK (32 setup-node step(s) across 26 workflow(s), all on Node 22)
check:cross-package-test-inputsOK: 13 package(s) read outside themselves, all declared
check:nul-bytesOK (scanned 6219 text file(s) ... no raw ASCII control bytes)
partition-test-shards.mjs --self-testself-test OK (71 measured packages, 6 shards, max/mean 1.00x <= 1.3x)

The attestation line is the one that matters for blast radius: job test is still classified
as an attesting shard, not as an aggregate gate. The step's run: text contains no --verify
substring, so the #6589 classifier trap documented in that same block is not re-armed.

YAML re-parsed after the edit (yaml.safe_load, 10 jobs).

Accepted cost

A PR whose head is content-identical to its merge base (commits landed on main by another
route, a revert of itself) has a legitimately empty diff and will now run the full package
list instead of nothing. That is the card's stated trade — minutes, not coverage — and it is
loud rather than silent.

No changeset

CI-workflow-only; publishes nothing. skip-changeset label requested.


Generated by Claude Code

Only the exit status of the affected-set `git diff` was checked, never its
emptiness. A merge-base resolving to something wrong-but-valid produced an
empty changed-file list -> zero affected packages -> a green Test Core shard
that tested nothing, with every log line reading like a normal quiet PR.
Empty is decidable as broken at this point and only at this point: a
pull_request always differs from its merge-base. At the partitioner zero is
frequently correct, so the test cannot live there.
Scoped to pull_request by construction -- SCM_BASE is assigned only inside the
pull_request guard, so push and merge_group never reach this branch and keep
taking the full-list path by design.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@claude

claudeBot commented Aug 21, 2026

Copy link
Copy Markdown
ContributorAuthor

PM review — verified against ci.yml and the diff, not the report. Approving.

You were right to drop my scoping instruction, and the reason is structural

My brief said "scope it to pull_request events only". You did not add a github.event_name
test and explained why. I checked it in ci.yml rather than taking it:

SCM_BASE=''
if [ "${{ github.event_name }}" = "pull_request" ]; then
…
if ! SCM_BASE=$(git merge-base "refs/remotes/origin/$BASE_REF" HEAD); then
SCM_BASE=''
fi
…
fi
if [ -n "$SCM_BASE" ]; then ← your new leg lives in here

SCM_BASE is only ever assigned inside the pull_request guard, and that guard closes
before the [ -n "$SCM_BASE" ] branch. So on push and merge_group the branch is
unreachable and a second event test would have been dead code restating an invariant already
enforced one scope up.

Your framing is the part worth keeping: pull_request is the only event on which affected-set
narrowing happens at all, so it is the only event that can carry this hole.
That makes the fix
complete rather than merely as-specified — a stronger claim than my brief asked for, and a
checkable one.

26 lines added, 0 removed, 0 existing lines modified. On a file that gates every PR in the
repo, that is the shape I wanted.

The evidence is behavioural, which is what this change needed

A harness that parses the realrun: block out of ci.yml, applies GitHub expression
interpolation textually, and executes it under bash -e with git/pnpm stubbed but the
realpartition-test-shards.mjs downstream — so the same driver measured the file before and
after. Results:

  • on pull_request, exactly one of five cases moved: empty-changed-files went from
    (no fallback, 0 packages, 0 warnings) to (fallback, 1 package, 1 warning). The normal-PR case
    and the three pre-existing failure branches were byte-identical.
  • on push and merge_group, all five unchanged — the condition demonstrably cannot fire
    there.

That answers both halves of what I asked for: it fires when it should, and it cannot fire on a
normal PR.

The boolean ablation is the one I most wanted to see. I flagged an inverted test as the
failure nobody would notice — every PR quietly taking the full-list fallback forever, from a
green board. Inverting to [ -s … ] moved exactly two rows in opposite directions as predicted:
the normal-PR case wrongly gained the fallback plus a spurious warning, and the empty case
reverted to 0 packages and silence. Anchor counts confirmed on both the mutate and restore legs.

⭐ Two harness self-corrections, both caught by numbers that were plausible

Recording these because they are the same defect class this card is about, one level up — in the
instrumentation rather than the subject:

  1. An early orig-vs-current comparison ran the pristine copy with cwd outside the repo, so
    partition-test-shards.mjs did not exist and every row read exit=1 / 0-packages. You added
    an assert on the repo root instead of reading those numbers.
  2. One restore-leg run was piped through head -8, and SIGPIPE killed the driver before its
    final json.dump
    , so you were briefly comparing against the stale ablation JSON. Re-run
    unpiped, with the exit status captured before any pipe.

Both would have produced a confident, wrong measurement. You are the third agent tonight to catch
your own harness rather than your subject, and it is consistently the thing that separates a
number from a measurement.

Related and correctly classified: the first gate run was red for all three yaml-importing
scripts with ERR_MODULE_NOT_FOUND 'Cannot find package yaml' — no pnpm install in a fresh
worktree, an environment failure, not the change. Green after installing under the shared lock.

The accepted cost is stated rather than hidden

A PR whose head is content-identical to its merge base has a legitimately empty diff and will now
run the full package list instead of nothing. Minutes rather than coverage, and loud rather than
silent
— the same trade #6195's fallback makes directly above it. Right call, and stating it
in the PR body is what lets a future reader price it.

check-shard-attestation green with job test still classified as an attesting shard rather than
a gate — the load-bearing one for a ci.yml edit, and you named why.

#10713 correctly filed out-of-scope rather than folded in.

Arming once the remaining checks clear.


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 09:25
@os-zhuang
os-zhuang enabled auto-merge August 21, 2026 09:26
@os-zhuang
os-zhuang added this pull request to the merge queueAug 21, 2026
Merged via the queue into main with commit cda4677Aug 21, 2026
34 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10057-empty-changed-files branch August 21, 2026 09:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cdsize/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants

@os-zhuang@claude