ci: fail format job when rustfmt rewrites any Rust file

[Copilot]

ci/format_pre.sh ran bazel run @rules_rust//:rustfmt (apply-mode), which always exits 0 — so the format CI job never actually caught rustfmt violations. Unformatted Rust landed on main at least three times as a result.

Change

After the apply-mode rustfmt run, check the working tree for changes and fail if any were made:

CURRENT=rustfmt
bazel "${BAZEL_STARTUP_OPTIONS[@]}" run "${BAZEL_BUILD_OPTIONS[@]}" @rules_rust//:rustfmt
if [[ -n "$(git status --porcelain)" ]]; then
    echo "ERROR: rustfmt produced changes — Rust code is not formatted." >&2
    echo "Run: bazel run @rules_rust//:rustfmt" >&2
    false
fi

• false triggers the existing ERR trap → rustfmt is appended to FAILED → script exits non-zero.
• The existing git diff > "$DIFF_OUTPUT" block at the end captures the rewrite diff for CI artifact upload, identical to check_format failure behavior.
• Clean trees are unaffected: git status --porcelain is empty, the block is skipped.

Original prompt

Problem

ci/format_pre.sh does not actually fail when Rust code is mis-formatted. The rustfmt step is currently:

CURRENT=rustfmt
bazel "${BAZEL_STARTUP_OPTIONS[@]}" run "${BAZEL_BUILD_OPTIONS[@]}" @rules_rust//:rustfmt

bazel run @rules_rust//:rustfmt is rules_rust's apply-mode runner — it rewrites files in place and exits 0 whether or not it had to change anything. The only place the script reports failure is via the ERR trap appending to the FAILED array, so a clean exit means the format job passes even when rustfmt has rewritten files.

This has already caused at least three real incidents where unformatted Rust code landed on main:

• dns: add a new RUST-based DNS resolver extension #44090 (the original hickory_dns PR) — landed with formatting issues.
• hickory_dns: slightly optimize how we maintain shared states #44175 — landed with formatting issues on top.
• deps/rust: Bump hickory-proto/resolver -> 0.26.1 #44858 — landed with formatting issues on top, surfacing a rustfmt diff in source/extensions/dynamic_modules/builtin_extensions/hickory_dns.rs only via the rustfmt_test Bazel target in a downstream test shard.

#44276 ("Fix format") had to clean these up after the fact.

The validating target already exists in source/extensions/dynamic_modules/builtin_extensions/BUILD:

rustfmt_test(
    name = "fmt_builtin_extensions",
    testonly = True,
    tags = ["nocoverage"],
    targets = [":builtin_extensions_static_lib"],
)

…and rules_rust ships @rules_rust//:rustfmt_check (the same runner in --check mode), but neither is invoked from the format CI job. So formatting issues only surface from a bazel test shard that happens to include the rustfmt_test target — which the format-only PR check does not run.

Goal

Ensure that ci/format_pre.sh (the format CI job that runs on every PR) fails when any Rust file under the repo is not rustfmt-clean, so we never again merge a PR that introduces a rustfmt diff.

Proposed change

Update the rustfmt step in ci/format_pre.sh so that it both attempts a fix (preserving today's helpful diff-upload-on-failure behaviour) and fails when there is anything to fix. The simplest, least-invasive way to do this is to run the apply-mode rustfmt and then check the working tree:

CURRENT=rustfmt
bazel "${BAZEL_STARTUP_OPTIONS[@]}" run "${BAZEL_BUILD_OPTIONS[@]}" @rules_rust//:rustfmt
if [[ -n "$(git status --porcelain)" ]]; then
    echo "ERROR: rustfmt produced changes — Rust code is not formatted." >&2
    echo "Run: bazel run @rules_rust//:rustfmt" >&2
    false
fi

The trailing false triggers the existing ERR trap, which appends to FAILED and causes the script to exit non‑zero at the end. The existing git diff > "$DIFF_OUTPUT" block at the bottom of the script will then upload the rustfmt diff as a CI artefact, exactly like it already does for check_format failures, so contributors get a copy-pasteable diff.

Alternatively (and equivalently in effect), invoke the check-mode runner directly, which exits non‑zero on diff without modifying files:

CURRENT=rustfmt
bazel "${BAZEL_STARTUP_OPTIONS[@]}" run "${BAZEL_BUILD_OPTIONS[@]}" @rules_rust//:rustfmt_check

The first option is preferred because it preserves the auto-fix-and-upload-diff UX that the other steps already use; please go with that unless rustfmt_check integrates more cleanly.

Acceptance criteria

1. ci/format_pre.sh exits non‑zero when any Rust source in the tree is not rustfmt-clean against the repo's rustfmt.toml.
2. The failure path produces a clear error message naming rustfmt as the failing step (the existing CURRENT=rustfmt + FAILED mechanism is sufficient).
3. The fix-up diff is still written to $DIFF_OUTPUT and uploaded as a CI artefact when rustfmt is the cause of failure (this should "just work" given the existing block at the bottom of the script — please verify).
4. The rustfmt step still passes on a clean tree (no false positives).
5. No other format steps regress.

Verification

• Locally: bazel run //ci:format (or run ci/format_pre.sh directly) on a clean checkout — should pass.
• Locally: introduce a deliberate rustfmt violation in a .rs file under source/extensions/dynamic_modules/ and re-run — the script must exit non‑zero with rustfmt listed in the TESTS FAILED block, and fix_format.diff must contain the expected rewrite.

Out of scope

• Reformatting any current rustfmt violations in the tree. If the tree is already dirty when this lands, fix the diff in the same PR (or a prerequisite) so CI is green; do not weaken the check.
• Changing rustfmt.toml or any rustfmt_test targets.
• Touching tools/code_format/check_format or the spelling/check_format steps.

Files to touch

• ci/format_pre.sh — the rustfmt step.

That should be the entire change.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: ```

diff --git a/source/extensions/dynamic_modules/builtin_extensions/hickory_dns.rs b/source/extensions/dynamic_modules/builtin_exte...

This pull request was created from Copilot chat.

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #44877 was opened by Copilot.

see: more, trace.