From af51d479f6df19abf81249315985c5c04c1364bf Mon Sep 17 00:00:00 2001 From: Chess Seventh Date: Sun, 26 Jul 2026 11:27:00 +0200 Subject: [PATCH] fix(exclude): match repo by origin remote name not worktree basename Repo exclusion identified the repository by its worktree directory basename (repo.workdir() -> file_name()), so a commit made from a git worktree (basename = lane name, not repo name) never matched the [exclude] list and flooded the Obsidian diary. This blocked clean use of the L49 wt worktree workflow. - resolve canonical identity from the origin remote URL (stable across worktrees) via new repo_name_from_url + canonical_repo_name + current_repo_canonical_name; run_commit_saver now uses the canonical name. Fall back to the workdir basename when there is no origin. - one "claude-src" exclude entry now covers all of its worktrees. - tests: worktree regression (origin name beats basename), URL-form variants + empty/sentinel, no-origin fallback; existing exclusion test switched to the canonical name. Gate: devenv pre-check green (clippy -D warnings + 127 tests + build). Refs: docs/feature/L58-exclude-by-remote/brief.md Co-Authored-By: Vulcan --- docs/feature/L58-exclude-by-remote/brief.md | 48 +++++++++ src/main.rs | 11 +- src/vim_commit.rs | 111 +++++++++++++++++++- 3 files changed, 163 insertions(+), 7 deletions(-) create mode 100644 docs/feature/L58-exclude-by-remote/brief.md diff --git a/docs/feature/L58-exclude-by-remote/brief.md b/docs/feature/L58-exclude-by-remote/brief.md new file mode 100644 index 0000000..7c0ff64 --- /dev/null +++ b/docs/feature/L58-exclude-by-remote/brief.md @@ -0,0 +1,48 @@ +# Bug-fix brief — L58 exclude-by-remote + +> Lean nWave motion (rigor: lean). Standalone defect in delivered behaviour, +> found while cleaning up hook artifacts in L57. Root cause already known; +> this brief records the defect, the gate, and the acceptance scenarios that +> drive RED → GREEN → COMMIT. + +## Defect + +`[exclude]` in the runtime ini fails to skip a repository when the commit is +made from a **git worktree**. `run_commit_saver` identified the repo via +`current_repo_workdir_name()` — `repo.workdir()` → `Path::file_name()`, i.e. the +**worktree directory basename** — and `is_repo_excluded()` compares it by exact +`==`. A worktree's basename is the lane name (e.g. `l58-wt`), not the repo name, +so `[exclude] repos = claude-src` never matches and every worktree commit floods +the Obsidian diary. This blocks clean use of the L49 `wt` worktree workflow. + +## Fix + +Resolve **canonical repo identity** from the `origin` remote URL (already read as +`repository_url` in `CommitSaver::from_repo`), which is stable across all +worktrees of a repo. Fall back to the workdir basename when there is no usable +`origin` (local-only repos). One `claude-src` exclude entry then covers every +worktree. + +- New (in `src/vim_commit.rs`): `repo_name_from_url`, `canonical_repo_name(&Repository)`, + `current_repo_canonical_name`. +- Changed: `run_commit_saver` (`src/main.rs`) calls `current_repo_canonical_name`; + `is_repo_excluded` doc updated (it now receives the canonical name). +- The 4 pure `is_repo_excluded` string-predicate tests are unchanged. + +## Gate (acceptance scenarios) + +1. A commit made in a **worktree** whose `origin` is `…/claude-src.git` (basename + ≠ `claude-src`) → **excluded**, writes no diary row. *(the regression)* +2. A commit in an **excluded repo** in its normal checkout → still excluded, + writes nothing. +3. A commit in a **non-excluded repo** → still journals normally. + +Verified by: `test_canonical_repo_name_prefers_origin_over_workdir` (1), +`test_run_commit_saver_skips_excluded_repo` updated to the canonical name (2), +plus `repo_name_from_url` variant/sentinel tests. Full gate: +`devenv shell -- pre-check` (clippy `-D warnings` + tests + build). + +## Deploy (Franci) + +Merge to master → the `🎯 Release` workflow bumps + tags from the conventional +`fix:` commit; then `up-hm` deploys the new binary. diff --git a/src/main.rs b/src/main.rs index c197cb3..f1a585a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,7 +7,7 @@ use rusty_commit_saver::vim_commit::CommitSaver; use rusty_commit_saver::vim_commit::check_diary_path_exists; use rusty_commit_saver::vim_commit::create_diary_file; use rusty_commit_saver::vim_commit::create_directories_for_new_entry; -use rusty_commit_saver::vim_commit::current_repo_workdir_name; +use rusty_commit_saver::vim_commit::current_repo_canonical_name; use rusty_commit_saver::vim_commit::is_repo_excluded; use rusty_commit_saver::config::GlobalVars; @@ -104,7 +104,7 @@ pub fn run_commit_saver( template_commit_date_path: &str, excluded_repos: &[String], ) -> Result<(), Box> { - if let Some(repo_name) = current_repo_workdir_name() { + if let Some(repo_name) = current_repo_canonical_name() { if is_repo_excluded(&repo_name, excluded_repos) { info!( "[run_commit_saver()]: repo '{repo_name}' is in the exclude list; skipping commit capture." @@ -505,9 +505,10 @@ mod main_tests { let commit_path = PathBuf::from("Diaries/Commits"); let date_template = "%Y/%m-%B/%F.md"; - // Exclude the repository the test suite itself runs in: the gate must - // short-circuit and write nothing to the Obsidian root. - if let Some(current_repo) = current_repo_workdir_name() { + // Exclude the repository the test suite itself runs in (by canonical + // name, so this holds in a worktree too): the gate must short-circuit + // and write nothing to the Obsidian root. + if let Some(current_repo) = current_repo_canonical_name() { if Repository::discover("./").is_ok() { let excluded = vec![current_repo]; let result = run_commit_saver( diff --git a/src/vim_commit.rs b/src/vim_commit.rs index d6a8989..b36d979 100644 --- a/src/vim_commit.rs +++ b/src/vim_commit.rs @@ -573,14 +573,67 @@ pub fn current_repo_workdir_name() -> Option { repo_workdir_name(&repo) } +/// Extracts the repository name from a Git remote URL. +/// +/// Handles the common remote forms — scp-style (`git@host:org/repo.git`), +/// https (`https://host/org/repo.git`), ssh (`ssh://git@host/org/repo.git`), +/// and bare local paths — by dropping a trailing `.git` and taking the final +/// path segment (splitting on both `/` and `:`). Returns `None` for an empty +/// input or the `no_url_set` sentinel that [`CommitSaver::from_repo`] uses when +/// no `origin` remote exists. +#[must_use] +pub fn repo_name_from_url(url: &str) -> Option { + let url = url.trim(); + if url.is_empty() || url == "no_url_set" { + return None; + } + let stem = url.trim_end_matches('/'); + let stem = stem.strip_suffix(".git").unwrap_or(stem); + stem.rsplit(['/', ':']) + .next() + .filter(|segment| !segment.is_empty()) + .map(str::to_owned) +} + +/// Returns the repository's canonical identity for exclusion matching. +/// +/// The name is taken from the `origin` remote URL (see [`repo_name_from_url`]), +/// which is stable across every worktree of the same repository — so a single +/// exclude entry covers a repo no matter what its worktree directories are +/// named. Falls back to the working-directory basename ([`repo_workdir_name`]) +/// when there is no usable `origin` remote (e.g. a local-only repository). +#[must_use] +pub fn canonical_repo_name(repo: &Repository) -> Option { + if let Ok(remote) = repo.find_remote("origin") { + if let Ok(url) = remote.url() { + if let Some(name) = repo_name_from_url(url) { + return Some(name); + } + } + } + repo_workdir_name(repo) +} + +/// Returns the canonical name of the repository discovered from the current path. +/// +/// Like [`current_repo_workdir_name`], but resolves the repository's canonical +/// identity via [`canonical_repo_name`] (its `origin` remote name) rather than +/// the ambient worktree basename. Returns `None` when no repository can be +/// discovered. +#[must_use] +pub fn current_repo_canonical_name() -> Option { + let repo = Repository::discover("./").ok()?; + canonical_repo_name(&repo) +} + /// Reports whether a repository name is present in the exclude list. /// /// Matching is an exact, case-sensitive comparison of the repository's -/// working-directory name against each configured entry. +/// canonical name against each configured entry. /// /// # Arguments /// -/// * `repo_name` - The repository's working-directory name (see [`repo_workdir_name`]) +/// * `repo_name` - The repository's canonical name (see [`canonical_repo_name`]) /// * `exclude_list` - The configured repository names to skip /// /// # Examples @@ -1347,4 +1400,58 @@ mod commit_saver_tests { let list = vec!["claude-src".to_string()]; assert!(!is_repo_excluded("Claude-Src", &list)); } + + #[test] + fn test_repo_name_from_url_variants() { + // Every common remote form for the same repo resolves to "claude-src". + for url in [ + "git@github.com:chess-seventh/claude-src.git", + "https://github.com/chess-seventh/claude-src.git", + "ssh://git@github.com/chess-seventh/claude-src.git", + "https://github.com/chess-seventh/claude-src", + "git@github.com:claude-src.git", + "/home/seventh/src/claude-src", + "/home/seventh/src/claude-src/", + ] { + assert_eq!( + repo_name_from_url(url).as_deref(), + Some("claude-src"), + "wrong repo name for url: {url}" + ); + } + } + + #[test] + fn test_repo_name_from_url_rejects_empty_and_sentinel() { + assert_eq!(repo_name_from_url(""), None); + assert_eq!(repo_name_from_url(" "), None); + assert_eq!(repo_name_from_url("no_url_set"), None); + } + + #[test] + fn test_canonical_repo_name_prefers_origin_over_workdir() { + // The regression this lane fixes: a repo checked out in a directory + // whose basename is NOT the repo name (e.g. a git worktree named after + // the lane) must still resolve to its canonical origin name, so one + // exclude entry covers every worktree. + let temp_dir = tempdir().unwrap(); + let repo_path = temp_dir.path().join("some-lane-worktree"); + fs::create_dir(&repo_path).unwrap(); + let repo = Repository::init(&repo_path).unwrap(); + repo.remote("origin", "git@github.com:chess-seventh/claude-src.git") + .unwrap(); + + assert_eq!(canonical_repo_name(&repo).as_deref(), Some("claude-src")); + } + + #[test] + fn test_canonical_repo_name_falls_back_to_workdir_without_origin() { + // No origin remote (local-only repo): fall back to the workdir basename. + let temp_dir = tempdir().unwrap(); + let repo_path = temp_dir.path().join("claude-src"); + fs::create_dir(&repo_path).unwrap(); + let repo = Repository::init(&repo_path).unwrap(); + + assert_eq!(canonical_repo_name(&repo).as_deref(), Some("claude-src")); + } }