Simplify dependency semantics with conservative auto-deps and branch commit pinning (#558)

* resolve: simplify fetch concurrency paths

* auto_deps: require materialization before adding deps

* auto_deps: make phase explicitly online-only

* auto_deps: pin branch deps and add branch regression tests

* changelog: note auto-deps online-only phase

* update: refresh branch+rev pins and cover with integration test

* auto_deps: drop lockfile URL mapping and simplify resolution flow

* cache_index: unify remote lookup API with explicit policy

* dedup resolve/update logic and streamline auto-deps

* test(pcb): add resilient auto-dep branch refresh coverage

* chore: drop branch-rev pinning spec draft

* refactor: simplify and dedupe dependency resolution paths

* fix(resolve): ignore stale lockfile pseudo-version when rev changed

* test(cache_index): avoid network on remote package miss assertion

Author: akhilles

CHANGELOG.md

@@ -11,6 +11,9 @@ and this project adheres to Semantic Versioning (https://semver.org/spec/v2.0.0.
 ### Fixed
 
 - Reduced `layout.sync` false positives in publish/check flows by normalizing `.kicad_pro` newline writes and ignoring trailing whitespace-only drift when comparing synced layout files.
+- Simplified dependency fetch/index concurrency paths and reuse a shared cache index during resolve/fetch phases to reduce open-file pressure on macOS.
+- Auto-deps is now conservative and online-only: it adds remote deps only after successful materialization, skips imports already covered by existing `dependencies`/`assets`, and no longer infers missing deps from `pcb.sum`.
+- Branch-based dependencies now require commit pinning for reproducibility: online resolve/update pins `branch` deps to `rev`, while `--locked`/`--offline` reject branch-only declarations.
 
 ## [0.3.43] - 2026-02-18
 

crates/pcb-zen/src/auto_deps.rs

@@ -8,9 +8,11 @@ use std::collections::{BTreeMap, HashMap};
 use std::path::{Path, PathBuf};
 
 use crate::ast_utils::{skip_vendor, visit_string_literals};
-use crate::cache_index::{cache_base, find_lockfile_entry, CacheIndex};
+use crate::cache_index::CacheIndex;
 use crate::git;
-use pcb_zen_core::config::{AssetDependencySpec, DependencySpec, Lockfile, PcbToml, KICAD_ASSETS};
+use crate::resolve::{fetch_asset_repo, fetch_package};
+use crate::workspace::WorkspaceInfo;
+use pcb_zen_core::config::{AssetDependencySpec, DependencySpec, PcbToml, KICAD_ASSETS};
 use pcb_zen_core::DefaultFileProvider;
 
 #[derive(Debug, Default)]
@@ -20,7 +22,6 @@ pub struct AutoDepsSummary {
     pub packages_updated: usize,
     pub unknown_aliases: Vec<(PathBuf, Vec<String>)>,
     pub unknown_urls: Vec<(PathBuf, Vec<String>)>,
-    pub discovered_remote: usize,
     pub stdlib_removed: usize,
 }
 
@@ -30,115 +31,82 @@ struct CollectedImports {
     urls: HashSet<String>,
 }
 
+#[derive(Debug, Clone)]
+struct ResolvedDep {
+    module_path: String,
+    version: String,
+    is_asset: bool,
+}
+
+impl ResolvedDep {
+    fn package(module_path: String, version: String) -> Self {
+        Self {
+            module_path,
+            version,
+            is_asset: false,
+        }
+    }
+
+    fn asset(module_path: String, version: String) -> Self {
+        Self {
+            module_path,
+            version,
+            is_asset: true,
+        }
+    }
+}
+
 /// Scan workspace for .zen files and auto-add missing dependencies to pcb.toml files
 ///
 /// Resolution order for URL imports:
 /// 1. Workspace members (local packages)
-/// 2. Lockfile entries (pcb.sum) - fast path, no git operations
-/// 3. Remote package discovery (git tags) - slow path, cached per repo (skipped when offline)
-pub fn auto_add_zen_deps(
-    workspace_root: &Path,
-    packages: &BTreeMap<String, crate::workspace::MemberPackage>,
-    lockfile: Option<&Lockfile>,
-    offline: bool,
-) -> Result<AutoDepsSummary> {
+/// 2. Remote package discovery (git tags) - slow path, cached per repo
+pub fn auto_add_zen_deps(workspace_info: &WorkspaceInfo) -> Result<AutoDepsSummary> {
+    let workspace_root = &workspace_info.root;
+    let packages = &workspace_info.packages;
     let package_imports = collect_imports_by_package(workspace_root, packages)?;
     let mut summary = AutoDepsSummary::default();
+    let file_provider = DefaultFileProvider::new();
 
-    let index = if !offline {
-        CacheIndex::open().ok()
-    } else {
-        None
-    };
+    let index = CacheIndex::open()?;
 
     for (pcb_toml_path, imports) in package_imports {
-        let mut deps_to_add: Vec<(String, String, bool)> = Vec::new();
-        let mut unknown_aliases: Vec<String> = Vec::new();
+        let existing_config = PcbToml::from_file(&file_provider, &pcb_toml_path)?;
+        let mut deps_to_add: Vec<ResolvedDep> = Vec::new();
+        let unknown_aliases: Vec<String> = imports
+            .aliases
+            .iter()
+            .filter(|alias| alias.as_str() != "stdlib")
+            .cloned()
+            .collect();
         let mut unknown_urls: Vec<String> = Vec::new();
 
-        // Process @alias imports
-        // Note: @stdlib is handled implicitly by the toolchain, no need to add to [dependencies]
-        for alias in &imports.aliases {
-            if alias != "stdlib" {
-                unknown_aliases.push(alias.clone());
-            }
-        }
-
         // Process URL imports
-        let cache = cache_base();
         for url in &imports.urls {
-            // Check if this is a known KiCad asset with subpath
-            if let Some(version) = get_kicad_asset_version(url) {
-                // Opportunistically verify path exists if we have the repo cached
-                let (repo_url, subpath) = git::split_asset_repo_and_subpath(url);
-                let repo_cache_dir = cache.join(repo_url).join(&version);
-                if repo_cache_dir.exists() && !subpath.is_empty() {
-                    let target_path = repo_cache_dir.join(subpath);
-                    if !target_path.exists() {
-                        // Path doesn't exist in cached repo, skip auto-dep
-                        unknown_urls.push(url.clone());
-                        continue;
-                    }
-                }
-                deps_to_add.push((url.clone(), version, true));
-                continue;
-            }
-
-            // Try workspace members first
-            if let Some((package_url, version)) = find_matching_workspace_member(url, packages) {
-                deps_to_add.push((package_url, version, false));
+            if is_url_covered_by_manifest(url, &existing_config) {
                 continue;
             }
 
-            // Try lockfile
-            if let Some(lf) = lockfile {
-                if let Some((module_path, version)) = find_lockfile_entry(url, lf) {
-                    deps_to_add.push((module_path, version, false));
-                    continue;
-                }
-            }
-
-            // Try sqlite cache
-            if let Some(ref idx) = index {
-                if let Some((module_path, version)) = idx.find_remote_package(url) {
-                    deps_to_add.push((module_path, version, false));
-                    continue;
-                }
-            }
+            let candidate = resolve_dep_candidate(url, packages, &index);
 
-            // Fetch and populate cache (only if online)
-            if offline {
+            let Some(candidate) = candidate else {
                 unknown_urls.push(url.clone());
                 continue;
-            }
+            };
 
-            if let Some(ref idx) = index {
-                match idx.find_or_discover_remote_package(url) {
-                    Ok(Some((module_path, version))) => {
-                        deps_to_add.push((module_path, version, false));
-                        summary.discovered_remote += 1;
-                    }
-                    Ok(None) => unknown_urls.push(url.clone()),
-                    Err(e) => {
-                        eprintln!("  Warning: Failed to discover package for {}: {}", url, e);
-                        unknown_urls.push(url.clone());
-                    }
-                }
+            if can_materialize_dep(workspace_info, &index, &candidate) {
+                deps_to_add.push(candidate);
             } else {
                 unknown_urls.push(url.clone());
             }
         }
 
-        if !unknown_aliases.is_empty() {
-            summary
-                .unknown_aliases
-                .push((pcb_toml_path.clone(), unknown_aliases));
-        }
-        if !unknown_urls.is_empty() {
-            summary
-                .unknown_urls
-                .push((pcb_toml_path.clone(), unknown_urls));
-        }
+        push_unknown(
+            &mut summary.unknown_aliases,
+            &pcb_toml_path,
+            unknown_aliases,
+        );
+        push_unknown(&mut summary.unknown_urls, &pcb_toml_path, unknown_urls);
 
         let (added, corrected) =
             add_and_correct_dependencies(&pcb_toml_path, &deps_to_add, packages)?;
@@ -155,6 +123,116 @@ pub fn auto_add_zen_deps(
     Ok(summary)
 }
 
+fn is_url_covered_by_manifest(url: &str, config: &PcbToml) -> bool {
+    config
+        .dependencies
+        .keys()
+        .any(|dep| dep_covers_url(dep, url))
+        || config.assets.keys().any(|asset| dep_covers_url(asset, url))
+}
+
+fn dep_covers_url(dep: &str, url: &str) -> bool {
+    if dep == url {
+        return true;
+    }
+
+    url.strip_prefix(dep)
+        .is_some_and(|rest| rest.starts_with('/'))
+}
+
+fn push_unknown(summary: &mut Vec<(PathBuf, Vec<String>)>, path: &Path, items: Vec<String>) {
+    if items.is_empty() {
+        return;
+    }
+    summary.push((path.to_path_buf(), items));
+}
+
+fn can_materialize_dep(
+    workspace_info: &WorkspaceInfo,
+    index: &CacheIndex,
+    dep: &ResolvedDep,
+) -> bool {
+    if dep.is_asset {
+        let (repo_url, subpath) = git::split_asset_repo_and_subpath(&dep.module_path);
+        let asset_key = dep.module_path.clone();
+        let result = fetch_asset_repo(workspace_info, repo_url, &dep.version, &[asset_key], false)
+            .and_then(|base| {
+                let target = if subpath.is_empty() {
+                    base
+                } else {
+                    base.join(subpath)
+                };
+                anyhow::ensure!(
+                    target.exists(),
+                    "Asset subpath '{}' not found in {}@{}",
+                    subpath,
+                    repo_url,
+                    dep.version
+                );
+                Ok(())
+            });
+
+        if let Err(e) = result {
+            log::debug!(
+                "Skipping auto-dep asset {}@{} (materialization failed): {}",
+                dep.module_path,
+                dep.version,
+                e
+            );
+            return false;
+        }
+        return true;
+    }
+
+    let Some(parsed_version) = crate::tags::parse_relaxed_version(&dep.version) else {
+        log::debug!(
+            "Skipping auto-dep package {}@{} (invalid version)",
+            dep.module_path,
+            dep.version
+        );
+        return false;
+    };
+
+    if let Err(e) = fetch_package(
+        workspace_info,
+        &dep.module_path,
+        &parsed_version,
+        index,
+        false,
+    ) {
+        log::debug!(
+            "Skipping auto-dep package {}@{} (materialization failed): {}",
+            dep.module_path,
+            dep.version,
+            e
+        );
+        return false;
+    }
+
+    true
+}
+
+fn resolve_dep_candidate(
+    url: &str,
+    packages: &BTreeMap<String, crate::workspace::MemberPackage>,
+    index: &CacheIndex,
+) -> Option<ResolvedDep> {
+    get_kicad_asset_version(url)
+        .map(|version| ResolvedDep::asset(url.to_string(), version))
+        .or_else(|| {
+            find_matching_workspace_member(url, packages)
+                .map(|(module_path, version)| ResolvedDep::package(module_path, version))
+        })
+        .or_else(|| match index.find_remote_package(url) {
+            Ok(Some(dep)) => Some(ResolvedDep::package(dep.module_path, dep.version)),
+            Ok(None) => None,
+            Err(e) => {
+                eprintln!("  Warning: Failed to discover package for {}: {}", url, e);
+                None
+            }
+        })
+}
+
 /// Get the version for a known KiCad asset URL (returns None if not a KiCad asset with subpath)
 fn get_kicad_asset_version(url: &str) -> Option<String> {
     for (_, base_url, version) in KICAD_ASSETS {
@@ -370,34 +448,28 @@ fn extract_from_str(s: &str, aliases: &mut HashSet<String>, urls: &mut HashSet<S
 /// Add dependencies to a pcb.toml file and correct workspace member versions
 fn add_and_correct_dependencies(
     pcb_toml_path: &Path,
-    deps: &[(String, String, bool)],
+    deps: &[ResolvedDep],
     packages: &BTreeMap<String, crate::workspace::MemberPackage>,
 ) -> Result<(usize, usize)> {
     let mut config = PcbToml::from_file(&DefaultFileProvider::new(), pcb_toml_path)?;
     let mut added = 0;
     let mut corrected = 0;
 
-    for (url, version, is_asset) in deps {
-        if config.dependencies.contains_key(url) || config.assets.contains_key(url) {
+    for dep in deps {
+        if is_url_covered_by_manifest(&dep.module_path, &config) {
             continue;
         }
 
-        // For assets, check if already satisfied by a whole-repo entry
-        if *is_asset {
-            let (repo_url, subpath) = git::split_asset_repo_and_subpath(url);
-            if !subpath.is_empty() && config.assets.contains_key(repo_url) {
-                continue;
-            }
-        }
-
-        if *is_asset {
-            config
-                .assets
-                .insert(url.clone(), AssetDependencySpec::Ref(version.clone()));
+        if dep.is_asset {
+            config.assets.insert(
+                dep.module_path.clone(),
+                AssetDependencySpec::Ref(dep.version.clone()),
+            );
         } else {
-            config
-                .dependencies
-                .insert(url.clone(), DependencySpec::Version(version.clone()));
+            config.dependencies.insert(
+                dep.module_path.clone(),
+                DependencySpec::Version(dep.version.clone()),
+            );
         }
         added += 1;
     }