Merge fix/exclude-weave-state: coordination state auto-excluded from the working tree

Author: Palanikannan1437

Cargo.lock

@@ -322,7 +322,10 @@ for the full flag list; the table below is what each one is for.
 
 `claim`/`release`/`status`/`apply` all operate on the same `.weave/state.automerge` CRDT
 document as the MCP tools below: the CLI and MCP server are two front ends onto one
-coordination state.
+coordination state. That document lives in the repo's working tree but is never repo
+content: the first time weave writes it, it adds `.weave/` to the repo's local
+`.git/info/exclude` (never your own `.gitignore`), so it never shows up in `git status`
+or gets swept into `git add -A`.
 
 ## MCP Server
 

README.md

@@ -1,4 +1,4 @@
-//! The `git` queries weave asks, and the four ways one can fail.
+//! The `git` queries weave asks, and the ways one can fail.
 //!
 //! Every function here shells out to `git`, and every one of them used to
 //! answer failure with `Box<dyn Error>` built from a `format!` string — so
@@ -50,6 +50,17 @@ pub enum GitError {
         status: i32,
         stderr: String,
     },
+
+    /// `git` answered fine, but a filesystem operation this module needed on
+    /// top of that answer (writing the local exclude file) failed. Distinct
+    /// from [`GitError::NotRunnable`] because the process that failed here is
+    /// our own I/O, not git's.
+    #[error("could not update {path}: {source}")]
+    Io {
+        path: String,
+        #[source]
+        source: std::io::Error,
+    },
 }
 
 /// Run `git -C <dir> <args>` and hand back its output, or say which way it
@@ -237,6 +248,82 @@ pub fn diff_files(
     Ok(files)
 }
 
+/// Make sure `entry` (a `.gitignore`-style pattern) is listed in this
+/// repository's *local* exclude file, without touching anything the
+/// repository itself tracks.
+///
+/// Weave writes its own coordination state (`.weave/`) into a repo's working
+/// tree, and that state must never show up in `git status`, get swept into
+/// `git add -A`, or ride along in a generated patch — it is local machine
+/// state, not repository content. `.git/info/exclude` is the git-native place
+/// for exactly that: it behaves like a `.gitignore` but lives inside `.git/`,
+/// so it is never committed, never pushed, and never collides with a
+/// `.gitignore` the repository's own maintainers write and version.
+///
+/// Resolved via `git rev-parse --git-path info/exclude` rather than assuming
+/// `<repo_root>/.git/info/exclude`, so this also lands in the right place for
+/// a linked worktree (whose private git dir is elsewhere) or a submodule
+/// (whose `.git` is a file, not a directory) — anywhere git itself would
+/// consider "the local exclude file for this working tree".
+///
+/// Idempotent: a second call with the same `entry` is a no-op, so this is
+/// safe to call on every save rather than only on first creation — which
+/// also means a repository whose `.weave/` directory predates this fix gets
+/// it retroactively on its very next save.
+///
+/// A best-effort courtesy, not a requirement: `repo_root` not being inside a
+/// git repository, or `git` not being runnable at all, comes back `Ok(())` —
+/// silently doing nothing is correct there, since there is no repository
+/// tracking to protect against. Only an I/O failure on a `.git` this call
+/// *did* manage to identify is reported, and even that is expected to be
+/// swallowed by callers for whom this is advisory (see
+/// `weave_crdt::EntityStateDoc::save`, which must never fail to write state
+/// just because the exclude file couldn't be touched).
+pub fn ensure_locally_excluded(repo_root: &Path, entry: &str) -> Result<(), GitError> {
+    let args = ["rev-parse", "--git-path", "info/exclude"];
+    let output = run(repo_root, &args)?;
+    if !output.status.success() {
+        // Not inside a git repository (or some other refusal) — nothing to
+        // exclude from, and that's fine.
+        return Ok(());
+    }
+    let printed = String::from_utf8_lossy(&output.stdout).trim().to_string();
+    if printed.is_empty() {
+        return Ok(());
+    }
+    let exclude_path = {
+        let p = PathBuf::from(&printed);
+        if p.is_absolute() {
+            p
+        } else {
+            repo_root.join(p)
+        }
+    };
+
+    let io_err = |source: std::io::Error| GitError::Io {
+        path: exclude_path.display().to_string(),
+        source,
+    };
+
+    if let Some(parent) = exclude_path.parent() {
+        std::fs::create_dir_all(parent).map_err(io_err)?;
+    }
+
+    let existing = std::fs::read_to_string(&exclude_path).unwrap_or_default();
+    if existing.lines().any(|line| line.trim() == entry) {
+        return Ok(()); // already excluded
+    }
+
+    let mut updated = existing;
+    if !updated.is_empty() && !updated.ends_with('\n') {
+        updated.push('\n');
+    }
+    updated.push_str(entry);
+    updated.push('\n');
+    std::fs::write(&exclude_path, updated).map_err(io_err)?;
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -286,4 +373,78 @@ mod tests {
             other => panic!("expected NoMergeBase, got {other:?}"),
         }
     }
+
+    fn init_repo(dir: &Path) {
+        for args in [
+            vec!["init", "-q"],
+            vec!["config", "user.email", "test@example.com"],
+            vec!["config", "user.name", "Test"],
+        ] {
+            let status = Command::new("git")
+                .arg("-C")
+                .arg(dir)
+                .args(&args)
+                .status()
+                .expect("git must be runnable for this test");
+            assert!(status.success(), "git {args:?} failed");
+        }
+    }
+
+    #[test]
+    fn ensure_locally_excluded_writes_the_entry_to_info_exclude_not_gitignore() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        init_repo(dir.path());
+
+        ensure_locally_excluded(dir.path(), ".weave/").expect("ensure_locally_excluded");
+
+        let exclude = std::fs::read_to_string(dir.path().join(".git/info/exclude"))
+            .expect("info/exclude should exist");
+        assert!(exclude.lines().any(|l| l == ".weave/"));
+        assert!(
+            !dir.path().join(".gitignore").exists(),
+            "must never create or touch the repository's own .gitignore"
+        );
+    }
+
+    #[test]
+    fn ensure_locally_excluded_is_idempotent() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        init_repo(dir.path());
+
+        ensure_locally_excluded(dir.path(), ".weave/").expect("first call");
+        ensure_locally_excluded(dir.path(), ".weave/").expect("second call");
+        ensure_locally_excluded(dir.path(), ".weave/").expect("third call");
+
+        let exclude = std::fs::read_to_string(dir.path().join(".git/info/exclude"))
+            .expect("info/exclude should exist");
+        let hits = exclude.lines().filter(|l| *l == ".weave/").count();
+        assert_eq!(hits, 1, "repeated calls must not duplicate the entry");
+    }
+
+    #[test]
+    fn ensure_locally_excluded_preserves_an_existing_exclude_file() {
+        let dir = tempfile::tempdir().expect("tempdir");
+        init_repo(dir.path());
+        let exclude_path = dir.path().join(".git/info/exclude");
+        std::fs::write(&exclude_path, "*.local-scratch\n").expect("seed existing exclude");
+
+        ensure_locally_excluded(dir.path(), ".weave/").expect("ensure_locally_excluded");
+
+        let exclude = std::fs::read_to_string(&exclude_path).expect("read exclude");
+        assert!(exclude.lines().any(|l| l == "*.local-scratch"));
+        assert!(exclude.lines().any(|l| l == ".weave/"));
+    }
+
+    #[test]
+    fn ensure_locally_excluded_is_a_silent_no_op_outside_a_repository() {
+        let dir = tempfile::tempdir().expect("tempdir (not a git repo)");
+
+        let result = ensure_locally_excluded(dir.path(), ".weave/");
+
+        assert!(result.is_ok(), "must not fail just because there's no repo");
+        assert!(
+            !dir.path().join(".git").exists(),
+            "must not create a .git directory as a side effect"
+        );
+    }
 }

crates/weave-core/src/git.rs

@@ -20,6 +20,7 @@ test-helpers = []
 # them on without shipping them to real consumers.
 [dev-dependencies]
 weave-crdt = { path = ".", features = ["test-helpers"] }
+tempfile = "3"
 
 [dependencies]
 automerge = "0.11"

crates/weave-crdt/Cargo.toml

@@ -209,6 +209,18 @@ impl EntityStateDoc {
         }
         if let Some(parent) = self.path.parent() {
             std::fs::create_dir_all(parent)?;
+            // `parent` is `.weave/` — weave's own coordination state, not
+            // repository content. It must never show up in `git status`,
+            // get swept into `git add -A`, or ride along in a generated
+            // patch (see `weave_core::git::ensure_locally_excluded`).
+            // Best-effort and idempotent, on every save rather than only the
+            // first: a repo we can't find, or a git we can't run, must never
+            // block writing the state a caller asked for, and a `.weave/`
+            // directory that predates this fix gets excluded retroactively
+            // on its very next save.
+            if let Some(repo_root) = parent.parent() {
+                let _ = weave_core::git::ensure_locally_excluded(repo_root, ".weave/");
+            }
         }
         let data = self.doc.save();
         std::fs::write(&self.path, data)?;

crates/weave-crdt/src/state.rs

@@ -0,0 +1,75 @@
+//! RED test for the `.weave/` working-tree leak.
+//!
+//! A 327-run transcript study found that `.weave/state.automerge` — the
+//! binary Automerge doc every `EntityStateDoc::save()` writes under the
+//! target repo's `.weave/` directory — was neither tracked nor ignored.
+//! `git status` reported it as `?? .weave/`, `git add -A` staged the binary
+//! blob, and any patch built from the staged diff carried it along,
+//! silently invalidating the patch.
+//!
+//! This test drives the real product path — `EntityStateDoc::open` +
+//! `save()` — against a fresh, real git repository (via the actual `git`
+//! binary, not a fake), and asserts the two user-visible symptoms are gone:
+//! `git status --porcelain` reports nothing under `.weave/`, and `git add -A`
+//! followed by `git diff --cached --name-only` stages nothing under it
+//! either.
+
+use std::path::Path;
+use std::process::Command;
+
+use weave_crdt::EntityStateDoc;
+
+fn git(dir: &Path, args: &[&str]) -> String {
+    let out = Command::new("git")
+        .arg("-C")
+        .arg(dir)
+        .args(args)
+        .output()
+        .expect("git must be runnable for this test");
+    assert!(
+        out.status.success(),
+        "git {args:?} failed: {}",
+        String::from_utf8_lossy(&out.stderr)
+    );
+    String::from_utf8_lossy(&out.stdout).trim().to_string()
+}
+
+#[test]
+fn weave_state_never_shows_up_in_git_status_or_a_staged_diff() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let repo = dir.path();
+
+    git(repo, &["init", "-q"]);
+    git(repo, &["config", "user.email", "test@example.com"]);
+    git(repo, &["config", "user.name", "Test"]);
+    // A repo needs at least one commit for `git status`/`git diff --cached`
+    // to have a baseline to compare against — an empty repo would pass this
+    // test's assertions trivially.
+    std::fs::write(repo.join("README.md"), "hello\n").expect("write readme");
+    git(repo, &["add", "README.md"]);
+    git(repo, &["commit", "-q", "-m", "initial"]);
+
+    // The exact door every weave-cli command and the weave-mcp server open:
+    // `<repo_root>/.weave/state.automerge`.
+    let state_path = repo.join(".weave").join("state.automerge");
+    let mut state = EntityStateDoc::open(&state_path).expect("open a fresh state doc");
+    state.save().expect("save creates .weave/ and writes the doc");
+
+    assert!(
+        state_path.exists(),
+        "the state file should actually have been written to disk"
+    );
+
+    let status = git(repo, &["status", "--porcelain"]);
+    assert!(
+        !status.lines().any(|l| l.contains(".weave")),
+        "`git status --porcelain` must not mention .weave/, got:\n{status}"
+    );
+
+    git(repo, &["add", "-A"]);
+    let staged = git(repo, &["diff", "--cached", "--name-only"]);
+    assert!(
+        !staged.lines().any(|l| l.starts_with(".weave/")),
+        "`git add -A` must not stage anything under .weave/, got:\n{staged}"
+    );
+}

crates/weave-crdt/tests/git_exclude.rs

@@ -186,7 +186,7 @@ <h2>Architecture</h2>
       <div class="arch-arrow">&darr;</div>
       <div class="arch-layer" style="border-color: var(--purple);">
         <div class="label" style="color: var(--purple);">weave-crdt</div>
-        <div class="desc">Automerge-backed entity state. Advisory claims, agent tracking, conflict detection. Persisted at <code style="color:var(--cyan)">.weave/state.automerge</code>. Not committed to git &mdash; local coordination state.</div>
+        <div class="desc">Automerge-backed entity state. Advisory claims, agent tracking, conflict detection. Persisted at <code style="color:var(--cyan)">.weave/state.automerge</code>. Not committed to git &mdash; local coordination state, auto-excluded via <code style="color:var(--cyan)">.git/info/exclude</code> on first write.</div>
       </div>
       <div class="arch-arrow">&darr;</div>
       <div class="arch-layer" style="border-color: var(--cyan);">

docs/docs.html

@@ -228,7 +228,7 @@ <h2>Automerge: a CRDT for JSON documents</h2>
             <li><strong>Operations:</strong> an audit log of claims, releases, and modifications</li>
           </ul>
 
-          <p>This is saved as a binary file at <code>.weave/state.automerge</code> in your repo root. It's not committed to git &mdash; it's local coordination state, like <code>.git/</code> itself.</p>
+          <p>This is saved as a binary file at <code>.weave/state.automerge</code> in your repo root. It's not committed to git &mdash; it's local coordination state, like <code>.git/</code> itself. The first time weave writes it, it also adds <code>.weave/</code> to your repo's local <code>.git/info/exclude</code> (never your own <code>.gitignore</code>), so <code>git status</code> and <code>git add -A</code> ignore it automatically &mdash; nothing for you to configure.</p>
 
           <h2>Why advisory locks, not hard locks?</h2>