[kyle-sexton]

Closes #9051

Summary

scripts/check-affected.sh --tests reported "0 changed files" on branches that changed files and then passed, so the ready phase's verify gate checked nothing. Now it checks the branch it belongs to whatever the working directory is. An empty changed set while HEAD is not the base exits 2 with a message instead of passing.

Fix

Root cause: the script picked the tree to check from the shell's working directory (git rev-parse --show-toplevel with no -C, scripts/check-affected.sh:97-101 on main), not from where the script lives. Every run in this repository's session transcripts that reported 0 changed files had the shape bash <worktree>/scripts/check-affected.sh --tests with the working directory in the main checkout. Runs started inside the worktree (env -C <worktree> bash scripts/check-affected.sh) listed the branch's files. In that shape the script checked the main checkout. The main checkout's HEAD sat at its merge-base with origin/main, so the diff was empty. The worktree counted as a foreign tree, so tests was skipped. Rerunning that shape against #9062's worktree printed exactly the 7 passed, 7 skipped over 0 changed files that #9062 reported. #9051 reports the same 0 changed files for #9041 and #9048. The explicit --base origin/main runs were made from the main checkout too, so --base was the same cause, not a second bug.

• scripts/check-affected.sh checks the tree CHECK_DATA_ROOT names, else the tree holding the script, which is the convention the other check scripts already follow (${CHECK_DATA_ROOT:-$SCRIPT_DIR/..}).
• An empty changed set while HEAD is not the base (HEAD behind the base, or commits that change nothing) exits 2 with a message. HEAD at its base still passes with nothing to check.
• --uncommitted passes CHECK_DATA_ROOT to its snapshot run, so the issue lane's wrapper, which sets CHECK_DATA_ROOT to the checkout, still checks the snapshot. With no edits, the snapshot is HEAD itself, so it still passes.
• .claude/hooks/check-affected-pre-push.sh passes CHECK_DATA_ROOT="$data" to the script it runs. Its merge-base copy sits outside any work tree, so without this the copy would exit 2, which the hook treats as a pass. The hook's short-flag glob is also reordered from [nd] to [dn] (same match), because typos 1.51 flags nd, and the pre-push check blocks any branch that touches the file.

Verification

Red: the new cases in scripts/check-affected.test.sh run against main's script:

FAIL: run from another checkout, it lists the script's own branch's change: rc=0 out=check-affected: 7 passed, 7 skipped over 0 changed files
FAIL: an explicit --base run from another checkout still lists the change: rc=0 out=check-affected: 7 passed, 6 skipped over 0 changed files
FAIL: HEAD behind its base with no changed files exits non-zero with a message: rc=0
FAIL: commits beyond the base that change no file exit non-zero with a message: rc=0
PASS=37 FAIL=9

The other 5 failures are the copy cases, which now pass CHECK_DATA_ROOT (the hook's new contract), and main's script ignores it.

Green, with the fix: scripts/check-affected.test.sh PASS=46 FAIL=0; .claude/hooks/check-affected-pre-push.test.sh PASS=51 FAIL=0, including the real merge-base copy blocking a shellcheck error; scripts/check-data-root.test.sh PASS=8 FAIL=0; scripts/lib/gate-entry.test.sh PASS=33 FAIL=0; scripts/check-affected-gates.test.sh PASS=7 FAIL=0.

Run in the shape that failed (working directory in the main checkout, the script in this worktree), the fixed script listed this branch's 3 files and ran the checks over them. That run exited 1 on two findings:

• typos flagged the pre-existing [nd] glob in the hook. e51d6e5 fixes it, and the pre-push check then passed.
• tests failed only on scripts/check-script-contract.test.sh, at check-html-assets.sh: htmlhint not found ... (run npm ci). This fresh worktree has no node_modules. That failure has nothing to do with this change.

Every other selected suite passed.

Related

• fix(pr-pipeline): match lane PRs to queued issues by branch and serialize work-unblock admissions #9041, fix(pr-pipeline): wait out a GITHUB_TOKEN rate limit before failing a dispatch #9048, test: declare test scopes for the selection audit gaps #9062: the PRs whose ready-phase verify gate passed over 0 changed files.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-22T18:01:14.857716Z	6b679e5	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed 6b679e5ab13d4ac5ec96069c247e26a86c766976: 0 finding(s), 0 as inline comments.

Security review of PR #9072 (head 6b679e5)

Findings: 0. The PR changes scripts/check-affected.sh, its test and the pre-push hook. I read the full diff and the changed script. No plugin manifest changed, and no standing instruction was deleted or softened. The protected-class register was not needed.

I looked at four places where the change could cross a trust boundary:

• Which tree is read. The checked tree is now git -C "${CHECK_DATA_ROOT:-$tools}" rev-parse --show-toplevel instead of the working directory. The pre-push hook sets CHECK_DATA_ROOT="$data" explicitly, and the --uncommitted path sets it to the scratch snapshot. Neither value comes from untrusted input. Anyone who can set the variable in the caller's environment can already run the script directly, so it adds no new control.
• Foreign-tree safeguards. The foreign test still compares the post-cd working directory with $tools. When the checked tree differs from the tool tree, the hardening stays in place: UV_NO_CONFIG=1, markdownlint reading only the tool tree's configs, and skipping the checks that run the branch's own code. In the --uncommitted re-entry, CHECK_DATA_ROOT="$snap/tree" still sends the inner run down the foreign path, so the snapshot's code is not run.
• Clean-tree shortcut. The new --uncommitted branch reuses HEAD as the snapshot commit when the tree matches HEAD^{tree}. It writes no new objects and takes no external input.
• Empty change set. Exiting 2 when no files changed but HEAD is not the base makes the check fail closed. Before, it passed having checked nothing.

The pre-push hook edit reorders -[nd]* to -[dn]* in a case pattern, which is equivalent. The test changes only add cases and use cd "$tmp" with an explicit CHECK_DATA_ROOT.

[github-actions]

claude-review reviewed 6b679e5ab13d4ac5ec96069c247e26a86c766976: 0 finding(s), 0 as inline comments.

No findings on PR #9072 at 6b679e5.

What I checked (by reading the diff, PR CONTEXT and the surrounding code in scripts/check-affected.sh and .claude/hooks/check-affected-pre-push.sh):

• Tree selection. The script now resolves the tree to check with git -C "${CHECK_DATA_ROOT:-$tools}" rev-parse --show-toplevel and cds there. This matches issue fix(repo): check-affected.sh can report 0 changed files and pass silently #9051's first fix bullet (resolve the repository from the script's location, not the shell's working directory).
• foreign detection. It still compares pwd -P with $tools after the cd. The copy-script cases and the --uncommitted snapshot still count as foreign trees, so markdownlint and the "skip checks that run the branch's own code" logic are unchanged.
• Empty changed set. The new guard exits 2 only when $all_raw is empty and git rev-list --count "$base...HEAD" is non-zero. The merge-base call just before it has already succeeded, so rev-list cannot fail spuriously there. HEAD at its base still passes. This satisfies the second acceptance criterion.
• Cases that now exit 2. HEAD behind the base with no own commits, empty-only commits, and a net-empty diff all exit 2. Both callers treat exit 2 as a skip, so they do not block a push.
• --uncommitted. Passing CHECK_DATA_ROOT="$snap/tree" drops the old cd into the snapshot and avoids an inherited CHECK_DATA_ROOT redirecting the inner run to the checkout. With no edits the snapshot is HEAD itself, so the base resolves to HEAD and the guard passes.
• Pre-push hook. CHECK_DATA_ROOT="$data" is needed because the merge-base copy sits outside any work tree. The [nd] to [dn] glob change matches the same set of flags.
• Tests. The new cases cover the working directory in another checkout, an explicit --base, HEAD behind its base, empty commits, HEAD at its base, and --uncommitted with and without edits. The copy cases now pass CHECK_DATA_ROOT.

Closing issue #9051. Both acceptance criteria are met, and the new tests would fail if either broke. The third fix bullet (why an explicit --base still gave 0) is covered by the PR's root-cause analysis and by a dedicated --base test.

Not verified. I did not run the test suites; the PASS counts in the PR body are author-claimed. My shell was restricted, so I could not grep the repository for other callers of check-affected.sh that might depend on the working directory. This PR updates the two it names: the pre-push hook and the --uncommitted snapshot run.