ci: clippy the platform-gated code no gate ever compiled by pawellisowski · Pull Request #469 · aware-aeco/aware

pawellisowski · 2026-08-28T01:57:09Z

Summary

ci.yml's gates job runs on ubuntu-latest, and clippy only ever sees the code the active cfg set compiles. That left 262 lines of shipped Rust behind #[cfg(windows)] / #[cfg(target_os = "macos")] checked by nothing at all — 182 of them in commands/model_reader_host.rs, the v0.131.0 managed Windows host, plus manifest/loader.rs, runtime/pidfile.rs, runtime/invoker.rs, render/blender.rs and commands/app.rs.
Nothing else reached them either. release.yml and bridge-windows-packaged do build the crate on Windows, but with cargo build, and rustc accepts a clippy:: tool lint and then ignores it. So all three of CLAUDE.md §Code style's mechanised rules applied to none of that code: cargo clippy -D warnings, the crate-root deny(clippy::unwrap_used, clippy::expect_used) in src/main.rs, and undocumented_unsafe_blocks in Cargo.toml.
Adds two gates — a check-only cargo clippy --target x86_64-pc-windows-gnu step in gates, and a gates-macos job on a real macos-latest runner — plus cli/tests/windows_target_gate.rs so neither can be quietly removed or narrowed later.

The finding, measured

Not inferred. An unwrap() was planted in the #[cfg(windows)] process_is_alive in src/runtime/pidfile.rs and both commands run against it:

command	result
`cargo clippy --all-targets --locked -- -D warnings` (what CI ran before this PR)	0 errors
the same, with `--target x86_64-pc-windows-gnu`	error: used `unwrap()` on a `Result` value

The plant was reverted; the branch contains no source change. The code behind those cfgs is clean today — the defect is that no gate would have told anyone otherwise.

Why a cross-check for Windows but a real runner for macOS

Both limits were measured, not assumed.

Windows — cross-checking x86_64-pc-windows-msvc from a Linux runner is not available: ring and zstd-sys compile C for the target and their build scripts refuse an msvc target under GNU cc ("GNU compiler is not supported for this target"). The -gnu triple needs only mingw, which the step installs. The two triples agree on windows, target_os and target_family and differ only under target_env — so -gnu is an exact proxy exactly as long as nothing in the crate branches on target_env, which no_target_env_cfg_leaves_the_gnu_proxy_faithful now forbids.
macOS — there is no proxy triple, because what is missing is a toolchain rather than a cfg. --target aarch64-apple-darwin dies inside zstd-sys's build script (cc: error: unrecognized command-line option '-arch') before reaching this crate at all. So macOS coverage has to be a macOS runner. release.yml already builds osx-arm64 there, but with cargo build — the same reason the Windows cross-check exists despite release.yml building for Windows.

Negative controls

Per the repo's existing gate-test convention (lockfile_gate.rs, agent_python_suites_gate.rs), every scan over artefacts that are correct today is paired with a classifier driven over synthetic input, so a scanner that has stopped matching anything fails loudly instead of reporting clean. Each was mutation-tested: retargeting the macOS job to ubuntu-latest fails it, dropping -D warnings fails it, a planted wrapped predicate fails the scanner, and a target_env branch planted in build.rs fails it by name.

The gate was vacuous twice over on first submission, and that is worth stating plainly — it is the exact failure mode the file exists to prevent. The job body kept its comment lines, and the comment block above the macOS step names every flag asserted over it; the step's own name: also reads cargo clippy -D warnings (macos). So the flag assertions were satisfied by prose, and deleting -D warnings from the real step left the test green. Flags are now read from run: commands only, and the_job_reader_matches_its_contract plants exactly that deletion.

Type of change

Other (specify): CI guardrail — closes a hole in an existing gate

Decalog check

This change respects all five decalog truths.

Quality bar

Not an agent, skill or app. On the pinned 1.95.0 toolchain, from cli/:

cargo fmt --all -- --check                                        pass
cargo clippy --all-targets --locked -- -D warnings                pass
cargo clippy --target x86_64-pc-windows-gnu --all-targets ...     pass
cargo test --locked                                               pass (53 binaries, 0 failed)

Plus the rest of the repo's own gates on this tree: the lockfile pair, no-hardcoded-string-offsets.py, sync_stats.py --check, run-agent-python-tests.py, no-claude-coauthor-trailers.py, and the steel-detailer-lookup crate's fmt/clippy/test. All 14 CI checks are green on 364ea3b1, clippy (macos) included — that job could not be run locally, so CI was its first real execution.

Notes for reviewers — two things left open, both deliberate

1. One Codex P2 is outstanding on the head, unfixed. "Cover Windows-gated build-script branches." The mechanism is real: a build script is compiled for the host, so a #[cfg(windows)] in cli/build.rs would be activated by the Windows release build and compiled by neither ubuntu clippy run, the -gnu cross-check included. But cli/build.rs contains no cfg at all today — it is one println! — so nothing is currently unlinted and this PR makes nothing worse. Codex's own suggested remedy is "lint the package on a Windows runner", which is point 2. The narrower alternative needs a judgement call about which of #[cfg(windows)], cfg!(windows), CARGO_CFG_TARGET_* and TARGET/HOST env reads should fail a build — a maintainer's design decision, not a sweeper's. Three earlier findings were fixed and their threads resolved (macOS coverage and multiline cfg scanning in e460a664, build.rs in the target_env inventory in 364ea3b1); this run stopped at its two-round limit because the findings were widening rather than converging.

2. Three #[cfg(windows)] tests still run nowhere, because no job runs cargo test on Windows:

tests/app_id_is_a_segment.rs::a_drive_relative_id_is_refused_as_well — a path-traversal regression test (C:evil carries no separator, and Path::join discards the base when the appended path has a prefix)
src/commands/model_reader_host.rs::provider_is_created_suspended_until_its_job_is_attached
the windows arm of the pidfile probe

This PR makes clippy compile all three; it does not make anything run them. Closing that means a windows-latest job, and the 53-binary suite could not be executed here to know whether it is green — pushing it unverified is the speculative red CI the engineering rules warn against. A windows-latest clippy job would also settle point 1, and is the smaller of the two.

Cost: roughly one to two minutes on gates (a second dependency graph checked, not built, nothing linked or run) plus a short macos-latest job.

`ci.yml`'s `gates` job runs on ubuntu-latest, and clippy only sees the code the active cfg set compiles. That left 262 lines of shipped Rust behind `#[cfg(windows)]` / `#[cfg(target_os = "macos")]` — 182 of them in `commands/model_reader_host.rs`, the v0.131.0 managed Windows host — checked by nothing at all. `release.yml` and `bridge-windows-packaged` do build the crate on Windows, but with `cargo build`, and rustc accepts a `clippy::` tool lint and then ignores it. So all three of CLAUDE.md §Code style's mechanised rules reached none of that code: `cargo clippy -D warnings`, the crate-root `deny(clippy::unwrap_used, clippy::expect_used)`, and the manifest's `undocumented_unsafe_blocks` deny. Measured rather than assumed: an `unwrap()` planted in the `#[cfg(windows)]` `process_is_alive` in `src/runtime/pidfile.rs` draws zero errors from the existing step and is rejected by the new one. Adds a check-only `cargo clippy --target x86_64-pc-windows-gnu` step to the same job, and `cli/tests/windows_target_gate.rs` to keep it honest: it asserts the step still carries `--target`, `--all-targets`, `--locked` and `-D warnings`, that the triple checked is the triple the toolchain step installs, that the host step was not merely retargeted, and that no `target_env` cfg exists — the condition that makes the gnu triple a faithful proxy for the shipped msvc one. Both classifiers carry negative controls over synthetic input.

pawellisowski · 2026-08-28T01:57:14Z

@codex review

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d7aa1bb07

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex review of 6d7aa1b, both findings P2, both real. 1. The windows cross-check closed only the windows half. A windows target compiles no `#[cfg(target_os = "macos")]` arm, so `commands/app.rs`'s macOS arm and `model_reader_host.rs`'s `not(target_os = "linux")` arm were still checked by nothing. A darwin cross-check from the ubuntu runner is not available, measured the same way `-msvc` was: `cargo clippy --target aarch64-apple-darwin` dies in `zstd-sys`'s build script with `cc: error: unrecognized command-line option '-arch'` — the runner's cc is not an Apple cross-compiler. Unlike msvc, darwin has no proxy triple, because what is missing is a toolchain and not a `target_env`. So macOS coverage is a `macos-latest` job, check-only, which is what this adds. 2. `target_env_branches` required `cfg` and `target_env` on one line, so a rustfmt-wrapped `all(...)` predicate — the ordinary way one arrives — read clean. It now accumulates complete predicates across lines, with quote-aware comment stripping and a runaway cap, and the negative control drives it over a wrapped fixture, a wrapped commented-out one, and a line carrying both a string and a trailing comment. Two flaws found while proving the new gate is not vacuous, and fixed: a job body kept its comment block, and a step's `name:` reads `cargo clippy -D warnings (macos)` — so the flag assertions were twice satisfied by prose, and deleting `-D warnings` from the real step left this file green. Flags are now read from `run:` commands only, and the negative control plants exactly that deletion. Also splits the empty-set guard in two: windows-gated code justifies the windows step, macOS-gated code the macOS job, so neither platform's lines excuse the other's missing gate. Verified on the pinned 1.95.0 toolchain: fmt --check, clippy host, clippy --target x86_64-pc-windows-gnu, and cargo test --locked (53 binaries, all ok). Mutations confirmed each new assertion fails when its gate is removed.

pawellisowski · 2026-08-28T06:01:37Z

Codex round 1 — both P2 findings fixed in `e460a664`

P2 — "Add clippy coverage for the macOS-only branches." Accepted; it was right that this closed only the windows half.

A darwin cross-check from the ubuntu runner is not available, and this is measured rather than assumed, the same way -msvc was:

$ cargo clippy --target aarch64-apple-darwin --all-targets --locked -- -D warnings
error occurred in cc-rs: command did not execute successfully ...
  cargo:warning=cc: error: unrecognized command-line option '-arch'
  cargo:warning=cc: error: unrecognized command-line option '-mmacosx-version-min=11.0'
  (zstd-sys build script; `ring` compiles C for the target too)

The runner's cc is not an Apple cross-compiler and has no SDK, so the run never reaches this crate's own code. Unlike msvc — whose stand-in is -gnu because the two triples differ only under target_env — darwin has no proxy triple, since what is missing is a toolchain rather than a cfg. So macOS coverage has to be a macOS runner.

Added a gates-macos job on macos-latest: clippy only, check-only, no --target, so the host cfg set is the macOS one. release.yml already builds this crate on macos-latest, but with cargo build, which accepts a clippy:: tool lint and ignores it — exactly the reason the windows cross-check exists despite release.yml building for Windows. It needs none of the apt deps: libsecret and libdbus are the Linux credential backend.

ci_still_lints_the_macos_cfg_set_on_a_macos_runner is its tripwire, and the empty-set guard is now split in two — windows-gated code justifies the windows step, macOS-gated code the macOS job — so neither platform's lines excuse the other's missing gate. That was the specific conflation flagged.

P2 — "Scan complete cfg attributes for target_env." Accepted, and reproduced: a rustfmt-wrapped all(...) predicate planted in src/runtime/pidfile.rs read clean under the old per-line scan. target_env_branches now accumulates complete predicates across lines (quote-aware comment stripping, paren balancing that ignores string literals, a 32-line runaway cap so a stray paren cannot swallow the file). The negative control gained a wrapped fixture, a wrapped commented-out one, a line carrying both a string and a trailing comment, and an unbalanced predicate. With the plant restored the gate fails and names it:

cli/src/runtime/pidfile.rs:395: #[cfg(all( windows, target_env = "msvc", feature = "nothing" ))]

Two flaws in the new gate, found while proving it is not vacuous

Worth calling out because the first one is the failure mode this whole file exists to prevent, and it bit here:

The job body kept its comment lines, and the comment block above the macOS step names every flag asserted over it.
The step's own name: reads cargo clippy -D warnings (macos).

So the flag assertions were satisfied twice over by prose, and deleting -D warnings from the real step left this file green. Flags are now read from run: commands only, and the_job_reader_matches_its_contract plants exactly that deletion.

Verification

On the pinned 1.95.0 toolchain, from cli/:

check	result
`cargo fmt --all -- --check`	pass
`cargo clippy --all-targets --locked -- -D warnings`	pass
`cargo clippy --target x86_64-pc-windows-gnu --all-targets --locked -- -D warnings`	pass
`cargo test --locked`	pass (53 binaries)

Plus sync_stats.py --check, no-hardcoded-string-offsets.py --self-test and the trailers gate over this branch. Each new assertion was mutation-tested: retargeting the macOS job to ubuntu-latest fails it, dropping -D warnings fails it, and the planted wrapped predicate fails the scanner.

The macOS job itself is the one thing not runnable here — no macOS host — so CI on this PR is its first real execution.

Generated by Claude Code

pawellisowski · 2026-08-28T06:01:40Z

@codex review

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e460a664d7

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Codex review of e460a66, P2, and real. `crate_sources` walked `src/` and `tests/` only, so `cli/build.rs` sat outside the scan that asserts nothing in the crate branches on `target_env` — the condition that makes the gnu triple a faithful proxy for the shipped msvc one. That is the worst file to miss, not an edge case. A build script is compiled for the HOST and never for `--target`, so a `#[cfg(target_env = "msvc")]` in it is invisible to both ubuntu clippy runs, host and windows cross-check alike, while a Windows release build — whose host IS msvc — compiles the unlinted arm. The invariant this file exists to enforce was breakable in the one file most likely to break it. The walk now covers crate-root Rust targets (non-recursively, so `target/` build output stays out) and `benches/`/`examples/` should either ever appear. Since the inventory is itself a scan over sources that are clean today, it gets a tripwire naming `build.rs`, so a walk that quietly stops reaching it fails instead of reporting clean. Verified on the pinned 1.95.0 toolchain: fmt --check, clippy host, clippy --target x86_64-pc-windows-gnu, cargo test --locked (53 binaries, all ok). Mutation-tested: a wrapped msvc predicate planted in build.rs fails the scan and is named at build.rs:16; it passed before this change.

pawellisowski · 2026-08-28T06:12:48Z

Codex round 2 — the one new P2 fixed in `364ea3b1`

P2 — "Include build.rs in the target_env scan." Accepted, and it is the sharpest of the three so far.

crate_sources walked src/ and tests/ only, so cli/build.rs sat outside the scan that asserts nothing in the crate branches on target_env — the exact condition that makes the gnu triple a faithful proxy for the shipped msvc one.

The reasoning about why it hides there is right and worth restating: a build script is compiled for the host and never for --target, so a #[cfg(target_env = "msvc")] in it is invisible to both ubuntu clippy runs — host check and windows cross-check alike — while a Windows release build, whose host is msvc, compiles the unlinted arm. The invariant this file exists to enforce was breakable in the one file most likely to break it.

The walk now covers crate-root Rust targets, non-recursively so target/ build output stays out, plus benches/ and examples/ should either ever appear. And because the inventory is itself a scan over sources that are clean today, it now carries a tripwire naming build.rs: a walk that quietly stops reaching it fails instead of reporting clean.

Mutation-tested both directions. With a wrapped msvc predicate appended to the real cli/build.rs:

no_target_env_cfg_leaves_the_gnu_proxy_faithful ... FAILED
/home/user/aware/cli/build.rs:16: #[cfg(all( windows, target_env = "msvc" ))]

That plant passed silently before this commit — both because the file was unscanned and because the predicate is rustfmt-wrapped, so it needed round 1's multiline fix as well.

Round 1 findings

Both fixed in e460a664 and unchanged since; details in the comment above. clippy (macos) has now had its first real execution on this PR and passed in about two minutes, which was the one thing I could not verify locally.

Verification

Full set re-run on the pinned 1.95.0 toolchain, from cli/:

check	result
`cargo fmt --all -- --check`	pass
`cargo clippy --all-targets --locked -- -D warnings`	pass
`cargo clippy --target x86_64-pc-windows-gnu --all-targets --locked -- -D warnings`	pass
`cargo test --locked`	pass (53 binaries, 0 failed)

Generated by Claude Code

pawellisowski · 2026-08-28T06:12:51Z

@codex review

Generated by Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 364ea3b114

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-08-28T06:18:12Z

+        let Ok(source) = std::fs::read_to_string(&path) else {
+            continue;
+        };
+        for (line, text) in target_env_branches(&source) {


Cover Windows-gated build-script branches

Even though build.rs is now included in the inventory, this loop only reports predicates containing target_env. If the build script adds an ordinary #[cfg(windows)] or cfg!(windows) branch, the Windows release activates it because the build script runs on a Windows host, while both Ubuntu clippy invocations compile the build script for their Linux host—even when the package target is Windows GNU—and this test remains green. Scan build.rs for all Windows-host predicates (and target information read from Cargo environment variables), or lint the package on a Windows runner.

Useful? React with 👍 / 👎.

pawellisowski · 2026-08-28T06:22:43Z

Round 3 — stopping here, not merging. One P2 open for a human.

Codex reviewed 364ea3b1, the current head, and raised a new P2: "Cover Windows-gated build-script branches." I am leaving it unfixed and the PR open. Two reasons, and the second is the one that matters.

The sweeper gets two rounds per run, and this is the third. The rule exists for exactly this shape: each fix has drawn a new, adjacent finding rather than converging. Round 1 asked for macOS coverage and a multiline cfg scan; round 2, once build.rs was in the inventory, asked for target_env in build.rs; round 3, now that build.rs is scanned, asks for windows predicates in build.rs too. A PR on its third lap needs a human more than another automated round.

The finding is sound but is a hardening ask, not a defect in this diff. Codex is right about the mechanism — a build script is compiled for the host, so a #[cfg(windows)] in cli/build.rs would be activated by the Windows release build and compiled by neither ubuntu clippy run, --target x86_64-pc-windows-gnu included. But cli/build.rs contains no cfg at all today; it is one println! for a rerun-if-env-changed. So nothing is currently unlinted, and this PR does not make anything worse. It is a guard against a branch nobody has written.

It also reaches past where the author drew the line. Codex's own suggested remedy is "lint the package on a Windows runner" — and the PR body already declines to add a Windows job, deliberately, because the full suite could not be executed to know whether it is green. The narrower alternative, scanning build.rs for every windows-host predicate, needs a judgement call about which of #[cfg(windows)], cfg!(windows), CARGO_CFG_TARGET_* and TARGET/HOST env reads should fail a build, and that is a design decision for the maintainer rather than something a sweeper should settle unattended.

State as it stands


Head	`364ea3b1`
Codex	reviewed this exact commit; one P2 outstanding, quoted above
CI	all 14 checks green on this commit, `clippy (macos)` included
Rounds used	2 of 2

Three earlier findings are fixed and their threads resolved: macOS clippy coverage and the multiline cfg scan in e460a664, build.rs in the target_env inventory in 364ea3b1. Each was mutation-tested, and one of them turned up a real flaw in the new gate itself — the flag assertions were being satisfied by the step's own name: and by the comment block above it, so deleting -D warnings from the real step left the test green.

Ready for a maintainer to either accept the residual build.rs risk and merge, or say how far the build-script scan should reach.

Generated by Claude Code

chatgpt-codex-connector Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread .github/workflows/ci.yml

Comment thread cli/tests/windows_target_gate.rs Outdated

chatgpt-codex-connector Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread cli/tests/windows_target_gate.rs

chatgpt-codex-connector Bot reviewed Aug 28, 2026

View reviewed changes

pawellisowski mentioned this pull request Aug 28, 2026

Routine log — cloud maintenance routines #342

Open

fix(ci): guard Windows-specific build scripts

76f8eb8

pawellisowski merged commit ff396db into main Aug 28, 2026
14 checks passed

pawellisowski deleted the routine/guardrails-2026-08-28 branch August 28, 2026 13:13

Navigation Menu

Uh oh!

ci: clippy the platform-gated code no gate ever compiled - #469

ci: clippy the platform-gated code no gate ever compiled#469
pawellisowski merged 4 commits into
mainfrom
routine/guardrails-2026-08-28

pawellisowski commented Aug 28, 2026 •

edited

Loading

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 28, 2026

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

pawellisowski commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The finding, measured

Why a cross-check for Windows but a real runner for macOS

Negative controls

Type of change

Decalog check

Quality bar

Notes for reviewers — two things left open, both deliberate

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

pawellisowski commented Aug 28, 2026

Codex round 1 — both P2 findings fixed in e460a664

Two flaws in the new gate, found while proving it is not vacuous

Verification

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

pawellisowski commented Aug 28, 2026

Codex round 2 — the one new P2 fixed in 364ea3b1

Round 1 findings

Verification

Uh oh!

pawellisowski commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

pawellisowski commented Aug 28, 2026

Round 3 — stopping here, not merging. One P2 open for a human.

State as it stands

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

pawellisowski commented Aug 28, 2026 •

edited

Loading

Codex round 1 — both P2 findings fixed in `e460a664`

Codex round 2 — the one new P2 fixed in `364ea3b1`