diff --git a/.agents/skills/arch-checks/SKILL.md b/.agents/skills/arch-checks/SKILL.md index d355a4fbd..a95347830 100644 --- a/.agents/skills/arch-checks/SKILL.md +++ b/.agents/skills/arch-checks/SKILL.md @@ -10,10 +10,16 @@ never weaken a check to get green. ## `check_layer_dependencies.sh` — layer DAG in `rustfs/src` -Enforces `interface (admin, storage/ecfs, storage/s3_api) → app → infra`; no -upward imports. Known legacy violations live in +Enforces `composition (server, startup/init) → interface (admin, +storage/ecfs, storage/s3_api) → app → infra`; no upward imports. Server source +files are composition roots, while imports of their exported HTTP contracts +are classified as interface dependencies. Known legacy violations live in `scripts/layer-dependency-baseline.txt`. +Dedicated `*_test.rs` and `tests/` modules are outside this production guard. +Inline `#[cfg(test)]` imports remain checked under their source file's layer; +move architecture-crossing test scaffolding into a dedicated test module. + - **New violation**: restructure your change so the dependency points downward (move the shared type/function to the lower layer). - **You legitimately removed a baseline entry**: run diff --git a/scripts/check_layer_dependencies.sh b/scripts/check_layer_dependencies.sh index 32f41fc07..bbb445feb 100755 --- a/scripts/check_layer_dependencies.sh +++ b/scripts/check_layer_dependencies.sh @@ -13,7 +13,14 @@ fi classify_source_layer() { local file="$1" - if [[ "$file" == rustfs/src/app/* ]]; then + if [[ "$file" == rustfs/src/server/* ]] || + [[ "$file" == rustfs/src/startup_*.rs ]] || + [[ "$file" == rustfs/src/init.rs ]] || + [[ "$file" == rustfs/src/main.rs ]] || + [[ "$file" == rustfs/src/lib.rs ]] || + [[ "$file" == rustfs/src/embedded.rs ]]; then + printf 'composition' + elif [[ "$file" == rustfs/src/app/* ]]; then printf 'app' elif [[ "$file" == rustfs/src/admin/* ]] || [[ "$file" == rustfs/src/storage/ecfs.rs ]] || [[ "$file" == rustfs/src/storage/s3_api/* ]]; then printf 'interface' @@ -30,6 +37,14 @@ classify_target_layer() { local storage_path case "$root" in + init | main | lib | embedded | startup_*) + printf 'composition' + ;; + server) + # Server files are composition roots when they import lower layers, but + # their exported HTTP contracts belong to the interface boundary. + printf 'interface' + ;; app) printf 'app' ;; @@ -53,6 +68,9 @@ classify_target_layer() { layer_rank() { case "$1" in + composition) + printf '4' + ;; interface) printf '3' ;; @@ -68,6 +86,50 @@ layer_rank() { esac } +is_reverse_dependency() { + local source_rank target_rank + + source_rank="$(layer_rank "$1")" + target_rank="$(layer_rank "$2")" + (( source_rank < target_rank )) +} + +assert_dependency_direction() { + local expected="$1" + local source="$2" + local target="$3" + local actual='allowed' + + if is_reverse_dependency "$source" "$target"; then + actual='reverse' + fi + if [[ "$actual" != "$expected" ]]; then + printf 'Layer dependency guard self-test failed: %s -> %s (expected %s, got %s)\n' \ + "$source" "$target" "$expected" "$actual" >&2 + exit 1 + fi +} + +run_layer_model_self_tests() { + local server_source app_source storage_source server_target admin_target app_target storage_target + + server_source="$(classify_source_layer rustfs/src/server/http.rs)" + app_source="$(classify_source_layer rustfs/src/app/bucket_usecase.rs)" + storage_source="$(classify_source_layer rustfs/src/storage/rpc/node_service.rs)" + server_target="$(classify_target_layer server::http)" + admin_target="$(classify_target_layer admin::router)" + app_target="$(classify_target_layer app::bucket_usecase)" + storage_target="$(classify_target_layer storage::rpc)" + + assert_dependency_direction 'allowed' "$server_source" "$admin_target" + assert_dependency_direction 'allowed' "$server_source" "$app_target" + assert_dependency_direction 'allowed' "$server_source" "$storage_target" + assert_dependency_direction 'reverse' "$app_source" "$server_target" + assert_dependency_direction 'reverse' "$storage_source" "$server_target" + assert_dependency_direction 'reverse' "$app_source" "$admin_target" + assert_dependency_direction 'reverse' "$storage_source" "$admin_target" +} + normalize_import_group_item() { local prefix="$1" local item="$2" @@ -182,6 +244,11 @@ normalize_import_path() { emit_crate_use_statements() { (cd "$ROOT_DIR" && rg --files -g '*.rs' rustfs/src | while IFS= read -r file; do + # The guard is file-scoped: dedicated test modules are excluded, while + # inline #[cfg(test)] imports remain subject to the source file's layer. + if [[ "$file" == *_test.rs ]] || [[ "$file" == */tests/* ]]; then + continue + fi perl -0777 -ne ' while (/\buse\s+crate::.*?;/sg) { my $statement = $&; @@ -194,6 +261,26 @@ emit_crate_use_statements() { done) } +write_baseline_file() { + local entries="$1" + + cat >"$BASELINE_FILE" <<'EOF' +# Layer dependency baseline for the rustfs binary crate. +# +# The guard models production imports as: +# composition -> interface -> app -> infra +# +# Canonical dependency entry: +# dep|source_file|source_layer->target_layer|crate::imported_symbol +# +# Canonical conceptual cycle entry: +# cycle|left_layer<->right_layer +EOF + cat "$entries" >>"$BASELINE_FILE" +} + +run_layer_model_self_tests + normalize_baseline_file() { local input="$1" local output="$2" @@ -265,10 +352,7 @@ while IFS= read -r line; do printf '%s->%s\n' "$source_layer" "$target_layer" >>"$EDGES_RAW" fi - source_rank="$(layer_rank "$source_layer")" - target_rank="$(layer_rank "$target_layer")" - - if (( source_rank < target_rank )); then + if is_reverse_dependency "$source_layer" "$target_layer"; then printf 'dep|%s|%s->%s|crate::%s\n' "$file" "$source_layer" "$target_layer" "$import_path" >>"$VIOLATIONS_RAW" fi done < <(normalize_import_path "$text") @@ -292,7 +376,7 @@ done <"${TMP_DIR}/edges_sorted.txt" | sort -u >"${TMP_DIR}/cycles_sorted.txt" cat "${TMP_DIR}/violations_sorted.txt" "${TMP_DIR}/cycles_sorted.txt" | sort -u >"$CURRENT_BASELINE" if [[ "$MODE" == "update" ]]; then - cp "$CURRENT_BASELINE" "$BASELINE_FILE" + write_baseline_file "$CURRENT_BASELINE" echo "Updated baseline: $BASELINE_FILE" exit 0 fi diff --git a/scripts/layer-dependency-baseline.txt b/scripts/layer-dependency-baseline.txt index 9b41b7b12..33354b8eb 100644 --- a/scripts/layer-dependency-baseline.txt +++ b/scripts/layer-dependency-baseline.txt @@ -1,35 +1,44 @@ # Layer dependency baseline for the rustfs binary crate. # -# These are intra-crate module references within rustfs/ that cross the -# conceptual layer boundaries (app, infra/server, interface/admin). -# Since they live inside one Cargo crate, Rust doesn't enforce separation. -# The list is maintained for architectural awareness during code review. +# The guard models production imports as: +# composition -> interface -> app -> infra # -# Format for dependency entries: -# status|source_file|direction|imported_symbol|reason +# Canonical dependency entry: +# dep|source_file|source_layer->target_layer|crate::imported_symbol # -# Format for conceptual cycles: -# status|cycle|direction_pair|reason -# -# Status: -# accepted - reviewed and intentionally allowed -# todo - should be resolved in a future refactor - +# Canonical conceptual cycle entry: +# cycle|left_layer<->right_layer cycle|app<->infra cycle|app<->interface +cycle|composition<->infra +cycle|composition<->interface cycle|infra<->interface +dep|rustfs/src/admin/handlers/scanner.rs|interface->composition|crate::startup_background::ENV_SCANNER_ENABLED +dep|rustfs/src/admin/handlers/scanner.rs|interface->composition|crate::startup_background::scanner_enabled_from_env +dep|rustfs/src/app/admin_usecase.rs|app->interface|crate::server::DependencyReadiness +dep|rustfs/src/app/admin_usecase.rs|app->interface|crate::server::collect_dependency_readiness_report dep|rustfs/src/app/bucket_usecase.rs|app->interface|crate::admin::handlers::site_replication::site_replication_bucket_meta_hook dep|rustfs/src/app/bucket_usecase.rs|app->interface|crate::admin::handlers::site_replication::site_replication_delete_bucket_hook dep|rustfs/src/app/bucket_usecase.rs|app->interface|crate::admin::handlers::site_replication::site_replication_make_bucket_hook -dep|rustfs/src/init.rs|infra->interface|crate::admin +dep|rustfs/src/app/bucket_usecase.rs|app->interface|crate::server::RemoteAddr +dep|rustfs/src/app/object_usecase.rs|app->interface|crate::server::convert_ecstore_object_info +dep|rustfs/src/cluster_snapshot.rs|infra->interface|crate::server::DependencyReadiness +dep|rustfs/src/cluster_snapshot.rs|infra->interface|crate::server::DependencyReadinessReport +dep|rustfs/src/cluster_snapshot.rs|infra->interface|crate::server::ReadinessDegradedReason +dep|rustfs/src/cluster_snapshot.rs|infra->interface|crate::server::snapshot_dependency_readiness_report dep|rustfs/src/runtime_sources.rs|infra->app|crate::app::context -dep|rustfs/src/server/http.rs|infra->interface|crate::admin -dep|rustfs/src/server/layer.rs|infra->interface|crate::admin::console::is_console_path +dep|rustfs/src/storage/access.rs|infra->interface|crate::server::RemoteAddr +dep|rustfs/src/storage/ecfs_extend.rs|infra->interface|crate::server::cors dep|rustfs/src/storage/ecfs_extend.rs|infra->interface|crate::storage::ecfs::ListObjectUnorderedQuery -dep|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::ecfs::FS -dep|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::ecfs::validate_object_lock_configuration_input -dep|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::s3_api::common::rustfs_initiator -dep|rustfs/src/storage/ecfs_test.rs|infra->interface|crate::storage::s3_api::common::rustfs_owner +dep|rustfs/src/storage/helper.rs|infra->interface|crate::server::convert_ecstore_object_info +dep|rustfs/src/storage/helper.rs|infra->interface|crate::server::is_audit_module_enabled +dep|rustfs/src/storage/helper.rs|infra->interface|crate::server::is_notify_module_enabled +dep|rustfs/src/storage/helper.rs|infra->interface|crate::server::refresh_audit_module_enabled +dep|rustfs/src/storage/helper.rs|infra->interface|crate::server::refresh_notify_module_enabled +dep|rustfs/src/storage/rpc/http_service.rs|infra->interface|crate::server::RPC_PREFIX dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::config::reload_dynamic_config_runtime_state dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::config::reload_runtime_config_snapshot dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::admin::service::site_replication::reload_site_replication_runtime_state +dep|rustfs/src/storage/rpc/node_service.rs|infra->interface|crate::server::MODULE_SWITCHES_SIGNAL_SUBSYSTEM +dep|rustfs/src/storage/rpc/node_service/heal.rs|infra->composition|crate::startup_background::heal_enabled_from_env +dep|rustfs/src/storage/rpc/node_service/heal.rs|infra->composition|crate::startup_background::scanner_enabled_from_env