Repository navigation
ci(selector): flag process-signal code as Windows-sensitive - #7503
Conversation
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. |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
|
✅ Deterministic PR hygiene checks passed. |
d8c4665 to
c31b08f
Compare
|
Merged with owner authority per maintainer decision 2026-10-21 (jun): the project owner authorized owner-path ( Exact-head verification on |
|
Windows verdict detail: |
Summary
The Windows-sensitive diff selector (
scripts/ci/windows-sensitive-diff.sh) missed process-signal code. #7484 added a test asserting POSIX signal semantics (SIGKILLafterSIGTERM). On Windows,kill("SIGTERM")is TerminateProcess, so the test failed ondevlane=all(windows 4/9, run 39693529639). The selector never flagged #7484, so nobody dispatchedlane=allbefore merge. #7502 fixes the test itself.This adds three fixed
grep -Fcontent markers:sigterm,sigkillandprocess.kill. They use the selector's existing lowercase matching. A diff that adds process-signal code now reportswindows-sensitive: true, and that shows the existing notice: dispatchlane=allbefore merge.Security note: this is a notice-only change to a CI selector script. CI policy is unchanged: PR CI still runs no Windows jobs (AGENTS.md owner CI policy), and no workflow permissions, triggers or job conditions change.
Verification
bun test tests/ci-workflows/: 1637 pass, 10 skip, 0 fail (local, macOS).tests/ci-workflows/ci-review-lanes.test.tsshow that a diff addingchild.kill("SIGTERM"), a"SIGKILL"assertion orprocess.kill(pid, 0)yieldstrue. The existing negative scenarios still yieldfalse.scripts/ci, so it is Windows-sensitive itself. Aci.yml lane=alldispatch on the exact head is required before merge.Checklist
devSummary by CodeRabbit
process.killcalls.