From 2dcef860a64b032a9a7044d09c3c758982fb18ac Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 17:17:35 +0100 Subject: [PATCH] fix(maestro): group worktree sessions under their repository The sidebar nested a worktree session under the worktree, so a repo grew a /worktrees/ folder holding one session of the same name, and the repo stopped being where you look for the work. The daemon recorded whatever base_repo the request named, which is the worktree itself when spawning from inside one; it now records the resolved main checkout. The web tree groups by that instead of the cwd. The TUI sidebar is a flat list with no directory grouping, so it needs no mirror. Closes #99 --- crates/maestro-daemon/src/session.rs | 6 +- crates/maestro-daemon/src/worktree.rs | 7 +++ .../maestro-daemon/tests/session_manager.rs | 57 +++++++++++++++++++ web/src/tree.test.ts | 56 ++++++++++++++++++ web/src/tree.ts | 6 +- web/src/types.ts | 23 +++++++- 6 files changed, 151 insertions(+), 4 deletions(-) diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index fd27f50..a8150bb 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -278,7 +278,11 @@ impl SessionManager { self.inner.worktree_mgr.add_existing(base_repo, branch) } .map_err(SessionError::Worktree)?; - (path.clone(), Some(path), Some(base_repo.clone())) + // Record the repository, not whatever the request named: a + // spawn from inside a linked worktree passes that worktree as + // `base_repo`, and clients group sessions by this path (#99). + let repo = crate::worktree::main_checkout_of(base_repo); + (path.clone(), Some(path), Some(repo)) } None => (req.cwd.clone(), None, None), }; diff --git a/crates/maestro-daemon/src/worktree.rs b/crates/maestro-daemon/src/worktree.rs index a5f0d1b..4f97c33 100644 --- a/crates/maestro-daemon/src/worktree.rs +++ b/crates/maestro-daemon/src/worktree.rs @@ -174,6 +174,13 @@ impl WorktreeManager { /// The repository's main checkout. `--git-common-dir` points at the shared /// `.git` for every linked worktree, so its parent is the main tree. Falls /// back to the input when git can't answer. +/// The repository a path belongs to: the main checkout, even when `path` is a +/// linked worktree. Clients group sessions by this (#99), and placement uses it +/// so a worktree is never nested inside another. +pub fn main_checkout_of(path: &Path) -> PathBuf { + main_checkout(path) +} + fn main_checkout(path: &Path) -> PathBuf { let out = Command::new("git") .arg("-C") diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 6b4f016..0e11df4 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -947,3 +947,60 @@ async fn liveness_watcher_leaves_running_sessions_alone() { fx.manager.kill(s.id, false).await.unwrap(); } + +/// #99: clients group a session under `worktree.base_repo`, so that field has +/// to name the repository even when the spawn request pointed at a linked +/// worktree. Recording the request verbatim put the session in a folder of its +/// own — `/worktrees/` — instead of under the repo. +#[tokio::test] +async fn worktree_base_repo_records_the_main_checkout_not_the_request() { + let fx = DaemonFixture::new(); + let repo = TempDir::new().unwrap(); + init_repo(repo.path()); + + // A linked worktree of that repo, then a spawn whose `base_repo` names it + // rather than the main checkout — what happens when the operator spawns + // from inside a worktree. + let first = fx + .manager + .create(CreateSessionRequest { + cwd: repo.path().to_path_buf(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: Some(WorktreeSpec { + base_repo: repo.path().to_path_buf(), + branch: "feat/one".into(), + create_branch: true, + }), + }) + .await + .expect("create first"); + let inner_worktree = first.worktree.expect("first has a worktree").path; + + let second = fx + .manager + .create(CreateSessionRequest { + cwd: inner_worktree.clone(), + command: "sleep 30".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: Some(WorktreeSpec { + base_repo: inner_worktree.clone(), + branch: "feat/two".into(), + create_branch: true, + }), + }) + .await + .expect("create second"); + + let wt = second.worktree.expect("second has a worktree"); + assert_eq!( + wt.base_repo, + repo.path() + .canonicalize() + .unwrap_or_else(|_| repo.path().to_path_buf()), + "base_repo must be the repository, not the worktree the request named", + ); + assert_ne!(wt.base_repo, inner_worktree, "the worktree is not the repo"); +} diff --git a/web/src/tree.test.ts b/web/src/tree.test.ts index a79aac2..8a1e40c 100644 --- a/web/src/tree.test.ts +++ b/web/src/tree.test.ts @@ -95,3 +95,59 @@ describe("mergeLinear", () => { expect(merged.path).toBe("a/b"); }); }); + +describe("worktree sessions group under their repository (#99)", () => { + /** A session the daemon spawned into a worktree of `repo`. */ + function worktreeSession(id: string, repo: string, wt: string): Session { + return { ...session(id, wt), worktree: { path: wt, base_repo: repo } } as Session; + } + + /** Top-level folder names, after linear chains are collapsed. */ + function topFolders(root: TreeFolder): string[] { + return root.children.filter((c) => c.kind === "folder").map((c) => (c as TreeFolder).name); + } + + it("nests under the repo, not the worktree directory", () => { + const root = buildTree( + [ + worktreeSession( + "a", + "/home/n/tea/agent-skills", + "/home/n/tea/agent-skills/worktrees/largo-clarinet", + ), + ], + "/home/n", + "", + ) as TreeFolder; + + expect(topFolders(root)).toEqual(["~/tea/agent-skills"]); + const repo = root.children[0] as TreeFolder; + expect(repo.children).toHaveLength(1); + expect(repo.children[0].kind).toBe("leaf"); + }); + + it("puts a repo's plain and worktree sessions side by side", () => { + const root = buildTree( + [ + session("plain", "/home/n/tea/agent-skills"), + worktreeSession( + "wt", + "/home/n/tea/agent-skills", + "/home/n/tea/agent-skills/worktrees/largo-clarinet", + ), + ], + "/home/n", + "", + ) as TreeFolder; + + expect(topFolders(root)).toEqual(["~/tea/agent-skills"]); + const repo = root.children[0] as TreeFolder; + expect(repo.children).toHaveLength(2); + expect(repo.children.every((c) => c.kind === "leaf")).toBe(true); + }); + + it("leaves a session without a worktree where it is", () => { + const root = buildTree([session("a", "/home/n/tea/arr")], "/home/n", "") as TreeFolder; + expect(topFolders(root)).toEqual(["~/tea/arr"]); + }); +}); diff --git a/web/src/tree.ts b/web/src/tree.ts index 93e098b..6f47ef7 100644 --- a/web/src/tree.ts +++ b/web/src/tree.ts @@ -2,7 +2,7 @@ // so it can be tested directly. import type { Session } from "./types.js"; -import { abbreviateCwd, sessionCwd, sessionName } from "./types.js"; +import { abbreviateCwd, sessionName, sessionTreePath } from "./types.js"; export interface TreeFolder { kind: "folder"; @@ -30,7 +30,9 @@ export function buildTree(sessions: Session[], home: string | undefined, filter: const lc = filter.trim().toLowerCase(); for (const s of sessions) { - const cwd = sessionCwd(s); + // Grouped by repository, so a worktree session lands under its repo + // rather than in a folder of its own (#99). + const cwd = sessionTreePath(s); const dir = cwd ? abbreviateCwd(cwd, home) : ""; if (lc) { const haystack = `${sessionName(s)} ${dir}`.toLowerCase(); diff --git a/web/src/types.ts b/web/src/types.ts index b036b5b..3f15719 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -36,6 +36,16 @@ export interface Session { * without a worktree; all counts default to 0 on the wire for older * daemons. */ blocked_work?: InTheWay | null; + /** The worktree the daemon created for this session, absent when it did not + * create one. `base_repo` is the repository's main checkout, which is what + * the session tree groups by (#99). */ + worktree?: SessionWorktree | null; +} + +/** A session's daemon-created worktree (`SessionWorktree` on the wire). */ +export interface SessionWorktree { + path: string; + base_repo: string; } /** One directory's immediate subdirectories (NAP-329). */ @@ -231,12 +241,23 @@ export function sessionName(s: Session): string { } /** The directory a session lives in: its live `current_cwd` if the daemon has - * observed one, else the spawn `cwd`. Drives the directory-based session tree. */ + * observed one, else the spawn `cwd`. */ export function sessionCwd(s: Session): string | undefined { if (typeof s.current_cwd === "string" && s.current_cwd) return s.current_cwd; return s.cwd || undefined; } +/** Where a session sits in the directory tree. A worktree session belongs to + * its repository, not to the worktree (#99): otherwise `~/tea/agent-skills` + * grows a `~/tea/agent-skills/worktrees/largo-clarinet` folder holding one + * session of the same name, and the repository stops being where you look for + * the work. `base_repo` is the main checkout — the daemon resolves it. */ +export function sessionTreePath(s: Session): string | undefined { + const repo = s.worktree?.base_repo; + if (typeof repo === "string" && repo) return repo; + return sessionCwd(s); +} + /** Collapse a daemon's home dir to `~`. `home` comes from `DaemonInfo.home_dir`; * absent/empty leaves the path untouched. Mirrors the CLI's `abbreviate_dir`. */ export function abbreviateCwd(cwd: string, home?: string): string {