From eae61b8bf579980ca0fbc3297f24fb49f4acc7bb Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:13:24 +0100 Subject: [PATCH 01/10] design: fix repo tagging for git worktrees and non-standard clone directories --- .../changes/fix-repo-tag-worktree/design.md | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 planflow/changes/fix-repo-tag-worktree/design.md diff --git a/planflow/changes/fix-repo-tag-worktree/design.md b/planflow/changes/fix-repo-tag-worktree/design.md new file mode 100644 index 0000000..9556da7 --- /dev/null +++ b/planflow/changes/fix-repo-tag-worktree/design.md @@ -0,0 +1,140 @@ +# Fix repo tagging for git worktrees and non-standard clone directories — Design + +## Summary + +The `repo:` tag emitted by `gather_env_tags` is currently derived from the basename of `git rev-parse --show-toplevel`. For git worktrees and non-standard clone directories this produces branch-name or ticket-name tags instead of the actual repository name. This change derives the `repo:` tag from the origin remote URL (the last path segment, stripping `.git`), reusing the same `git remote get-url origin` fetch already used for the `org:` tag. It falls back to the existing show-toplevel basename when no origin remote exists or the URL is unparseable. A new private `repo_from_remote_url` function mirrors the existing `org_from_remote_url`, keeping the change minimal and the existing org-tag tests untouched. + +## Definition of Done + +- repo tag for a clone of e.g. `git@github.com:org/hbf.git` is `hbf` regardless of the directory name it lives in +- repo tag still falls back to the toplevel directory basename when no origin remote exists (matches the existing no-org-tag-without-remote test design) +- unit tests in `src/tags.rs` covering: worktree-style directory with an origin remote, no-remote fallback, ssh/https/git-protocol URL forms (mirror the existing `org_*` test cases) +- `cargo test` and `cargo clippy` pass +- note for reviewers: existing Langfuse traces keep the historical bad tags; no data migration in scope, but consider mentioning it in the release notes + +## Architecture + +### Current state + +`src/tags.rs` `gather_env_tags()` produces the `repo:` tag at lines 83-87: + +```rust +// Git repo name +if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } +} +``` + +For a linked git worktree, `--show-toplevel` returns the worktree directory, which users typically name after the branch or ticket (e.g. `wt579b`, `hbf-578-idfix`). The same problem affects any clone directory with an unusual name. The tag is written once at trace start, so downstream consumers (code-trace-analyze) cannot fix it. + +The `org:` tag (lines 90-94) already fetches the origin remote URL and passes it to `org_from_remote_url` (lines 42-71), which normalises SCP-style / ssh:// / https:// URLs and returns the second-to-last path segment. + +### Change + +1. **New function `repo_from_remote_url`** — a sibling to `org_from_remote_url` that normalises the remote URL the same way but returns the **last** path segment (the repo name) instead of the second-to-last. Returns `None` for unparseable URLs (local paths, etc.), same as `org_from_remote_url`. + +2. **Restructure the repo + org tag blocks in `gather_env_tags`** — fetch `git remote get-url origin` once. If a URL is obtained: + - Derive `repo:` from `repo_from_remote_url(&url)`. If that returns `None` (unparseable URL), fall back to the show-toplevel basename. + - Derive `org:` from `org_from_remote_url(&url)` as before. + If no remote URL is obtained (no origin remote): + - Fall back to the show-toplevel basename for `repo:`. + - No `org:` tag (existing behaviour). + + This replaces the current two independent blocks (lines 82-87 for repo, 90-94 for org) with a single coordinated block that fetches the remote URL once and derives both tags from it, with the show-toplevel fetch as the repo fallback. + +### Fallback ordering + +``` +repo tag: origin remote URL → repo_from_remote_url → repo name + ↓ (no remote or unparseable) + show-toplevel basename + +org tag: origin remote URL → org_from_remote_url → org name + ↓ (no remote or unparseable) + (no org tag) +``` + +The `show-toplevel` git command is only needed for the fallback path now. When a remote URL is present and parseable, the `show-toplevel` call is unnecessary. The implementation fetches the remote URL first; if it yields a valid repo name, `show-toplevel` is not called. If the remote URL is absent or unparseable, `show-toplevel` is called for the fallback. This avoids a redundant git subprocess in the common (remote-present) case. + +## Existing Patterns followed + +- **Sibling extraction functions** — `repo_from_remote_url` mirrors the structure and URL-normalisation logic of `org_from_remote_url` exactly, returning a different path segment. This follows the existing pattern of small, single-purpose, private functions with dedicated unit tests. +- **Test mirroring** — new `repo_*` test cases mirror the existing `org_*` test cases (`org_from_ssh_url`, `org_from_https_url`, `org_from_ssh_protocol_url`, `org_from_https_url_without_git_suffix`, `org_from_gitlab_subgroup_url`, `org_from_local_path_returns_none`) one-to-one, asserting on the repo name instead of the org. +- **Graceful fallback** — the no-remote fallback to show-toplevel basename follows the same design as the existing `no_org_tag_without_remote` test: when git data is unavailable, the tag is simply not emitted or falls back to a local alternative. +- **`git_cmd` helper** — all git subprocess calls use the existing `git_cmd(&[...], cwd)` helper at lines 5-22. + +## The loop + +**What the agent runs:** `cargo test` and `cargo clippy` in the repo root — standard Rust toolchain, genuine coverage. New unit tests in `src/tags.rs` exercise `repo_from_remote_url` directly (pure-function tests, no git subprocess) and `gather_env_tags` end-to-end via temp-dir git repos with remotes (mirroring `org_tag_emitted_when_remote_present`). No falsework. No integration tests need changes — no existing test asserts on the `repo:` tag value from `gather_env_tags`. + +**What stays human-verified:** the release-notes note about historical Langfuse traces retaining bad tags is a documentation concern, not a testable code property. + +## Implementation Phases + +1. **Add `repo_from_remote_url`** — new private function in `src/tags.rs`, sibling to `org_from_remote_url`. Normalises the URL identically, returns the last path segment instead of the second-to-last. Returns `None` for unparseable URLs. + +2. **Add `repo_from_remote_url` unit tests** — mirror the 6 existing `org_*` test cases (`org_from_ssh_url` → `repo_from_ssh_url`, etc.) plus `org_from_gitlab_subgroup_url` and `org_from_local_path_returns_none`. Assert on the repo name (last segment). + +3. **Restructure `gather_env_tags` repo + org blocks** — replace the current two separate blocks (lines 82-94) with a single block: fetch the origin remote URL once; if present and parseable, derive `repo:` from `repo_from_remote_url` and `org:` from `org_from_remote_url`; if absent or unparseable for repo, fall back to `show-toplevel` basename for `repo:` only. + +4. **Add `gather_env_tags` integration-style tests** — two new tests mirroring `org_tag_emitted_when_remote_present` and `no_org_tag_without_remote`: one that creates a temp git repo with an origin remote and asserts both `repo:widgets` and `org:acme` are present (proving the repo tag comes from the URL, not the temp-dir name); one that creates a repo with no remote and asserts `repo:` falls back to the temp-dir basename. + +5. **Run `cargo test` and `cargo clippy`** — verify all tests pass and no clippy warnings. Fix any issues. + +## Additional Considerations + +- **Historical data** — existing Langfuse traces retain the directory-name-derived repo tags. No data migration is in scope. The release notes should mention this so reviewers understand the tag values change going forward but historical traces are unaffected. +- **code-trace-analyze (cta)** — not touched. Its aggregation is correct; the fix is upstream at the tag-derivation source. +- **Python reference implementation** (`docs/langfuse_hook.py`) — uses the old show-toplevel logic and has no `org:` tag. It is already out of sync with the Rust implementation and is historical reference material; no change needed. +- **README documentation** — `README.md` and `plugin/README.md` list `repo:` in tag tables without describing derivation semantics. No doc change strictly required; the example `repo:my-project` remains valid in form. +- **URL-normalisation duplication** — `repo_from_remote_url` and `org_from_remote_url` both normalise the URL independently. This is acceptable for two small functions; if a third URL form ever appears, a refactor to a shared `parse_remote_url` returning both segments (Approach B from brainstorming) is straightforward. + +## Acceptance Criteria + +### DoD 1: repo tag derived from remote URL regardless of directory name + +- **fix-repo-tag-worktree.AC1.1 (success):** A git repo cloned into a directory named `wt579b` with origin `git@github.com:org/hbf.git` produces the tag `repo:hbf`, not `repo:wt579b`. +- **fix-repo-tag-worktree.AC1.2 (success):** A git repo cloned into a directory named `hbf-578-idfix` with origin `https://github.com/org/hbf.git` produces the tag `repo:hbf`, not `repo:hbf-578-idfix`. +- **fix-repo-tag-worktree.AC1.3 (success):** A git repo with origin `ssh://git@github.com/org/hbf.git` produces `repo:hbf` regardless of its directory name. +- **fix-repo-tag-worktree.AC1.4 (failure):** A git repo with an origin remote that is a local path (e.g. `/home/doug/projects/hbf`) does not produce a remote-derived repo tag; it falls back to the show-toplevel basename (see AC2). + +### DoD 2: fallback to show-toplevel basename when no origin remote + +- **fix-repo-tag-worktree.AC2.1 (success):** A git repo with no origin remote produces `repo:` where toplevel-basename is the directory name from `git rev-parse --show-toplevel`. +- **fix-repo-tag-worktree.AC2.2 (success):** A git repo with an origin remote that is a local path (unparseable by `repo_from_remote_url`) falls back to `repo:`. +- **fix-repo-tag-worktree.AC2.3 (failure):** A git repo with no origin remote does not produce an `org:` tag (existing behaviour preserved — `no_org_tag_without_remote` test still passes). + +### DoD 3: unit tests covering URL forms and fallback + +- **fix-repo-tag-worktree.AC3.1 (success):** `repo_from_ssh_url` test asserts `repo_from_remote_url("git@github.com:acme/widgets.git") == Some("widgets")`. +- **fix-repo-tag-worktree.AC3.2 (success):** `repo_from_https_url` test asserts `repo_from_remote_url("https://github.com/acme/widgets.git") == Some("widgets")`. +- **fix-repo-tag-worktree.AC3.3 (success):** `repo_from_ssh_protocol_url` test asserts `repo_from_remote_url("ssh://git@github.com/acme/widgets.git") == Some("widgets")`. +- **fix-repo-tag-worktree.AC3.4 (success):** `repo_from_https_url_without_git_suffix` test asserts `repo_from_remote_url("https://github.com/acme/widgets") == Some("widgets")`. +- **fix-repo-tag-worktree.AC3.5 (success):** `repo_from_gitlab_subgroup_url` test asserts `repo_from_remote_url("git@gitlab.com:acme/platform/widgets.git") == Some("widgets")`. +- **fix-repo-tag-worktree.AC3.6 (success):** `repo_from_local_path_returns_none` test asserts `repo_from_remote_url("/home/doug/projects/widgets") == None`. +- **fix-repo-tag-worktree.AC3.7 (success):** `repo_tag_from_remote_when_present` test creates a temp git repo with origin `git@github.com:acme/widgets.git` and asserts `gather_env_tags` returns both `repo:widgets` and `org:acme` (repo tag is URL-derived, not temp-dir-name-derived). +- **fix-repo-tag-worktree.AC3.8 (success):** `repo_tag_falls_back_to_toplevel_without_remote` test creates a temp git repo with no remote and asserts `gather_env_tags` returns `repo:`. + +### DoD 4: cargo test and cargo clippy pass + +- **fix-repo-tag-worktree.AC4.1 (success):** `cargo test` passes with zero failures (all existing tests plus new tests). +- **fix-repo-tag-worktree.AC4.2 (success):** `cargo clippy` passes with zero warnings. +- **fix-repo-tag-worktree.AC4.3 (failure):** If `cargo test` or `cargo clippy` fails, the change is not complete. + +### DoD 5: release-notes note about historical traces + +- **fix-repo-tag-worktree.AC5.1 (success):** The design document (this file) notes that existing Langfuse traces retain historical bad repo tags and no data migration is in scope. (This is satisfied by the Additional Considerations section above; the release-notes mention itself is a human action at release time, not a code-testable property.) + +## Glossary + +- **repo tag** — the `repo:` Langfuse trace tag emitted by `gather_env_tags`, intended to identify the git repository the traced session was running in. +- **org tag** — the `org:` Langfuse trace tag emitted by `gather_env_tags`, derived from the origin remote URL's second-to-last path segment (the GitHub/GitLab owner or organisation). +- **show-toplevel** — `git rev-parse --show-toplevel`, which returns the absolute path of the working tree root. For a linked worktree this is the worktree directory, not the main repository. +- **linked worktree** — a git worktree created with `git worktree add`, which checks out a branch in a separate directory that shares the main repository's `.git` store. Users typically name the worktree directory after the branch or ticket. +- **remote URL** — the URL of the `origin` git remote, fetched via `git remote get-url origin`. Common forms: SCP-style (`git@github.com:org/repo.git`), ssh:// (`ssh://git@github.com/org/repo.git`), https:// (`https://github.com/org/repo.git`). +- **`org_from_remote_url`** — existing private function in `src/tags.rs` that extracts the organisation (second-to-last path segment) from a remote URL. +- **`repo_from_remote_url`** — new private function in `src/tags.rs` that extracts the repository name (last path segment) from a remote URL, mirroring `org_from_remote_url`. +- **`gather_env_tags`** — public function in `src/tags.rs` that collects environment-derived tags (repo, org, branch, user, host, os, agent version) for a Langfuse trace. +- **code-trace-analyze (cta)** — the downstream tool that aggregates Langfuse traces by repo/branch. Not in scope for this change; its aggregation logic is correct. From d82047903c615869f42b4846443bd100efdec395 Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:15:54 +0100 Subject: [PATCH 02/10] plan: implementation plan and spec deltas for fix-repo-tag-worktree --- .../fix-repo-tag-worktree/implementation.md | 395 ++++++++++++++++++ .../fix-repo-tag-worktree/specs/tags/spec.md | 104 +++++ 2 files changed, 499 insertions(+) create mode 100644 planflow/changes/fix-repo-tag-worktree/implementation.md create mode 100644 planflow/changes/fix-repo-tag-worktree/specs/tags/spec.md diff --git a/planflow/changes/fix-repo-tag-worktree/implementation.md b/planflow/changes/fix-repo-tag-worktree/implementation.md new file mode 100644 index 0000000..a4f806a --- /dev/null +++ b/planflow/changes/fix-repo-tag-worktree/implementation.md @@ -0,0 +1,395 @@ +# Fix repo tagging for git worktrees and non-standard clone directories — Implementation Plan + +Source: `planflow/changes/fix-repo-tag-worktree/design.md` + +## Codebase verification summary + +All design assumptions confirmed against the current code (verified 2026-09-25): + +- `src/tags.rs` is 287 lines; `org_from_remote_url` at lines 42–71, `git_cmd` at lines 5–22, `gather_env_tags` at lines 73–151. +- `repo:` block confirmed at lines 82–87 (show-toplevel basename, no remote consultation). +- `org:` block confirmed at lines 89–94 (remote URL → `org_from_remote_url`). +- Existing `org_*` test functions at lines 214–287 (6 unit tests + 2 integration tests). +- No existing `repo_from_remote_url` function or `repo_*` URL-parsing tests. +- No URL-parsing crates; manual string-splitting is the pattern. +- `git_cmd` returns `None` on failure/non-zero/empty — tests build real temp git repos. +- The only import needed (`std::process::Command`, `crate::source::Source`) is already present; no new imports required. + +No discrepancies found. The plan is grounded in the code as it is now. + +## Acceptance-criteria traceability + +| AC | Task(s) | +|----|---------| +| AC1.1 (repo:hbf from SSH URL in worktree-named dir) | T2, T3, T4 | +| AC1.2 (repo:hbf from HTTPS URL in ticket-named dir) | T2, T3, T4 | +| AC1.3 (repo:hbf from ssh:// URL) | T2 | +| AC1.4 (local-path remote falls back to toplevel) | T2, T4 | +| AC2.1 (no remote → toplevel basename) | T3, T4 | +| AC2.2 (unparseable remote → toplevel basename) | T2, T4 | +| AC2.3 (no remote → no org tag, existing behaviour) | T3, T4 | +| AC3.1–AC3.6 (unit tests for URL forms) | T2 | +| AC3.7 (integration: repo+org from remote) | T4 | +| AC3.8 (integration: repo fallback without remote) | T4 | +| AC4.1 (cargo test passes) | T5 | +| AC4.2 (cargo clippy passes) | T5 | +| AC4.3 (failure means not complete) | T5 | +| AC5.1 (release-notes note — human action) | — (design doc already satisfies; no code task) | + +## Tasks + +### T1: Add `repo_from_remote_url` function + +**File:** `src/tags.rs` + +**What:** Add a new private function `repo_from_remote_url` as a sibling to `org_from_remote_url`, placed immediately after `org_from_remote_url` (after line 71, before `gather_env_tags` at line 73). Include a `///` doc comment mirroring the style of `org_from_remote_url`'s doc comment (lines 32–41), documenting the URL forms handled. + +**Implementation detail:** + +The function normalises the URL identically to `org_from_remote_url` but returns the **last** path segment instead of the second-to-last. The normalisation logic is the same: + +1. Trim whitespace, strip trailing `.git`. +2. If URL contains `://`, take the path after the first `/` following the scheme part (discards `user@host`). +3. Otherwise split on `:` for SCP-style (`git@github.com:org/repo`); if no `:` and no `://`, return `None` (local path). +4. Split path on `/`, filter empty segments. +5. Require `segments.len() >= 2` (same guard — a bare repo name with no org is not a valid remote URL for this purpose). +6. Return `segments[segments.len() - 1]` (the repo name, last segment). Return `None` if empty. + +```rust +/// Extract the repository name from a git remote URL. +/// +/// Handles SCP-style (`git@github.com:org/repo.git`), +/// ssh:// (`ssh://git@github.com/org/repo.git`), +/// and https:// (`https://github.com/org/repo.git`) forms. +/// Returns `None` for local paths or unparseable URLs. +fn repo_from_remote_url(url: &str) -> Option { + let url = url.trim().trim_end_matches(".git"); + + let path = match url.split_once("://") { + Some((_, rest)) => { + let after_host = rest.split_once('/').map(|(_, h)| h).unwrap_or(rest); + after_host + } + None => match url.split_once(':') { + Some((_, rest)) => rest, + None => return None, + }, + }; + + let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + if segments.len() < 2 { + return None; + } + let repo = segments[segments.len() - 1]; + if repo.is_empty() { + None + } else { + Some(repo.to_string()) + } +} +``` + +**Dependencies:** None (first task). + +**Test:** T2 (unit tests for this function). + +**Verification command:** +```bash +cargo build 2>&1 +``` +Expected: compiles with no errors (function is unused until T3 wires it in, so there may be a dead-code warning — that's acceptable and resolved by T3). + +**Commit:** `feat: add repo_from_remote_url function` + +--- + +### T2: Add `repo_from_remote_url` unit tests + +**File:** `src/tags.rs` (in the `#[cfg(test)] mod tests` block, after the existing `org_from_local_path_returns_none` test at line 257 and before the `org_tag_emitted_when_remote_present` test at line 259) + +**What:** Add 6 unit test functions mirroring the existing `org_*` URL-parsing tests one-to-one. Each asserts on the repo name (last path segment) instead of the org (second-to-last). + +**Tests to add:** + +```rust +#[test] +fn repo_from_ssh_url() { + assert_eq!( + repo_from_remote_url("git@github.com:acme/widgets.git"), + Some("widgets".to_string()) + ); +} + +#[test] +fn repo_from_https_url() { + assert_eq!( + repo_from_remote_url("https://github.com/acme/widgets.git"), + Some("widgets".to_string()) + ); +} + +#[test] +fn repo_from_ssh_protocol_url() { + assert_eq!( + repo_from_remote_url("ssh://git@github.com/acme/widgets.git"), + Some("widgets".to_string()) + ); +} + +#[test] +fn repo_from_https_url_without_git_suffix() { + assert_eq!( + repo_from_remote_url("https://github.com/acme/widgets"), + Some("widgets".to_string()) + ); +} + +#[test] +fn repo_from_gitlab_subgroup_url() { + assert_eq!( + repo_from_remote_url("git@gitlab.com:acme/platform/widgets.git"), + Some("widgets".to_string()) + ); +} + +#[test] +fn repo_from_local_path_returns_none() { + assert_eq!( + repo_from_remote_url("/home/doug/projects/widgets"), + None + ); +} +``` + +**Dependencies:** T1 (function must exist). + +**Verification command:** +```bash +cargo test repo_from 2>&1 +``` +Expected: 6 tests pass, 0 failures: +``` +running 6 tests +test tags::tests::repo_from_ssh_url ... ok +test tags::tests::repo_from_https_url ... ok +test tags::tests::repo_from_ssh_protocol_url ... ok +test tags::tests::repo_from_https_url_without_git_suffix ... ok +test tags::tests::repo_from_gitlab_subgroup_url ... ok +test tags::tests::repo_from_local_path_returns_none ... ok + +test result: ok. 6 passed; 0 failed +``` + +**Commit:** `test: add repo_from_remote_url unit tests` + +--- + +### T3: Restructure `gather_env_tags` repo + org blocks + +**File:** `src/tags.rs` (lines 82–94 of the current code) + +**What:** Replace the current two independent blocks (repo at lines 82–87, org at lines 89–94) with a single coordinated block that fetches the origin remote URL once and derives both tags. The show-toplevel git command becomes the fallback path only. + +**Current code to replace (lines 82–94):** + +```rust + // Git repo name + if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } + } + + // Git organisation (owner) from the origin remote URL. + if let Some(url) = git_cmd(&["remote", "get-url", "origin"], cwd) { + if let Some(org) = org_from_remote_url(&url) { + tags.push(format!("org:{org}")); + } + } +``` + +**Replacement code:** + +```rust + // Git repo name and organisation from the origin remote URL. + // Falls back to the show-toplevel basename for repo when no origin + // remote exists or the URL is unparseable (e.g. local path). + if let Some(url) = git_cmd(&["remote", "get-url", "origin"], cwd) { + match repo_from_remote_url(&url) { + Some(repo) => tags.push(format!("repo:{repo}")), + None => { + if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } + } + } + } + if let Some(org) = org_from_remote_url(&url) { + tags.push(format!("org:{org}")); + } + } else { + // No origin remote: fall back to show-toplevel basename for repo. + if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } + } + } +``` + +**Key behavioural properties of this restructure:** +- When a remote URL is present and parseable: `repo:` comes from `repo_from_remote_url`, `org:` comes from `org_from_remote_url`, and `show-toplevel` is never called (avoids a redundant git subprocess in the common case). +- When a remote URL is present but unparseable (local path): `repo:` falls back to show-toplevel basename, `org:` is not emitted (same as current `org_from_remote_url` returning `None`). +- When no origin remote exists: `repo:` falls back to show-toplevel basename, no `org:` tag (existing behaviour preserved). +- Tag ordering is preserved: `repo:` before `org:`, matching the current order. + +**Dependencies:** T1 (uses `repo_from_remote_url`). + +**Test:** T4 (integration tests exercise `gather_env_tags` with and without remotes). Existing `org_tag_emitted_when_remote_present` and `no_org_tag_without_remote` tests must still pass. + +**Verification command:** +```bash +cargo test 2>&1 +``` +Expected: all existing tests pass (including `org_tag_emitted_when_remote_present` and `no_org_tag_without_remote`). No new tests yet — those come in T4. The build should have no dead-code warning now (function is used). + +**Commit:** `feat: derive repo tag from origin remote URL with show-toplevel fallback` + +--- + +### T4: Add `gather_env_tags` integration-style tests for repo tag + +**File:** `src/tags.rs` (in the `#[cfg(test)] mod tests` block, after the existing `no_org_tag_without_remote` test) + +**What:** Add two new integration-style tests that exercise `gather_env_tags` end-to-end via temp-dir git repos, mirroring the existing `org_tag_emitted_when_remote_present` (line 259) and `no_org_tag_without_remote` (line 279) patterns. + +**Tests to add:** + +```rust +#[test] +fn repo_tag_from_remote_when_present() { + let dir = tempfile::tempdir().unwrap(); + let dir_path = dir.path().to_str().unwrap(); + // Initialise a git repo and add an origin remote. + Command::new("git") + .args(["init"]) + .current_dir(dir_path) + .output() + .unwrap(); + Command::new("git") + .args(["remote", "add", "origin", "git@github.com:acme/widgets.git"]) + .current_dir(dir_path) + .output() + .unwrap(); + + let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); + + // Repo tag comes from the remote URL, not the temp-dir name. + assert!( + tags.iter().any(|t| t == "repo:widgets"), + "expected repo:widgets in tags: {tags:?}" + ); + // Org tag should also be present. + assert!( + tags.iter().any(|t| t == "org:acme"), + "expected org:acme in tags: {tags:?}" + ); + // Should NOT contain repo:. + let dir_basename = std::path::Path::new(dir_path) + .file_name() + .unwrap() + .to_string_lossy() + .to_string(); + assert!( + !tags.iter().any(|t| t == format!("repo:{dir_basename}")), + "should not contain repo:{dir_basename} in tags: {tags:?}" + ); +} + +#[test] +fn repo_tag_falls_back_to_toplevel_without_remote() { + let dir = tempfile::tempdir().unwrap(); + let dir_path = dir.path().to_str().unwrap(); + // Initialise a git repo with no remote. + Command::new("git") + .args(["init"]) + .current_dir(dir_path) + .output() + .unwrap(); + + let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); + + let dir_basename = std::path::Path::new(dir_path) + .file_name() + .unwrap() + .to_string_lossy() + .to_string(); + assert!( + tags.iter().any(|t| t == format!("repo:{dir_basename}")), + "expected repo:{dir_basename} in tags: {tags:?}" + ); + // No org tag should be present without a remote. + assert!( + !tags.iter().any(|t| t.starts_with("org:")), + "should not contain any org: tag in tags: {tags:?}" + ); +} +``` + +**Dependencies:** T3 (the restructured `gather_env_tags` that derives repo from remote URL). + +**Verification command:** +```bash +cargo test repo_tag 2>&1 +``` +Expected: 2 tests pass, 0 failures: +``` +running 2 tests +test tags::tests::repo_tag_from_remote_when_present ... ok +test tags::tests::repo_tag_falls_back_to_toplevel_without_remote ... ok + +test result: ok. 2 passed; 0 failed +``` + +**Commit:** `test: add gather_env_tags integration tests for repo tag derivation` + +--- + +### T5: Run full test suite and clippy + +**What:** Run the complete verification suite per the design's loop section. Fix any failures or warnings that arise from the changes in T1–T4. + +**Verification commands:** +```bash +cargo test 2>&1 +``` +Expected: all tests pass (existing + 6 new unit tests + 2 new integration tests), 0 failures. + +```bash +cargo clippy 2>&1 +``` +Expected: zero warnings. + +**If failures occur:** fix the code or tests until both commands pass clean. Common issues to watch for: +- Clippy may suggest using `unwrap_or_else` instead of `match` in the fallback — follow the suggestion if it arises. +- The `repo_from_remote_url` function's normalisation logic must produce exactly the same path-parsing behaviour as `org_from_remote_url` for all tested URL forms — if a test fails, check segment indexing (`segments.len() - 1` vs `segments.len() - 2`). +- Ensure the `tempfile` crate is already a dev-dependency (it is used by existing tests at lines 259 and 279, so it should already be available). + +**Dependencies:** T1, T2, T3, T4 (all code and test changes must be in place). + +**Commit:** (only if fixes were needed) `fix: resolve test/clippy issues from repo tag restructure` + +--- + +## Task dependency graph + +``` +T1 (add repo_from_remote_url) +├── T2 (add unit tests) depends on T1 +└── T3 (restructure gather_env_tags) depends on T1 + └── T4 (add integration tests) depends on T3 + └── T5 (full test + clippy) depends on T1, T2, T3, T4 +``` + +T1 is the root. T2 and T3 can proceed in parallel after T1. T4 depends on T3. T5 depends on everything. diff --git a/planflow/changes/fix-repo-tag-worktree/specs/tags/spec.md b/planflow/changes/fix-repo-tag-worktree/specs/tags/spec.md new file mode 100644 index 0000000..dad7b56 --- /dev/null +++ b/planflow/changes/fix-repo-tag-worktree/specs/tags/spec.md @@ -0,0 +1,104 @@ +# Delta for tags + +## ADDED Requirements + +### Requirement: Repo tag derived from origin remote URL +The system SHALL derive the `repo:` tag from the last path segment of the origin remote URL (with trailing `.git` stripped), not from the working-tree directory basename. + +#### Scenario: Clone in a worktree-named directory with SSH remote +- GIVEN a git repository whose origin remote is `git@github.com:org/hbf.git` +- AND the working tree directory is named `wt579b` (a linked worktree) +- WHEN `gather_env_tags` collects tags +- THEN the tag `repo:hbf` SHALL be emitted, not `repo:wt579b` + +#### Scenario: Clone in a ticket-named directory with HTTPS remote +- GIVEN a git repository whose origin remote is `https://github.com/org/hbf.git` +- AND the working tree directory is named `hbf-578-idfix` +- WHEN `gather_env_tags` collects tags +- THEN the tag `repo:hbf` SHALL be emitted, not `repo:hbf-578-idfix` + +#### Scenario: ssh:// protocol URL form +- GIVEN a git repository whose origin remote is `ssh://git@github.com/org/hbf.git` +- WHEN `gather_env_tags` collects tags +- THEN the tag `repo:hbf` SHALL be emitted regardless of the working-tree directory name + +#### Scenario: GitLab subgroup URL +- GIVEN a git repository whose origin remote is `git@gitlab.com:acme/platform/widgets.git` +- WHEN `gather_env_tags` collects tags +- THEN the tag `repo:widgets` SHALL be emitted (last path segment, ignoring subgroups) + +#### Scenario: HTTPS URL without .git suffix +- GIVEN a git repository whose origin remote is `https://github.com/acme/widgets` (no trailing `.git`) +- WHEN `gather_env_tags` collects tags +- THEN the tag `repo:widgets` SHALL be emitted + +### Requirement: Repo tag fallback to show-toplevel basename +The system SHALL fall back to the `git rev-parse --show-toplevel` basename for the `repo:` tag when no origin remote exists, or when the origin remote URL is unparseable (e.g. a local filesystem path). + +#### Scenario: No origin remote +- GIVEN a git repository with no origin remote configured +- WHEN `gather_env_tags` collects tags +- THEN the tag `repo:` SHALL be emitted, where toplevel-basename is the directory name from `git rev-parse --show-toplevel` +- AND no `org:` tag SHALL be emitted + +#### Scenario: Origin remote is a local path +- GIVEN a git repository whose origin remote is `/home/doug/projects/widgets` (a local filesystem path, no scheme and no SCP-style colon) +- WHEN `gather_env_tags` collects tags +- THEN `repo_from_remote_url` SHALL return `None` for the local path +- AND the tag `repo:` SHALL be emitted as a fallback + +### Requirement: Show-toplevel not called when remote URL is parseable +The system SHALL NOT invoke `git rev-parse --show-toplevel` when the origin remote URL is present and successfully parsed by `repo_from_remote_url`, to avoid a redundant git subprocess in the common case. + +#### Scenario: Remote present and parseable +- GIVEN a git repository with a valid origin remote URL (SCP-style, ssh://, or https://) +- WHEN `gather_env_tags` collects tags +- THEN `repo:` SHALL be derived from `repo_from_remote_url` +- AND `git rev-parse --show-toplevel` SHALL NOT be invoked + +### Requirement: repo_from_remote_url URL normalisation +The system SHALL provide a `repo_from_remote_url` function that normalises git remote URLs identically to `org_from_remote_url` but returns the last path segment (repository name) instead of the second-to-last (organisation). It SHALL return `None` for local paths and unparseable URLs. + +#### Scenario: SCP-style URL +- GIVEN `repo_from_remote_url("git@github.com:acme/widgets.git")` +- WHEN the function is called +- THEN it SHALL return `Some("widgets")` + +#### Scenario: HTTPS URL +- GIVEN `repo_from_remote_url("https://github.com/acme/widgets.git")` +- WHEN the function is called +- THEN it SHALL return `Some("widgets")` + +#### Scenario: ssh:// protocol URL +- GIVEN `repo_from_remote_url("ssh://git@github.com/acme/widgets.git")` +- WHEN the function is called +- THEN it SHALL return `Some("widgets")` + +#### Scenario: HTTPS URL without .git suffix +- GIVEN `repo_from_remote_url("https://github.com/acme/widgets")` +- WHEN the function is called +- THEN it SHALL return `Some("widgets")` + +#### Scenario: GitLab subgroup URL +- GIVEN `repo_from_remote_url("git@gitlab.com:acme/platform/widgets.git")` +- WHEN the function is called +- THEN it SHALL return `Some("widgets")` + +#### Scenario: Local path +- GIVEN `repo_from_remote_url("/home/doug/projects/widgets")` +- WHEN the function is called +- THEN it SHALL return `None` + +## MODIFIED Requirements + +### Requirement: Org tag derivation unchanged +The system SHALL derive the `org:` tag from `org_from_remote_url` applied to the origin remote URL, identically to the pre-change behaviour. The restructure of the repo and org tag blocks into a single coordinated block SHALL NOT alter the org tag's value, ordering (after `repo:`), or fallback behaviour (no org tag when no remote or unparseable URL). + +Previously: The org tag was derived independently in a separate block that fetched `git remote get-url origin` and called `org_from_remote_url`. The repo tag was derived independently in a separate block that fetched `git rev-parse --show-toplevel` and took the basename. + +## REMOVED Requirements + +### Requirement: Repo tag derived from show-toplevel basename +The system no longer derives the `repo:` tag from the basename of `git rev-parse --show-toplevel` as the primary method. This is now the fallback only, used when no origin remote URL is available or the URL is unparseable. The primary derivation is from the origin remote URL's last path segment. + +(Reason: the show-toplevel basename produces incorrect repo tags for git worktrees and non-standard clone directory names, as described in the design document.) From de1fdaa0c7e0a3f4491f63db0f0ef1476610c58d Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:17:55 +0100 Subject: [PATCH 03/10] work: task graph and allocation for fix-repo-tag-worktree --- .../changes/fix-repo-tag-worktree/work.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 planflow/changes/fix-repo-tag-worktree/work.md diff --git a/planflow/changes/fix-repo-tag-worktree/work.md b/planflow/changes/fix-repo-tag-worktree/work.md new file mode 100644 index 0000000..5e38862 --- /dev/null +++ b/planflow/changes/fix-repo-tag-worktree/work.md @@ -0,0 +1,43 @@ +# Work Plan: Fix repo tagging for git worktrees and non-standard clone directories + +Source: `planflow/changes/fix-repo-tag-worktree/implementation.md` + +## Summary + +Five tasks, all agent-owned, touching a single file (`src/tags.rs`). The change adds a `repo_from_remote_url` sibling function to the existing `org_from_remote_url`, wires it into `gather_env_tags` with a show-toplevel fallback, and adds unit + integration tests. A final task runs the full test suite and clippy. + +## Dependency graph + +``` +T1 (add repo_from_remote_url) [agent, S] +├── T2 (add unit tests) [agent, S] depends on T1 +└── T3 (restructure gather_env_tags) [agent, M] depends on T1 + └── T4 (add integration tests) [agent, S] depends on T3 + └── T5 (full test + clippy) [agent, S] depends on T1, T2, T3, T4 + ── checkpoint: human review before merge +``` + +### Parallel streams + +- **T1** is the single entry point. Nothing can start until it lands. +- **T2 and T3** are both unblocked after T1. They touch different parts of `src/tags.rs` (T2 adds tests at the bottom of the test module; T3 replaces the repo/org blocks inside `gather_env_tags`). Since `planflow-apply` commits per-task and both edit the same file, the implementor applies them sequentially — but they can be dispatched together and are logically independent. +- **T4** depends on T3 (needs the restructured `gather_env_tags`). +- **T5** depends on everything and is the final verification gate. + +### Human checkpoints + +- **After T5**: pause for human review of the full diff before merge. This is the only checkpoint — the change is small, test-verified at each step, and carries no client-facing or credentials risk. + +## Allocation + +All tasks are `agent`-owned. No human-owned tasks in the code pipeline; the release-notes note about historical traces (AC5.1) is a human action at release time, not a code task, and is already documented in the design. + +## Task table + +| id | title | owner | depends_on | size | checkpoint | +|----|-------|-------|------------|------|------------| +| T1 | Add `repo_from_remote_url` function | agent | [] | S | false | +| T2 | Add `repo_from_remote_url` unit tests | agent | [T1] | S | false | +| T3 | Restructure `gather_env_tags` repo + org blocks | agent | [T1] | M | false | +| T4 | Add `gather_env_tags` integration tests for repo tag | agent | [T3] | S | false | +| T5 | Run full test suite and clippy | agent | [T1, T2, T3, T4] | S | true | From 2195f55159c1c827a818c174f0a14c12d47a4791 Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:18:58 +0100 Subject: [PATCH 04/10] feat: add repo_from_remote_url function Task: T1 --- src/tags.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/tags.rs b/src/tags.rs index 096fb91..53d781f 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -70,6 +70,38 @@ fn org_from_remote_url(url: &str) -> Option { } } +/// Extract the repository name from a git remote URL. +/// +/// Handles SCP-style (`git@github.com:org/repo.git`), +/// ssh:// (`ssh://git@github.com/org/repo.git`), +/// and https:// (`https://github.com/org/repo.git`) forms. +/// Returns `None` for local paths or unparseable URLs. +fn repo_from_remote_url(url: &str) -> Option { + let url = url.trim().trim_end_matches(".git"); + + let path = match url.split_once("://") { + Some((_, rest)) => { + let after_host = rest.split_once('/').map(|(_, h)| h).unwrap_or(rest); + after_host + } + None => match url.split_once(':') { + Some((_, rest)) => rest, + None => return None, + }, + }; + + let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + if segments.len() < 2 { + return None; + } + let repo = segments[segments.len() - 1]; + if repo.is_empty() { + None + } else { + Some(repo.to_string()) + } +} + pub fn gather_env_tags(source: Source, cwd: Option<&str>, agent_version: Option<&str>) -> Vec { let mut tags = vec![source.agent_tag().to_string()]; From 31199fab7e0d3646c2ed941d8b44109fcc8e2808 Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:19:41 +0100 Subject: [PATCH 05/10] test: add repo_from_remote_url unit tests Task: T2 --- src/tags.rs | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/tags.rs b/src/tags.rs index 53d781f..63e1281 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -287,6 +287,51 @@ mod tests { assert_eq!(org_from_remote_url("/home/doug/projects/widgets"), None); } + #[test] + fn repo_from_ssh_url() { + assert_eq!( + repo_from_remote_url("git@github.com:acme/widgets.git"), + Some("widgets".to_string()) + ); + } + + #[test] + fn repo_from_https_url() { + assert_eq!( + repo_from_remote_url("https://github.com/acme/widgets.git"), + Some("widgets".to_string()) + ); + } + + #[test] + fn repo_from_ssh_protocol_url() { + assert_eq!( + repo_from_remote_url("ssh://git@github.com/acme/widgets.git"), + Some("widgets".to_string()) + ); + } + + #[test] + fn repo_from_https_url_without_git_suffix() { + assert_eq!( + repo_from_remote_url("https://github.com/acme/widgets"), + Some("widgets".to_string()) + ); + } + + #[test] + fn repo_from_gitlab_subgroup_url() { + assert_eq!( + repo_from_remote_url("git@gitlab.com:acme/platform/widgets.git"), + Some("widgets".to_string()) + ); + } + + #[test] + fn repo_from_local_path_returns_none() { + assert_eq!(repo_from_remote_url("/home/doug/projects/widgets"), None); + } + #[test] fn org_tag_emitted_when_remote_present() { let repo = tempfile::TempDir::new().unwrap(); From a75053732fab60df40ac3c31ff612d1f01b25ba1 Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:22:42 +0100 Subject: [PATCH 06/10] feat: derive repo tag from origin remote URL with show-toplevel fallback Task: T3 --- src/tags.rs | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/tags.rs b/src/tags.rs index 63e1281..3e80144 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -111,18 +111,30 @@ pub fn gather_env_tags(source: Source, cwd: Option<&str>, agent_version: Option< } } - // Git repo name - if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { - if let Some(name) = std::path::Path::new(&toplevel).file_name() { - tags.push(format!("repo:{}", name.to_string_lossy())); - } - } - - // Git organisation (owner) from the origin remote URL. + // Git repo name and organisation from the origin remote URL. + // Falls back to the show-toplevel basename for repo when no origin + // remote exists or the URL is unparseable (e.g. local path). if let Some(url) = git_cmd(&["remote", "get-url", "origin"], cwd) { + match repo_from_remote_url(&url) { + Some(repo) => tags.push(format!("repo:{repo}")), + None => { + if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } + } + } + } if let Some(org) = org_from_remote_url(&url) { tags.push(format!("org:{org}")); } + } else { + // No origin remote: fall back to show-toplevel basename for repo. + if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } + } } // Git branch From e09c83f443300952a890e4ed03fe1058d793f49b Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:23:42 +0100 Subject: [PATCH 07/10] test: add gather_env_tags integration tests for repo tag derivation Task: T4 --- src/tags.rs | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/tags.rs b/src/tags.rs index 3e80144..29fb9cb 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -373,4 +373,73 @@ mod tests { let tags = gather_env_tags(Source::ClaudeCode, Some(&repo_path), None); assert!(!tags.iter().any(|t| t.starts_with("org:"))); } + + #[test] + fn repo_tag_from_remote_when_present() { + let dir = tempfile::tempdir().unwrap(); + let dir_path = dir.path().to_str().unwrap(); + // Initialise a git repo and add an origin remote. + Command::new("git") + .args(["init"]) + .current_dir(dir_path) + .output() + .unwrap(); + Command::new("git") + .args(["remote", "add", "origin", "git@github.com:acme/widgets.git"]) + .current_dir(dir_path) + .output() + .unwrap(); + + let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); + + // Repo tag comes from the remote URL, not the temp-dir name. + assert!( + tags.iter().any(|t| t == "repo:widgets"), + "expected repo:widgets in tags: {tags:?}" + ); + // Org tag should also be present. + assert!( + tags.iter().any(|t| t == "org:acme"), + "expected org:acme in tags: {tags:?}" + ); + // Should NOT contain repo:. + let dir_basename = std::path::Path::new(dir_path) + .file_name() + .unwrap() + .to_string_lossy() + .to_string(); + assert!( + !tags.iter().any(|t| *t == format!("repo:{dir_basename}")), + "should not contain repo:{dir_basename} in tags: {tags:?}" + ); + } + + #[test] + fn repo_tag_falls_back_to_toplevel_without_remote() { + let dir = tempfile::tempdir().unwrap(); + let dir_path = dir.path().to_str().unwrap(); + // Initialise a git repo with no remote. + Command::new("git") + .args(["init"]) + .current_dir(dir_path) + .output() + .unwrap(); + + let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); + + let dir_basename = std::path::Path::new(dir_path) + .file_name() + .unwrap() + .to_string_lossy() + .to_string(); + assert!( + tags.iter().any(|t| *t == format!("repo:{dir_basename}")), + "expected repo:{dir_basename} in tags: {tags:?}" + ); + // No org tag should be present without a remote. + assert!( + !tags.iter().any(|t| t.starts_with("org:")), + "should not contain any org: tag in tags: {tags:?}" + ); + } } \ No newline at end of file From f45cb6464450957facdb91f59175ef1331ddaceb Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:26:50 +0100 Subject: [PATCH 08/10] test: assert git setup success in repo tag tests Task: T4 (review fix) --- planflow/housekeeping.md | 3 +++ src/tags.rs | 9 ++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 planflow/housekeeping.md diff --git a/planflow/housekeeping.md b/planflow/housekeeping.md new file mode 100644 index 0000000..a0b6758 --- /dev/null +++ b/planflow/housekeeping.md @@ -0,0 +1,3 @@ +# Housekeeping + +- [ ] lint | src/tags.rs:102 | `gather_env_tags` signature exceeds rustfmt line width (pre-existing from T3 restructure; `cargo fmt --check` fails there and across most `tests/*` files) | fix | observed in fix-repo-tag-worktree diff --git a/src/tags.rs b/src/tags.rs index 29fb9cb..2ec71a6 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -379,16 +379,18 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let dir_path = dir.path().to_str().unwrap(); // Initialise a git repo and add an origin remote. - Command::new("git") + let output = Command::new("git") .args(["init"]) .current_dir(dir_path) .output() .unwrap(); - Command::new("git") + assert!(output.status.success()); + let output = Command::new("git") .args(["remote", "add", "origin", "git@github.com:acme/widgets.git"]) .current_dir(dir_path) .output() .unwrap(); + assert!(output.status.success()); let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); @@ -419,11 +421,12 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let dir_path = dir.path().to_str().unwrap(); // Initialise a git repo with no remote. - Command::new("git") + let output = Command::new("git") .args(["init"]) .current_dir(dir_path) .output() .unwrap(); + assert!(output.status.success()); let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); From e64f58e9a92a135f8bc1ae55352b37273b00a4ad Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:28:53 +0100 Subject: [PATCH 09/10] test: use git_cmd helper for setup in repo tag tests Task: T4 (review fix 2) --- src/tags.rs | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/tags.rs b/src/tags.rs index 2ec71a6..797f26d 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -378,19 +378,14 @@ mod tests { fn repo_tag_from_remote_when_present() { let dir = tempfile::tempdir().unwrap(); let dir_path = dir.path().to_str().unwrap(); - // Initialise a git repo and add an origin remote. - let output = Command::new("git") - .args(["init"]) - .current_dir(dir_path) - .output() - .unwrap(); - assert!(output.status.success()); - let output = Command::new("git") + assert!(git_cmd(&["init"], Some(dir_path)).is_some()); + // `git remote add` produces no stdout, so check exit status directly. + let status = Command::new("git") .args(["remote", "add", "origin", "git@github.com:acme/widgets.git"]) .current_dir(dir_path) - .output() + .status() .unwrap(); - assert!(output.status.success()); + assert!(status.success()); let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); @@ -421,12 +416,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let dir_path = dir.path().to_str().unwrap(); // Initialise a git repo with no remote. - let output = Command::new("git") - .args(["init"]) - .current_dir(dir_path) - .output() - .unwrap(); - assert!(output.status.success()); + assert!(git_cmd(&["init"], Some(dir_path)).is_some()); let tags = gather_env_tags(Source::Opencode, Some(dir_path), None); From fb42c8f3bf5522b1ae5e0bb7d8b901209a0e6501 Mon Sep 17 00:00:00 2001 From: winjer Date: Fri, 25 Sep 2026 11:34:39 +0100 Subject: [PATCH 10/10] refactor: extract show-toplevel fallback into closure Task: T3 (pr-agent review suggestion) --- src/tags.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/tags.rs b/src/tags.rs index 797f26d..ee87a02 100644 --- a/src/tags.rs +++ b/src/tags.rs @@ -114,27 +114,24 @@ pub fn gather_env_tags(source: Source, cwd: Option<&str>, agent_version: Option< // Git repo name and organisation from the origin remote URL. // Falls back to the show-toplevel basename for repo when no origin // remote exists or the URL is unparseable (e.g. local path). + let mut push_repo_from_toplevel = || { + if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { + if let Some(name) = std::path::Path::new(&toplevel).file_name() { + tags.push(format!("repo:{}", name.to_string_lossy())); + } + } + }; if let Some(url) = git_cmd(&["remote", "get-url", "origin"], cwd) { match repo_from_remote_url(&url) { Some(repo) => tags.push(format!("repo:{repo}")), - None => { - if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { - if let Some(name) = std::path::Path::new(&toplevel).file_name() { - tags.push(format!("repo:{}", name.to_string_lossy())); - } - } - } + None => push_repo_from_toplevel(), } if let Some(org) = org_from_remote_url(&url) { tags.push(format!("org:{org}")); } } else { // No origin remote: fall back to show-toplevel basename for repo. - if let Some(toplevel) = git_cmd(&["rev-parse", "--show-toplevel"], cwd) { - if let Some(name) = std::path::Path::new(&toplevel).file_name() { - tags.push(format!("repo:{}", name.to_string_lossy())); - } - } + push_repo_from_toplevel(); } // Git branch