fork-ci: fail clippy only on files the PR changes

[thedancingdeveloper]

Summary

• `cargo clippy -p -- -D warnings` fails a PR on upstream lint debt in files it never touched. hooks: per-handler failClosed and PreToolUse on write_stdin #6 changed `codex-app-server` and was failed for a `let_and_return` in `message_processor.rs:372` that is already on `fork/main`. A fork patch must not have to fix upstream debt.
• Clippy now runs without `-D` and emits `--message-format=json`. The step fails only when a warning or error has its primary span in a file the PR changed (paths compared `codex-rs/`-relative), and it prints every matching diagnostic. Compile errors stay fatal everywhere, which the `cargo check --workspace` step already enforces.
• This is smaller than carrying a one-hunk lint fix on `fork/main`: that fix would itself be a carried patch to rebase forever, and the next upstream lint would need another.

Test plan

• Self-test branch `fork/ci-lint` adds one `let_and_return` in `codex-rs/cli/src/main.rs`, a file the branch changes, so its `fork-ci` run must fail and name that file.
• hooks: per-handler failClosed and PreToolUse on write_stdin #6's run against this workflow must pass: its clippy finding is in a file the PR does not change.

[thedancingdeveloper]

Review: blocker (head 83364de). The logic is fine; the branch base isn't.

1. The branch is based on the pre-merge fork/ci history. git merge-base with fork/main is c138238, and the PR carries 472cc14, 8e7029c and 27bc640 again, which #4 already landed as a77b522, 1aa8ff2 and 08f5389. Rebase onto origin/fork/main so the PR is one commit: the clippy-step change and the files output. git diff origin/fork/main..HEAD should then show only fork-ci.yml (+ the script if it changed).

2. The requested self-test is missing. Link two runs:

   • (a) a probe branch that adds one clippy lint in a changed file, and fork-ci fails on it;
   • (b) #6 rebased without 72e7308, and fork-ci passes despite the upstream let_and_return in message_processor.rs:371.

   Those two runs are the test of this file.

Logic checked:

• JSON clippy with no -D;
• only diagnostics whose primary span is in a changed file fail the job;
• paths are stripped of codex-rs/ to match clippy's workspace-relative file_name;
• || true on clippy is fine, because compile errors are already fatal in cargo check.

Non-blocking: errors from upstream deny lints in unchanged files are now ignored too. That's correct, but say so in the comment.

[thedancingdeveloper]

Review: no blockers in the code (head 5f9d3ef)

It's now one commit on fork/main (c8a5628) that touches only fork-ci.yml (+25/-1), so blocker 1 is resolved. The logic is as reviewed before.

Merge conditions (blocker 2, the self-test):

• (a) the fork/ci-lint probe run exists and fails, naming codex-rs/cli/src/main.rs. No fork-ci run exists for that branch yet;
• (b) this PR's own run 37937522191 is green.

After merge, #6 is rebased without 72e7308, and its green run is the second half of the test.

[thedancingdeveloper]

Epic lead, on the self-test: probe run 37937788126 failed at cargo check (E0308 in cli/src/main.rs:4970: fn _fork_ci_lint_probe() returns value with no return type). It never reached the clippy step, so it doesn't show the filter firing. Fix the probe to compile, e.g. fn _fork_ci_lint_probe() -> i32 { let value = 1; value }, which is a pure let_and_return lint. Re-run it and show it fails at the clippy step, naming main.rs. This PR's own run 37937522191 is green. Once the probe is correct, #11 can merge.

[thedancingdeveloper]

Review: blocker, the gate never fires (head 5f9d3ef).

The corrected probe (run 37940613906, probe head 16f7723, fn _fork_ci_lint_probe() -> i32 { let value = 1; value }) got through cargo check and the clippy step, then failed in cargo test. So the clippy gate passed a real let_and_return in a changed file.

Root cause, reproduced in scratch (clippy 1.95 on a one-file crate, jq 1.8.1):

• Clippy does emit warning clippy::let_and_return src/main.rs:6 in the JSON stream.
• --slurpfile changed <(jq -R . /tmp/changed-files) makes $changed an array of strings, so [$changed[][]] tries to iterate a string: jq: error: Cannot iterate over string ("src/main.rs").
• jq exits 0 after that runtime error, so offenders is empty and the step passes. Every PR passes this gate regardless of its lints.

Fix (verified both ways in scratch: the changed file is reported, an unchanged file isn't):

select(any($changed[]; . == $s.file_name))

And make a jq failure fail the step. jq exited 0 here, so capture its stderr and fail if it's non-empty, or run it as its own command before the offenders= assignment and check that.

Then re-run the probe: it must fail at the cargo clippy step, naming cli/src/main.rs:4970 and let_and_return.

Separate finding, not this PR's logic: cargo test -p codex-cli failed on packaged_daemon_bootstrap_seeds_local_package in the probe run. Check whether it also fails on a fork/main baseline dispatch. If it's environment-dependent upstream behaviour (the daemon/package bootstrap on a hosted runner), skip that one test in fork-ci with --skip, and say why in a comment. Otherwise every PR touching codex-cli will be red.

[thedancingdeveloper]

Review (head 8c64e9b): the filter fix is right; one small blocker; the probe still needs a run.

Resolved: select(any($changed[]; . == $s.file_name)), the same filter I verified in scratch. The codex-cli skip of packaged_daemon_bootstrap_seeds_local_package (upstream asserts autoUpdateEnabled == false while default_auto_update_enabled() is true) is acceptable with that comment. Non-blocking: link the fork/main baseline run that shows it failing unmodified.

1. Blocker: 2> >(tee /tmp/jq-err >&2) is asynchronous. The process substitution can still be writing when [[ -s /tmp/jq-err ]] runs, so a jq error can again pass unnoticed, the same fail-open class as before. Use a synchronous redirect, 2>/tmp/jq-err, then cat /tmp/jq-err >&2 and fail if it's non-empty. Also clear /tmp/jq-err at the top of each loop iteration (or use one file per crate).
2. Merge condition: rebase the fork/ci-lint probe onto this head and re-run. It must fail at the cargo clippy step, printing cli/src/main.rs:4970 and the let_and_return message. The last probe run (37940613906, 16f7723) used the broken filter.

[thedancingdeveloper]

Review: no blockers (head 285c950)

Both merge conditions are verified:

• (a) The gate fires on a real lint. Probe run 37944444440 (fork/ci-lint at ed1bca1, descended from 285c950) fails at the cargo clippy step with clippy findings in files this PR changes and cli/src/main.rs:4970:5: warning: returning the result of a \let` binding from a block (let_and_return). It's listed twice because clippy checks the bin and test targets. cargo check` passed first, so this is a lint, not a compile error.
• (b) This PR's own run 37944232480 on 285c950 is green.

Code:

• select(any($changed[]; . == $s.file_name));
• a synchronous 2>/tmp/jq-err, cleared per iteration, where any jq stderr fails the step;
• clippy without -D, judged only on files the PR changed;
• compile errors stay fatal via cargo check --workspace;
• the commented skip for the upstream-drifted packaged_daemon_bootstrap_seeds_local_package.

After merge, re-run fork-ci on #10 and #12 and rebase #6 (without 72e7308). Delete the fork/ci-lint probe branch; its job is done.