From 5f9d3efe8b3f75a3dbd687aea928e07596874596 Mon Sep 17 00:00:00 2001 From: theDancingDeveloper Date: Fri, 9 Oct 2026 13:30:20 +0000 Subject: [PATCH 1/3] Fail clippy only on diagnostics in files the PR changes cargo clippy -p -- -D warnings fails a PR on upstream lint debt in files it never touched: a change to codex-app-server flagged a redundant binding in message_processor.rs that is already on fork/main, and a fork patch must not have to fix upstream debt. Clippy now runs without -D and emits JSON. The step fails only when a warning or error has its primary span in a file the PR changed, and it prints each matching diagnostic. Compile errors stay fatal everywhere, which the workspace cargo check already enforces. --- .github/workflows/fork-ci.yml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/fork-ci.yml b/.github/workflows/fork-ci.yml index 693d41cfcf35..25dbef2dcf0a 100644 --- a/.github/workflows/fork-ci.yml +++ b/.github/workflows/fork-ci.yml @@ -65,6 +65,9 @@ jobs: echo "list<> "$GITHUB_OUTPUT" # v8's build.rs otherwise downloads librusty_v8 from denoland, which does @@ -92,13 +95,34 @@ jobs: cargo fmt -p "$crate" -- --check done <<< "${{ steps.crates.outputs.list }}" + # -D warnings would fail the PR on upstream lint debt in files it never + # touched, and a fork patch must not have to fix that. Clippy runs without + # it, and only a warning or error whose primary span is in a file this PR + # changed fails the build. Compile errors are fatal regardless, which + # `cargo check` above already enforces. - name: cargo clippy shell: bash run: | set -euo pipefail + printf '%s\n' "${{ steps.crates.outputs.files }}" | sed 's|^codex-rs/||' > /tmp/changed-files while IFS= read -r crate; do [[ -z "$crate" ]] && continue - cargo clippy -p "$crate" --all-targets -- -D warnings + cargo clippy -p "$crate" --all-targets --message-format=json -- -W clippy::all \ + > "/tmp/clippy-${crate}.json" || true + offenders="$(jq -r --slurpfile changed <(jq -R . /tmp/changed-files) ' + select(.reason == "compiler-message") + | .message + | select(.level == "warning" or .level == "error") + | . as $m + | ($m.spans[] | select(.is_primary)) as $s + | select([$changed[][]] | index($s.file_name)) + | "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" + ' "/tmp/clippy-${crate}.json")" + if [[ -n "$offenders" ]]; then + echo "$offenders" + echo "clippy findings in files this PR changes" >&2 + exit 1 + fi done <<< "${{ steps.crates.outputs.list }}" - name: cargo test From 8c64e9b24f4c919317309ca3d3d0822c07dacaf0 Mon Sep 17 00:00:00 2001 From: theDancingDeveloper Date: Fri, 9 Oct 2026 14:24:25 +0000 Subject: [PATCH 2/3] Make the clippy gate actually fail, and skip an upstream-failing test --slurpfile yields an array of strings, so `[$changed[][]]` was a jq runtime error ("Cannot iterate over string"). jq exits 0 on a runtime error, so the filter matched nothing and every PR passed the clippy step. Compare the file names with `any($changed[]; . == $s.file_name)`, and fail the step on any jq stderr so a broken filter cannot pass silently again. cargo test -p codex-cli fails packaged_daemon_bootstrap_seeds_local_package on unmodified fork/main: it asserts autoUpdateEnabled == false while default_auto_update_enabled() returns true. Skip it with that reason rather than carry a fix for upstream drift. --- .github/workflows/fork-ci.yml | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/workflows/fork-ci.yml b/.github/workflows/fork-ci.yml index 25dbef2dcf0a..606d83a5b397 100644 --- a/.github/workflows/fork-ci.yml +++ b/.github/workflows/fork-ci.yml @@ -109,15 +109,23 @@ jobs: [[ -z "$crate" ]] && continue cargo clippy -p "$crate" --all-targets --message-format=json -- -W clippy::all \ > "/tmp/clippy-${crate}.json" || true + # --slurpfile yields an array of strings. Iterating each element again + # (`[$changed[][]]`) is a jq runtime error, and jq exits 0 on those, + # so the gate would pass every PR. `any` compares strings directly, + # and jq's stderr is surfaced so a filter error fails the step. offenders="$(jq -r --slurpfile changed <(jq -R . /tmp/changed-files) ' select(.reason == "compiler-message") | .message | select(.level == "warning" or .level == "error") | . as $m | ($m.spans[] | select(.is_primary)) as $s - | select([$changed[][]] | index($s.file_name)) + | select(any($changed[]; . == $s.file_name)) | "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" - ' "/tmp/clippy-${crate}.json")" + ' "/tmp/clippy-${crate}.json" 2> >(tee /tmp/jq-err >&2))" + if [[ -s /tmp/jq-err ]]; then + echo "jq failed to read the clippy output" >&2 + exit 1 + fi if [[ -n "$offenders" ]]; then echo "$offenders" echo "clippy findings in files this PR changes" >&2 @@ -131,5 +139,13 @@ jobs: set -euo pipefail while IFS= read -r crate; do [[ -z "$crate" ]] && continue - cargo test -p "$crate" + # packaged_daemon_bootstrap_seeds_local_package asserts + # autoUpdateEnabled == false, but default_auto_update_enabled() + # returns true in this tree. It fails on unmodified fork/main, so + # it is upstream drift rather than anything a fork PR introduced. + extra=() + if [[ "$crate" == "codex-cli" ]]; then + extra+=(-- --skip packaged_daemon_bootstrap_seeds_local_package) + fi + cargo test -p "$crate" "${extra[@]}" done <<< "${{ steps.crates.outputs.list }}" From 285c9503845c2faa0638cf4384e06636bb008680 Mon Sep 17 00:00:00 2001 From: theDancingDeveloper Date: Fri, 9 Oct 2026 14:26:37 +0000 Subject: [PATCH 3/3] Capture jq stderr synchronously so a filter error cannot pass --- .github/workflows/fork-ci.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/fork-ci.yml b/.github/workflows/fork-ci.yml index 606d83a5b397..4eed6177a0dc 100644 --- a/.github/workflows/fork-ci.yml +++ b/.github/workflows/fork-ci.yml @@ -109,10 +109,13 @@ jobs: [[ -z "$crate" ]] && continue cargo clippy -p "$crate" --all-targets --message-format=json -- -W clippy::all \ > "/tmp/clippy-${crate}.json" || true - # --slurpfile yields an array of strings. Iterating each element again - # (`[$changed[][]]`) is a jq runtime error, and jq exits 0 on those, - # so the gate would pass every PR. `any` compares strings directly, - # and jq's stderr is surfaced so a filter error fails the step. + # --slurpfile yields an array of strings. Iterating each element + # again (`[$changed[][]]`) is a jq runtime error, and jq exits 0 on + # those, so the gate would pass every PR. `any` compares strings + # directly. stderr goes to a file synchronously: a process + # substitution can still be writing when the check runs, which + # would let a jq error pass again. + rm -f /tmp/jq-err offenders="$(jq -r --slurpfile changed <(jq -R . /tmp/changed-files) ' select(.reason == "compiler-message") | .message @@ -121,8 +124,9 @@ jobs: | ($m.spans[] | select(.is_primary)) as $s | select(any($changed[]; . == $s.file_name)) | "\($s.file_name):\($s.line_start):\($s.column_start): \($m.level): \($m.message)" - ' "/tmp/clippy-${crate}.json" 2> >(tee /tmp/jq-err >&2))" + ' "/tmp/clippy-${crate}.json" 2>/tmp/jq-err)" if [[ -s /tmp/jq-err ]]; then + cat /tmp/jq-err >&2 echo "jq failed to read the clippy output" >&2 exit 1 fi