From 5726a8916e7300c238379e6900f47f4302737af4 Mon Sep 17 00:00:00 2001 From: Rodrigo Mourey <38763157+Mourey@users.noreply.github.com> Date: Thu, 9 Jul 2026 21:29:20 +0200 Subject: [PATCH 1/2] fix(kanban): branch dispatched worktrees from fresh remote tip The interactive `hermes -w` path branches new worktrees from the freshly-fetched remote tip via `_resolve_worktree_base` (gated by `worktree_sync`, default on, with a fail-soft fallback to local HEAD). The kanban DISPATCH path never inherited this: `_ensure_git_worktree` hardcoded "HEAD" for the new-branch case, so a dispatched card branched from the standalone clone's (often stale) local HEAD. That roots the new branch on an old merge base, which later surfaces as textual merge conflicts against a moved origin/main and inflates the PR diff. Port the base-freshness logic into the dispatch path: - Extract `_resolve_worktree_base` from cli.py into a new shared module `hermes_cli/worktree_sync.py` (verbatim body + its own module logger). kanban_db must not import cli (cli imports kanban_db, not vice versa), so the helper lives where both can import it. cli.py now imports it and re-exports the same name, keeping behavior and the public symbol identical. - `_ensure_git_worktree` now resolves the base ref for the NEW-branch case (gated by `worktree_sync`, defensively defaulting to True on config error) and branches from it. It mirrors the interactive path's fail-soft retry: if `worktree add ` fails and base_ref != HEAD, it retries once from local HEAD before raising, so a fetch hiccup never hard-fails worktree creation. The existing-branch resume path is unchanged. Tests: new `tests/hermes_cli/test_kanban_worktree_base.py` proves the new-branch worktree contains the remote-only commit (branched from the fetched tip, not stale HEAD), the offline/unusable-ref fallbacks still succeed from HEAD, sync-off branches from local HEAD, and the existing-branch resume path is unchanged. Co-Authored-By: Claude Opus 4.8 --- cli.py | 140 +----------- hermes_cli/kanban_db.py | 67 +++++- hermes_cli/worktree_sync.py | 156 +++++++++++++ tests/hermes_cli/test_kanban_worktree_base.py | 214 ++++++++++++++++++ 4 files changed, 427 insertions(+), 150 deletions(-) create mode 100644 hermes_cli/worktree_sync.py create mode 100644 tests/hermes_cli/test_kanban_worktree_base.py diff --git a/cli.py b/cli.py index 26e202711d47..7e17f164afa3 100644 --- a/cli.py +++ b/cli.py @@ -55,6 +55,7 @@ from hermes_cli.cli_commands_mixin import CLICommandsMixin from hermes_cli.cli_billing_mixin import CLIBillingMixin from agent.interrupt_compat import request_hard_interrupt +from hermes_cli.worktree_sync import _resolve_worktree_base # prompt_toolkit for fixed input area TUI from prompt_toolkit.history import FileHistory @@ -1466,145 +1467,6 @@ def _path_is_within_root(path: Path, root: Path) -> bool: return False -def _resolve_worktree_base( - repo_root: str, - fetch_timeout: float = 5, - freshness_window: float = 300, -) -> tuple: - """Resolve the freshest base ref to branch a new worktree from. - - The standalone clone's ``HEAD`` can lag the remote by hundreds of commits - (the ``~/.hermes/hermes-agent`` clone is updated only by ``hermes update``, - not on every session). Branching a worktree from that stale ``HEAD`` roots - every new branch on an old base — so the PR diff GitHub computes against - current ``main`` balloons with unrelated changes, and the agent has to - discover the staleness via the pre-push gate and rebase. Branching from the - freshly-fetched remote tip instead means the worktree starts current. - - Strategy (each step falls back to the next on failure): - 1. If the current branch tracks an upstream, refresh and use that - upstream ref — so a deliberate feature-branch worktree tracks its own - remote, not the default branch. - 2. Else refresh the remote's default branch (``origin/HEAD`` → e.g. - ``origin/main``) and use it. - 3. Else fall back to ``HEAD`` (offline, no remote, or detached) — the - old behavior, never worse than before. - - "Refresh" is deliberately cheap on the startup path (the fetch here used - to stall ``hermes -w`` launches for 30-60s on flaky smart-HTTP - connections): - - - The fetch is SKIPPED entirely when the repo's ``FETCH_HEAD`` is younger - than *freshness_window* seconds — a base fetched moments ago cannot have - meaningfully moved, so repeated launches don't re-pay a network round - trip. - - The fetch is capped at *fetch_timeout* seconds. On timeout or failure we - fall back to the locally-known remote-tracking ref (labelled "cached") - instead of cascading into a second fetch attempt. Genuine staleness is - backstopped by the pre-push stale-base gate. - - Returns ``(base_ref, label)`` where *base_ref* is a git revision suitable - for ``git worktree add ... `` and *label* is a short - human-readable description for the session banner. - """ - import subprocess - - from hermes_cli._subprocess_compat import noninteractive_git_env - - def _git(args, timeout: float = 20): - return subprocess.run( - ["git", *args], - capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout, cwd=repo_root, - stdin=subprocess.DEVNULL, - env=noninteractive_git_env(), - ) - - def _ref_exists(ref: str) -> bool: - try: - return _git(["rev-parse", "--verify", "--quiet", ref + "^{commit}"]).returncode == 0 - except Exception: - return False - - def _fetch_head_age() -> Optional[float]: - """Seconds since the last fetch in this repo, or None if unknown.""" - try: - gd = _git(["rev-parse", "--git-dir"]) - if gd.returncode != 0: - return None - git_dir = Path(gd.stdout.strip()) - if not git_dir.is_absolute(): - git_dir = Path(repo_root) / git_dir - fetch_head = git_dir / "FETCH_HEAD" - if not fetch_head.exists(): - return None - return max(0.0, time.time() - fetch_head.stat().st_mtime) - except Exception: - return None - - def _refresh(remote: str, branch: str, ref: str) -> tuple: - """Return (ref, label) after a cheap best-effort refresh of *ref*. - - Never raises, never fetches twice, never blocks longer than - *fetch_timeout*. - """ - age = _fetch_head_age() - if age is not None and age < freshness_window and _ref_exists(ref): - return ref, f"{ref} (fetched {int(age)}s ago)" - try: - fetched = _git(["fetch", remote, branch], timeout=fetch_timeout) - if fetched.returncode == 0: - return ref, f"{ref} (fetched)" - reason = "fetch failed" - except subprocess.TimeoutExpired: - reason = f"fetch timed out after {fetch_timeout:g}s" - except Exception as e: - reason = f"fetch error: {e}" - if _ref_exists(ref): - logger.debug("worktree base: %s — using cached %s", reason, ref) - return ref, f"{ref} (cached — {reason})" - return "HEAD", f"HEAD (local — {reason}, no cached {ref})" - - # 1. Current branch's upstream, if it tracks one. - try: - up = _git(["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"]) - if up.returncode == 0: - upstream = up.stdout.strip() # e.g. "origin/main" - if upstream and "/" in upstream: - remote, branch = upstream.split("/", 1) - return _refresh(remote, branch, upstream) - except Exception as e: - logger.debug("worktree base: upstream resolution failed: %s", e) - - # 2. Remote default branch (origin/HEAD). - try: - # Resolve the remote's default branch symref. - head_ref = _git(["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"]) - default_ref = "" - if head_ref.returncode == 0: - default_ref = head_ref.stdout.strip().replace("refs/remotes/", "", 1) - if not default_ref: - # origin/HEAD not set locally; ask the remote (network — capped - # like the fetch so a stalled connection can't hang startup). - show = _git(["remote", "show", "origin"], timeout=max(fetch_timeout, 5)) - for line in show.stdout.splitlines(): - line = line.strip() - if line.startswith("HEAD branch:"): - _branch = line.split(":", 1)[1].strip() - # A remote with no default branch reports "(unknown)"; - # don't construct a bogus "origin/(unknown)" ref from it. - if _branch and _branch != "(unknown)": - default_ref = "origin/" + _branch - break - if default_ref and "/" in default_ref: - remote, branch = default_ref.split("/", 1) - return _refresh(remote, branch, default_ref) - except Exception as e: - logger.debug("worktree base: default-branch resolution failed: %s", e) - - # 3. Fall back to local HEAD (offline / no remote / detached). - return "HEAD", "HEAD (local — could not reach remote)" - - def _setup_worktree(repo_root: str = None, sync_base: bool = True) -> Optional[Dict[str, str]]: """Create an isolated git worktree for this CLI session. diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 113e34842ec5..671fd5a9da63 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -90,6 +90,7 @@ from typing import Any, Iterable, Mapping, Optional from hermes_cli.sqlite_util import add_column_if_missing as _add_column_if_missing +from hermes_cli.worktree_sync import _resolve_worktree_base from toolsets import get_toolset_names _log = logging.getLogger(__name__) @@ -6495,19 +6496,63 @@ def _ensure_git_worktree(repo_root: Path, target: Path, branch_name: str) -> Non return target.parent.mkdir(parents=True, exist_ok=True) if _git_branch_exists(repo_root, branch_name): + # Resume an existing branch: check it out as-is, no new base to pick. cmd = ["git", "-C", str(repo_root), "worktree", "add", str(target), branch_name] + result = subprocess.run( + cmd, + capture_output=True, + text=True, encoding='utf-8', errors='replace', + timeout=60, + check=False, + ) + if result.returncode != 0: + stderr = (result.stderr or result.stdout or "").strip() + raise RuntimeError( + f"git worktree add failed for {target} on branch {branch_name}: {stderr}" + ) + return + + # New-branch case: branch from the freshly-fetched remote tip by default so + # a dispatched card starts current with the project, not the standalone + # clone's (possibly stale) local HEAD. Branching from a stale HEAD roots the + # branch on an old merge base, which surfaces later as textual merge + # conflicts against a moved origin/main. This mirrors the interactive + # ``hermes -w`` path (cli._setup_worktree). Opt out with worktree_sync: + # false in config. + try: + from hermes_cli.config import load_config + sync_base = bool(load_config().get("worktree_sync", True)) + except Exception: + sync_base = True + if sync_base: + try: + base_ref, _label = _resolve_worktree_base(str(repo_root)) + except Exception: + base_ref = "HEAD" else: - cmd = [ - "git", "-C", str(repo_root), "worktree", "add", "-b", branch_name, - str(target), "HEAD", - ] - result = subprocess.run( - cmd, - capture_output=True, - text=True, encoding='utf-8', errors='replace', - timeout=60, - check=False, - ) + base_ref = "HEAD" + + def _worktree_add(base: str) -> subprocess.CompletedProcess: + return subprocess.run( + ["git", "-C", str(repo_root), "worktree", "add", "-b", branch_name, + str(target), base], + capture_output=True, + text=True, encoding='utf-8', errors='replace', + timeout=60, + check=False, + ) + + result = _worktree_add(base_ref) + if result.returncode != 0 and base_ref != "HEAD": + # Fail-soft: a partial fetch can leave the resolved remote ref unusable. + # Retry from local HEAD so worktree creation never hard-fails on a sync + # hiccup — same fallback the interactive _setup_worktree uses. + stderr = (result.stderr or result.stdout or "").strip() + _log.warning( + "worktree add for %s from %s failed (%s); retrying from local HEAD", + target, base_ref, stderr, + ) + result = _worktree_add("HEAD") if result.returncode != 0: stderr = (result.stderr or result.stdout or "").strip() raise RuntimeError( diff --git a/hermes_cli/worktree_sync.py b/hermes_cli/worktree_sync.py new file mode 100644 index 000000000000..d125678b027b --- /dev/null +++ b/hermes_cli/worktree_sync.py @@ -0,0 +1,156 @@ +"""Shared worktree base-ref resolution. + +Extracted from ``cli.py`` so both the interactive ``hermes -w`` path +(``cli.py``) and the kanban dispatch path (``hermes_cli/kanban_db.py``) can +branch a new worktree from the freshly-fetched remote tip. ``kanban_db`` must +not import ``cli`` (``cli`` imports ``kanban_db``, not vice versa), so this +helper lives in a module both can import. + +The body is ``cli._resolve_worktree_base`` verbatim; ``cli`` now imports the +symbol from here, so the two paths cannot drift apart again. +""" +import logging +import time +from pathlib import Path +from typing import Optional + +logger = logging.getLogger(__name__) + + +def _resolve_worktree_base( + repo_root: str, + fetch_timeout: float = 5, + freshness_window: float = 300, +) -> tuple: + """Resolve the freshest base ref to branch a new worktree from. + + The standalone clone's ``HEAD`` can lag the remote by hundreds of commits + (the ``~/.hermes/hermes-agent`` clone is updated only by ``hermes update``, + not on every session). Branching a worktree from that stale ``HEAD`` roots + every new branch on an old base — so the PR diff GitHub computes against + current ``main`` balloons with unrelated changes, and the agent has to + discover the staleness via the pre-push gate and rebase. Branching from the + freshly-fetched remote tip instead means the worktree starts current. + + Strategy (each step falls back to the next on failure): + 1. If the current branch tracks an upstream, refresh and use that + upstream ref — so a deliberate feature-branch worktree tracks its own + remote, not the default branch. + 2. Else refresh the remote's default branch (``origin/HEAD`` → e.g. + ``origin/main``) and use it. + 3. Else fall back to ``HEAD`` (offline, no remote, or detached) — the + old behavior, never worse than before. + + "Refresh" is deliberately cheap on the startup path (the fetch here used + to stall ``hermes -w`` launches for 30-60s on flaky smart-HTTP + connections): + + - The fetch is SKIPPED entirely when the repo's ``FETCH_HEAD`` is younger + than *freshness_window* seconds — a base fetched moments ago cannot have + meaningfully moved, so repeated launches don't re-pay a network round + trip. + - The fetch is capped at *fetch_timeout* seconds. On timeout or failure we + fall back to the locally-known remote-tracking ref (labelled "cached") + instead of cascading into a second fetch attempt. Genuine staleness is + backstopped by the pre-push stale-base gate. + + Returns ``(base_ref, label)`` where *base_ref* is a git revision suitable + for ``git worktree add ... `` and *label* is a short + human-readable description for the session banner. + """ + import subprocess + + from hermes_cli._subprocess_compat import noninteractive_git_env + + def _git(args, timeout: float = 20): + return subprocess.run( + ["git", *args], + capture_output=True, text=True, encoding="utf-8", errors="replace", timeout=timeout, cwd=repo_root, + stdin=subprocess.DEVNULL, + env=noninteractive_git_env(), + ) + + def _ref_exists(ref: str) -> bool: + try: + return _git(["rev-parse", "--verify", "--quiet", ref + "^{commit}"]).returncode == 0 + except Exception: + return False + + def _fetch_head_age() -> Optional[float]: + """Seconds since the last fetch in this repo, or None if unknown.""" + try: + gd = _git(["rev-parse", "--git-dir"]) + if gd.returncode != 0: + return None + git_dir = Path(gd.stdout.strip()) + if not git_dir.is_absolute(): + git_dir = Path(repo_root) / git_dir + fetch_head = git_dir / "FETCH_HEAD" + if not fetch_head.exists(): + return None + return max(0.0, time.time() - fetch_head.stat().st_mtime) + except Exception: + return None + + def _refresh(remote: str, branch: str, ref: str) -> tuple: + """Return (ref, label) after a cheap best-effort refresh of *ref*. + + Never raises, never fetches twice, never blocks longer than + *fetch_timeout*. + """ + age = _fetch_head_age() + if age is not None and age < freshness_window and _ref_exists(ref): + return ref, f"{ref} (fetched {int(age)}s ago)" + try: + fetched = _git(["fetch", remote, branch], timeout=fetch_timeout) + if fetched.returncode == 0: + return ref, f"{ref} (fetched)" + reason = "fetch failed" + except subprocess.TimeoutExpired: + reason = f"fetch timed out after {fetch_timeout:g}s" + except Exception as e: + reason = f"fetch error: {e}" + if _ref_exists(ref): + logger.debug("worktree base: %s — using cached %s", reason, ref) + return ref, f"{ref} (cached — {reason})" + return "HEAD", f"HEAD (local — {reason}, no cached {ref})" + + # 1. Current branch's upstream, if it tracks one. + try: + up = _git(["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"]) + if up.returncode == 0: + upstream = up.stdout.strip() # e.g. "origin/main" + if upstream and "/" in upstream: + remote, branch = upstream.split("/", 1) + return _refresh(remote, branch, upstream) + except Exception as e: + logger.debug("worktree base: upstream resolution failed: %s", e) + + # 2. Remote default branch (origin/HEAD). + try: + # Resolve the remote's default branch symref. + head_ref = _git(["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"]) + default_ref = "" + if head_ref.returncode == 0: + default_ref = head_ref.stdout.strip().replace("refs/remotes/", "", 1) + if not default_ref: + # origin/HEAD not set locally; ask the remote (network — capped + # like the fetch so a stalled connection can't hang startup). + show = _git(["remote", "show", "origin"], timeout=max(fetch_timeout, 5)) + for line in show.stdout.splitlines(): + line = line.strip() + if line.startswith("HEAD branch:"): + _branch = line.split(":", 1)[1].strip() + # A remote with no default branch reports "(unknown)"; + # don't construct a bogus "origin/(unknown)" ref from it. + if _branch and _branch != "(unknown)": + default_ref = "origin/" + _branch + break + if default_ref and "/" in default_ref: + remote, branch = default_ref.split("/", 1) + return _refresh(remote, branch, default_ref) + except Exception as e: + logger.debug("worktree base: default-branch resolution failed: %s", e) + + # 3. Fall back to local HEAD (offline / no remote / detached). + return "HEAD", "HEAD (local — could not reach remote)" diff --git a/tests/hermes_cli/test_kanban_worktree_base.py b/tests/hermes_cli/test_kanban_worktree_base.py new file mode 100644 index 000000000000..aeb4f25bbf4a --- /dev/null +++ b/tests/hermes_cli/test_kanban_worktree_base.py @@ -0,0 +1,214 @@ +"""Tests for the kanban dispatch path branching worktrees from the fresh tip. + +The interactive ``hermes -w`` path already branches new worktrees from the +freshly-fetched remote tip (``cli._setup_worktree`` → ``_resolve_worktree_base``). +The kanban DISPATCH path (``kanban_db._ensure_git_worktree``) historically +hardcoded ``HEAD`` for the new-branch case, so a dispatched card branched from +the standalone clone's (possibly stale) local ``HEAD`` — rooting the branch on +an old merge base and surfacing later as textual merge conflicts against a +moved ``origin/main``. + +These tests exercise the REAL ``kanban_db._ensure_git_worktree`` against a real +local "remote" repo (so ``git fetch`` works offline in the hermetic sandbox), +proving the new-branch worktree includes commits that exist on the remote but +not on the stale local ``HEAD`` — and that it still fails soft to ``HEAD`` when +the remote is unreachable. +""" + +import subprocess +from pathlib import Path + +import pytest + +from hermes_cli import kanban_db + + +def _run(args, cwd): + return subprocess.run(args, cwd=cwd, capture_output=True, text=True, timeout=30) + + +def _commit(repo, name, msg): + (Path(repo) / name).write_text(msg + "\n") + _run(["git", "add", "."], repo) + _run(["git", "commit", "-m", msg], repo) + + +def _head(repo): + return _run(["git", "rev-parse", "HEAD"], repo).stdout.strip() + + +@pytest.fixture(autouse=True) +def _sync_on(monkeypatch): + """Default every test to worktree_sync ON unless it overrides the flag. + + ``_ensure_git_worktree`` reads ``worktree_sync`` via + ``hermes_cli.config.load_config``; pin it so the test does not depend on + the developer's real ~/.hermes/config.yaml. + """ + monkeypatch.setattr( + "hermes_cli.config.load_config", lambda: {"worktree_sync": True} + ) + + +@pytest.fixture +def remote_and_clone(tmp_path): + """A bare 'remote' + a clone that is intentionally BEHIND the remote. + + Returns (clone_path, remote_head_sha, stale_local_head_sha). + """ + remote = tmp_path / "remote.git" + seed = tmp_path / "seed" + seed.mkdir() + _run(["git", "init"], seed) + _run(["git", "config", "user.email", "t@t.com"], seed) + _run(["git", "config", "user.name", "T"], seed) + # Pin the seed repo's branch name so push + remote default are 'main'. + _run(["git", "checkout", "-b", "main"], seed) + _commit(seed, "README.md", "base commit") + _run(["git", "init", "--bare", str(remote)], tmp_path) + _run(["git", "remote", "add", "origin", str(remote)], seed) + _run(["git", "push", "origin", "main"], seed) + # Set the bare remote's default branch so a clone gets origin/HEAD -> + # origin/main and a tracking branch (mirrors a real GitHub remote). + _run(["git", "symbolic-ref", "HEAD", "refs/heads/main"], remote) + + # Clone it (this clone tracks origin/main). + clone = tmp_path / "clone" + _run(["git", "clone", str(remote), str(clone)], tmp_path) + _run(["git", "config", "user.email", "t@t.com"], clone) + _run(["git", "config", "user.name", "T"], clone) + stale_local_head = _head(clone) + + # Advance the REMOTE past the clone (simulating other merges landing on + # main while this clone sat stale). + _commit(seed, "feature.txt", "remote-only commit") + _run(["git", "push", "origin", "main"], seed) + remote_head = _head(seed) + + assert remote_head != stale_local_head + return clone, remote_head, stale_local_head + + +class TestEnsureGitWorktreeSyncBase: + def test_new_branch_branches_from_remote_tip(self, remote_and_clone): + """worktree_sync on: a NEW-branch dispatch worktree starts from the + fetched remote tip, not the stale local HEAD — so it contains the + remote-only commit.""" + clone, remote_head, stale_local_head = remote_and_clone + target = clone / ".worktrees" / "card-1" + kanban_db._ensure_git_worktree(clone, target, "wt/card-1") + + assert target.exists() + wt_head = _head(target) + assert wt_head == remote_head, ( + "dispatched worktree should start from the fetched remote tip" + ) + assert wt_head != stale_local_head + # And it must contain the remote-only file. + assert (target / "feature.txt").exists() + + def test_sync_disabled_branches_from_local_head(self, remote_and_clone, monkeypatch): + """worktree_sync off: opt back into the old behavior — branch from the + stale local HEAD (no remote-only commit).""" + monkeypatch.setattr( + "hermes_cli.config.load_config", lambda: {"worktree_sync": False} + ) + clone, remote_head, stale_local_head = remote_and_clone + target = clone / ".worktrees" / "card-off" + kanban_db._ensure_git_worktree(clone, target, "wt/card-off") + + assert _head(target) == stale_local_head + assert not (target / "feature.txt").exists() + + +class TestEnsureGitWorktreeOfflineFallback: + def test_unreachable_remote_falls_back_to_head(self, remote_and_clone): + """A fetch hiccup must never hard-fail worktree creation: with the + remote pointed at a nonexistent path, resolution/fetch can't reach the + tip, so creation falls back to local HEAD and still succeeds.""" + clone, _remote_head, stale_local_head = remote_and_clone + # Break the remote so `git fetch` inside _resolve_worktree_base fails. + _run(["git", "remote", "set-url", "origin", + str(clone.parent / "does-not-exist.git")], clone) + + target = clone / ".worktrees" / "card-offline" + # Must not raise. + kanban_db._ensure_git_worktree(clone, target, "wt/card-offline") + + assert target.exists() + # Fetch failed, so the base falls back to the stale local HEAD. + assert _head(target) == stale_local_head + + def test_unusable_base_ref_retries_from_head(self, remote_and_clone, monkeypatch): + """If base resolution yields a ref that ``git worktree add`` can't use + (e.g. a partial fetch left it dangling), creation retries once from + local HEAD rather than hard-failing — mirrors _setup_worktree.""" + clone, _remote_head, stale_local_head = remote_and_clone + # Force a base ref that does not exist so the first `worktree add` fails. + monkeypatch.setattr( + kanban_db, "_resolve_worktree_base", + lambda root: ("origin/does-not-exist", "bogus"), + ) + target = clone / ".worktrees" / "card-retry" + # Must not raise; the HEAD retry succeeds. + kanban_db._ensure_git_worktree(clone, target, "wt/card-retry") + + assert target.exists() + assert _head(target) == stale_local_head + + def test_no_remote_repo_still_creates(self, tmp_path): + """A repo with no remote at all resolves base to HEAD and creates the + new-branch worktree without error.""" + repo = tmp_path / "no-remote" + repo.mkdir() + _run(["git", "init"], repo) + _run(["git", "config", "user.email", "t@t.com"], repo) + _run(["git", "config", "user.name", "T"], repo) + _commit(repo, "README.md", "only commit") + head = _head(repo) + + target = repo / ".worktrees" / "card-lonely" + kanban_db._ensure_git_worktree(repo, target, "wt/card-lonely") + + assert target.exists() + assert _head(target) == head + + +class TestEnsureGitWorktreeExistingBranch: + def test_existing_branch_resume_unchanged(self, remote_and_clone): + """The existing-branch resume path is untouched by the base-freshness + change: when the branch already exists, the worktree checks it out + as-is (no new base is picked).""" + clone, _remote_head, stale_local_head = remote_and_clone + # Create the branch first (off the stale local HEAD), with a commit that + # is unique to it so we can prove the worktree checks out THIS branch. + _run(["git", "branch", "wt/existing", "HEAD"], clone) + # Put a commit on the branch via a throwaway worktree, then remove it, + # so the branch tip diverges from both HEAD and the remote tip. + tmp_wt = clone / ".worktrees" / "seed-existing" + _run(["git", "-C", str(clone), "worktree", "add", str(tmp_wt), "wt/existing"], clone) + _commit(tmp_wt, "branch_only.txt", "commit only on wt/existing") + branch_tip = _head(tmp_wt) + _run(["git", "-C", str(clone), "worktree", "remove", "--force", str(tmp_wt)], clone) + + assert branch_tip != stale_local_head + + target = clone / ".worktrees" / "card-resume" + kanban_db._ensure_git_worktree(clone, target, "wt/existing") + + assert target.exists() + assert _head(target) == branch_tip, ( + "resume path should check out the existing branch tip as-is" + ) + assert (target / "branch_only.txt").exists() + + def test_already_materialized_worktree_is_noop(self, remote_and_clone): + """If the target is already a linked worktree of the same repo, + _ensure_git_worktree returns early without re-adding (idempotent).""" + clone, remote_head, _stale = remote_and_clone + target = clone / ".worktrees" / "card-idem" + kanban_db._ensure_git_worktree(clone, target, "wt/idem") + first_head = _head(target) + # Second call with the same target+branch must not raise. + kanban_db._ensure_git_worktree(clone, target, "wt/idem") + assert _head(target) == first_head From 51f4c0aa9e843b804d3dd9692d286f13d8d10d21 Mon Sep 17 00:00:00 2001 From: Rodrigo Mourey <38763157+Mourey@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:50:05 +0200 Subject: [PATCH 2/2] test(kanban): make the offline worktree-base case discriminating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offline test asserted the resulting worktree matched local `HEAD`, but the fixture left local `HEAD` and the stale `origin/main` tracking ref pointing at the same commit — so it passed no matter which base resolution picked, and could not detect a regression in the fallback. Advance local `HEAD` past the tracking ref first, then assert the base actually chosen. Current `_resolve_worktree_base` falls back to the cached remote-tracking ref when a fetch fails (HEAD only when no cached ref exists), so the rewritten case pins that contract from the dispatch path and additionally asserts the worktree does NOT inherit the primary checkout's local-only commits. A sibling case covers the no-cached-ref path resolving to `HEAD` without hard-failing worktree creation. Addresses the hermes-sweeper review on #61626, which flagged the non-discriminating fixture. Co-Authored-By: Claude Opus 5 --- tests/hermes_cli/test_kanban_worktree_base.py | 60 ++++++++++++++++--- 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/tests/hermes_cli/test_kanban_worktree_base.py b/tests/hermes_cli/test_kanban_worktree_base.py index aeb4f25bbf4a..82687846315b 100644 --- a/tests/hermes_cli/test_kanban_worktree_base.py +++ b/tests/hermes_cli/test_kanban_worktree_base.py @@ -11,8 +11,8 @@ These tests exercise the REAL ``kanban_db._ensure_git_worktree`` against a real local "remote" repo (so ``git fetch`` works offline in the hermetic sandbox), proving the new-branch worktree includes commits that exist on the remote but -not on the stale local ``HEAD`` — and that it still fails soft to ``HEAD`` when -the remote is unreachable. +not on the stale local ``HEAD`` — and that an unreachable remote fails soft +(cached remote-tracking ref, then ``HEAD``) instead of hard-failing. """ import subprocess @@ -122,11 +122,25 @@ def test_sync_disabled_branches_from_local_head(self, remote_and_clone, monkeypa class TestEnsureGitWorktreeOfflineFallback: - def test_unreachable_remote_falls_back_to_head(self, remote_and_clone): - """A fetch hiccup must never hard-fail worktree creation: with the - remote pointed at a nonexistent path, resolution/fetch can't reach the - tip, so creation falls back to local HEAD and still succeeds.""" - clone, _remote_head, stale_local_head = remote_and_clone + def test_unreachable_remote_uses_cached_tracking_ref(self, remote_and_clone): + """A fetch hiccup must never hard-fail worktree creation. + + With the remote pointed at a nonexistent path the fetch fails, so + ``_resolve_worktree_base`` falls back to the locally-cached + ``origin/main`` — not local ``HEAD``, which on a dispatch host is + "whatever branch the primary checkout is parked on". Genuine staleness + of the cached ref is backstopped by the pre-push stale-base gate. + + Local ``HEAD`` is advanced past the cached tracking ref first: without + that divergence both candidates resolve to the same commit and the + assertion cannot tell which one was chosen. + """ + clone, _remote_head, _stale = remote_and_clone + _commit(clone, "local_only.txt", "commit only in the local clone") + local_head = _head(clone) + cached_ref = _run(["git", "rev-parse", "origin/main"], clone).stdout.strip() + assert local_head != cached_ref + # Break the remote so `git fetch` inside _resolve_worktree_base fails. _run(["git", "remote", "set-url", "origin", str(clone.parent / "does-not-exist.git")], clone) @@ -136,8 +150,36 @@ def test_unreachable_remote_falls_back_to_head(self, remote_and_clone): kanban_db._ensure_git_worktree(clone, target, "wt/card-offline") assert target.exists() - # Fetch failed, so the base falls back to the stale local HEAD. - assert _head(target) == stale_local_head + assert _head(target) == cached_ref, ( + "a failed fetch should branch from the cached remote-tracking ref" + ) + assert not (target / "local_only.txt").exists(), ( + "the dispatched worktree must not inherit the primary checkout's " + "local-only commits" + ) + + def test_unreachable_remote_without_cached_ref_falls_back_to_head(self, tmp_path): + """No usable tracking ref + failed fetch: base is local ``HEAD`` and + worktree creation still succeeds rather than hard-failing on a bogus + ref.""" + repo = tmp_path / "broken-upstream" + repo.mkdir() + _run(["git", "init"], repo) + _run(["git", "config", "user.email", "t@t.com"], repo) + _run(["git", "config", "user.name", "T"], repo) + _run(["git", "checkout", "-b", "main"], repo) + _commit(repo, "README.md", "base") + # A branch that claims an upstream which has no tracking ref locally. + _run(["git", "remote", "add", "origin", str(tmp_path / "nonexistent.git")], repo) + _run(["git", "config", "branch.main.remote", "origin"], repo) + _run(["git", "config", "branch.main.merge", "refs/heads/main"], repo) + head = _head(repo) + + target = repo / ".worktrees" / "card-no-cache" + kanban_db._ensure_git_worktree(repo, target, "wt/card-no-cache") + + assert target.exists() + assert _head(target) == head def test_unusable_base_ref_retries_from_head(self, remote_and_clone, monkeypatch): """If base resolution yields a ref that ``git worktree add`` can't use