refactor: enforce check_x → CheckStatus convention + toolchain supply-chain pin by brettdavies · Pull Request #17 · brettdavies/agentnative-cli

brettdavies · 2026-04-16T17:05:40Z

Summary

Two related hardening efforts bundled:

Convention enforcement — standardizes all source checks on a single pattern (check_x() returns CheckStatus;
run() is the sole CheckResult constructor, using self.id()/self.group()/self.layer()). Documents the
convention in CLAUDE.md, refactors all 16 Rust source checks to match, and adds a Rust-only integration test that
walks the source tree and fails CI if any check_x() still returns CheckResult.
Toolchain supply-chain pin — rust-toolchain.toml now pins to a specific X.Y.Z release with a trailing rustc
commit-SHA comment. Rustup verifies component SHA256s from the distribution manifest, so the pin is effectively a
SHA pin (the manifest is the toolchain's lockfile). Both local and CI read the same file and install identical
bits. Motivated by a CI-only clippy failure during this PR's own review: local clippy 1.94 passed a lint that CI
clippy 1.95 rejected.

Convention enforcement was identified during the PR #15 code review
— the 3 new Python checks were fixed in that PR, but the 16 existing Rust checks still used the old pattern. The
toolchain pin was added mid-review after the clippy divergence exposed that floating channel = "stable" leaves
local and CI on different rustc versions.

Changelog

Type of Change

refactor: Code refactoring (no functional changes)
chore: Maintenance tasks (supply-chain toolchain pin)
docs: Documentation update (CLAUDE.md conventions + toolchain policy)
test: Adding or updating tests (convention enforcement test)

Related Issues/Stories

Related PRs: feat(python): Python starter checks, fixture test coverage, and real-world validation #15 (review finding that identified the convention drift)
Follow-up: dot-github todo ci: bootstrap repo with workflows, rulesets, and project config #4 — determine the automated bump workflow for rust-toolchain.toml across repos
(central reusable workflow vs. local script vs. hybrid)

Testing

Unit tests added/updated
Integration tests added/updated
All tests passing

Test Summary:

Unit tests: 304 passing (unchanged count — refactor preserves behavior)
Integration tests: 39 passing (+1 new: convention_check_x_returns_check_status_not_check_result)
Clippy: clean with -Dwarnings on rustc 1.94.1 (pinned)

Files Modified

Modified:

CLAUDE.md — added Source Check Convention section (Tier 1) and Toolchain Pin section documenting the 7-day
quarantine policy
rust-toolchain.toml — pinned channel = "1.94.1" with trailing comment naming the rustc commit SHA and release
date; bumped from floating channel = "stable"
scripts/hooks/pre-push — removed rustup update stable step; pin + rustup's manifest verification now handle
toolchain integrity
16 files in src/checks/source/rust/ — check_x() returns CheckStatus, run() uses self.id()/self.group()/
self.layer() instead of duplicated string/enum literals
src/checks/source/rust/no_pager.rs — collapsed nested if inside a match arm into a match guard (fixes clippy
collapsible_match lint introduced in clippy 1.95)
tests/integration.rs — added convention_check_x_returns_check_status_not_check_result enforcement test

Key Details

Tier 1 (Document): CLAUDE.md now codifies the source check structure. Covers the Check trait impl shape, the
check_x() → CheckStatus separation, and the convention that run() is the sole CheckResult constructor. Also adds
a new Toolchain Pin section explaining the supply-chain pin policy.

Tier 2 (Refactor): All 16 Rust source checks aligned to the convention. Net -132 lines by removing duplicated
CheckResult construction (previously the id / label / group / layer fields were hardcoded in both run()
and every check_x() helper).

Tier 3 (Enforce): Integration test walks src/checks/source/ with std::fs (no shell dependencies), finds every
fn check_ signature (any visibility, single-line or multi-line), and fails if any return CheckResult instead of
CheckStatus. Uses CARGO_MANIFEST_DIR for a stable absolute path.

Toolchain pin format: channel = "1.94.1" # rustc e408947bfd200af42db322daf0fadfe7e26d3bd1, released 2026-03-25.
Comment mirrors the GitHub Actions SHA-pin pattern (action@<sha> # vN.N.N). Policy: bump only via reviewed PR after
the new stable has aged ≥7 days (supply-chain quarantine consistent with UV_EXCLUDE_NEWER / bun
minimumReleaseAge / npm_config_min_release_age in dotfiles).

Benefits

Eliminates ID string literal triplication (3 copies per check → 1 authoritative source in fn id())
New checks can't drift — the convention is documented AND enforced by CI
-132 lines of boilerplate removed
Local and CI toolchains are guaranteed identical — no more "local clippy older than CI clippy" false greens
Toolchain updates route through reviewed PRs with a 7-day quarantine, matching the broader brettdavies supply-chain
posture

Breaking Changes

No breaking changes

Deployment Notes

No special deployment steps required

Checklist

Code follows project conventions and style guidelines
Commit messages follow Conventional Commits
Self-review of code completed
Tests added/updated and passing
No new warnings or errors introduced
Changes are backward compatible

…checks Tier 1: Document the Source Check Convention in CLAUDE.md — check_x() returns CheckStatus (not CheckResult), run() is the sole CheckResult constructor using self.id()/self.group()/self.layer(). Tier 2: Apply the convention to all 16 Rust source checks. Each check_x() function now returns CheckStatus directly, eliminating ID/group/layer string literal triplication. Net -132 lines. Tier 3: Add convention_check_x_returns_check_status_not_check_result integration test that greps source check files for the old pattern and fails if any check_x() still returns CheckResult.

Rewrite convention_check_x_returns_check_status_not_check_result as pure Rust using std::fs. The previous implementation shelled out to rg which is not available in GitHub Actions runners, causing the test to panic with "No such file or directory" and blocking all CI runs. The new implementation: - Uses CARGO_MANIFEST_DIR for a stable absolute path (no CWD fragility) - Walks src/checks/source/ recursively with only std::fs - Catches all visibility modifiers (pub, pub(crate), pub(super), private) - Handles multi-line signatures by accumulating text until the opening { - Reports specific file:line:code references on failure Also corrects CLAUDE.md Source Check Convention claims: - check_x() signature is (source: &str) or (source: &str, file: &str) - "Every" → "Most" with noted exceptions for output_module.rs and error_types.rs which use different helper shapes

clippy 1.95 flags the nested `if` inside `CheckStatus::Pass =>` as collapsible. Use a match guard instead. Local toolchain was 1.94 so the lint didn't fire pre-push — ran into this as a CI-only failure.

Run `rustup update stable --no-self-update` before the other checks so local clippy/rustc match what CI will use. CI reinstalls stable on every run, so if the local toolchain is behind, a newer lint can fail CI while pre-push reports green — exactly what happened with the collapsible_match lint that landed in clippy 1.95. Fast no-op when already current; takes a few seconds on first push after a new stable release.

Pin rust-toolchain.toml to a specific release instead of floating `stable`. Rustup verifies component SHA256s from the distribution manifest — the version pin is effectively a SHA pin. Trailing comment documents the rustc commit SHA for audit, mirroring the GitHub Actions pin pattern (action@<sha> # vN.N.N). Both local and CI now read rust-toolchain.toml and install identical bits. Removes the pre-push rustup-update hook introduced earlier in this branch — pinning is the right fix, not runtime sync. Policy: bump the channel only via reviewed PR, after the new stable has aged ≥7 days. This matches the UV_EXCLUDE_NEWER / bun minimumReleaseAge / npm_config_min_release_age quarantine already in dotfiles, and guards against the `tj-actions/changed-files`-class supply-chain attack on the toolchain itself.

## Summary Pin `rust-toolchain.toml` to a specific `X.Y.Z` release with a trailing rustc commit-SHA comment, replacing the floating `channel = "stable"`. Rustup verifies component SHA256s from the distribution manifest, so the version pin is effectively a SHA pin (the manifest is the toolchain's lockfile). Both local and CI now read the same file and install identical bits. Mirrors the pattern just shipped in [`agentnative` PR #17](brettdavies/agentnative-cli#17), which was motivated by a real CI-only clippy failure during PR review: local clippy 1.94 passed a lint that CI clippy 1.95 rejected because `channel = "stable"` let rustc drift between environments. ## Changelog  ## Type of Change - [x] `chore`: Maintenance tasks (supply-chain toolchain pin) ## Related Issues/Stories - Related PRs: [brettdavies/agentnative-cli#17](brettdavies/agentnative-cli#17) — pattern source ## Testing - [x] All tests passing **Test Summary:** - Unit tests: 185 passing - Integration tests: 38 + 14 = 52 passing - Live integration tests: 1 ignored (requires live X API) - Clippy: clean with `-Dwarnings` on rustc 1.94.1 (pinned) - Fmt: clean - Pre-push hook: passed (fmt, clippy, test, windows compat) ## Files Modified **Modified:** - `rust-toolchain.toml` — pinned `channel = "1.94.1"` with trailing comment naming the rustc commit SHA and release date; bumped from floating `channel = "stable"`. Added 4 lines of header comments documenting the supply-chain rationale and the ≥7-day quarantine policy. ## Key Details **Toolchain pin format:** `channel = "1.94.1" # rustc e408947bfd200af42db322daf0fadfe7e26d3bd1, released 2026-03-25`. Comment mirrors the GitHub Actions SHA-pin pattern (`action@<sha> # vN.N.N`). **Policy:** bump the channel only via reviewed PR, after the new stable has aged ≥7 days. Matches the broader brettdavies supply-chain posture (`UV_EXCLUDE_NEWER`, bun `minimumReleaseAge`, `npm_config_min_release_age` already in dotfiles). **MSRV compatibility:** bird's `rust-version = "1.87"`; 1.94.1 satisfies it comfortably. **No hook / workflow changes needed:** neither `scripts/hooks/pre-push`, `.githooks/pre-push`, nor any `.github/workflows/*.yml` file references `rustup update` or pins a specific toolchain version — rustup reads `rust-toolchain.toml` automatically on every `cargo` invocation, so the single-file change is sufficient. ## Benefits - Local and CI toolchains are guaranteed identical — no more "local clippy older than CI clippy" false greens - Toolchain updates route through reviewed PRs with a 7-day quarantine - Consistent with the supply-chain-pin pattern applied across brettdavies repos (GitHub Actions SHA pins, Docker image digest pins, package-manager lockfiles) ## Breaking Changes - [x] No breaking changes ## Deployment Notes - [x] No special deployment steps required ## Checklist - [x] Code follows project conventions and style guidelines - [x] Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) - [x] Self-review of code completed - [x] No new warnings or errors introduced - [x] Changes are backward compatible

## Summary Pins `rust-toolchain.toml` to a specific `X.Y.Z` release instead of floating `channel = "stable"`. Rustup verifies component SHA256s from the distribution manifest, so the version pin is effectively a SHA pin (the manifest is the toolchain's lockfile). Trailing comment documents the rustc commit SHA for audit, mirroring the GitHub Actions SHA-pin pattern (`action@<sha> # vN.N.N`). Both local and CI now read `rust-toolchain.toml` and install identical bits. Prevents "local clippy older than CI clippy" drift. Matches the pattern applied in [agentnative PR #17](brettdavies/agentnative-cli#17). ## Changelog  ## Type of Change - [x] `chore`: Maintenance task (supply-chain toolchain pin) ## Testing - [x] All tests passing - [x] Clippy clean with `-Dwarnings` - [x] `cargo fmt --check` clean **Test Summary:** - Unit tests: 17 passing (unchanged — same code, new pinned toolchain) - Clippy: clean on rustc 1.94.1 ## Policy Bump the channel only via reviewed PR, after the new stable has aged ≥7 days. This matches the `UV_EXCLUDE_NEWER` / bun `minimumReleaseAge` / `npm_config_min_release_age` quarantine already in dotfiles. A central automated bump workflow is tracked as a follow-up in `brettdavies/dot-github` todo #4. - [x] No breaking changes - [x] No special deployment steps required

…-chain pin (#17) ## Summary Two related hardening efforts bundled: 1. **Convention enforcement** — standardizes all source checks on a single pattern (`check_x()` returns `CheckStatus`; `run()` is the sole `CheckResult` constructor, using `self.id()`/`self.group()`/`self.layer()`). Documents the convention in CLAUDE.md, refactors all 16 Rust source checks to match, and adds a Rust-only integration test that walks the source tree and fails CI if any `check_x()` still returns `CheckResult`. 2. **Toolchain supply-chain pin** — `rust-toolchain.toml` now pins to a specific `X.Y.Z` release with a trailing rustc commit-SHA comment. Rustup verifies component SHA256s from the distribution manifest, so the pin is effectively a SHA pin (the manifest is the toolchain's lockfile). Both local and CI read the same file and install identical bits. Motivated by a CI-only clippy failure during this PR's own review: local clippy 1.94 passed a lint that CI clippy 1.95 rejected. Convention enforcement was identified during the [PR #15 code review](#15) — the 3 new Python checks were fixed in that PR, but the 16 existing Rust checks still used the old pattern. The toolchain pin was added mid-review after the clippy divergence exposed that floating `channel = "stable"` leaves local and CI on different rustc versions. ## Changelog  ## Type of Change - [x] `refactor`: Code refactoring (no functional changes) - [x] `chore`: Maintenance tasks (supply-chain toolchain pin) - [x] `docs`: Documentation update (CLAUDE.md conventions + toolchain policy) - [x] `test`: Adding or updating tests (convention enforcement test) ## Related Issues/Stories - Related PRs: #15 (review finding that identified the convention drift) - Follow-up: `dot-github` todo #4 — determine the automated bump workflow for `rust-toolchain.toml` across repos (central reusable workflow vs. local script vs. hybrid) ## Testing - [x] Unit tests added/updated - [x] Integration tests added/updated - [x] All tests passing **Test Summary:** - Unit tests: 304 passing (unchanged count — refactor preserves behavior) - Integration tests: 39 passing (+1 new: `convention_check_x_returns_check_status_not_check_result`) - Clippy: clean with `-Dwarnings` on rustc 1.94.1 (pinned) ## Files Modified **Modified:** - `CLAUDE.md` — added Source Check Convention section (Tier 1) and Toolchain Pin section documenting the 7-day quarantine policy - `rust-toolchain.toml` — pinned `channel = "1.94.1"` with trailing comment naming the rustc commit SHA and release date; bumped from floating `channel = "stable"` - `scripts/hooks/pre-push` — removed `rustup update stable` step; pin + rustup's manifest verification now handle toolchain integrity - 16 files in `src/checks/source/rust/` — `check_x()` returns `CheckStatus`, `run()` uses `self.id()`/`self.group()`/ `self.layer()` instead of duplicated string/enum literals - `src/checks/source/rust/no_pager.rs` — collapsed nested `if` inside a `match` arm into a match guard (fixes clippy `collapsible_match` lint introduced in clippy 1.95) - `tests/integration.rs` — added `convention_check_x_returns_check_status_not_check_result` enforcement test ## Key Details **Tier 1 (Document):** CLAUDE.md now codifies the source check structure. Covers the `Check` trait impl shape, the `check_x() → CheckStatus` separation, and the convention that `run()` is the sole `CheckResult` constructor. Also adds a new Toolchain Pin section explaining the supply-chain pin policy. **Tier 2 (Refactor):** All 16 Rust source checks aligned to the convention. Net -132 lines by removing duplicated `CheckResult` construction (previously the `id` / `label` / `group` / `layer` fields were hardcoded in both `run()` and every `check_x()` helper). **Tier 3 (Enforce):** Integration test walks `src/checks/source/` with `std::fs` (no shell dependencies), finds every `fn check_` signature (any visibility, single-line or multi-line), and fails if any return `CheckResult` instead of `CheckStatus`. Uses `CARGO_MANIFEST_DIR` for a stable absolute path. **Toolchain pin format:** `channel = "1.94.1" # rustc e408947bfd200af42db322daf0fadfe7e26d3bd1, released 2026-03-25`. Comment mirrors the GitHub Actions SHA-pin pattern (`action@<sha> # vN.N.N`). Policy: bump only via reviewed PR after the new stable has aged ≥7 days (supply-chain quarantine consistent with `UV_EXCLUDE_NEWER` / bun `minimumReleaseAge` / `npm_config_min_release_age` in dotfiles). ## Benefits - Eliminates ID string literal triplication (3 copies per check → 1 authoritative source in `fn id()`) - New checks can't drift — the convention is documented AND enforced by CI - -132 lines of boilerplate removed - Local and CI toolchains are guaranteed identical — no more "local clippy older than CI clippy" false greens - Toolchain updates route through reviewed PRs with a 7-day quarantine, matching the broader brettdavies supply-chain posture ## Breaking Changes - [x] No breaking changes ## Deployment Notes - [x] No special deployment steps required ## Checklist - [x] Code follows project conventions and style guidelines - [x] Commit messages follow Conventional Commits - [x] Self-review of code completed - [x] Tests added/updated and passing - [x] No new warnings or errors introduced - [x] Changes are backward compatible

brettdavies added 6 commits April 16, 2026 12:04

style: format convention enforcement test

1fd6afd

fix(clippy): collapse nested if into match guard in no_pager

4d4e7e5

clippy 1.95 flags the nested `if` inside `CheckStatus::Pass =>` as collapsible. Use a match guard instead. Local toolchain was 1.94 so the lint didn't fire pre-push — ran into this as a CI-only failure.

brettdavies merged commit 07091e2 into dev Apr 16, 2026
6 checks passed

brettdavies deleted the refactor/check-status-convention branch April 16, 2026 17:08

This was referenced Apr 16, 2026

chore(supply-chain): pin Rust toolchain to 1.94.1 with commit SHA brettdavies/bird#30

Merged

chore(supply-chain): pin Rust toolchain to 1.94.1 with commit SHA brettdavies/xurl-rs#22

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: enforce check_x → CheckStatus convention + toolchain supply-chain pin#17

refactor: enforce check_x → CheckStatus convention + toolchain supply-chain pin#17
brettdavies merged 6 commits intodevfrom
refactor/check-status-convention

brettdavies commented Apr 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

brettdavies commented Apr 16, 2026

Summary

Changelog

Type of Change

Related Issues/Stories

Testing

Files Modified

Key Details

Benefits

Breaking Changes

Deployment Notes

Checklist

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant