Repository navigation
fix(ci): run the selected suites for uncommitted changes as for committed ones - #10681
Conversation
…tted ones `check-affected.sh --uncommitted --tests` re-runs itself on a scratch snapshot of the work tree, so the inner run saw a data tree other than its own and skipped tests as foreign code. A digest pin over an uncommitted skill edit then went unchecked. The snapshot is the tooling tree's own work tree, so the outer run now marks it, and the inner run runs the suites from the tooling tree, whose selector reads the same uncommitted edits. A copy checking another tree still skips them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k-affected-uncommitted-selection
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
claude-review reviewed No findings. I read the full diff ( What I checked:
No closing issue (the PR states none). No manifests changed. No cost or critical-security hold applies. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
claude-security-review reviewed Security review of #10681 ( What the change does. The outer Checked and not a finding.
critical-findings: 0 Finding (SUGGESTION): a standing guard now rests on a premise that no longer holds for one invocation. Spend: none that bills. The change only makes local and lane pre-push self-checks run more test suites. |
Post-merge mutation testingKilled 1, survived 0, timeout 0, no coverage 0, invalid 0, not run 7.
The run's summary has the full report. |
No related issue: a peer found it while checking #10675 before its push; no issue was filed.
Summary
scripts/check-affected.sh --uncommitted --testsnever ran the selected suites. It re-runs itself on a scratch snapshot of the work tree withCHECK_DATA_ROOTnaming that snapshot, so the inner run treats the snapshot as a foreign tree and skips tests withSKIP tests (runs the checked tree's own test code). That skip is the pre-push hook's guard for a merge-base copy. Here it hid two failing digest pins inplugins/planning/tests/interview-defenses.test.shover an uncommittedplugins/planning/skills/interview/SKILL.mdedit. Test selection was fine:scripts/affected-tests.shalready reads the work tree and picks that suite.Fix
--uncommittedrun passesCHECK_AFFECTED_SNAPSHOT_OF=<its own tooling tree>, but only when it is checking its own tree. The inner run clears the tests skip only when that value equals its own tooling tree.affected()step a committed run uses.affected-tests.shreads the work tree's uncommitted and untracked edits there, the same content as the snapshot, and the scratch snapshot has nonode_modules.setup-env.sh --nodenow gets--root "$tools", so it never installs into the scratch snapshot.Verification
bash scripts/check-affected.test.sh:PASS=79 FAIL=0. The new case--uncommitted --tests runs the suites over the uncommitted changefailed before the fix withSKIP tests (runs the checked tree's own test code)(PASS=78 FAIL=1).--uncommitted --testsprintedSKIP tests (runs the checked tree's own test code). After the fix it printedFAIL tests (exit 1), from the Stance section's digest pin ininterview-defenses.test.sh.bash scripts/check-affected.sh --testson the commit:11 passed, 5 skipped. Tests is a SKIP only because 2 selected Node suites are left to their CI lane.validate-plugins, because no plugin is touched. Plain--uncommittedwithout--testsstill runs no suite, as before.Related
N/A
🤖 Generated with Claude Code