ci: stabilize architecture layer guard (#3255)

docs/architecture/migration-progress.md

@@ -5,13 +5,13 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
 ## Current Context
 
 - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660)
-- Branch: `overtrue/arch-migration-guardrails`
-- Baseline: `upstream/main` at `61f0dfbc40f748be313be84d834d8259cf3e19c9`
-- PR type for this branch: `docs-only`
+- Branch: `overtrue/arch-migration-ci-gates`
+- Baseline: `upstream/main` at `f00898d0703ac94a4a215a54b6666c747ebe6622`
+- PR type for this branch: `ci-gate`
 - Runtime behavior changes: none
 - Rust code changes: none
-- Repository metadata changes: `.gitignore` now allows tracking only
-  `docs/architecture/` under the otherwise ignored `docs` tree.
+- Script changes: stabilize `scripts/check_layer_dependencies.sh` against the
+  current accepted baseline.
 
 ## Phase 0 Tasks
 
@@ -25,11 +25,11 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
     allowed PR type per PR.
 - [x] `G-004` Define re-export and wrapper policy.
   - Acceptance: temporary compatibility code must use `RUSTFS_COMPAT_TODO`.
-- [~] `G-005` Add dependency direction guard.
-  - Current branch: documentation only.
-  - Next PR: add a `ci-gate` check for forbidden dependency edges.
+- [x] `G-005` Add dependency direction guard.
+  - Acceptance: `./scripts/check_layer_dependencies.sh` passes on current
+    `upstream/main` while still rejecting new unaccepted layer dependencies.
 - [~] `G-006` Create migration loss-prevention checks.
-  - Current branch: documentation only.
+  - Current branch: keeps the existing layer guard stable.
   - Next PR: add checks for public re-export, route matrix, and storage trait
     coverage before pure moves.
 - [x] `G-009` Enforce pre-push three-expert review.
@@ -37,48 +37,56 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
     quality/architecture, migration-preservation, and testing/verification review
     before push.
 - [~] `TEST-PRTYPE-001` Check PR type enum consistency.
-  - Current branch: documentation only.
+  - Current branch: not in scope.
   - Next PR: add a mechanical check that all migration docs use the same PR type
     vocabulary.
 
 ## Next PRs
 
-1. `ci-gate`: extend `scripts/check_layer_dependencies.sh` or add a nearby check
-   for architecture-migration guardrails.
-2. `docs-only` or `test-only`: capture startup timeline and admin route-action
+1. `docs-only` or `test-only`: capture startup timeline and admin route-action
    snapshot.
-3. `docs-only`: inventory `ecstore::config::{Config, KV, KVS}` consumers.
+2. `docs-only`: inventory `ecstore::config::{Config, KV, KVS}` consumers.
+3. `ci-gate`: add focused checks for PR type vocabulary and temporary
+   compatibility marker/register consistency.
 
 ## Pre-Push Review Log
 
 | Expert | Status | Notes |
 |---|---|---|
-| Quality/architecture | pass | Staged diff is scoped to docs, `.gitignore`, and the root architecture index; no over-abstraction or target drift found |
-| Migration preservation | pass | No runtime files changed; notify/audit and IAM/KMS no-drift bullets were added after review feedback |
-| Testing/verification | pass-with-nonblocking-follow-up | Docs-only verification is sufficient; suggested commands are recorded below |
+| Quality/architecture | pass | Final review confirmed the guard scope, baseline logic, and storage target classification; the segment-level `storage::ecfs` / `storage::s3_api` follow-up passed |
+| Migration preservation | pass | Final review confirmed no Rust, runtime, storage hot-path, or global-state logic changes and no missing `RUSTFS_COMPAT_TODO` marker |
+| Testing/verification | pass | Final review accepted the full gate, focused guard checks, stale-baseline check, and negative baseline-removal check as sufficient for this `ci-gate` PR |
 
 ## Verification Notes
 
 Passed:
 
-- `git diff --cached --check`
-- `git diff --cached --name-only -- '*.rs' 'Cargo.toml' 'Cargo.lock' '.github/**' 'scripts/**' 'Makefile' 'Justfile'`
-- `git diff --cached --exit-code -- '*.rs' 'Cargo.toml' 'Cargo.lock' '.github/**' 'scripts/**' 'Makefile' 'Justfile'`
-- `printf '%s\n' docs/architecture/overview.md docs/foo.md docs/other/file.md | git check-ignore -v --stdin --no-index`
-- `git rev-parse upstream/main`
-- `git log -1 --format='%H %s' upstream/main`
+- `./scripts/check_layer_dependencies.sh`
 - `./scripts/check_metrics_migration_refs.sh`
-
-Known unrelated baseline issue:
-
-- `./scripts/check_layer_dependencies.sh` currently fails on `upstream/main`
-  because the script output and `scripts/layer-dependency-baseline.txt` format are
-  out of sync. Keep that fix in the next `ci-gate` PR.
+- `bash -n scripts/check_layer_dependencies.sh`
+- `make pre-commit`
+- temporary negative check that adds an unaccepted reverse dependency to a copied
+  fixture and confirms the guard fails
+- temporary negative check that adds single-line root grouped
+  `use crate::{admin::bad};` and confirms the guard fails
+- temporary negative check that adds multiline root grouped
+  `use crate::{ ... }` and confirms the guard fails
+- temporary negative check that adds root grouped alias
+  `use crate::{admin as admin_alias};` and confirms the guard fails
+- temporary negative check that adds grouped `self` imports and confirms the
+  guard fails
+- temporary negative check that adds a stale baseline row and confirms the guard
+  fails
+- temporary negative check that removes the accepted
+  `crate::storage::ecfs::FS` baseline row for `rustfs/src/protocols/client.rs`
+  and confirms the guard fails
+- `git diff --check`
+- focused shell review of changed scripts
 
 ## Handoff Notes
 
 - Keep Phase 0 PRs small. Do not start Config, Storage API, Runtime, or ECStore
-  movement inside this docs branch.
+  movement inside this `ci-gate` branch.
 - Keep CI checks in a separate `ci-gate` PR so the PR type rule remains enforceable.
 - Do not add temporary compatibility code without a matching
   `RUSTFS_COMPAT_TODO(<task-id>)` marker and cleanup-register entry.

scripts/check_layer_dependencies.sh

@@ -27,6 +27,7 @@ classify_source_layer() {
 classify_target_layer() {
   local path="$1"
   local root="${path%%::*}"
+  local storage_path
 
   case "$root" in
     app)
@@ -36,7 +37,9 @@ classify_target_layer() {
       printf 'interface'
       ;;
     storage)
-      if [[ "$path" == ecfs* ]] || [[ "$path" == s3_api* ]]; then
+      storage_path="${path#storage::}"
+      if [[ "$storage_path" == "ecfs" ]] || [[ "$storage_path" == ecfs::* ]] ||
+        [[ "$storage_path" == "s3_api" ]] || [[ "$storage_path" == s3_api::* ]]; then
         printf 'interface'
       else
         printf 'infra'
@@ -65,6 +68,168 @@ layer_rank() {
   esac
 }
 
+normalize_import_group_item() {
+  local prefix="$1"
+  local item="$2"
+  local path nested_prefix nested_items
+
+  item="$(sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//; s/[[:space:]]+as[[:space:]].*$//' <<<"$item")"
+  item="$(sed -E 's/[[:space:]]//g' <<<"$item")"
+  prefix="$(sed -E 's/[[:space:]]//g' <<<"$prefix")"
+
+  [[ -z "$item" ]] && return 0
+  if [[ "$item" == "self" ]]; then
+    prefix="${prefix%::}"
+    [[ -n "$prefix" ]] && printf '%s\n' "$prefix"
+    return 0
+  fi
+
+  if [[ "$item" == *"{"* ]]; then
+    nested_prefix="${item%%\{*}"
+    nested_items="${item#*\{}"
+    nested_items="${nested_items%\}*}"
+    nested_prefix="${nested_prefix%::}"
+    if [[ -n "$prefix" ]]; then
+      nested_prefix="${prefix}::${nested_prefix}"
+    fi
+    normalize_import_group "$nested_prefix" "$nested_items"
+    return 0
+  fi
+
+  if [[ -n "$prefix" ]]; then
+    path="${prefix}::${item}"
+  else
+    path="$item"
+  fi
+
+  if [[ "$path" == *"::*" ]]; then
+    path="${path%::*}"
+  fi
+  while [[ "$path" == *"::" ]]; do
+    path="${path%::}"
+  done
+
+  [[ -n "$path" ]] && printf '%s\n' "$path"
+}
+
+normalize_import_group() {
+  local prefix="$1"
+  local group="$2"
+  local item="" char depth=0 i
+
+  for ((i = 0; i < ${#group}; i++)); do
+    char="${group:i:1}"
+    case "$char" in
+      "{")
+        depth=$((depth + 1))
+        ;;
+      "}")
+        depth=$((depth - 1))
+        ;;
+      ",")
+        if (( depth == 0 )); then
+          normalize_import_group_item "$prefix" "$item"
+          item=""
+          continue
+        fi
+        ;;
+    esac
+    item+="$char"
+  done
+
+  normalize_import_group_item "$prefix" "$item"
+}
+
+normalize_import_path() {
+  local text="$1"
+  local path
+
+  path="$(sed -E 's/.*use[[:space:]]+crate::([^;]+);?.*/\1/' <<<"$text")"
+  if [[ -z "$path" ]] || [[ "$path" == "$text" ]]; then
+    return 0
+  fi
+
+  path="$(sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//' <<<"$path")"
+  if [[ "$path" == *"{"* ]]; then
+    local prefix group
+
+    prefix=""
+    group="$path"
+    if [[ "$path" != \{* ]]; then
+      prefix="${path%%\{*}"
+      group="${path#*\{}"
+    else
+      group="${path#\{}"
+    fi
+    group="${group%\}*}"
+    prefix="${prefix%::}"
+    normalize_import_group "$prefix" "$group"
+    return 0
+  fi
+
+  path="$(sed -E 's/[[:space:]]+as[[:space:]].*$//' <<<"$path")"
+  path="$(sed -E 's/[[:space:]]//g' <<<"$path")"
+
+  if [[ "$path" == *"::*" ]]; then
+    path="${path%::*}"
+  fi
+  while [[ "$path" == *"::" ]]; do
+    path="${path%::}"
+  done
+
+  printf '%s\n' "$path"
+}
+
+emit_crate_use_statements() {
+  (cd "$ROOT_DIR" && rg --files -g '*.rs' rustfs/src | while IFS= read -r file; do
+    perl -0777 -ne '
+      while (/\buse\s+crate::.*?;/sg) {
+        my $statement = $&;
+        my $line = substr($_, 0, $-[0]) =~ tr/\n//;
+        $line += 1;
+        $statement =~ s/\s+/ /g;
+        print "$ARGV:$line:$statement\n";
+      }
+    ' "$file"
+  done)
+}
+
+normalize_baseline_file() {
+  local input="$1"
+  local output="$2"
+  local line status first second third
+
+  : >"$output"
+
+  while IFS= read -r line; do
+    [[ -z "$line" ]] && continue
+    [[ "$line" == \#* ]] && continue
+
+    IFS='|' read -r status first second third _ <<<"$line"
+    case "$status" in
+      dep)
+        if [[ -n "$first" ]] && [[ -n "$second" ]] && [[ -n "$third" ]]; then
+          printf 'dep|%s|%s|%s\n' "$first" "$second" "$third" >>"$output"
+        fi
+        ;;
+      cycle)
+        if [[ -n "$first" ]]; then
+          printf 'cycle|%s\n' "$first" >>"$output"
+        fi
+        ;;
+      accepted | todo)
+        if [[ "$first" == "cycle" ]] && [[ -n "$second" ]]; then
+          printf 'cycle|%s\n' "$second" >>"$output"
+        elif [[ -n "$first" ]] && [[ -n "$second" ]] && [[ -n "$third" ]]; then
+          printf 'dep|%s|%s|%s\n' "$first" "$second" "$third" >>"$output"
+        fi
+        ;;
+    esac
+  done <"$input"
+
+  sort -u -o "$output" "$output"
+}
+
 TMP_DIR="$(mktemp -d)"
 trap 'rm -rf "$TMP_DIR"' EXIT
 
@@ -86,28 +251,28 @@ while IFS= read -r line; do
     continue
   fi
 
-  import_path="$(sed -E 's/.*use[[:space:]]+crate::([^;{ ]+).*/\1/' <<<"$text")"
-  if [[ -z "$import_path" ]] || [[ "$import_path" == "$text" ]]; then
-    continue
-  fi
+  while IFS= read -r import_path; do
+    if [[ -z "$import_path" ]]; then
+      continue
+    fi
 
-  target_layer="$(classify_target_layer "$import_path")"
-  if [[ "$target_layer" == "unknown" ]]; then
-    continue
-  fi
+    target_layer="$(classify_target_layer "$import_path")"
+    if [[ "$target_layer" == "unknown" ]]; then
+      continue
+    fi
 
-  if [[ "$source_layer" != "$target_layer" ]]; then
-    printf '%s->%s\n' "$source_layer" "$target_layer" >>"$EDGES_RAW"
-  fi
+    if [[ "$source_layer" != "$target_layer" ]]; then
+      printf '%s->%s\n' "$source_layer" "$target_layer" >>"$EDGES_RAW"
+    fi
 
-  source_rank="$(layer_rank "$source_layer")"
-  target_rank="$(layer_rank "$target_layer")"
+    source_rank="$(layer_rank "$source_layer")"
+    target_rank="$(layer_rank "$target_layer")"
 
-  if (( source_rank < target_rank )); then
-    printf 'dep|%s|%s->%s|crate::%s\n' "$file" "$source_layer" "$target_layer" "$import_path" >>"$VIOLATIONS_RAW"
-    printf '%s:%s reverse dependency %s->%s via crate::%s\n' "$file" "$lineno" "$source_layer" "$target_layer" "$import_path"
-  fi
-done < <(cd "$ROOT_DIR" && rg -n --no-heading -g '*.rs' 'use[[:space:]]+crate::' rustfs/src)
+    if (( source_rank < target_rank )); then
+      printf 'dep|%s|%s->%s|crate::%s\n' "$file" "$source_layer" "$target_layer" "$import_path" >>"$VIOLATIONS_RAW"
+    fi
+  done < <(normalize_import_path "$text")
+done < <(emit_crate_use_statements)
 
 sort -u "$VIOLATIONS_RAW" >"${TMP_DIR}/violations_sorted.txt"
 
@@ -138,15 +303,24 @@ if [[ ! -f "$BASELINE_FILE" ]]; then
   exit 1
 fi
 
-sort -u "$BASELINE_FILE" >"${TMP_DIR}/baseline_sorted.txt"
+normalize_baseline_file "$BASELINE_FILE" "${TMP_DIR}/baseline_sorted.txt"
 
 NEW_ITEMS="${TMP_DIR}/new_items.txt"
 comm -13 "${TMP_DIR}/baseline_sorted.txt" "$CURRENT_BASELINE" >"$NEW_ITEMS"
 
+STALE_ITEMS="${TMP_DIR}/stale_items.txt"
+comm -23 "${TMP_DIR}/baseline_sorted.txt" "$CURRENT_BASELINE" >"$STALE_ITEMS"
+
 if [[ -s "$NEW_ITEMS" ]]; then
   echo "Layer dependency guard failed: new reverse dependencies or cycles detected"
   cat "$NEW_ITEMS"
   exit 1
 fi
 
+if [[ -s "$STALE_ITEMS" ]]; then
+  echo "Layer dependency guard failed: stale baseline entries detected"
+  cat "$STALE_ITEMS"
+  exit 1
+fi
+
 echo "Layer dependency guard passed (no new reverse dependencies/cycles)."

scripts/layer-dependency-baseline.txt

@@ -5,15 +5,56 @@
 # Since they live inside one Cargo crate, Rust doesn't enforce separation.
 # The list is maintained for architectural awareness during code review.
 #
-# Format: status|source_file|direction|imported_symbol
+# Format for dependency entries:
+#   status|source_file|direction|imported_symbol|reason
+#
+# Format for conceptual cycles:
+#   status|cycle|direction_pair|reason
 #
 # Status:
 #   accepted - reviewed and intentionally allowed
 #   todo     - should be resolved in a future refactor
 
-accepted|rustfs/src/main.rs|infra->app|crate::app::context::*|main wires all layers
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::admin::handlers::site_replication::site_replication_bucket_meta_hook|bucket use-case calls site replication hooks
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::admin::handlers::site_replication::site_replication_delete_bucket_hook|bucket use-case calls site replication hooks
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::admin::handlers::site_replication::site_replication_make_bucket_hook|bucket use-case calls site replication hooks
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::ListObjectVersionsParams|bucket use-case uses current S3 API bucket helper types
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::ListObjectsV2Params|bucket use-case uses current S3 API bucket helper types
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::build_list_buckets_output|bucket use-case uses current S3 API bucket response helpers
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::build_list_object_versions_output|bucket use-case uses current S3 API bucket response helpers
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::build_list_objects_output|bucket use-case uses current S3 API bucket response helpers
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::build_list_objects_v2_output|bucket use-case uses current S3 API bucket response helpers
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::parse_list_object_versions_params|bucket use-case uses current S3 API bucket query parser
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::bucket::parse_list_objects_v2_params|bucket use-case uses current S3 API bucket query parser
+accepted|rustfs/src/app/bucket_usecase.rs|app->interface|crate::storage::s3_api::common::rustfs_owner|bucket use-case uses current S3 API owner helper
+accepted|rustfs/src/app/lifecycle_transition_api_test.rs|app->interface|crate::storage::ecfs::FS|app test exercises ECFS through current interface path
+accepted|rustfs/src/app/multipart_usecase.rs|app->interface|crate::storage::s3_api::multipart::ListMultipartUploadsParams|multipart use-case uses current S3 API multipart helper types
+accepted|rustfs/src/app/multipart_usecase.rs|app->interface|crate::storage::s3_api::multipart::build_list_multipart_uploads_output|multipart use-case uses current S3 API multipart response helpers
+accepted|rustfs/src/app/multipart_usecase.rs|app->interface|crate::storage::s3_api::multipart::build_list_parts_output|multipart use-case uses current S3 API multipart response helpers
+accepted|rustfs/src/app/multipart_usecase.rs|app->interface|crate::storage::s3_api::multipart::parse_list_multipart_uploads_params|multipart use-case uses current S3 API multipart query parser
+accepted|rustfs/src/app/multipart_usecase.rs|app->interface|crate::storage::s3_api::multipart::parse_list_parts_params|multipart use-case uses current S3 API multipart query parser
+accepted|rustfs/src/app/multipart_usecase.rs|app->interface|crate::storage::s3_api::multipart::parse_upload_part_number|multipart use-case uses current S3 API multipart part parser
+accepted|rustfs/src/app/object_usecase.rs|app->interface|crate::storage::ecfs|object use-case uses current ECFS interface helpers
+accepted|rustfs/src/app/object_usecase.rs|app->interface|crate::storage::s3_api::multipart::parse_list_parts_params|object use-case uses current S3 API multipart query parser
+accepted|rustfs/src/init.rs|infra->interface|crate::admin|init wires the admin module during process setup
+accepted|rustfs/src/protocols/client.rs|infra->interface|crate::storage::ecfs::FS|protocol client delegates to the current ECFS S3 implementation
 accepted|rustfs/src/server/audit.rs|infra->app|crate::app::context::resolve_server_config|config resolution uses global AppContext
 accepted|rustfs/src/server/event.rs|infra->app|crate::app::context::resolve_server_config|config resolution uses global AppContext
 accepted|rustfs/src/server/http.rs|infra->interface|crate::admin|HTTP server routes to admin handlers
 accepted|rustfs/src/server/layer.rs|infra->interface|crate::admin::console::is_console_path|pure path predicate for routing
-accepted|rustfs/src/storage/objects/put_object.rs|infra->app|crate::app::context::resolve_bucket_metadata_handle|metadata resolution uses global AppContext
+accepted|rustfs/src/server/layer.rs|infra->interface|crate::admin::handlers::health::HealthProbe|HTTP layer builds health responses
+accepted|rustfs/src/server/layer.rs|infra->interface|crate::admin::handlers::health::build_health_response_parts|HTTP layer builds health responses
+accepted|rustfs/src/startup_iam.rs|infra->app|crate::app::context::AppContext|startup wires IAM bootstrap through AppContext
+accepted|rustfs/src/startup_iam.rs|infra->app|crate::app::context::get_global_app_context|startup wires IAM bootstrap through AppContext
+accepted|rustfs/src/startup_iam.rs|infra->app|crate::app::context::init_global_app_context|startup wires IAM bootstrap through AppContext
+accepted|rustfs/src/storage/ecfs_extend.rs|infra->interface|crate::storage::ecfs::ListObjectUnorderedQuery|storage extension uses current ECFS interface query type
+accepted|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::ecfs::FS|storage tests exercise current ECFS interface path
+accepted|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::ecfs::validate_object_lock_configuration_input|storage tests exercise current ECFS object-lock validator
+accepted|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::s3_api::common::rustfs_initiator|storage tests use current S3 API initiator helper
+accepted|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::s3_api::common::rustfs_owner|storage tests use current S3 API owner helper
+accepted|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::config::reload_dynamic_config_runtime_state|node RPC reloads admin runtime state
+accepted|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::config::reload_runtime_config_snapshot|node RPC reloads admin runtime state
+accepted|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::site_replication::reload_site_replication_runtime_state|node RPC reloads admin runtime state
+accepted|cycle|app<->infra|existing intra-crate cross-layer references
+accepted|cycle|app<->interface|existing intra-crate cross-layer references
+accepted|cycle|infra<->interface|existing intra-crate cross-layer references