mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-10-20 05:15:31 +00:00
Merge pull request #71795 from NousResearch/bb/dead-worktree-projects
fix(projects): stop deleted worktrees showing up as their own sidebar projects
This commit is contained in:
commit
dacd8d5416
3 changed files with 155 additions and 12 deletions
|
|
@ -486,6 +486,86 @@ def test_deleted_sibling_worktree_folds_into_parent_home_checkout():
|
|||
assert len(main["sessions"]) == 2
|
||||
|
||||
|
||||
def test_deleted_sibling_worktree_subdir_folds_into_parent_home_checkout():
|
||||
# Same as above, but the session's cwd is a SUBDIR of the deleted worktree
|
||||
# (an agent that cd-ed into `<repo>-<suffix>/apps/desktop`). The leaf name
|
||||
# ("desktop") shares nothing with the repo, so the sibling probe has to walk
|
||||
# the ancestors — otherwise the dead path is minted as its own project.
|
||||
resolve = _resolver({"/www/hermes-agent": ("/www/hermes-agent", "/www/hermes-agent")})
|
||||
sessions = [
|
||||
_session("/www/hermes-agent", branch="main"),
|
||||
_session("/www/hermes-agent-guiperf/apps/desktop"),
|
||||
]
|
||||
|
||||
tree = pt.build_tree([], sessions, [], resolve, hydrate=True)
|
||||
|
||||
assert [p["id"] for p in tree["projects"]] == ["/www/hermes-agent"]
|
||||
project = tree["projects"][0]
|
||||
assert _lane_ids(project) == ["/www/hermes-agent::branch::main"]
|
||||
assert len(project["repos"][0]["groups"][0]["sessions"]) == 2
|
||||
|
||||
|
||||
def test_deleted_unrelated_workspace_does_not_become_a_project():
|
||||
# A deleted dir the sibling probe can't reach by name (`hermes-salvage-drafts`
|
||||
# shares no prefix with `hermes-agent`; `/tmp/scratch` was never a worktree)
|
||||
# must not be promoted to a phantom project — it can never be opened and can
|
||||
# only be dismissed by hand. The session stays in flat Recents.
|
||||
resolve = _resolver({"/www/hermes-agent": ("/www/hermes-agent", "/www/hermes-agent")})
|
||||
sessions = [
|
||||
_session("/www/hermes-agent", branch="main"),
|
||||
_session("/www/hermes-salvage-drafts/apps/desktop"),
|
||||
_session("/tmp/scratch"),
|
||||
]
|
||||
on_disk = {"/www/hermes-agent"}
|
||||
|
||||
tree = pt.build_tree([], sessions, [], resolve, hydrate=True, exists=lambda p: p in on_disk)
|
||||
|
||||
assert [p["id"] for p in tree["projects"]] == ["/www/hermes-agent"]
|
||||
|
||||
|
||||
def test_existing_non_git_workspace_still_becomes_a_project():
|
||||
# The existence guard keys on the DIRECTORY, not on git-ness: a plain folder
|
||||
# that's still on disk is a legitimate workspace and must keep its project.
|
||||
sessions = [_session("/www/notes")]
|
||||
|
||||
tree = pt.build_tree([], sessions, [], lambda _cwd: None, hydrate=True, exists=lambda _p: True)
|
||||
|
||||
assert [p["id"] for p in tree["projects"]] == ["/www/notes"]
|
||||
|
||||
|
||||
def test_stale_persisted_repo_root_does_not_become_a_project():
|
||||
# A session carrying a git_repo_root whose repo has since been deleted must
|
||||
# not resurrect it as a project on the strength of the persisted value alone.
|
||||
sessions = [_session("/tmp/gone/sub", repo_root="/tmp/gone")]
|
||||
|
||||
tree = pt.build_tree([], sessions, [], lambda _cwd: None, hydrate=True, exists=lambda _p: False)
|
||||
|
||||
assert tree["projects"] == []
|
||||
|
||||
|
||||
def test_exists_defaults_to_keeping_everything():
|
||||
# Omitting `exists` (remote backends, which can't stat) preserves the old
|
||||
# behavior: guessing "gone" would wrongly hide a project on the other host.
|
||||
sessions = [_session("/remote/workspace")]
|
||||
|
||||
tree = pt.build_tree([], sessions, [], lambda _cwd: None, hydrate=True)
|
||||
|
||||
assert [p["id"] for p in tree["projects"]] == ["/remote/workspace"]
|
||||
|
||||
|
||||
def test_sibling_probe_is_bounded():
|
||||
# Each miss costs a git invocation, and this runs per session, so a deeply
|
||||
# nested unresolvable cwd must not fan out into an unbounded probe storm.
|
||||
probed = []
|
||||
|
||||
def resolve(cwd):
|
||||
probed.append(cwd)
|
||||
return None
|
||||
|
||||
assert pt._probe_sibling_worktree("/a-b-c/d-e-f/g-h-i/j-k-l", resolve) == ""
|
||||
assert len(probed) <= pt._MAX_SIBLING_PROBES
|
||||
|
||||
|
||||
def test_colliding_repo_basenames_disambiguate_labels():
|
||||
resolve = _resolver(
|
||||
{
|
||||
|
|
|
|||
|
|
@ -34,6 +34,13 @@ from typing import Any, Callable, Optional
|
|||
# cwd is not in a git repo (or cannot be probed, e.g. a remote backend).
|
||||
Resolve = Callable[[str], Optional[dict]]
|
||||
|
||||
# A "does this directory still exist?" predicate, injected for the same reason
|
||||
# ``Resolve`` is: the builder stays pure and unit-testable. Defaults to assuming
|
||||
# everything exists, which preserves the previous behavior for callers that
|
||||
# can't stat (remote backends), where guessing "gone" would wrongly hide a
|
||||
# project that lives on the other host.
|
||||
Exists = Callable[[str], bool]
|
||||
|
||||
# Only KANBAN-TASK worktrees (`<repo>/.worktrees/t_<hex>`, the `t_…` id kanban_db
|
||||
# mints) collapse into one lane; user-named "New worktree" dirs under
|
||||
# `.worktrees/` stay as their own lanes.
|
||||
|
|
@ -151,20 +158,40 @@ def _placement(
|
|||
}
|
||||
|
||||
|
||||
def _parent_dir(path: str) -> str:
|
||||
"""The containing directory of ``path`` (``""`` once the root is passed)."""
|
||||
stripped = re.sub(r"[/\\]+$", "", path or "")
|
||||
return re.sub(r"[/\\]+$", "", re.sub(r"[^/\\]+$", "", stripped))
|
||||
|
||||
|
||||
def _probe_sibling_worktree(cwd: str, resolve: Resolve) -> str:
|
||||
"""The parent repo root of a deleted ``<repo>-<suffix>`` worktree, else ``""``.
|
||||
|
||||
A deleted worktree dir can't be probed, so walk back up its name — trimming
|
||||
one ``-<segment>`` at a time — and return the first sibling that resolves.
|
||||
Probes are bounded and served from the shared git-probe cache.
|
||||
"""
|
||||
parts = base_name(cwd).split("-")
|
||||
floor = max(0, len(parts) - 1 - _MAX_SIBLING_PROBES)
|
||||
|
||||
for i in range(len(parts) - 1, floor, -1):
|
||||
info = resolve(_with_base_name(cwd, "-".join(parts[:i])))
|
||||
if info and info.get("repo_root"):
|
||||
return (info["repo_root"] or "").strip()
|
||||
The session's cwd is frequently a SUBDIR of the deleted worktree (an agent
|
||||
that ``cd``-ed into ``<repo>-<suffix>/apps/desktop``), whose basename shares
|
||||
nothing with the repo. So the trim is applied to each ANCESTOR, deepest
|
||||
first, not just to the leaf — otherwise the probe silently no-ops and the
|
||||
dead path gets minted as its own top-level project. Probes are bounded in
|
||||
total (each costs a git invocation) and served from the shared probe cache.
|
||||
"""
|
||||
probes = 0
|
||||
path = re.sub(r"[/\\]+$", "", cwd or "")
|
||||
|
||||
while path and probes < _MAX_SIBLING_PROBES:
|
||||
parts = base_name(path).split("-")
|
||||
|
||||
for i in range(len(parts) - 1, 0, -1):
|
||||
if probes >= _MAX_SIBLING_PROBES:
|
||||
break
|
||||
probes += 1
|
||||
info = resolve(_with_base_name(path, "-".join(parts[:i])))
|
||||
if info and info.get("repo_root"):
|
||||
return (info["repo_root"] or "").strip()
|
||||
|
||||
path = _parent_dir(path)
|
||||
|
||||
return ""
|
||||
|
||||
|
|
@ -506,6 +533,7 @@ def build_tree(
|
|||
hydrate: bool = False,
|
||||
is_junk_root: Optional[Callable[[str], bool]] = None,
|
||||
is_junk_cwd: Optional[Callable[[str], bool]] = None,
|
||||
exists: Optional[Exists] = None,
|
||||
) -> dict:
|
||||
"""Build the authoritative project tree.
|
||||
|
||||
|
|
@ -517,7 +545,10 @@ def build_tree(
|
|||
bare home dir, the HERMES_HOME subtree). ``is_junk_cwd`` is the narrower
|
||||
policy for non-git session folders: selected descendants may be intentional
|
||||
workspaces even when their parent tree contains Hermes state. User-created
|
||||
projects are honored regardless.
|
||||
projects are honored regardless. ``exists`` reports whether a directory is
|
||||
still on disk, so a session whose workspace was DELETED (a removed worktree,
|
||||
a scratch dir under /tmp) doesn't get promoted to a phantom AUTO project;
|
||||
omit it (remote backends) to keep every candidate.
|
||||
|
||||
Returns ``{"projects": [...], "scoped_session_ids": [...]}``. When
|
||||
``hydrate`` is False (overview), lane ``sessions`` arrays are emptied but
|
||||
|
|
@ -527,6 +558,7 @@ def build_tree(
|
|||
active_projects = [p for p in projects if not p.get("archived")]
|
||||
_junk = is_junk_root or (lambda _root: False)
|
||||
_junk_cwd = is_junk_cwd or (lambda _cwd: False)
|
||||
_exists = exists or (lambda _path: True)
|
||||
folder_index = _FolderIndex(active_projects)
|
||||
|
||||
by_project: dict[str, list[dict]] = {}
|
||||
|
|
@ -589,8 +621,10 @@ def build_tree(
|
|||
root = _session_repo_root(session, resolve)
|
||||
if root:
|
||||
# A real git root uses the stricter repo policy. Do not reinterpret a
|
||||
# filtered internal repo as a cwd-only project.
|
||||
if not _junk(root):
|
||||
# filtered internal repo as a cwd-only project. A root that no longer
|
||||
# exists is a stale persisted value (the repo was deleted after the
|
||||
# session ran) and must not resurrect as a project.
|
||||
if not _junk(root) and _exists(root):
|
||||
_add_auto(root, session)
|
||||
continue
|
||||
|
||||
|
|
@ -603,7 +637,13 @@ def build_tree(
|
|||
resolve,
|
||||
(session.get("git_repo_root") or "").strip(),
|
||||
)
|
||||
if placement:
|
||||
# A placement that only echoes back the unresolvable cwd is the
|
||||
# path-only heuristic guessing — it never found a repo. When that dir is
|
||||
# also gone from disk (a deleted worktree whose name shares no prefix
|
||||
# with its parent, a removed /tmp scratch dir), promoting it mints a
|
||||
# phantom project that can never be opened and can only be dismissed by
|
||||
# hand. The session keeps its place in flat Recents.
|
||||
if placement and _exists(placement["repo_key"]):
|
||||
_add_auto(placement["repo_key"], session)
|
||||
|
||||
seen: set[str] = set()
|
||||
|
|
|
|||
|
|
@ -13888,12 +13888,34 @@ def _project_tree_inputs(
|
|||
return sessions, projects, discovered, active_id
|
||||
|
||||
|
||||
# Per-build memo for `_dir_exists_cached`. Cleared at the top of every
|
||||
# `_build_project_tree`, so a dir created or deleted between sidebar refreshes
|
||||
# is seen on the next one.
|
||||
_DIR_EXISTS_CACHE: dict[str, bool] = {}
|
||||
|
||||
|
||||
def _dir_exists_cached(path: str) -> bool:
|
||||
"""``os.path.isdir`` for the project tree, memoized per build.
|
||||
|
||||
``build_tree`` asks per SESSION, not per distinct path, so a power user with
|
||||
hundreds of sessions across a handful of dirs would otherwise fire hundreds
|
||||
of redundant stats on every sidebar open. The memo is per build, so a dir
|
||||
created or deleted between refreshes is picked up on the next one.
|
||||
"""
|
||||
hit = _DIR_EXISTS_CACHE.get(path)
|
||||
if hit is None:
|
||||
hit = os.path.isdir(path)
|
||||
_DIR_EXISTS_CACHE[path] = hit
|
||||
return hit
|
||||
|
||||
|
||||
def _build_project_tree(
|
||||
db, *, preview_limit: int, hydrate: bool, session_limit: int, include_discovered: bool
|
||||
) -> tuple[dict, str | None]:
|
||||
"""Gather inputs and run the one authoritative builder. Returns (tree, active_id)."""
|
||||
from tui_gateway import project_tree
|
||||
|
||||
_DIR_EXISTS_CACHE.clear()
|
||||
sessions, projects, discovered, active_id = _project_tree_inputs(
|
||||
db, session_limit, include_discovered=include_discovered
|
||||
)
|
||||
|
|
@ -13906,6 +13928,7 @@ def _build_project_tree(
|
|||
hydrate=hydrate,
|
||||
is_junk_root=_is_repo_junk,
|
||||
is_junk_cwd=_is_session_cwd_junk,
|
||||
exists=_dir_exists_cached,
|
||||
)
|
||||
return tree, active_id
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue