ENG-4687 pre-commit hook: cargo fmt --check + hygiene

[jimbofreedman]

Summary

Adds a pre-commit configuration that runs cargo fmt --check plus standard hygiene hooks on every commit, and exposes cargo clippy on the manual stage. Includes contributor docs and machine-verified invariant + e2e tests so the hook contract cannot drift silently.

• New .pre-commit-config.yaml — two cargo fmt --check hooks (root crate via cargo-fmt-check, nested tests/downstream_consumer sub-package via cargo-fmt-check-downstream, both on the default stage), a manual-stage cargo-clippy hook, and the pre-commit-hooks v5.0.0 hygiene baseline. Both cargo fmt invocations are direct entries (no shell), so the config works on Windows runners.
• CONTRIBUTING.md updated with pipx install pre-commit && pre-commit install instructions (plus pip/brew alternatives) and a "before opening a PR" reminder.
• New CI job pre-commit installs pre-commit==4.6.0 and runs cargo test --test pre_commit_e2e — AC2 inside that test invokes pre-commit run --all-files on the repo root and asserts the tree stays drift-free, so the standalone pre-commit run step was unnecessary.
• Invariant tests (tests/pre_commit_config.rs, 11 cases) lock the YAML shape including the no-shell requirement, the dual-hook structure, and per-hook attribute presence; e2e tests (tests/pre_commit_e2e.rs, 4 cases) exercise the hooks against ephemeral git repos and skip cleanly when pre-commit, cargo fmt, or cargo clippy is absent.

Acceptance criteria

• AC1 — pre-commit install succeeds on a fresh clone. Verified by the pre-commit CI job, which performs pip install pre-commit==4.6.0 followed by cargo test --test pre_commit_e2e (whose AC2 case shells out to pre-commit against the repo root).
• AC2 — pre-commit run --all-files passes on the current tree. Verified by tests/pre_commit_e2e.rs::ac2_pre_commit_run_all_files_passes_on_current_tree, which also fails if any hook silently rewrites files (auto-fixer with a zero exit), and skips cleanly when the worktree has uncommitted local changes.
• AC3 — committing misformatted Rust fails the hook with rustfmt's diff. Verified by tests/pre_commit_e2e.rs::ac3_cargo_fmt_check_fails_on_misformatted_rust, which requires rustfmt's Diff in signature in the output (no broad fallback) and skips with a rustup component add rustfmt hint if the toolchain component is absent.
• AC4 — pre-commit run --hook-stage manual runs clippy (and clippy does NOT run on default stage). Verified by tests/pre_commit_e2e.rs::ac4_manual_stage_invokes_clippy (anchored on cargo's own Checking ... banner — not pre-commit's hook-name banner, which would fire even if clippy never ran) and ac4_clippy_does_not_run_on_default_stage (requires pre-commit's explicit No hook with id diagnostic plus a non-zero exit, and asserts Checking e2e_root is absent).
• AC5 — invariants locked against silent drift. Verified by cargo test --test pre_commit_config (11/11 passing). The fmt-hook contract is now asserted per-hook (locate each - id: cargo-fmt-check* block, then check each attribute inside it) so a silently dropped attribute on either fmt hook trips the invariant.

Test plan

• cargo fmt --check — clean
• cargo clippy --no-deps -- -D warnings — clean
• cargo test --test pre_commit_config — 11/11 pass
• pipx install pre-commit (or pip install pre-commit) then cargo test --test pre_commit_e2e — 4/4 pass
• pre-commit run --all-files — all hooks green
• CI: all jobs green (fmt, clippy, test-matrix, features-matrix, coverage, docs, pre-commit)

ClickUp

ENG-4687: https://app.clickup.com/t/86c9umm8t

[Copilot]

Pull request overview

This PR introduces a repository-wide pre-commit gate to enforce cargo fmt --check plus hygiene hooks on every commit, with cargo clippy exposed as an opt-in manual-stage hook. It also adds invariant + end-to-end tests and a CI job to ensure the hook contract can’t drift unnoticed.

Changes:

• Add .pre-commit-config.yaml with hygiene hooks, cargo fmt --check (default stage), and cargo clippy (manual stage).
• Add invariant tests (tests/pre_commit_config.rs) and e2e tests (tests/pre_commit_e2e.rs) to lock configuration and behavior.
• Update contributor documentation and CI to install/run pre-commit and execute the e2e suite.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/pre_commit_e2e.rs	Adds end-to-end tests that shell out to pre-commit to validate AC2/AC3/AC4.
tests/pre_commit_config.rs	Adds invariant tests that lock the .pre-commit-config.yaml shape via string assertions.
tests/downstream_consumer/src/lib.rs	Applies formatting changes so cargo fmt --check passes.
CONTRIBUTING.md	Documents pre-commit usage in the local validation workflow.
CODE_OF_CONDUCT.md	Removes trailing blank line at EOF (hygiene).
.pre-commit-config.yaml	Introduces the pre-commit hook configuration (hygiene + fmt + manual clippy).
.github/workflows/ci.yaml	Adds a dedicated CI job to install/run pre-commit and execute e2e hook tests.

Comments suppressed due to low confidence (2)

tests/pre_commit_e2e.rs:342

• augmented_path is built with a hard-coded : separator here as well, which is not portable to Windows PATH semantics. Use std::env::join_paths to construct the PATH value in an OS-appropriate way.

    let cargo = cargo_bin();
    let path = std::env::var("PATH").unwrap_or_default();
    let cargo_dir = Path::new(&cargo)
        .parent()
        .map(|p| p.to_string_lossy().into_owned())
        .unwrap_or_default();
    let augmented_path = if cargo_dir.is_empty() {
        path.clone()
    } else {
        format!("{cargo_dir}:{path}")
    };

tests/pre_commit_e2e.rs:412

• Same PATH concatenation issue here: using format!("{cargo_dir}:{path}") is not portable on Windows. Prefer std::env::join_paths/split_paths so this test behaves consistently across platforms.

    let cargo = cargo_bin();
    let path = std::env::var("PATH").unwrap_or_default();
    let cargo_dir = Path::new(&cargo)
        .parent()
        .map(|p| p.to_string_lossy().into_owned())
        .unwrap_or_default();
    let augmented_path = if cargo_dir.is_empty() {
        path.clone()
    } else {
        format!("{cargo_dir}:{path}")
    };

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.