refactor: move package graph review to review boundary

Author: olwang

crates/rsscript-compiler/src/package.rs

@@ -46,7 +46,27 @@ mod check;
 mod dependency {
     pub(super) use rsscript_package_review::*;
 }
-mod graph;
+// Legacy composition only: graph evidence is package-review-owned. The
+// compiler supplies the captured-input authorization and native wrapper seam.
+mod graph {
+    use std::path::Path;
+
+    use rsscript_package_model::{PackageGraphCheck, PackageTree};
+
+    pub(super) fn package_tree_captured(package_dir: &Path) -> Result<PackageTree, String> {
+        rsscript_package_review::package_tree_captured(
+            package_dir,
+            super::native::package_native_rust_review,
+        )
+    }
+
+    pub(super) fn check_package_graph(package_dir: &Path) -> Result<PackageGraphCheck, String> {
+        rsscript_package_review::check_package_graph(
+            package_dir,
+            super::native::package_native_rust_review,
+        )
+    }
+}
 // Legacy composition only: package lock semantics and hashing are owned by
 // `rsscript-package-review`; the compiler supplies native-wrapper callbacks.
 mod lock {
@@ -55,8 +75,7 @@ mod lock {
     use rsscript_package_model::{PackageLock, PackageLockDiff};
 
     pub(super) use rsscript_package_review::{
-        compare_locked_packages, effective_interface_hash, package_lock_diff_reasons,
-        parse_package_lock, read_package_lock,
+        compare_locked_packages, package_lock_diff_reasons, parse_package_lock, read_package_lock,
     };
     pub(super) const PACKAGE_LOCK_MAX_BYTES: u64 = rsscript_package_review::PACKAGE_LOCK_MAX_BYTES;
 
@@ -145,11 +164,14 @@ pub use authorization::{
     prepare_package_for_execution,
 };
 pub use check::check_package_dir;
-use dependency::{
-    PackageDependencySpec, collect_dependency_interface_sources,
-    collect_dependency_lowering_sources,
-};
-pub use graph::package_tree;
+use dependency::{collect_dependency_interface_sources, collect_dependency_lowering_sources};
+pub fn package_tree(package_dir: &Path) -> Result<PackageTree, String> {
+    let snapshot = authorization::snapshot_package_graph_inputs(package_dir)?;
+    let mut tree = graph::package_tree_captured(snapshot.root())
+        .map_err(|error| snapshot.remap_error(error))?;
+    authorization::remap_tree(&snapshot, &mut tree);
+    Ok(tree)
+}
 pub(super) use lock_format::package_lock_toml;
 pub use metadata::package_lowering_input;
 pub(crate) use native::package_native_plugin_build_dependencies;
@@ -214,14 +236,6 @@ pub(super) fn dedup_diagnostics(diagnostics: &mut Vec<Diagnostic>) {
     });
 }
 
-fn package_identity(manifest: &Manifest) -> PackageIdentity {
-    PackageIdentity {
-        name: manifest.package.name.clone(),
-        version: manifest.package.version.clone(),
-        edition: manifest.package.edition.clone(),
-    }
-}
-
 #[cfg(test)]
 mod preparation_limit_tests {
     use super::*;

…s/rsscript-compiler/src/package/graph.rs …tes/rsscript-package-review/src/graph.rscrates/rsscript-compiler/src/package/graph.rs renamed to crates/rsscript-package-review/src/graph.rs crates/rsscript-compiler/src/package/graph.rs renamed to crates/rsscript-package-review/src/graph.rs

@@ -1,46 +1,60 @@
 use std::collections::{BTreeMap, BTreeSet};
 use std::path::Path;
 
-use super::dependency::{
+use crate::dependency::{
     DependencyResolutionScope, ResolvedDependencyEdge, ResolvedDependencyGraph,
     resolve_dependency_graph,
 };
-use super::lock::effective_interface_hash;
-use super::review::review_package_dir_captured_with_features;
-use super::source_set::{
+use crate::lock::effective_interface_hash;
+use crate::review::{NativeRustReviewFn, review_package_dir_captured_with_features};
+use crate::source_set::{
     ManifestDependencyBudget, ManifestProviderChoice, load_package_manifest,
     load_package_with_features,
 };
-use super::{
-    PackageDependencyKind, PackageDependencySpec, PackageGraphCheck, PackageRisk, PackageTree,
-    PackageTreeNode, PackageTreeSummary, package_identity,
+use rsscript_package_model::{
+    PackageDependencyKind, PackageGraphCheck, PackageIdentity, PackageProviderImplementation,
+    PackageReviewFileKind, PackageRisk, PackageTree, PackageTreeNode, PackageTreeSummary,
+    PackageVirtual,
 };
 
-pub fn package_tree(package_dir: &Path) -> Result<PackageTree, String> {
-    let snapshot = super::authorization::snapshot_package_graph_inputs(package_dir)?;
-    let mut tree =
-        package_tree_captured(snapshot.root()).map_err(|error| snapshot.remap_error(error))?;
-    super::authorization::remap_tree(&snapshot, &mut tree);
-    Ok(tree)
+use crate::{Manifest, PackageDependencySpec, PackageSource};
+
+fn package_identity(manifest: &Manifest) -> PackageIdentity {
+    PackageIdentity {
+        name: manifest.package.name.clone(),
+        version: manifest.package.version.clone(),
+        edition: manifest.package.edition.clone(),
+    }
 }
 
-pub(super) fn package_tree_captured(package_dir: &Path) -> Result<PackageTree, String> {
+/// Build a package tree from captured project inputs. Native Rust wrapper
+/// inspection stays outside this boundary and is injected by the legacy
+/// compatibility host.
+pub fn package_tree_captured(
+    package_dir: &Path,
+    native_rust_review: NativeRustReviewFn,
+) -> Result<PackageTree, String> {
     let graph = resolve_dependency_graph(package_dir, DependencyResolutionScope::Development)?;
     let root = package_tree_node(
         &graph,
         &graph.root,
         PackageDependencyKind::Root,
         None,
         &mut BTreeMap::new(),
+        native_rust_review,
     )?;
     let mut summary = PackageTreeSummary::default();
     collect_package_tree_summary(&root, &mut summary, &mut BTreeSet::new());
     Ok(PackageTree { root, summary })
 }
 
-pub(super) fn check_package_graph(package_dir: &Path) -> Result<PackageGraphCheck, String> {
+/// Validate graph-level review facts from captured project inputs.
+pub fn check_package_graph(
+    package_dir: &Path,
+    native_rust_review: NativeRustReviewFn,
+) -> Result<PackageGraphCheck, String> {
     let root_manifest = load_package_manifest(package_dir)?;
-    let tree = package_tree_captured(package_dir)?;
+    let tree = package_tree_captured(package_dir, native_rust_review)?;
     let mut packages_by_name: BTreeMap<String, BTreeSet<String>> = BTreeMap::new();
     collect_package_graph_identities(&tree.root, &mut packages_by_name);
 
@@ -273,7 +287,10 @@ fn canonical_graph_source(source: &str) -> String {
     let Some(path) = source.strip_prefix("path+") else {
         return source.to_string();
     };
-    format!("path+{}", super::canonical_path_label(Path::new(path)))
+    format!(
+        "path+{}",
+        rsscript_project::canonical_project_path_label(Path::new(path))
+    )
 }
 
 fn package_tree_node(
@@ -282,6 +299,7 @@ fn package_tree_node(
     dependency_kind: PackageDependencyKind,
     incoming: Option<&ResolvedDependencyEdge>,
     cache: &mut BTreeMap<(String, PackageDependencyKind), PackageTreeNode>,
+    native_rust_review: NativeRustReviewFn,
 ) -> Result<PackageTreeNode, String> {
     let cache_key = (key.to_string(), dependency_kind);
     if let Some(cached) = cache.get(&cache_key) {
@@ -295,7 +313,11 @@ fn package_tree_node(
     let package_dir = &resolved.package_dir;
     let features = resolved.features.clone();
     let package = load_package_with_features(package_dir, Some(&features))?;
-    let review = review_package_dir_captured_with_features(package_dir, Some(&features))?;
+    let review = review_package_dir_captured_with_features(
+        package_dir,
+        Some(&features),
+        native_rust_review,
+    )?;
     let interface_effective_hash = effective_interface_hash(&package.sources, &features);
     let identity = package_identity(&package.manifest);
     let mut dependencies = Vec::new();
@@ -306,7 +328,14 @@ fn package_tree_node(
             edge.kind
         };
         dependencies.push(match &edge.target {
-            Some(target) => package_tree_node(graph, target, child_kind, Some(edge), cache)?,
+            Some(target) => package_tree_node(
+                graph,
+                target,
+                child_kind,
+                Some(edge),
+                cache,
+                native_rust_review,
+            )?,
             None => {
                 unresolved_dependency_node(edge.spec.clone(), child_kind, unresolved_reasons(edge))
             }
@@ -319,7 +348,7 @@ fn package_tree_node(
         name: identity.name,
         version: Some(identity.version),
         requirement: spec.and_then(|spec| spec.requirement.clone()),
-        source: super::package_path_source(package_dir),
+        source: rsscript_project::project_path_source(package_dir),
         risk: review.risk,
         features,
         native: review.native_rust.is_some(),
@@ -391,14 +420,12 @@ fn unresolved_dependency_node(
     }
 }
 
-fn package_provider_implementations(
-    manifest: &super::Manifest,
-) -> Vec<super::PackageProviderImplementation> {
+fn package_provider_implementations(manifest: &Manifest) -> Vec<PackageProviderImplementation> {
     manifest
         .implements
         .iter()
         .map(
-            |(interface_package, implementation)| super::PackageProviderImplementation {
+            |(interface_package, implementation)| PackageProviderImplementation {
                 interface_package: interface_package.clone(),
                 version: implementation.version.clone(),
                 interface_features: implementation.interface_features.clone(),
@@ -408,23 +435,23 @@ fn package_provider_implementations(
         .collect()
 }
 
-fn package_virtual(manifest: &super::Manifest) -> Option<super::PackageVirtual> {
+fn package_virtual(manifest: &Manifest) -> Option<PackageVirtual> {
     manifest
         .virtual_package
         .as_ref()
-        .map(|virtual_package| super::PackageVirtual {
+        .map(|virtual_package| PackageVirtual {
             has_default: virtual_package.has_default,
             provider: virtual_package.provider.clone(),
         })
 }
 
-fn package_is_interface_only(sources: &[super::PackageSource]) -> bool {
+fn package_is_interface_only(sources: &[PackageSource]) -> bool {
     let has_interface = sources
         .iter()
-        .any(|source| source.kind == super::PackageReviewFileKind::Interface);
+        .any(|source| source.kind == PackageReviewFileKind::Interface);
     let has_source = sources
         .iter()
-        .any(|source| source.kind == super::PackageReviewFileKind::Source);
+        .any(|source| source.kind == PackageReviewFileKind::Source);
     has_interface && !has_source
 }
 

crates/rsscript-package-review/src/lib.rs

@@ -12,6 +12,7 @@ mod contract;
 mod dependency;
 mod diff;
 mod execution_facts;
+mod graph;
 mod lock;
 mod policy;
 mod review;
@@ -25,6 +26,7 @@ pub use contract::*;
 pub use dependency::*;
 pub use diff::*;
 pub use execution_facts::*;
+pub use graph::*;
 pub use lock::*;
 pub use policy::*;
 pub use review::*;

crates/rsscript-sdk/tests/architecture.rs

@@ -2069,6 +2069,23 @@ fn native_package_dependency_model_is_not_owned_by_aot_lowering() {
             && lock.contains("rsscript_project::project_path_source"),
         "package lock must own hashing/comparison and receive native path resolution explicitly"
     );
+    let graph_path = root.join("crates/rsscript-package-review/src/graph.rs");
+    assert!(
+        graph_path.is_file()
+            && !root
+                .join("crates/rsscript-compiler/src/package/graph.rs")
+                .exists(),
+        "package graph evidence must be physically owned by the package-review boundary"
+    );
+    let graph = read(&graph_path);
+    assert!(
+        graph.contains("pub fn package_tree_captured")
+            && graph.contains("pub fn check_package_graph")
+            && graph.contains("NativeRustReviewFn")
+            && graph.contains("rsscript_project::project_path_source")
+            && !graph.contains("authorization::"),
+        "package graph evaluation must consume captured review input and receive native inspection explicitly"
+    );
     assert!(
         dependency.contains("load_package_from_manifest_source")
             && !dependency.contains("load_package_with_features("),
@@ -2083,6 +2100,12 @@ fn native_package_dependency_model_is_not_owned_by_aot_lowering() {
             && !package_module.contains("fn open_regular_file_within_root"),
         "compiler compatibility code must consume project-owned generic I/O primitives instead of reimplementing bounded traversal"
     );
+    assert!(
+        package_module.contains("rsscript_package_review::package_tree_captured")
+            && package_module.contains("snapshot_package_graph_inputs")
+            && package_module.contains("remap_tree"),
+        "compiler package graph compatibility must only authorize captured input and remap public paths"
+    );
 }
 
 #[test]

docs/architecture/adr/0225-own-captured-package-review-inputs-outside-compiler.md

@@ -60,15 +60,21 @@ are review-owned as well. The one native-sensitive operation is an explicit
 rooted-path resolver callback; compiler compatibility retains only that adapter
 and snapshot remapping rather than lock semantics or hashing implementation.
 
+Package graph construction and graph-level review validation are also
+review-owned. They consume the project-captured manifest graph and receive the
+legacy native Rust inspection only as the same explicit callback. Compiler
+compatibility now only authorizes the captured input and remaps snapshot paths
+for its legacy public result.
+
 The compiler's opt-in `package` compatibility feature has a private forwarding
 module during the staged migration so existing authorization, native, lock, and
 review callers retain their established behavior. The reviewed compiler default
 closure remains unchanged and does not select this crate.
 
 ## Consequences
 
-This is the first physical S05.3 migration step, not its completion. Review
-execution, risk/policy, lock/check/diff, and public compatibility composition
-must move next before the forwarding module can be removed. Architecture tests
-assert that the source-set file cannot return under `rsscript-compiler` and
-that its loader continues to consume project-owned bounded capture APIs.
+This is a staged physical S05.3 migration. The remaining package check and
+final public compatibility composition must move before the forwarding module
+can be removed. Architecture tests assert that captured review implementation
+files cannot return under `rsscript-compiler`, while compiler compatibility
+continues to consume project-owned bounded capture APIs.