From 4f4d26815513b2d9c574c5d3b844c9ca521a30d6 Mon Sep 17 00:00:00 2001
From: Zhengchao An <anzhengchao@gmail.com>
Date: Wed, 26 Aug 2026 10:21:25 +0800
Subject: [PATCH] ci: ratchet ecstore ::other(format!) error construction
 shrink-only (#6614)

Backlog#1845 step 2. reduce_errs buckets per-disk errors by equality, and Io equality compares the rendered message, so an other(format!(..)) error embedding per-disk detail makes N same-cause failures count as N distinct errors during quorum aggregation. The census that opened the issue counted 1,609 such sites; the production count in crates/ecstore/src is 657 today and was still growing.

Freeze it: scripts/check_error_other_format_ratchet.sh counts ::other(format! sites per file (trailing #[cfg(test)] modules excluded) against a shrink-only per-file baseline, failing on any growth and on stale entries after a shrink, following the layer-dependency-baseline model. Wired into make pre-commit / pre-pr / dev-check and the CI Quick Checks job.

Ref rustfs/backlog#1845
---
 .config/make/lint-fmt.mak                   |   5 +
 .config/make/pre-commit.mak                 |   6 +-
 .github/workflows/ci.yml                    |   3 +
 scripts/check_error_other_format_ratchet.sh | 141 ++++++++++++++++++++
 scripts/error-other-format-baseline.txt     |  79 +++++++++++
 5 files changed, 231 insertions(+), 3 deletions(-)
 create mode 100755 scripts/check_error_other_format_ratchet.sh
 create mode 100644 scripts/error-other-format-baseline.txt

diff --git a/.config/make/lint-fmt.mak b/.config/make/lint-fmt.mak
index dda47ae88..673010369 100644
--- a/.config/make/lint-fmt.mak
+++ b/.config/make/lint-fmt.mak
@@ -45,6 +45,11 @@ logging-guardrails-check: ## Check logging guardrails for redaction and noise re
 	@echo "🪵 Checking logging guardrails..."
 	./scripts/check_logging_guardrails.sh
 
+.PHONY: error-other-ratchet-check
+error-other-ratchet-check: ## Check the ecstore ::other(format!) quorum-bucketing ratchet stays shrink-only
+	@echo "🪣 Checking error other(format!) ratchet..."
+	./scripts/check_error_other_format_ratchet.sh
+
 .PHONY: tokio-io-uring-check
 tokio-io-uring-check: ## Check tokio io-uring runtime feature stays removed
 	@echo "🚫 Checking tokio io-uring feature guard..."
diff --git a/.config/make/pre-commit.mak b/.config/make/pre-commit.mak
index 54b54e34c..cbf7a1c83 100644
--- a/.config/make/pre-commit.mak
+++ b/.config/make/pre-commit.mak
@@ -19,13 +19,13 @@ planning-docs-check: ## Check that no planning-type documents are committed
 	./scripts/check_no_planning_docs.sh
 
 .PHONY: pre-commit
-pre-commit: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check s3s-footprint-check fips-wording-check embedded-secrets-check test-wiring-check doc-paths-check planning-docs-check quick-check ## Run fast pre-commit checks without clippy/full tests
+pre-commit: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check error-other-ratchet-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check s3s-footprint-check fips-wording-check embedded-secrets-check test-wiring-check doc-paths-check planning-docs-check quick-check ## Run fast pre-commit checks without clippy/full tests
 	@echo "✅ All pre-commit checks passed!"
 
 .PHONY: pre-pr
-pre-pr: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check s3s-footprint-check fips-wording-check embedded-secrets-check test-wiring-check doc-paths-check planning-docs-check log-analyzer-rules-check clippy-check test ## Run full pre-PR checks with clippy and tests
+pre-pr: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check error-other-ratchet-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check s3s-footprint-check fips-wording-check embedded-secrets-check test-wiring-check doc-paths-check planning-docs-check log-analyzer-rules-check clippy-check test ## Run full pre-PR checks with clippy and tests
 	@echo "✅ All pre-PR checks passed!"
 
 .PHONY: dev-check
-dev-check: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check s3s-footprint-check fips-wording-check embedded-secrets-check test-wiring-check doc-paths-check planning-docs-check quick-check ## Run fast local development checks
+dev-check: fmt-check unsafe-code-check architecture-migration-check logging-guardrails-check error-other-ratchet-check tokio-io-uring-check extension-schema-check body-cache-whitelist-check s3s-footprint-check fips-wording-check embedded-secrets-check test-wiring-check doc-paths-check planning-docs-check quick-check ## Run fast local development checks
 	@echo "✅ Fast development checks passed!"
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 8c532ba3f..d1ae8e44a 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -142,6 +142,9 @@ jobs:
       - name: Check logging guardrails
         run: ./scripts/check_logging_guardrails.sh
 
+      - name: Check error other(format!) ratchet
+        run: ./scripts/check_error_other_format_ratchet.sh
+
       - name: Check tokio io-uring feature guard
         run: ./scripts/check_no_tokio_io_uring.sh
 
diff --git a/scripts/check_error_other_format_ratchet.sh b/scripts/check_error_other_format_ratchet.sh
new file mode 100755
index 000000000..254cc4f78
--- /dev/null
+++ b/scripts/check_error_other_format_ratchet.sh
@@ -0,0 +1,141 @@
+#!/usr/bin/env bash
+# Ratchet guard for `…::other(format!` error construction in rustfs-ecstore
+# (rustfs/backlog#1845, PR2).
+#
+# Quorum aggregation (`reduce_errs` in crates/ecstore/src/disk/error_reduce.rs)
+# buckets errors by equality, and `DiskError::Io` / `StorageError::Io` equality
+# compares the *rendered message*. An `other(format!(…))` error that embeds
+# per-disk / per-peer detail therefore makes N same-cause failures count as N
+# distinct errors, starving quorum decisions and heal retry classification
+# (pinned by crates/ecstore/src/error/conversion_roundtrip_tests.rs).
+#
+# This guard freezes the existing `::other(format!` call sites in
+# crates/ecstore/src as a per-file baseline and fails when any file GROWS its
+# count (or a new file introduces one). New code must use a typed error
+# variant, or keep the formatted detail out of the bucketed message (e.g. put
+# it in a wrapped source error with a stable Display).
+#
+# The baseline is SHRINK-ONLY, following the layer-dependency-baseline model
+# (backlog#1834): when a PR removes call sites, regenerate the baseline in the
+# same PR via --update-baseline; a diff that raises a count or adds a file is
+# baselining a brand-new bucketing hazard and must carry an explicit exemption
+# rationale in the PR description.
+#
+# Trailing `#[cfg(test)] mod … {` blocks are excluded from the counts: test
+# construction of other(format!) never reaches production quorum paths, and
+# the repository convention keeps inline test modules at the end of the file.
+#
+# Usage:
+#   scripts/check_error_other_format_ratchet.sh                  # check
+#   scripts/check_error_other_format_ratchet.sh --update-baseline
+
+set -euo pipefail
+
+ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+BASELINE_FILE="${ROOT_DIR}/scripts/error-other-format-baseline.txt"
+SCOPE="crates/ecstore/src"
+PATTERN='::other\(\s*format!'
+MODE="check"
+
+if [[ "${1:-}" == "--update-baseline" ]]; then
+  MODE="update"
+fi
+
+TMP_DIR="$(mktemp -d)"
+trap 'rm -rf "$TMP_DIR"' EXIT
+
+count_file_sites() {
+  # Strip a trailing `#[cfg(test)]\nmod … {` region (repo convention keeps the
+  # test module last), then count pattern occurrences across line breaks.
+  perl -0777 -pe 's/\n#\[cfg\(test\)\]\s*\nmod\s+[A-Za-z0-9_]+\s*\{.*$/\n/s' "$1" |
+    perl -0777 -ne 'my $c = () = /::other\(\s*format!/g; print "$c\n";'
+}
+
+CURRENT="${TMP_DIR}/current.txt"
+: >"$CURRENT"
+
+while IFS= read -r file; do
+  count="$(count_file_sites "${ROOT_DIR}/${file}")"
+  if (( count > 0 )); then
+    printf '%s|%s\n' "$count" "$file" >>"$CURRENT"
+  fi
+done < <(cd "$ROOT_DIR" && rg -U -l "$PATTERN" --type rust "$SCOPE" | LC_ALL=C sort)
+
+total="$(awk -F'|' '{sum += $1} END {print sum + 0}' "$CURRENT")"
+
+write_baseline_file() {
+  cat >"$BASELINE_FILE" <<'EOF'
+# `::other(format!` ratchet baseline for crates/ecstore/src (backlog#1845 PR2).
+#
+# SHRINK-ONLY: entries are `count|file`. A PR may lower a count or drop a file
+# (after replacing the call sites with typed variants) by re-running
+# scripts/check_error_other_format_ratchet.sh --update-baseline. A PR that
+# raises a count or adds a file is introducing a new quorum-bucketing hazard
+# and must carry an explicit exemption rationale in its description.
+EOF
+  cat "$CURRENT" >>"$BASELINE_FILE"
+}
+
+if [[ "$MODE" == "update" ]]; then
+  write_baseline_file
+  echo "Updated baseline: $BASELINE_FILE (total call sites: $total)"
+  exit 0
+fi
+
+if [[ ! -f "$BASELINE_FILE" ]]; then
+  echo "Baseline file missing: $BASELINE_FILE"
+  echo "Run: scripts/check_error_other_format_ratchet.sh --update-baseline"
+  exit 1
+fi
+
+BASELINE_SORTED="${TMP_DIR}/baseline.txt"
+grep -v '^#' "$BASELINE_FILE" | grep -v '^$' | LC_ALL=C sort -t'|' -k2 >"$BASELINE_SORTED"
+LC_ALL=C sort -t'|' -k2 -o "$CURRENT" "$CURRENT"
+
+STATUS=0
+GREW="${TMP_DIR}/grew.txt"
+SHRANK="${TMP_DIR}/shrank.txt"
+: >"$GREW"
+: >"$SHRANK"
+
+# Compare per-file counts; report growth and staleness separately.
+awk -F'|' -v grew="$GREW" -v shrank="$SHRANK" '
+  NR == FNR { baseline[$2] = $1; next }
+  {
+    current[$2] = $1
+    if (!($2 in baseline)) {
+      printf "%s: %s call sites (new file, baseline has none)\n", $2, $1 >> grew
+    } else if ($1 + 0 > baseline[$2] + 0) {
+      printf "%s: %s call sites (baseline %s)\n", $2, $1, baseline[$2] >> grew
+    } else if ($1 + 0 < baseline[$2] + 0) {
+      printf "%s: %s call sites (baseline %s)\n", $2, $1, baseline[$2] >> shrank
+    }
+  }
+  END {
+    for (file in baseline) {
+      if (!(file in current)) {
+        printf "%s: baseline lists %s call sites but the file now has none\n", file, baseline[file] >> shrank
+      }
+    }
+  }
+' "$BASELINE_SORTED" "$CURRENT"
+
+if [[ -s "$GREW" ]]; then
+  echo "error(format!) ratchet failed: new '::other(format!' call sites in crates/ecstore/src"
+  echo "Use a typed error variant instead — formatted per-disk detail fragments reduce_errs quorum buckets (backlog#1845):"
+  cat "$GREW"
+  STATUS=1
+fi
+
+if [[ -s "$SHRANK" ]]; then
+  echo "error(format!) ratchet: counts went DOWN (good) but the baseline is stale."
+  echo "Re-run scripts/check_error_other_format_ratchet.sh --update-baseline and commit the shrunken baseline:"
+  cat "$SHRANK"
+  STATUS=1
+fi
+
+if (( STATUS == 0 )); then
+  echo "error(format!) ratchet passed (total call sites: $total)."
+fi
+
+exit "$STATUS"
diff --git a/scripts/error-other-format-baseline.txt b/scripts/error-other-format-baseline.txt
new file mode 100644
index 000000000..dc9bd8c21
--- /dev/null
+++ b/scripts/error-other-format-baseline.txt
@@ -0,0 +1,79 @@
+# `::other(format!` ratchet baseline for crates/ecstore/src (backlog#1845 PR2).
+#
+# SHRINK-ONLY: entries are `count|file`. A PR may lower a count or drop a file
+# (after replacing the call sites with typed variants) by re-running
+# scripts/check_error_other_format_ratchet.sh --update-baseline. A PR that
+# raises a count or adds a file is introducing a new quorum-bucketing hazard
+# and must carry an explicit exemption rationale in its description.
+2|crates/ecstore/src/bucket/bucket_target_sys.rs
+4|crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs
+3|crates/ecstore/src/bucket/lifecycle/durable_namespace.rs
+2|crates/ecstore/src/bucket/lifecycle/metadata_boundary.rs
+4|crates/ecstore/src/bucket/lifecycle/tier_delete_journal.rs
+1|crates/ecstore/src/bucket/lifecycle/transition_transaction.rs
+27|crates/ecstore/src/bucket/metadata.rs
+22|crates/ecstore/src/bucket/metadata_sys.rs
+3|crates/ecstore/src/bucket/msgp_decode.rs
+1|crates/ecstore/src/bucket/object_lock/objectlock_sys.rs
+3|crates/ecstore/src/bucket/quota/reservation.rs
+1|crates/ecstore/src/bucket/replication/replication_object_config.rs
+2|crates/ecstore/src/bucket/replication/replication_pool.rs
+2|crates/ecstore/src/bucket/replication/replication_target_boundary.rs
+1|crates/ecstore/src/client/api_put_object_multipart.rs
+2|crates/ecstore/src/client/api_put_object_streaming.rs
+1|crates/ecstore/src/client/api_s3_datatypes.rs
+1|crates/ecstore/src/client/signer_error.rs
+5|crates/ecstore/src/client/transition_api.rs
+3|crates/ecstore/src/cluster/rpc/http_auth.rs
+2|crates/ecstore/src/cluster/rpc/internode_data_transport.rs
+17|crates/ecstore/src/cluster/rpc/peer_rest_client.rs
+11|crates/ecstore/src/cluster/rpc/peer_s3_client.rs
+43|crates/ecstore/src/cluster/rpc/remote_disk.rs
+7|crates/ecstore/src/config/com.rs
+14|crates/ecstore/src/config/storageclass.rs
+185|crates/ecstore/src/core/pools.rs
+8|crates/ecstore/src/data_movement/mod.rs
+2|crates/ecstore/src/data_usage/local_snapshot.rs
+12|crates/ecstore/src/data_usage/mod.rs
+1|crates/ecstore/src/diagnostics/admin_server_info.rs
+5|crates/ecstore/src/disk/local.rs
+1|crates/ecstore/src/disk/mod.rs
+5|crates/ecstore/src/erasure/codec/bridge.rs
+1|crates/ecstore/src/erasure/coding/decode_reader.rs
+10|crates/ecstore/src/erasure/coding/encode.rs
+25|crates/ecstore/src/erasure/coding/erasure.rs
+4|crates/ecstore/src/layout/disks_layout.rs
+2|crates/ecstore/src/layout/endpoint.rs
+17|crates/ecstore/src/layout/endpoints.rs
+1|crates/ecstore/src/layout/format.rs
+1|crates/ecstore/src/layout/pool_space.rs
+1|crates/ecstore/src/layout/set_layout.rs
+19|crates/ecstore/src/object_api/readers.rs
+1|crates/ecstore/src/object_api/types.rs
+3|crates/ecstore/src/runtime/sources.rs
+4|crates/ecstore/src/services/batch_processor.rs
+14|crates/ecstore/src/services/notification_sys.rs
+16|crates/ecstore/src/services/rebalance/control.rs
+1|crates/ecstore/src/services/rebalance/entry.rs
+8|crates/ecstore/src/services/rebalance/meta.rs
+8|crates/ecstore/src/services/rebalance/runtime.rs
+19|crates/ecstore/src/services/rebalance/worker.rs
+33|crates/ecstore/src/services/tier/tier.rs
+1|crates/ecstore/src/services/tier/tier_config.rs
+1|crates/ecstore/src/services/tier/warm_backend_gcs.rs
+1|crates/ecstore/src/services/tier/warm_backend_s3.rs
+1|crates/ecstore/src/services/tier/warm_backend_wasabi.rs
+7|crates/ecstore/src/set_disk/core/io_primitives.rs
+1|crates/ecstore/src/set_disk/mod.rs
+3|crates/ecstore/src/set_disk/ops/bitrot_self_verify.rs
+2|crates/ecstore/src/set_disk/ops/heal.rs
+8|crates/ecstore/src/set_disk/ops/multipart.rs
+2|crates/ecstore/src/set_disk/ops/object.rs
+3|crates/ecstore/src/set_disk/read.rs
+5|crates/ecstore/src/store/bucket.rs
+1|crates/ecstore/src/store/heal_walk.rs
+12|crates/ecstore/src/store/init.rs
+2|crates/ecstore/src/store/init_format.rs
+3|crates/ecstore/src/store/multipart.rs
+8|crates/ecstore/src/store/object.rs
+5|crates/ecstore/src/store/rebalance/support.rs