Fix doctor on fresh clones and pull lock contention (knit)

Cargo.toml

@@ -2,7 +2,7 @@
 # Published as `knit-cli` (the crates.io name `knit` is taken); the installed
 # binary is still `knit`.
 name = "knit-cli"
-version = "0.1.0-alpha.49"
+version = "0.1.0-alpha.50"
 edition = "2021"
 license = "Apache-2.0"
 description = "A local-first CLI for coordinating cross-repo feature bundles."
@@ -26,7 +26,7 @@ members = [".", "crates/knit-runtime"]
 
 [dependencies]
 anyhow = "1"
-knit-runtime = { version = "0.1.0-alpha.49", path = "crates/knit-runtime" }
+knit-runtime = { version = "0.1.0-alpha.50", path = "crates/knit-runtime" }
 chrono = { version = "0.4", features = ["serde"] }
 clap = { version = "4.5", features = ["derive"] }
 ctrlc = { version = "3.4", features = ["termination"] }

crates/knit-runtime/Cargo.toml

@@ -4,7 +4,7 @@
 # version-control shaped; the CLI compiles it in and adapts bundle state into
 # the crate's RuntimeContext contract.
 name = "knit-runtime"
-version = "0.1.0-alpha.49"
+version = "0.1.0-alpha.50"
 edition = "2021"
 license = "Apache-2.0"
 description = "Per-bundle docker-compose runtime for knit cross-repo bundles."

docs/release-0.1.0-alpha.50.md

@@ -0,0 +1,8 @@
+# Knit 0.1.0-alpha.50
+
+`knit doctor`, `knit migrate` and `knit project list` read only
+`*.project.json` files in `.knit/projects/` as projects, so the known-repos
+file a fresh clone writes there no longer makes `knit doctor` fail. `knit pull
+--bundles` no longer fails open bundles with "Another Knit process is updating
+this state" naming its own process: the landing plan syncs it runs for each
+bundle now take turns instead of colliding on the workspace lock.

src/commands/doctor.rs

@@ -2,7 +2,8 @@ use crate::commands::bundle::bundle_state;
 use crate::model::{BundleState, ChangeGroup, KnitConfig, KnitProject};
 use crate::output as out;
 use crate::store::{
-    find_knit_root, global_config_path, load_effective_config, read_json, write_json,
+    find_knit_root, global_config_path, is_project_file, load_effective_config, read_json,
+    write_json,
 };
 use crate::tracking::ledger_recorded_head_sha;
 use anyhow::{bail, Context, Result};
@@ -28,8 +29,13 @@ pub fn doctor_workspace() -> Result<()> {
     if let Ok(effective) = load_effective_config(&root) {
         inspect_effective_remotes(&effective, &mut issues);
     }
-    inspect_json_dir::<ChangeGroup>(&root.join(".knit/bundles"), "bundle", &mut issues);
-    inspect_json_dir::<KnitProject>(&root.join(".knit/projects"), "project", &mut issues);
+    inspect_json_dir::<ChangeGroup>(&root.join(".knit/bundles"), is_json, "bundle", &mut issues);
+    inspect_json_dir::<KnitProject>(
+        &root.join(".knit/projects"),
+        is_project_file,
+        "project",
+        &mut issues,
+    );
     inspect_operational_json_dir(&root.join(".knit/merge-runs"), "merge run", &mut issues);
     inspect_operational_json_dir(&root.join(".knit/land-runs"), "land run", &mut issues);
     inspect_locks(&root, &mut issues);
@@ -52,7 +58,12 @@ pub fn migrate_workspace(check: bool) -> Result<()> {
     let mut changed = Vec::new();
     migrate_one::<KnitConfig>(&root.join(".knit/config.json"), check, &mut changed)?;
     migrate_bundles(&root.join(".knit/bundles"), check, &mut changed)?;
-    migrate_dir::<KnitProject>(&root.join(".knit/projects"), check, &mut changed)?;
+    migrate_dir::<KnitProject>(
+        &root.join(".knit/projects"),
+        is_project_file,
+        check,
+        &mut changed,
+    )?;
     let mut removed = Vec::new();
     migrate_remove_legacy(&root.join(".knit/contexts.json"), check, &mut removed)?;
 
@@ -129,8 +140,16 @@ fn inspect_effective_remotes(config: &KnitConfig, issues: &mut Vec<String>) {
     }
 }
 
-fn inspect_json_dir<T>(dir: &Path, label: &str, issues: &mut Vec<String>)
-where
+fn is_json(path: &Path) -> bool {
+    path.extension().and_then(|extension| extension.to_str()) == Some("json")
+}
+
+fn inspect_json_dir<T>(
+    dir: &Path,
+    include: fn(&Path) -> bool,
+    label: &str,
+    issues: &mut Vec<String>,
+) where
     T: serde::de::DeserializeOwned,
 {
     if !dir.exists() {
@@ -142,7 +161,7 @@ where
     };
     for entry in entries.flatten() {
         let path = entry.path();
-        if path.extension().and_then(|extension| extension.to_str()) != Some("json") {
+        if !include(&path) {
             continue;
         }
         if let Err(error) = read_json::<T>(&path) {
@@ -250,7 +269,12 @@ fn inspect_bundle_paths(root: &Path, issues: &mut Vec<String>) {
     }
 }
 
-fn migrate_dir<T>(dir: &Path, check: bool, changed: &mut Vec<PathBuf>) -> Result<()>
+fn migrate_dir<T>(
+    dir: &Path,
+    include: fn(&Path) -> bool,
+    check: bool,
+    changed: &mut Vec<PathBuf>,
+) -> Result<()>
 where
     T: serde::de::DeserializeOwned + serde::Serialize,
 {
@@ -259,7 +283,7 @@ where
     }
     for entry in fs::read_dir(dir).with_context(|| format!("failed to read {}", dir.display()))? {
         let path = entry?.path();
-        if path.extension().and_then(|extension| extension.to_str()) == Some("json") {
+        if include(&path) {
             migrate_one::<T>(&path, check, changed)?;
         }
     }

src/commands/project.rs

@@ -13,7 +13,6 @@ use crate::store::{
 use crate::time::now_iso;
 use anyhow::{bail, Context, Result};
 use std::collections::BTreeMap;
-use std::ffi::OsStr;
 use std::ffi::OsString;
 use std::fs;
 use std::path::Path;
@@ -390,7 +389,7 @@ pub fn list_projects() -> Result<()> {
         .with_context(|| format!("failed to read {}", dir.display()))?
         .filter_map(|entry| entry.ok())
         .map(|entry| entry.path())
-        .filter(|path| path.extension() == Some(OsStr::new("json")))
+        .filter(|path| crate::store::is_project_file(path))
         .collect::<Vec<_>>();
     entries.sort();
 

src/commands/remote/pull.rs

@@ -119,6 +119,10 @@ pub struct RemotePullContext {
     /// bundles on parallel threads, and bounded server memory — one payload
     /// built at a time — is the entire point of the incremental fetch.
     fetch_gate: Mutex<()>,
+    /// Serializes per-bundle landing plan syncs. Each takes the workspace
+    /// `landing-sync` file lock, which refuses a second holder outright, even
+    /// a sibling thread of this same pull.
+    plans_gate: Mutex<()>,
 }
 
 impl RemotePullContext {
@@ -131,6 +135,19 @@ impl RemotePullContext {
             .unwrap_or_else(|poisoned| poisoned.into_inner());
         resolve_export_bundle_payload(&self.remote, self.token.as_deref(), entry)
     }
+
+    fn pull_bundle_plans(&self, bundle_id: &str) -> Result<()> {
+        let _gate = self
+            .plans_gate
+            .lock()
+            .unwrap_or_else(|poisoned| poisoned.into_inner());
+        super::landing::pull_plans_scoped(
+            Some(&self.project.id),
+            &self.remote_name,
+            false,
+            Some(bundle_id),
+        )
+    }
 }
 
 /// Outcome of pulling a single bundle's recorded state from the remote.
@@ -226,6 +243,7 @@ pub fn prepare_remote_pull(
                     remote,
                     token: Some(token),
                     fetch_gate: Mutex::new(()),
+                    plans_gate: Mutex::new(()),
                 }));
             }
             Err(error) => {
@@ -1192,12 +1210,7 @@ pub fn pull_bundle_remote_state(
             "no remote artifact".to_string(),
         ));
     };
-    super::landing::pull_plans_scoped(
-        Some(&context.project.id),
-        &context.remote_name,
-        false,
-        Some(bundle_id),
-    )?;
+    context.pull_bundle_plans(bundle_id)?;
     // The export only carries artifact metadata. When its hash is the one this
     // artifact was last reconciled with, the remote holds nothing new and the
     // payload is never downloaded — only the checkouts are refreshed.

src/store.rs

@@ -269,6 +269,14 @@ pub fn bundle_path(root: &Path, bundle_id: &str) -> PathBuf {
         .join(format!("{bundle_id}.bundle.json"))
 }
 
+/// True for a project document in `.knit/projects/`. Per-project sidecars
+/// (known repos, history sync state) share that directory under other names.
+pub fn is_project_file(path: &Path) -> bool {
+    path.file_name()
+        .and_then(|name| name.to_str())
+        .is_some_and(|name| name.ends_with(".project.json"))
+}
+
 pub fn project_path(root: &Path, project_id: &str) -> PathBuf {
     root.join(".knit/projects")
         .join(format!("{project_id}.project.json"))

tests/bundle.rs

@@ -52,6 +52,24 @@ fn doctor_ignores_missing_recorded_paths_only_for_archived_bundles() {
     fs::remove_dir_all(root).unwrap();
 }
 
+#[test]
+fn doctor_migrate_and_project_list_skip_project_sidecar_files() {
+    let root = unique_temp_dir();
+    let workspace = root.join("workspace");
+    fs::create_dir_all(&workspace).unwrap();
+    knit(&workspace, ["init", "demo"]);
+    let sidecar = workspace.join(".knit/projects/demo.known-repos.json");
+    let sidecar_body = "{\n  \"docs\": \"https://example.com/docs.git\"\n}\n";
+    fs::write(&sidecar, sidecar_body).unwrap();
+
+    assert!(knit(&workspace, ["doctor"]).contains("Knit doctor: ok"));
+    assert!(knit(&workspace, ["project", "list"]).contains("demo"));
+    knit(&workspace, ["migrate"]);
+    assert_eq!(fs::read_to_string(&sidecar).unwrap(), sidecar_body);
+
+    fs::remove_dir_all(root).unwrap();
+}
+
 #[test]
 fn doctor_reports_archived_ledger_inconsistency_with_missing_paths() {
     let root = unique_temp_dir();