From a8d693c0a371f2168633a2ae3550824cf9f5f682 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 13:57:01 +0100 Subject: [PATCH 01/20] fix(web): bump pnpmDeps hash for lockfile change --- flake.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake.nix b/flake.nix index 5f11588..61ff9da 100644 --- a/flake.nix +++ b/flake.nix @@ -125,7 +125,7 @@ pnpmDeps = (pkgs.fetchPnpmDeps.override { pnpm = pkgs.pnpm_10; }) { inherit (finalAttrs) pname version src; fetcherVersion = 3; - hash = "sha256-Lhi41AtCK69SM4+7zih19+Njf7o2hyXrAV0W6VO9YxU="; + hash = "sha256-D9JdgI92vzAY8en2zGEEPd4hwOLVi3sr1joT4b17Be4="; }; buildPhase = '' From 3029de667e146d9c76faf3f4814c8f8b47f72f8e Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:39:10 +0100 Subject: [PATCH 02/20] fix(maestro): honour the project cache TTL in the background rescan (#75) --- crates/maestro-daemon/src/projects.rs | 48 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/crates/maestro-daemon/src/projects.rs b/crates/maestro-daemon/src/projects.rs index aab747e..78db18f 100644 --- a/crates/maestro-daemon/src/projects.rs +++ b/crates/maestro-daemon/src/projects.rs @@ -261,17 +261,13 @@ impl ProjectScanner { } /// Cached snapshot, rescanning (off-thread) when the TTL has lapsed. + /// Also the background timer's entry point: a tick inside the TTL is a + /// no-op (#75). pub async fn repos(&self) -> std::sync::Arc> { - self.repos_inner(false).await - } - - async fn repos_inner(&self, force: bool) -> std::sync::Arc> { let mut cache = self.cache.lock().await; - if !force { - if let Some((at, repos)) = cache.as_ref() { - if at.elapsed() < self.ttl { - return repos.clone(); - } + if let Some((at, repos)) = cache.as_ref() { + if at.elapsed() < self.ttl { + return repos.clone(); } } let roots = self.roots.clone(); @@ -287,11 +283,11 @@ impl ProjectScanner { repos } - /// Rescan now, ignoring the TTL. Called by the background timer — going - /// through `repos()` would hit the cache and make most ticks no-ops, - /// leaving requests to pay for the rescan instead (#54 review). + /// One background-refresh tick: rescan only if the cached snapshot has + /// passed its TTL (#75) — the timer used to bypass it and re-walk the + /// roots every [`REFRESH_INTERVAL`]. async fn refresh(&self) { - self.repos_inner(true).await; + self.repos().await; } /// Spawn the slow background refresh task. Drop the returned handle to @@ -544,6 +540,32 @@ mod tests { assert_eq!(fresh[0].name, "late"); } + #[tokio::test] + async fn background_refresh_honours_the_ttl() { + let tmp = tempfile::tempdir().unwrap(); + let mut scanner = ProjectScanner::new( + vec![tmp.path().to_path_buf()], + 3, + PathBuf::from("/nonexistent"), + ); + + let empty = scanner.repos().await; + assert!(empty.is_empty()); + + make_repo(tmp.path(), "late"); + // A background tick inside the TTL must not re-walk the filesystem: + // the new repo stays invisible until the cache expires. + scanner.refresh().await; + assert!(scanner.repos().await.is_empty()); + + // Once the TTL lapses, the next tick picks it up. + scanner.ttl = Duration::ZERO; + scanner.refresh().await; + let fresh = scanner.repos().await; + assert_eq!(fresh.len(), 1); + assert_eq!(fresh[0].name, "late"); + } + #[test] fn disabled_scanner_always_serves_empty() { let scanner = ProjectScanner::disabled(); From 9c6e06ec4f6ca83705a3e25019356e940f9555cf Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:39:51 +0100 Subject: [PATCH 03/20] fix(maestro): reuse empty draft pane and keep its seed (#72) openDraft's reuse check was `!target.draft`, dead after first render: createSpawnForm auto-syncs its picked defaults into the leaf's draft once its project load settles, so every empty pane has a non-null draft a second later. `n` and Spawn always split instead of reusing the pane on screen. Reuse now checks only `!target.sessionId`, matching the codebase's own definition of "empty" pane elsewhere. Reusing also drops the leaf's stale form instance so a new seed is read by the rebuilt form instead of being ignored by the old one. --- web/src/multiview.test.ts | 42 +++++++++++++++++++++++++++++++++++++++ web/src/multiview.ts | 10 ++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/web/src/multiview.test.ts b/web/src/multiview.test.ts index 8067d5e..36e2a09 100644 --- a/web/src/multiview.test.ts +++ b/web/src/multiview.test.ts @@ -147,6 +147,48 @@ describe("multiview openDraft (#62)", () => { expect((mv.el.querySelector(".mv-draft input[name=name]") as HTMLInputElement).value).toBe("wip"); }); + it("reuses an already-synced empty pane instead of splitting (#72)", async () => { + const layout = emptyLayout(); + const tab = layout.tabs[0]; + const first = splitLeaf(tab, "", "s1", "row", true); + const empty = splitLeaf(tab, first, null, "row", true); + tab.focusedLeafId = empty; + const mv = makeMv(layout); + document.body.appendChild(mv.el); + mv.render(); + // Let the mounted form's loadProjects() settle and auto-sync its picked + // defaults into leaf.draft before openDraft runs (#72's dead-branch window). + await flush(); + expect(leaves(tab.root).find((l) => l.id === empty)?.draft).not.toBeNull(); + + mv.openDraft(); + await flush(); + + expect(leaves(tab.root)).toHaveLength(2); + expect(tab.focusedLeafId).toBe(empty); + }); + + it("keeps a seed after reusing an already-synced empty pane (#72)", async () => { + const layout = emptyLayout(); + const tab = layout.tabs[0]; + const first = splitLeaf(tab, "", "s1", "row", true); + const empty = splitLeaf(tab, first, null, "row", true); + tab.focusedLeafId = empty; + const mv = makeMv(layout); + document.body.appendChild(mv.el); + mv.render(); + await flush(); + + mv.openDraft({ cwd: "/home/user/proj", command: "claude" }); + await flush(); + + const leaf = leaves(tab.root).find((l) => l.id === empty)!; + expect(leaf.draft?.cwd).toBe("/home/user/proj"); + expect(leaf.draft?.command).toBe("claude"); + const cwdRaw = mv.el.querySelector(".mv-draft .cwd-raw") as HTMLInputElement; + expect(cwdRaw.value).toBe("/home/user/proj"); + }); + it("seeds a clone's cwd and command into the draft", async () => { const layout = emptyLayout(); const mv = makeMv(layout); diff --git a/web/src/multiview.ts b/web/src/multiview.ts index b837c6e..cfd00b9 100644 --- a/web/src/multiview.ts +++ b/web/src/multiview.ts @@ -423,9 +423,15 @@ export function createMultiView(opts: MultiViewOptions): MultiView { return; } const target = findLeaf(tab.root, tab.focusedLeafId)!; - if (!target.sessionId && !target.draft) { - // An already-empty pane becomes the draft in place. + if (!target.sessionId) { + // Empty = no session, not `!draft` (#72): the mounted form auto-syncs + // picked defaults into `draft` once ready, so that's never reliably null. target.draft = draft(); + const stale = draftForms.get(target.id); + if (stale) { + stale.destroy(); + draftForms.delete(target.id); + } } else { // Split a fresh draft beside the focused pane. tab.focusedLeafId = splitLeaf(tab, target.id, null, "row", true); From a85773cef0a28be68fa3c808d5d00447cb1df683 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:39:48 +0100 Subject: [PATCH 04/20] fix(maestro): make shift-S resolve to the fast-forward spawn binding crossterm attaches KeyModifiers::SHIFT to every uppercase char, so a real Shift+S delivered Char { c: 'S', shift: true }. canonical() turned that into "shift-S", which the default "S" binding never matched, so FastForwardSpawn was unreachable from the keyboard. canonical() now drops the shift- prefix when the char is already uppercase, since the case already says "shifted". Regression test drives an actual crossterm KeyEvent through canonicalise() and the keymap, rather than asserting on the binding string, per #70. Co-Authored-By: Claude Sonnet 5 --- crates/maestro-tui/src/config.rs | 20 ++++++++++++++++++++ crates/maestro-tui/src/input.rs | 22 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/maestro-tui/src/config.rs b/crates/maestro-tui/src/config.rs index 9f4eb2b..790e6bd 100644 --- a/crates/maestro-tui/src/config.rs +++ b/crates/maestro-tui/src/config.rs @@ -238,6 +238,26 @@ mod tests { assert!(s.contains("ctrl-glorb"), "diagnostic: {s}"); } + /// Regression for #70: the previous canonical form of a real Shift+S + /// keypress was `"shift-S"`, which the default `"S"` binding never + /// matched. Drive the actual crossterm event, not the binding string, + /// so the dead-key bug can't come back unnoticed. + #[test] + fn real_shift_s_keypress_resolves_to_fast_forward_spawn() { + use crossterm::event::{Event as CtEvent, KeyCode, KeyEvent, KeyModifiers}; + + use crate::input::{canonicalise, Region}; + + let cfg = TuiConfig::load(&TuiSection::default()).unwrap(); + let ev = CtEvent::Key(KeyEvent::new(KeyCode::Char('S'), KeyModifiers::SHIFT)); + let key = canonicalise(&ev, |_, _| Region::Unknown).expect("key event canonicalises"); + + assert_eq!( + cfg.keymap.resolve(Mode::Normal, &key.canonical()), + Some(Action::FastForwardSpawn), + ); + } + #[test] fn custom_chord_timeout_is_applied() { let mut section = TuiSection::default(); diff --git a/crates/maestro-tui/src/input.rs b/crates/maestro-tui/src/input.rs index 34c4d4a..137458f 100644 --- a/crates/maestro-tui/src/input.rs +++ b/crates/maestro-tui/src/input.rs @@ -117,7 +117,9 @@ impl InputKey { if *alt { out.push_str("alt-"); } - if *shift { + // Uppercase already implies shift; crossterm sets the flag + // on every such char, so skip the redundant prefix (#70). + if *shift && !c.is_uppercase() { out.push_str("shift-"); } out.push(*c); @@ -580,12 +582,28 @@ mod tests { #[test] fn canonical_round_trips_simple_chars() { - for s in ["a", "ctrl-a", "ctrl-w", "alt-x", "shift-A"] { + for s in ["a", "ctrl-a", "ctrl-w", "alt-x"] { let k = InputKey::parse(s).unwrap(); assert_eq!(k.canonical(), s, "round-trip {s:?}"); } } + /// crossterm 0.28 attaches `KeyModifiers::SHIFT` to any uppercase char, + /// so a real Shift+S delivers `Char { c: 'S', shift: true }`. Its + /// canonical form must be the bare `"S"` a keymap binds, not `"shift-S"` — + /// the uppercase letter already says "shifted"; a redundant prefix would + /// make the default `S` binding unreachable from the keyboard (#70). + #[test] + fn shift_flag_is_redundant_for_an_already_uppercase_char() { + let k = InputKey::Char { + c: 'S', + ctrl: false, + alt: false, + shift: true, + }; + assert_eq!(k.canonical(), "S"); + } + #[test] fn canonical_round_trips_specials() { for s in [ From 73e9ecfc85841cd56684d2a90099e4de2a8774bf Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:48:11 +0100 Subject: [PATCH 05/20] fix(maestro): cap and centre the spawn form's width The form filled the full pane width, so on a 4K display every input stretched thousands of pixels wide. Bound it to 60ch (in line with the 48ch/42ch precedents already in the type scale) and centre it with margin auto; panes narrower than the cap still fill edge to edge. --- web/src/styles.css | 3 +++ 1 file changed, 3 insertions(+) diff --git a/web/src/styles.css b/web/src/styles.css index 065d7e1..afe24c3 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1215,6 +1215,9 @@ textarea { /* ---------- forms ---------- */ .spawn-form { + width: 100%; + max-width: 60ch; + margin: 0 auto; padding: var(--sp-3); overflow: auto; display: flex; From b80d604408e37bea63382176b888f4c8f9782165 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:52:17 +0100 Subject: [PATCH 06/20] fix(maestro): default the worktree toggle off when the directory is itself a worktree (#91) Picking a project row that is itself a linked git worktree used to default 'Create git worktree' on, so spawning asked for a second worktree of an already-checked-out branch and git refused. Both clients now default the toggle off there: the session just runs in that existing worktree with no worktree record (not adoption). Turning it back on by hand still tries, per the issue's non-goal. --- crates/maestro-tui/src/draft.rs | 39 +++++++++++++++++++++++++++++++ web/src/spawnForm.ts | 16 ++++++++++--- web/src/spawnFormWorktree.test.ts | 31 +++++++++++++++++++++++- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/crates/maestro-tui/src/draft.rs b/crates/maestro-tui/src/draft.rs index 3e26386..6604326 100644 --- a/crates/maestro-tui/src/draft.rs +++ b/crates/maestro-tui/src/draft.rs @@ -654,6 +654,11 @@ impl DraftPane { let project = visible[sel].project.clone(); self.picked_cwd = Some(project.path.clone()); self.cwd_raw_visible = false; + // A directory that *is* a linked worktree defaults to spawning in + // place (#91): creating a second worktree of its branch fails. + // Turning it back on by hand still tries (git's own message), + // which is deliberate (#91 non-goal). + self.worktree_enabled = !project.is_worktree; // The scan already knows this directory's repo (#61): no probe // round trip needed, exactly like the web form. let repo = project @@ -1689,6 +1694,40 @@ mod tests { assert!(wt.create_branch); } + #[test] + fn picking_a_worktree_project_defaults_the_toggle_off() { + let mut draft = DraftPane::default(); + let mut wt = repo_project("wt-91", "/repos/wt91", "/repos/main", 1, None); + wt.is_worktree = true; + draft.set_projects( + vec![ + repo_project("main", "/repos/main", "/repos/main", 2, None), + wt, + ], + Utc::now(), + ); + draft.focused_field = DraftField::DirPicker; + typed(&mut draft, "wt-91"); + draft.on_input(&InputKey::Enter); + + assert!( + !draft.worktree_enabled, + "the directory is already a worktree; spawning in place" + ); + assert!( + draft.build_request().unwrap().worktree.is_none(), + "no worktree record — the session just runs there (#91)" + ); + + // Deliberately re-enabling still works, and the scan-known base repo + // survives so the toggle-on path needs no new probe. + draft.focused_field = DraftField::WorktreeToggle; + draft.on_input(&InputKey::Enter); + assert!(draft.worktree_enabled); + assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/repos/main"))); + assert!(draft.take_gitroot_probe().is_none()); + } + #[test] fn raw_path_targets_probe_gitroot_and_resolution_feeds_the_spec() { let mut draft = DraftPane { diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index 194d71c..1b130ce 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -430,9 +430,19 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { cwdRawWrap.hidden = true; markActiveProject(project.path); // The scan already knows this directory is inside a repo (#61): the - // toggle defaults back on and no /api/gitroot round trip is needed. - wtEnabled.checked = true; - applyProjectRepo(project.base_repo ?? project.path); + // toggle defaults back on and no /api/gitroot round trip is needed — + // unless the directory *is* a linked worktree itself (#91): spawning + // in place is the only thing that works there, so default the toggle + // off. Turning it back on by hand still tries (and fails with git's + // own message), which is deliberate (#91 non-goal). + wtEnabled.checked = !project.is_worktree; + if (project.is_worktree) { + inferredBaseRepo = null; + wtBody.hidden = true; + wtHint.hidden = true; + } else { + applyProjectRepo(project.base_repo ?? project.path); + } emit(); // Picking a project pre-selects the command it was last used with. const match = presets.find((p) => p.command === project.last_command && p.command !== ""); diff --git a/web/src/spawnFormWorktree.test.ts b/web/src/spawnFormWorktree.test.ts index 76fd12c..b8012e1 100644 --- a/web/src/spawnFormWorktree.test.ts +++ b/web/src/spawnFormWorktree.test.ts @@ -18,6 +18,7 @@ const BRANCHES = [ * its branch list is still in flight. */ function stubFetch(opts?: { gateBranches?: (release: () => void) => void; + projects?: unknown[]; }): string[] { const urls: string[] = []; const gated = new Promise((resolve) => { @@ -37,7 +38,7 @@ function stubFetch(opts?: { return new Response(JSON.stringify(BRANCHES), { status: 200 }); } if (url.includes("/api/projects")) { - return new Response(JSON.stringify(PROJECTS), { status: 200 }); + return new Response(JSON.stringify(opts?.projects ?? PROJECTS), { status: 200 }); } return new Response( JSON.stringify({ hostname: "h", version: "0", home_dir: "/home/user", presets: [] }), @@ -100,6 +101,34 @@ describe("spawnForm worktree defaults and branch picker (#61)", () => { ); }); + it("defaults the worktree toggle off for a row that is itself a worktree (#91)", async () => { + urls = stubFetch({ + projects: [ + { + path: "/home/user/wt-91", + name: "wt-91", + is_worktree: true, + base_repo: "/home/user/maestro", + last_activity: "2026-08-20T10:00:00Z", + }, + ], + }); + handle = createSpawnForm({ onSuccess() {} }); + document.body.appendChild(handle.el); + for (let i = 0; i < 40; i++) await Promise.resolve(); + + handle.el + .querySelectorAll(".project-row") + .forEach((row) => row.dataset.cwd === "/home/user/wt-91" && row.click()); + await drain(); + + expect(handle.el.querySelector("input[name=worktree_enabled]")!.checked).toBe( + false, + ); + expect(urls.some((u) => u.includes("/api/gitroot"))).toBe(false); + expect(handle.el.querySelector(".wt-body")!.hidden).toBe(true); + }); + it("still probes the gitroot for a free-text directory", async () => { const form = await mount(); From 2d5e2f1718d1ade558f888c49cb34d893339eeba Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:54:18 +0100 Subject: [PATCH 07/20] fix(maestro): move draft forms along with swapped pane data Center-zone pane drag swapped dragged.draft/target.draft but left draftForms cached under the old leaf id, so both panes kept showing their pre-swap form values while writing into the swapped leaf. Tear down both leaves' cached forms at swap time (same pattern as openDraft's stale-form teardown, #72) so render() remounts fresh forms seeded from the swapped data. Fixes #73 --- web/src/multiview.test.ts | 80 +++++++++++++++++++++++++++++++++++++++ web/src/multiview.ts | 10 +++++ 2 files changed, 90 insertions(+) diff --git a/web/src/multiview.test.ts b/web/src/multiview.test.ts index 36e2a09..a08bf79 100644 --- a/web/src/multiview.test.ts +++ b/web/src/multiview.test.ts @@ -202,3 +202,83 @@ describe("multiview openDraft (#62)", () => { expect(leaf.draft?.command).toBe("claude"); }); }); + +describe("multiview pane-drag swap (#73)", () => { + beforeEach(() => { + stubFetch(); + document.body.replaceChildren(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + /** Drag `sourcePane`'s title onto `targetPane`'s center, as the real pane + * drag handler does — modelled after the pointer sequence in runDrag()/ + * updateDock() in multiview.ts, since neither is exposed on MultiView. */ + async function dragPaneOntoCenter(sourcePane: HTMLElement, targetPane: HTMLElement): Promise { + // The real drag-start gate requires the pointerdown target to sit inside + // an `.attach-header` (attach.ts's session title bar). Draft panes mount + // no such element, so stand one in for the gate check itself. + const marker = document.createElement("div"); + marker.className = "attach-header"; + sourcePane.appendChild(marker); + + vi.spyOn(targetPane, "getBoundingClientRect").mockReturnValue({ + left: 100, + top: 0, + width: 100, + height: 100, + right: 200, + bottom: 100, + x: 100, + y: 0, + toJSON() {}, + } as DOMRect); + (document as unknown as { elementsFromPoint: (x: number, y: number) => Element[] }).elementsFromPoint = () => [ + targetPane, + ]; + + marker.dispatchEvent( + new PointerEvent("pointerdown", { clientX: 0, clientY: 0, bubbles: true, cancelable: true }), + ); + window.dispatchEvent( + new PointerEvent("pointermove", { clientX: 150, clientY: 50, bubbles: true, cancelable: true }), + ); + window.dispatchEvent(new PointerEvent("pointerup", { clientX: 150, clientY: 50, bubbles: true, cancelable: true })); + await flush(); + } + + it("moves each draft's form along with its data when two draft panes are swapped", async () => { + const layout = emptyLayout(); + const tab = layout.tabs[0]; + const leaf1 = splitLeaf(tab, "", null, "row", true); + const leaf2 = splitLeaf(tab, leaf1, null, "row", true); + const mv = makeMv(layout); + document.body.appendChild(mv.el); + mv.render(); + await flush(); + + const paneOf = (leafId: string) => mv.el.querySelector(`[data-leaf-id="${leafId}"]`) as HTMLElement; + const nameInputOf = (leafId: string) => paneOf(leafId).querySelector("input[name=name]") as HTMLInputElement; + + nameInputOf(leaf1).value = "west"; + nameInputOf(leaf1).dispatchEvent(new Event("input")); + nameInputOf(leaf2).value = "east"; + nameInputOf(leaf2).dispatchEvent(new Event("input")); + await flush(); + expect(leaves(tab.root).find((l) => l.id === leaf1)?.draft?.name).toBe("west"); + expect(leaves(tab.root).find((l) => l.id === leaf2)?.draft?.name).toBe("east"); + + await dragPaneOntoCenter(paneOf(leaf1), paneOf(leaf2)); + + // The leaf data swaps (already correct pre-fix)... + expect(leaves(tab.root).find((l) => l.id === leaf1)?.draft?.name).toBe("east"); + expect(leaves(tab.root).find((l) => l.id === leaf2)?.draft?.name).toBe("west"); + // ...and the mounted forms must follow, not stay behind showing the old + // pane's values (#73). + expect(nameInputOf(leaf1).value).toBe("east"); + expect(nameInputOf(leaf2).value).toBe("west"); + }); +}); diff --git a/web/src/multiview.ts b/web/src/multiview.ts index cfd00b9..3ea9a94 100644 --- a/web/src/multiview.ts +++ b/web/src/multiview.ts @@ -701,6 +701,16 @@ export function createMultiView(opts: MultiViewOptions): MultiView { const dragged = findLeaf(tab.root, self)!; [dragged.sessionId, target.sessionId] = [target.sessionId, dragged.sessionId]; [dragged.draft, target.draft] = [target.draft, dragged.draft]; + // The leaf data moved but each pane's mounted form is still keyed to + // its old leaf (#73) — tear both down like openDraft's reuse case + // (#72) so render() remounts fresh forms seeded from the swapped data. + for (const id of [dragged.id, target.id]) { + const stale = draftForms.get(id); + if (stale) { + stale.destroy(); + draftForms.delete(id); + } + } tab.focusedLeafId = target.id; } else if (sessionId) { const loc = locateSession(layout, sessionId); From 2c8758ed147bccff989dd7ed7cc303060bacc37e Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:58:07 +0100 Subject: [PATCH 08/20] fix(maestro): stop a late /api/info resolution from clobbering a picked preset A tile clicked, or a custom command typed, during the /api/info round trip was discarded: applyInitialCommand() ran unconditionally a second time once presets refined, re-selecting the first non-custom preset with no seed to protect the operator's pick. Guard it with a commandTouched flag, mirroring the nameTouched pattern already used for the name-batch race. Fixes #76. --- web/src/spawnForm.ts | 27 ++++++- web/src/spawnFormPreset.test.ts | 124 ++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 web/src/spawnFormPreset.test.ts diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index 1b130ce..8053f98 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -337,6 +337,15 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { emit(); } + /** Re-highlight the operator's already-picked tile after `renderCommandTiles` + * rebuilds the row from scratch (new tiles carry no `is-active` class) — + * without touching `command`, since that was already settled. */ + function reapplyActiveTile(): void { + for (const tile of cmdTiles.querySelectorAll(".tile")) { + tile.classList.toggle("is-active", (tile as HTMLElement).dataset.id === selectedPresetId); + } + } + function renderCommandTiles(): void { cmdTiles.replaceChildren(); for (const p of presets) { @@ -345,13 +354,23 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { tile.className = "tile"; tile.dataset.id = p.id; tile.innerHTML = `${escapeHtml(p.label)}`; - tile.addEventListener("click", () => selectPreset(p.id)); + tile.addEventListener("click", () => { + commandTouched = true; + selectPreset(p.id); + }); cmdTiles.appendChild(tile); } } + // Set once the operator picks a tile or types a custom command — guards + // `applyInitialCommand()` the same way `nameTouched` guards a late name + // batch: a late `/api/info` resolution must never overwrite a field the + // operator has already touched (#76). + let commandTouched = false; + cmdRaw.addEventListener("input", () => { if (selectedPresetId === "custom" || presets.find((p) => p.id === selectedPresetId)?.command === "") { + commandTouched = true; command = cmdRaw.value.trim(); emit(); } @@ -367,7 +386,11 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { if (info.presets?.length) { presets = info.presets; renderCommandTiles(); - applyInitialCommand(); + if (commandTouched) { + reapplyActiveTile(); + } else { + applyInitialCommand(); + } } }) .catch(() => { diff --git a/web/src/spawnFormPreset.test.ts b/web/src/spawnFormPreset.test.ts new file mode 100644 index 0000000..e7fe7bc --- /dev/null +++ b/web/src/spawnFormPreset.test.ts @@ -0,0 +1,124 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { createSpawnForm, type SpawnFormHandle, type SpawnFormValues } from "./spawnForm.js"; + +/** Stubs fetch so `/api/info` only resolves once `resolveInfo()` is called — + * everything else (names, projects, gitroot) answers immediately with an + * empty/default body. Lets a test pick a preset (or type a custom command) + * mid-flight, then observe what the late `/api/info` resolution does to it. */ +function stubDeferredInfo(): { resolveInfo: () => void } { + let releaseInfo: () => void = () => {}; + const infoGate = new Promise((resolve) => { + releaseInfo = resolve; + }); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/api/info")) { + await infoGate; + return new Response( + JSON.stringify({ + hostname: "h", + version: "0", + home_dir: "/home/user", + presets: [ + { id: "claude", label: "Claude", command: "claude" }, + { id: "fish", label: "Fish", command: "fish" }, + { id: "custom", label: "Custom…", command: "" }, + ], + }), + { status: 200 }, + ); + } + if (url.includes("/api/names")) { + return new Response(JSON.stringify({ names: ["amber-fox"] }), { status: 200 }); + } + return new Response(JSON.stringify([]), { status: 200 }); + }), + ); + return { resolveInfo: releaseInfo }; +} + +describe("spawnForm preset selection vs. late /api/info (#76)", () => { + let handle: SpawnFormHandle | null = null; + + afterEach(() => { + handle?.destroy(); + handle = null; + vi.unstubAllGlobals(); + }); + + async function drain(): Promise { + for (let i = 0; i < 40; i++) await Promise.resolve(); + } + + it("keeps an operator-picked tile after /api/info resolves late", async () => { + const { resolveInfo } = stubDeferredInfo(); + let values: SpawnFormValues | undefined; + handle = createSpawnForm({ onChange: (v) => (values = v), onSuccess() {} }); + const form = handle.el; + document.body.appendChild(form); + await drain(); + + // Fallback presets are up; the operator picks "zsh" before /api/info lands. + const zshTile = [...form.querySelectorAll(".tile")].find( + (t) => t.dataset.id === "zsh", + )!; + zshTile.click(); + expect(zshTile.classList.contains("is-active")).toBe(true); + + // /api/info lands late with a preset list that doesn't even include "zsh". + resolveInfo(); + await drain(); + + // The operator's pick must not be discarded back to the new list's first + // entry — `onChange` only starts firing once the initial load settles + // (`ready`), which happens right here, so this is the first place the + // published command is observable. + expect(values?.command).toBe("zsh"); + }); + + it("keeps an operator-typed custom command after /api/info resolves late", async () => { + const { resolveInfo } = stubDeferredInfo(); + let values: SpawnFormValues | undefined; + handle = createSpawnForm({ onChange: (v) => (values = v), onSuccess() {} }); + const form = handle.el; + document.body.appendChild(form); + await drain(); + + const customTile = [...form.querySelectorAll(".tile")].find( + (t) => t.dataset.id === "custom", + )!; + customTile.click(); + const cmdRaw = form.querySelector(".cmd-raw")!; + cmdRaw.value = "htop"; + cmdRaw.dispatchEvent(new Event("input")); + + resolveInfo(); + await drain(); + + expect(values?.command).toBe("htop"); + expect(cmdRaw.value).toBe("htop"); + }); + + it("still applies the real presets' first entry when the operator never touched the command", async () => { + const { resolveInfo } = stubDeferredInfo(); + let values: SpawnFormValues | undefined; + handle = createSpawnForm({ onChange: (v) => (values = v), onSuccess() {} }); + const form = handle.el; + document.body.appendChild(form); + await drain(); + + resolveInfo(); + await drain(); + + // Untouched: the refined preset list is still free to settle the command. + expect(values?.command).toBe("claude"); + const fishTile = [...form.querySelectorAll(".tile")].find( + (t) => t.dataset.id === "fish", + ); + expect(fishTile).toBeDefined(); + }); +}); From 16609eff9e94c5bdaad9d14fec4c993ec0eef64e Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:59:56 +0100 Subject: [PATCH 09/20] feat(maestro): debounce the gitroot probe in the spawn form Typing in the raw cwd field fired one /api/gitroot request per keystroke (#74). Every keystroke now resets a 250ms window shared with the TUI draft (GITROOT_PROBE_DEBOUNCE_MS there); probeToken still owns ordering so a late answer can't overwrite a newer input. --- web/src/spawnForm.ts | 23 ++++- web/src/spawnFormProbe.test.ts | 148 ++++++++++++++++++++++++++++++ web/src/spawnFormWorktree.test.ts | 4 +- 3 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 web/src/spawnFormProbe.test.ts diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index 1b130ce..60049c0 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -94,6 +94,11 @@ export const FALLBACK_PRESETS: SpawnPreset[] = [ { id: "custom", label: "Custom…", command: "" }, ]; +/** Quiet period after raw-path typing before `/api/gitroot` fires (#74). + * Must stay equal to `GITROOT_PROBE_DEBOUNCE_MS` in + * crates/maestro-tui/src/draft.rs so both clients debounce alike. */ +const GITROOT_PROBE_DEBOUNCE_MS = 250; + export interface SpawnFormHandle { el: HTMLFormElement; /** Remove global listeners (dir autocomplete) — call when unmounting. */ @@ -413,7 +418,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { cwd = cwdRaw.value.trim(); branchCache = null; emit(); - if (wtEnabled.checked) probeGitRoot(); + if (wtEnabled.checked) scheduleGitRootProbe(); }); function markActiveProject(activeCwd: string | null): void { @@ -579,7 +584,20 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { applyProjectRepo(project.base_repo ?? project.path); return; } - void probeGitRoot(); + scheduleGitRootProbe(); + } + + // Every keystroke resets this timer, so a burst of typing issues one + // request once the operator pauses. `probeToken` inside probeGitRoot still + // owns ordering: a scheduled probe that fires after a newer one started + // loses the token race and is dropped. + let probeTimer: number | undefined; + function scheduleGitRootProbe(): void { + window.clearTimeout(probeTimer); + probeTimer = window.setTimeout(() => { + probeTimer = undefined; + void probeGitRoot(); + }, GITROOT_PROBE_DEBOUNCE_MS); } async function probeGitRoot(): Promise { @@ -740,6 +758,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { el: form, destroy() { destroyed = true; + window.clearTimeout(probeTimer); autocomplete.destroy(); form.remove(); }, diff --git a/web/src/spawnFormProbe.test.ts b/web/src/spawnFormProbe.test.ts new file mode 100644 index 0000000..48eb074 --- /dev/null +++ b/web/src/spawnFormProbe.test.ts @@ -0,0 +1,148 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { createSpawnForm, type SpawnFormHandle } from "./spawnForm.js"; + +/** Debounce window under test — must equal GITROOT_PROBE_DEBOUNCE_MS in + * spawnForm.ts (#74). */ +const DEBOUNCE_MS = 250; + +const PROJECTS = [ + { path: "/home/user/maestro", name: "maestro", last_activity: "2026-08-20T10:00:00Z" }, +]; + +/** Stub the fetches the form makes. Gitroot responses for a given path can be + * held until `releaseGitRoot(path)` is called, so tests can land answers out + * of order. */ +function stubFetch(opts?: { holdGitRootFor?: string[] }): { + urls: string[]; + gitRootCalls: string[]; + releaseGitRoot: (path: string) => void; +} { + const urls: string[] = []; + const gitRootCalls: string[] = []; + const releases = new Map void>(); + const held = new Map>(); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + urls.push(url); + if (url.includes("/api/dirs")) { + return new Response(JSON.stringify({ entries: [] }), { status: 200 }); + } + if (url.includes("/api/gitroot")) { + const path = new URL(url, "http://localhost").searchParams.get("path") ?? ""; + gitRootCalls.push(path); + if (opts?.holdGitRootFor?.includes(path)) { + let release!: () => void; + held.set( + path, + new Promise((r) => { + release = r; + }), + ); + releases.set(path, release); + } + await held.get(path); + return new Response(JSON.stringify({ root: path }), { status: 200 }); + } + if (url.includes("/api/names")) { + return new Response(JSON.stringify({ names: ["amber-fox"] }), { status: 200 }); + } + if (url.includes("/api/projects")) { + return new Response(JSON.stringify(PROJECTS), { status: 200 }); + } + return new Response( + JSON.stringify({ hostname: "h", version: "0", home_dir: "/home/user", presets: [] }), + { status: 200 }, + ); + }), + ); + return { + urls, + gitRootCalls, + releaseGitRoot: (path: string) => releases.get(path)?.(), + }; +} + +describe("spawnForm gitroot probe debounce (#74)", () => { + let handle: SpawnFormHandle | null = null; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + handle?.destroy(); + handle = null; + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + async function mount(opts?: Parameters[0]) { + const stub = stubFetch(opts); + handle = createSpawnForm({ onSuccess() {} }); + document.body.appendChild(handle.el); + for (let i = 0; i < 40; i++) await Promise.resolve(); + return { form: handle.el, stub }; + } + + const drain = async () => { + for (let i = 0; i < 40; i++) await Promise.resolve(); + }; + + function typeRaw(form: HTMLFormElement, value: string): void { + const raw = form.querySelector("input[name=cwd_raw]")!; + raw.value = value; + raw.dispatchEvent(new Event("input")); + } + + it("a keystroke burst fires exactly one probe after the quiet period", async () => { + const { form, stub } = await mount(); + + form.querySelector(".project-row.project-other")!.click(); + // Burst: every keystroke reschedules; nothing may hit /api/gitroot yet. + for (let i = 1; i <= "/opt/somewhere".length; i++) { + typeRaw(form, "/opt/somewhere".slice(0, i)); + await drain(); + expect(stub.gitRootCalls).toEqual([]); + } + + await vi.advanceTimersByTimeAsync(DEBOUNCE_MS); + await drain(); + + expect(stub.gitRootCalls).toEqual(["/opt/somewhere"]); + expect(stub.urls.filter((u) => u.includes("/api/gitroot")).length).toBe(1); + }); + + it("a stale probe answer cannot overwrite a newer input", async () => { + const { form, stub } = await mount({ holdGitRootFor: ["/repo/old"] }); + + form.querySelector(".project-row.project-other")!.click(); + + // Probe for the old target, held in flight. + typeRaw(form, "/repo/old"); + await vi.advanceTimersByTimeAsync(DEBOUNCE_MS); + + // Newer target supersedes it and answers immediately. + typeRaw(form, "/repo/new"); + await vi.advanceTimersByTimeAsync(DEBOUNCE_MS); + await drain(); + + expect(stub.gitRootCalls).toEqual(["/repo/old", "/repo/new"]); + expect(form.querySelector(".wt-repo")!.textContent).toContain("/repo/new"); + expect( + form.querySelector("input[name=worktree_enabled]")!.checked, + ).toBe(true); + + // The late answer to the old target lands last but must lose. + stub.releaseGitRoot("/repo/old"); + await drain(); + + expect(form.querySelector(".wt-repo")!.textContent).toContain("/repo/new"); + expect( + form.querySelector("input[name=worktree_enabled]")!.checked, + ).toBe(true); + }); +}); diff --git a/web/src/spawnFormWorktree.test.ts b/web/src/spawnFormWorktree.test.ts index b8012e1..bb04b96 100644 --- a/web/src/spawnFormWorktree.test.ts +++ b/web/src/spawnFormWorktree.test.ts @@ -132,11 +132,13 @@ describe("spawnForm worktree defaults and branch picker (#61)", () => { it("still probes the gitroot for a free-text directory", async () => { const form = await mount(); - // Escape hatch: switch to a free-text dir; only now does /api/gitroot fire. + // Escape hatch: switch to a free-text dir; only now does /api/gitroot + // fire — debounced, so the quiet period must elapse first (#74). form.querySelector(".project-row.project-other")!.click(); const raw = form.querySelector("input[name=cwd_raw]")!; raw.value = "/opt/somewhere"; raw.dispatchEvent(new Event("input")); + await vi.advanceTimersByTimeAsync(250); await drain(); expect(urls.some((u) => u.includes("/api/gitroot"))).toBe(true); From 596e7e2f49acec14759ffaa356287b8b548f65ca Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 15:04:47 +0100 Subject: [PATCH 10/20] feat(maestro): debounce the gitroot probe in the TUI draft MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing in the raw path field fired one /api/gitroot probe per keystroke (#74). The draft now schedules the probe 250ms out — matching GITROOT_PROBE_DEBOUNCE_MS in web/src/spawnForm.ts — and every keystroke resets it; the 30Hz heartbeat polls for due probes so the burst's one request fires after typing stops. The seq guard in resolve_gitroot is untouched, so a late answer still cannot overwrite a newer input. --- crates/maestro-tui/src/app.rs | 55 ++++++++++++---- crates/maestro-tui/src/draft.rs | 113 ++++++++++++++++++++++++++++---- 2 files changed, 145 insertions(+), 23 deletions(-) diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index cecf43e..1894082 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -175,7 +175,8 @@ impl App { // Render on next loop iteration. } _ = tokio::time::sleep(Duration::from_millis(33)) => { - // 30 Hz heartbeat. + let effects = self.poll_due_probes_at(std::time::Instant::now()); + self.perform(effects).await; } } } @@ -439,7 +440,7 @@ impl App { if draft.take_name_refill() { effects.push(Effect::RefillNames(tab_idx)); } - if let Some((seq, path)) = draft.take_gitroot_probe() { + if let Some((seq, path)) = draft.take_gitroot_probe(std::time::Instant::now()) { effects.push(Effect::ProbeGitRoot { tab: tab_idx, seq, @@ -452,6 +453,26 @@ impl App { effects } + /// Fire any draft gitroot probes whose debounce window has elapsed (#74). + /// The 30 Hz heartbeat calls this so a burst of typing issues its one + /// probe even after the last keystroke. + fn poll_due_probes_at(&mut self, now: std::time::Instant) -> Vec { + let mut effects = Vec::new(); + for tab_idx in 0..self.state.tabs.len() { + let Some(PaneRef::Draft(draft)) = self.state.tabs[tab_idx].pane.as_mut() else { + continue; + }; + if let Some((seq, path)) = draft.take_gitroot_probe(now) { + effects.push(Effect::ProbeGitRoot { + tab: tab_idx, + seq, + path, + }); + } + } + effects + } + /// Populate a freshly opened draft: presets from `/api/info` (falling back /// to the built-ins already in place), projects from `/api/projects`, and /// a first name batch. Applies only if that tab still shows the draft — @@ -1347,10 +1368,11 @@ mod tests { } } - /// Editing the raw path with the worktree toggle on schedules a - /// `/api/gitroot` probe so the section learns its base repo (#65). + /// Editing the raw path schedules a `/api/gitroot` probe only once the + /// keystroke burst settles (#74): nothing fires inside the debounce + /// window; the heartbeat picks the due probe up afterwards. #[test] - fn typing_a_raw_path_schedules_a_gitroot_probe() { + fn typing_a_raw_path_schedules_a_debounced_gitroot_probe() { let mut app = make_app(); app.dispatch(Action::NewDraftPane); let PaneRef::Draft(draft) = app.state.active_tab_mut().pane.as_mut().unwrap() else { @@ -1359,13 +1381,24 @@ mod tests { draft.cwd_raw_visible = true; draft.focused_field = DraftField::CwdRaw; - let effects = app.handle_draft_input(&InputKey::char('/')); + // A burst of keystrokes fires nothing. + for c in "/opt/tooling".chars() { + assert!(app.handle_draft_input(&InputKey::char(c)).is_empty()); + assert!(app.poll_due_probes_at(std::time::Instant::now()).is_empty()); + } + + // Once the quiet period passes, exactly one probe is due. + let due = std::time::Instant::now() + + std::time::Duration::from_millis(crate::draft::GITROOT_PROBE_DEBOUNCE_MS) * 10; + match app.poll_due_probes_at(due).as_slice() { + [Effect::ProbeGitRoot { tab: 0, path, .. }] => { + assert_eq!(path, &std::path::PathBuf::from("/opt/tooling")); + } + other => panic!("expected one ProbeGitRoot effect, got {other:?}"), + } assert!( - matches!( - effects.as_slice(), - [Effect::ProbeGitRoot { tab: 0, seq: 2, .. }] - ), - "{effects:?}" + app.poll_due_probes_at(due).is_empty(), + "the probe is consumed once" ); } diff --git a/crates/maestro-tui/src/draft.rs b/crates/maestro-tui/src/draft.rs index 6604326..0224662 100644 --- a/crates/maestro-tui/src/draft.rs +++ b/crates/maestro-tui/src/draft.rs @@ -17,6 +17,7 @@ use std::collections::VecDeque; use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; use chrono::{DateTime, Utc}; use maestro_config::{builtin_presets, custom_preset}; @@ -35,6 +36,12 @@ use crate::projects; /// dry. pub const NAME_BATCH_SIZE: usize = 20; +/// Quiet period after raw-path typing before a `/api/gitroot` probe fires +/// (#74). Every keystroke resets it, so a burst issues one request once the +/// operator pauses. Must stay equal to `GITROOT_PROBE_DEBOUNCE_MS` in +/// web/src/spawnForm.ts so both clients debounce alike. +pub const GITROOT_PROBE_DEBOUNCE_MS: u64 = 250; + /// Width of the fixed label gutter every form line starts with (`cmd: `, /// `dir: `, …). const GUTTER: usize = 5; @@ -149,6 +156,9 @@ pub struct DraftPane { /// Bumped on every target change; answers carrying an older seq are stale. probe_seq: u32, needs_probe: bool, + /// When the pending probe may fire (debounce, #74). Typing into the raw + /// path resets it; a discrete action like the toggle makes it due at once. + probe_due_at: Option, /// Set when a probe found no repository (drives the rendered hint). repo_missing: bool, /// New-branch name. Tracks [`Self::name`] until edited, like the web form. @@ -198,6 +208,7 @@ impl Default for DraftPane { probed_cwd: None, probe_seq: 0, needs_probe: false, + probe_due_at: None, repo_missing: false, branch: crate::text_input::TextInput::default(), branch_edited: false, @@ -320,18 +331,23 @@ impl DraftPane { self.repo_missing = false; self.probe_seq += 1; self.needs_probe = false; + self.probe_due_at = None; self.probed_cwd = self.cwd(); self.branches_for = None; self.schedule_branch_load(); } /// A target with an unknown repo (raw path edit or the Other… hatch). - /// Cancels any in-flight probe and requests a fresh one. + /// Cancels any in-flight probe and schedules a fresh one after the + /// debounce window (#74). fn note_unknown_repo(&mut self) { self.base_repo = None; self.repo_missing = false; self.probe_seq += 1; self.needs_probe = self.worktree_enabled && self.cwd().is_some(); + self.probe_due_at = self + .needs_probe + .then(|| Instant::now() + Duration::from_millis(GITROOT_PROBE_DEBOUNCE_MS)); self.branches_for = None; self.schedule_branch_load(); } @@ -342,14 +358,19 @@ impl DraftPane { } } - /// Consume the gitroot-probe request: the caller fetches + /// Consume the gitroot-probe request once its debounce window has passed + /// (`now` at or after [`Self::probe_due_at`]): the caller fetches /// `GET /api/gitroot?path=…` and reports back via [`Self::resolve_gitroot`] - /// with the same seq. - pub fn take_gitroot_probe(&mut self) -> Option<(u32, PathBuf)> { + /// with the same seq. Returns `None` while typing is still in flight. + pub fn take_gitroot_probe(&mut self, now: Instant) -> Option<(u32, PathBuf)> { if !self.needs_probe { return None; } + if now < self.probe_due_at? { + return None; + } self.needs_probe = false; + self.probe_due_at = None; let cwd = self.cwd()?; self.probe_seq += 1; Some((self.probe_seq, cwd)) @@ -363,6 +384,7 @@ impl DraftPane { return; } self.needs_probe = false; + self.probe_due_at = None; self.probed_cwd = self.cwd(); match root { Some(repo) => { @@ -434,7 +456,9 @@ impl DraftPane { } if self.base_repo.is_none() { self.probe_seq += 1; + // A discrete toggle, not a keystroke burst: due immediately. self.needs_probe = cwd.is_some(); + self.probe_due_at = self.needs_probe.then(Instant::now); } self.schedule_branch_load(); } @@ -1277,6 +1301,11 @@ mod tests { } } + /// A moment comfortably past the probe debounce window (#74). + fn probe_due() -> Instant { + Instant::now() + Duration::from_millis(GITROOT_PROBE_DEBOUNCE_MS) * 10 + } + #[test] fn defaults_fall_back_to_builtin_presets_with_custom_last() { let draft = DraftPane::default(); @@ -1464,7 +1493,7 @@ mod tests { typed(&mut draft, "/opt/tooling"); assert_eq!(draft.cwd(), Some(PathBuf::from("/home/u/opt/tooling"))); // The raw target probed clean, so the worktree spec carries its repo. - let (seq, _) = draft.take_gitroot_probe().unwrap(); + let (seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); draft.resolve_gitroot(seq, Some(PathBuf::from("/home/u/opt/tooling"))); draft.seed_names(vec!["amber-falcon".into()]); let req = draft.build_request().unwrap(); @@ -1685,7 +1714,7 @@ mod tests { Some(Path::new("/repos/maestro")), "a picked project resolves its base repo without a probe" ); - assert!(draft.take_gitroot_probe().is_none()); + assert!(draft.take_gitroot_probe(probe_due()).is_none()); draft.seed_names(vec!["crimson-otter".into()]); let wt = draft.build_request().unwrap().worktree.unwrap(); @@ -1725,7 +1754,7 @@ mod tests { draft.on_input(&InputKey::Enter); assert!(draft.worktree_enabled); assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/repos/main"))); - assert!(draft.take_gitroot_probe().is_none()); + assert!(draft.take_gitroot_probe(probe_due()).is_none()); } #[test] @@ -1737,13 +1766,18 @@ mod tests { draft.on_input(&InputKey::Enter); // no projects → Other directory… typed(&mut draft, "/opt/tool"); - let (seq, path) = draft.take_gitroot_probe().expect("probe requested"); + let (seq, path) = draft + .take_gitroot_probe(probe_due()) + .expect("probe requested"); assert_eq!(path, PathBuf::from("/opt/tool")); - assert!(draft.take_gitroot_probe().is_none(), "one probe at a time"); + assert!( + draft.take_gitroot_probe(probe_due()).is_none(), + "one probe at a time" + ); // Further typing supersedes the in-flight probe… typed(&mut draft, "-x"); - let (seq2, _) = draft.take_gitroot_probe().unwrap(); + let (seq2, _) = draft.take_gitroot_probe(probe_due()).unwrap(); assert_ne!(seq2, seq); // …so a late answer to the old one must be ignored. draft.resolve_gitroot(seq, Some(PathBuf::from("/somewhere/else"))); @@ -1764,7 +1798,7 @@ mod tests { }; draft.on_input(&InputKey::Enter); typed(&mut draft, "/not/a/repo"); - let (seq, _) = draft.take_gitroot_probe().unwrap(); + let (seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); draft.resolve_gitroot(seq, None); assert!(!draft.worktree_enabled); @@ -1908,6 +1942,61 @@ mod tests { ); } + #[test] + fn a_keystroke_burst_schedules_one_debounced_gitroot_probe() { + let mut draft = DraftPane { + focused_field: DraftField::DirPicker, + ..DraftPane::default() + }; + draft.on_input(&InputKey::Enter); // no projects → Other directory… + + // Burst: every keystroke reschedules; nothing may fire inside the + // debounce window (#74). + for c in "/opt/tooling".chars() { + draft.on_input(&InputKey::char(c)); + assert!( + draft.take_gitroot_probe(Instant::now()).is_none(), + "no probe while typing is still in flight" + ); + } + + // Once the quiet period passes, exactly one probe fires. + let (seq, path) = draft + .take_gitroot_probe(probe_due()) + .expect("probe due after the quiet period"); + assert_eq!(path, PathBuf::from("/opt/tooling")); + assert!( + draft.take_gitroot_probe(probe_due()).is_none(), + "one probe total" + ); + + // The answer still lands through the unchanged seq guard. + draft.resolve_gitroot(seq, Some(PathBuf::from("/opt/tooling"))); + assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/opt/tooling"))); + } + + #[test] + fn a_stale_probe_answer_cannot_overwrite_a_newer_target_after_the_debounce() { + let mut draft = DraftPane { + focused_field: DraftField::DirPicker, + ..DraftPane::default() + }; + draft.on_input(&InputKey::Enter); // Other directory… + + typed(&mut draft, "/repo/old"); + let (old_seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); + + // Retyping supersedes the in-flight probe and gets a newer seq… + typed(&mut draft, "/repo/new"); + let (new_seq, _) = draft.take_gitroot_probe(probe_due()).unwrap(); + assert_ne!(new_seq, old_seq); + + // …so the late answer to the old target must be ignored. + draft.resolve_gitroot(new_seq, Some(PathBuf::from("/repo/new"))); + draft.resolve_gitroot(old_seq, Some(PathBuf::from("/repo/old"))); + assert_eq!(draft.base_repo.as_deref(), Some(Path::new("/repo/new"))); + } + #[test] fn toggling_worktree_off_spawns_in_the_checkout_itself() { let mut draft = DraftPane::default(); @@ -1933,7 +2022,7 @@ mod tests { draft.on_input(&InputKey::Enter); assert!(draft.worktree_enabled); assert!( - draft.take_gitroot_probe().is_none(), + draft.take_gitroot_probe(probe_due()).is_none(), "re-enabling reuses the known repo for an unchanged target" ); } From 2c89902829d3f2eeb2549096f069153010030162 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 15:08:05 +0100 Subject: [PATCH 11/20] feat(maestro): pick a project, then its worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The working-directory picker was a flat list mixing repositories with their worktrees, each row repeating the project name as an abbreviated path. It is now two steps: a grid of project boxes carrying the full unabbreviated path, then — only once a project is picked — that repository's worktrees with "+ new" as the default. "+ new" creates a worktree; picking an existing one runs in it and leaves the create-worktree toggle off, since the daemon never adopts (docs/WORKTREES.md). That is #91's rule, now an explicit choice rather than an inference from which row was clicked. Also fixes `.raw-wrap[hidden]`: `display: flex` outranked the UA sheet's `[hidden]`, so the free-text cwd input showed even when hidden. The interaction rules are listed in spawnForm.ts for the TUI mirror (#96) to follow. Refs #95 --- web/src/projectPicker.test.ts | 66 +++++---- web/src/spawnForm.ts | 226 ++++++++++++++++++++--------- web/src/spawnFormWorktree.test.ts | 12 +- web/src/styles.css | 159 +++++++++++++++----- web/src/worktreeStep.test.ts | 233 ++++++++++++++++++++++++++++++ 5 files changed, 559 insertions(+), 137 deletions(-) create mode 100644 web/src/worktreeStep.test.ts diff --git a/web/src/projectPicker.test.ts b/web/src/projectPicker.test.ts index ba6b821..6932566 100644 --- a/web/src/projectPicker.test.ts +++ b/web/src/projectPicker.test.ts @@ -34,13 +34,13 @@ function stubFetch(info: unknown, projects: Project[]): void { ); } -function rowPaths(form: HTMLFormElement): string[] { - return [...form.querySelectorAll(".project-row:not(.project-other)")].map( - (r) => r.dataset.cwd ?? "", +function boxPaths(form: HTMLFormElement): string[] { + return [...form.querySelectorAll(".project-box")].map( + (b) => b.dataset.cwd ?? "", ); } -describe("spawnForm project picker", () => { +describe("spawnForm project picker (#95: boxes, full paths, mains only)", () => { let handle: SpawnFormHandle | null = null; beforeEach(() => { @@ -65,7 +65,7 @@ describe("spawnForm project picker", () => { return handle; } - it("renders one row per project, fresh before stale", async () => { + it("renders one box per project, fresh before stale", async () => { const fresh = project({ name: "fresh", path: "/home/user/fresh", last_activity: NOW }); const stale = project({ name: "stale", @@ -73,40 +73,56 @@ describe("spawnForm project picker", () => { last_activity: "2026-01-01T00:00:00Z", }); const form = (await mount([stale, fresh])).el; - expect(rowPaths(form)).toEqual(["/home/user/fresh", "/home/user/stale"]); - const rows = [...form.querySelectorAll(".project-row")]; - expect(rows[1].classList.contains("is-stale")).toBe(true); - expect(rows[0].classList.contains("is-stale")).toBe(false); + expect(boxPaths(form)).toEqual(["/home/user/fresh", "/home/user/stale"]); + const boxes = [...form.querySelectorAll(".project-box")]; + expect(boxes[1].classList.contains("is-stale")).toBe(true); + expect(boxes[0].classList.contains("is-stale")).toBe(false); }); - it("nests worktrees under their base repository", async () => { + it("shows the name as the primary line and the full path unabbreviated", async () => { + const p = project({ name: "nixos-config", path: "/home/user/tea/nixos-config" }); + const form = (await mount([p])).el; + const box = form.querySelector(".project-box")!; + expect(box.querySelector(".project-name")!.textContent).toBe("nixos-config"); + // Not `~/t/nixos-config`: the abbreviation hid what distinguishes two + // projects sharing a basename (#95). + expect(box.querySelector(".project-path")!.textContent).toBe("/home/user/tea/nixos-config"); + }); + + it("lists only main checkouts as boxes — worktrees belong to step two", async () => { const main = project({ name: "maestro", path: "/home/user/maestro" }); const wt = project({ name: "wt", - path: "/elsewhere/wt", + path: "/home/user/maestro/worktrees/wt", is_worktree: true, base_repo: main.path, }); const form = (await mount([wt, main])).el; - expect(rowPaths(form)).toEqual(["/home/user/maestro", "/elsewhere/wt"]); - const [mainRow, wtRow] = [ - ...form.querySelectorAll(".project-row:not(.project-other)"), - ]; - expect(wtRow.classList.contains("is-worktree")).toBe(true); - expect(mainRow.classList.contains("is-worktree")).toBe(false); + expect(boxPaths(form)).toEqual(["/home/user/maestro"]); + expect(form.querySelector(".project-box.is-worktree")).toBeNull(); }); - it("filters rows as the operator types", async () => { + it("filters boxes as the operator types, keeping a repo its worktree matched", async () => { const a = project({ name: "maestro", path: "/home/user/maestro" }); const b = project({ name: "tea", path: "/home/user/work/tea" }); - const form = (await mount([a, b])).el; + const wt = project({ + name: "bulgarians", + path: "/home/user/maestro/worktrees/bulgarians", + is_worktree: true, + base_repo: a.path, + }); + const form = (await mount([a, b, wt])).el; const filter = form.querySelector(".project-filter")!; filter.value = "tea"; filter.dispatchEvent(new Event("input")); - expect(rowPaths(form)).toEqual(["/home/user/work/tea"]); + expect(boxPaths(form)).toEqual(["/home/user/work/tea"]); + // A query that only matches a worktree still surfaces its repository box. + filter.value = "bulgarians"; + filter.dispatchEvent(new Event("input")); + expect(boxPaths(form)).toEqual(["/home/user/maestro"]); filter.value = "zzz"; filter.dispatchEvent(new Event("input")); - expect(rowPaths(form)).toEqual([]); + expect(boxPaths(form)).toEqual([]); expect(form.querySelector(".picker-empty")).toBeTruthy(); }); @@ -135,17 +151,17 @@ describe("spawnForm project picker", () => { for (let i = 0; i < 20; i++) await Promise.resolve(); onChange.mockClear(); - const row = form.querySelector(".project-row")!; - row.click(); + const box = form.querySelector(".project-box")!; + box.click(); const values = form.querySelector("input[name=name]")!.value; const last = onChange.mock.lastCall![0]; expect(last.cwd).toBe("/home/user/maestro"); expect(last.command).toBe("claude"); expect(values).toBeTruthy(); - // The raw input stays hidden — the picked row is the source of truth. + // The raw input stays hidden — the picked box is the source of truth. expect((form.querySelector(".cwd-raw-wrap") as HTMLElement).hidden).toBe(true); - expect(row.classList.contains("is-active")).toBe(true); + expect(box.classList.contains("is-active")).toBe(true); }); it("keeps an 'Other directory…' escape hatch that reveals the raw input", async () => { diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index 1b130ce..16198b5 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -9,8 +9,8 @@ import { } from "./api.js"; import { attachDirAutocomplete } from "./dirAutocomplete.js"; import { filterProjects, groupProjects, isStale, sortProjects } from "./projectList.js"; +import type { ProjectGroup } from "./projectList.js"; import type { BranchInfo, CreateSessionRequest, Project, Session, SpawnPreset } from "./types.js"; -import { abbreviateCwd, abbreviateFolderName } from "./types.js"; /** Where a worktree's branch comes from: freshly created (default) or picked * from the repository's existing branches (#61). */ @@ -125,15 +125,20 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { placeholder="bash -l" autocomplete="off" />
- Working directory + Project
+
+
Name
@@ -182,8 +187,11 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { const errEl = form.querySelector(".form-error") as HTMLElement; const cmdTiles = form.querySelector(".cmd-tiles") as HTMLElement; const cmdRaw = form.querySelector(".cmd-raw") as HTMLInputElement; - const recentsEl = form.querySelector(".projects") as HTMLElement; + const projectsEl = form.querySelector(".projects") as HTMLElement; const filterInput = form.querySelector(".project-filter") as HTMLInputElement; + const otherBtn = form.querySelector(".project-other") as HTMLButtonElement; + const wtStep = form.querySelector(".wt-step") as HTMLElement; + const wtChoices = form.querySelector(".wt-choices") as HTMLElement; const cwdRawWrap = form.querySelector(".cwd-raw-wrap") as HTMLElement; const cwdRaw = form.querySelector(".cwd-raw") as HTMLInputElement; const wtEnabled = form.querySelector("input[name=worktree_enabled]") as HTMLInputElement; @@ -394,12 +402,36 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { if (first) selectPreset(first.id, false); } - // ---- working-directory picker --------------------------------------- + // ---- working-directory picker (#95) ---------------------------------- + // Two steps, and the TUI mirror (#96) must match these rules verbatim: + // 1. Step one is one box per repository (name, then full path). A worktree + // is a box only when its main checkout is missing from the scan. + // 2. The filter keeps a repository when the query matches it or any of its + // worktrees, so a worktree name still finds its box. + // 3. Picking a box reveals step two: "+ new", then that repository's + // worktrees newest-first. "Other directory…" clears both steps. + // 4. "+ new" is the default selection — except on a box that is itself a + // linked worktree, which defaults to that worktree. + // 5. "+ new" means create: cwd is the repository, create-worktree on. + // An existing worktree means run in it: cwd is that worktree, + // create-worktree OFF. The daemon never adopts (docs/WORKTREES.md), so + // these are separate acts — #91's rule, now an explicit choice. + // 6. Neither step probes /api/gitroot; the scan already reports repository + // membership. Only the free-text hatch pays for a probe. + + /** "new" (create a worktree) or the path of an existing worktree to run in. */ + type WorktreeChoice = "new" | string; + + let discovered: Project[] = []; // daemon snapshot, daemon-sorted + let groups: ProjectGroup[] = []; // rebuilt on every render + let selectedGroup: ProjectGroup | null = null; + let worktreeChoice: WorktreeChoice = "new"; + function showRawCwd(prefillValue?: string, focus = true): void { cwdRawWrap.hidden = false; if (prefillValue != null) cwd = prefillValue; cwdRaw.value = cwd; - markActiveProject(null); + clearProjectSelection(); if (focus) { cwdRaw.focus(); cwdRaw.select(); @@ -416,62 +448,89 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { if (wtEnabled.checked) probeGitRoot(); }); - function markActiveProject(activeCwd: string | null): void { - for (const row of recentsEl.querySelectorAll(".project-row")) { - row.classList.toggle( - "is-active", - (row as HTMLElement).dataset.cwd === activeCwd, - ); + /** Every worktree of a group, including the group's own box when the scan + * found no main checkout to nest it under. */ + function worktreesOf(group: ProjectGroup): Project[] { + return [group.main, ...group.worktrees].filter((p) => p.is_worktree); + } + + function markActiveBox(): void { + const active = selectedGroup?.main.path ?? null; + for (const box of projectsEl.querySelectorAll(".project-box")) { + box.classList.toggle("is-active", (box as HTMLElement).dataset.cwd === active); } } - function selectProject(project: Project): void { - cwd = project.path; + function markActiveChoice(): void { + for (const btn of wtChoices.querySelectorAll(".wt-choice")) { + const el = btn as HTMLElement; + const value = el.dataset.kind === "new" ? "new" : (el.dataset.cwd ?? ""); + el.classList.toggle("is-active", value === worktreeChoice); + } + } + + function clearProjectSelection(): void { + selectedGroup = null; + worktreeChoice = "new"; + wtChoices.replaceChildren(); + wtStep.hidden = true; + markActiveBox(); + } + + function selectProject(group: ProjectGroup): void { + selectedGroup = group; cwdRawWrap.hidden = true; - markActiveProject(project.path); - // The scan already knows this directory is inside a repo (#61): the - // toggle defaults back on and no /api/gitroot round trip is needed — - // unless the directory *is* a linked worktree itself (#91): spawning - // in place is the only thing that works there, so default the toggle - // off. Turning it back on by hand still tries (and fails with git's - // own message), which is deliberate (#91 non-goal). - wtEnabled.checked = !project.is_worktree; - if (project.is_worktree) { + renderWorktreeChoices(group); + markActiveBox(); + // Rule 4: a box that is itself a linked worktree defaults to running there. + applyWorktreeChoice(group.main.is_worktree ? group.main.path : "new"); + } + + function applyWorktreeChoice(choice: WorktreeChoice): void { + const group = selectedGroup; + if (!group) return; + const target = + choice === "new" ? group.main : worktreesOf(group).find((p) => p.path === choice); + if (!target) return; + worktreeChoice = choice; + cwd = target.path; + branchCache = null; + markActiveChoice(); + if (choice === "new") { + wtEnabled.checked = true; + applyProjectRepo(group.main.base_repo ?? group.main.path); + } else { + wtEnabled.checked = false; inferredBaseRepo = null; wtBody.hidden = true; wtHint.hidden = true; - } else { - applyProjectRepo(project.base_repo ?? project.path); } emit(); - // Picking a project pre-selects the command it was last used with. - const match = presets.find((p) => p.command === project.last_command && p.command !== ""); + // The chosen directory pre-selects the command it was last used with. + const match = presets.find((p) => p.command === target.last_command && p.command !== ""); if (match) selectPreset(match.id); } - let discovered: Project[] = []; // daemon snapshot, daemon-sorted - function renderProjects(): void { - recentsEl.replaceChildren(); - const sorted = sortProjects(filterProjects(discovered, filterInput.value), new Date()); - for (const group of groupProjects(sorted)) { - recentsEl.appendChild(projectRow(group.main)); - for (const wt of group.worktrees) recentsEl.appendChild(projectRow(wt)); - } - if (discovered.length > 0 && sorted.length === 0) { + projectsEl.replaceChildren(); + groups = groupProjects(sortProjects(discovered, new Date())); + const query = filterInput.value; + const visible = groups.filter((g) => filterProjects([g.main, ...g.worktrees], query).length > 0); + for (const group of visible) projectsEl.appendChild(projectBox(group)); + if (discovered.length > 0 && visible.length === 0) { const empty = document.createElement("p"); empty.className = "picker-empty"; empty.textContent = "No projects match."; - recentsEl.appendChild(empty); + projectsEl.appendChild(empty); } - recentsEl.appendChild(otherDirRow()); - markActiveProject(cwdRawWrap.hidden ? cwd : null); + markActiveBox(); } filterInput.addEventListener("input", renderProjects); + otherBtn.addEventListener("click", () => showRawCwd(cwd || homeDir)); async function loadProjects(): Promise { - recentsEl.replaceChildren(loadingRow()); + projectsEl.replaceChildren(loadingRow()); try { discovered = await fetchProjects(); } catch { @@ -482,12 +541,13 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { if (seedCwd) { // A seed sets the directory verbatim, regardless of what the scan found. // `cwdRawVisible === false` marks a draft restore whose cwd came from a - // picked row: keep the picker and don't steal focus. Otherwise land + // picked box: keep the picker and don't steal focus. Otherwise land // focused in the raw input (clone / no-match flows). if (prefill.cwdRawVisible === false) { cwd = seedCwd; cwdRaw.value = seedCwd; - markActiveProject(cwd); + // Restoring must not touch the worktree toggle — the seed carries it. + if (!restoreSelection(seedCwd)) clearProjectSelection(); } else { showRawCwd(seedCwd, false); } @@ -504,34 +564,66 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { emit(); } - function projectRow(project: Project): HTMLElement { - const row = document.createElement("button"); - row.type = "button"; - row.className = `project-row${project.is_worktree ? " is-worktree" : ""}${ - isStale(project, new Date()) ? " is-stale" : "" - }`; - row.dataset.cwd = project.path; - // Abbreviate exactly like the sidebar: home → `~`, then collapse leading - // segments to initials (`~/g/n/maestro`). The full path stays in the tooltip - // and `dataset.cwd` (what's actually submitted). - const display = abbreviateFolderName(abbreviateCwd(project.path, homeDir)); - row.innerHTML = ` - - ${escapeHtml(project.name)} - ${display} - - ${escapeHtml(project.last_command ?? "")}`; - row.addEventListener("click", () => selectProject(project)); - return row; + /** Put the two steps back on a restored draft's cwd, without re-running the + * side effects of a fresh pick. */ + function restoreSelection(path: string): boolean { + for (const group of groups) { + const wt = worktreesOf(group).find((p) => p.path === path); + const choice: WorktreeChoice | null = + wt ? wt.path : group.main.path === path ? "new" : null; + if (choice === null) continue; + selectedGroup = group; + worktreeChoice = choice; + renderWorktreeChoices(group); + markActiveChoice(); + markActiveBox(); + return true; + } + return false; } - function otherDirRow(): HTMLElement { - const row = document.createElement("button"); - row.type = "button"; - row.className = "project-row project-other"; - row.innerHTML = `Other directory…`; - row.addEventListener("click", () => showRawCwd(cwd || homeDir)); - return row; + function projectBox(group: ProjectGroup): HTMLElement { + const project = group.main; + const box = document.createElement("button"); + box.type = "button"; + box.className = `project-box${isStale(project, new Date()) ? " is-stale" : ""}`; + box.dataset.cwd = project.path; + const trees = worktreesOf(group).length; + // The full path, not the sidebar's `~/t/n/maestro` abbreviation: it is what + // tells two projects sharing a basename apart (#95). + const meta = [trees > 0 ? `${trees} worktree${trees === 1 ? "" : "s"}` : "", project.last_command ?? ""] + .filter(Boolean) + .join(" · "); + box.innerHTML = ` + ${escapeHtml(project.name)} + ${escapeHtml(project.path)} + ${escapeHtml(meta)}`; + box.addEventListener("click", () => selectProject(group)); + return box; + } + + function renderWorktreeChoices(group: ProjectGroup): void { + wtChoices.replaceChildren(); + wtChoices.appendChild( + choiceButton("new", "+ new", `new worktree of ${group.main.name}`), + ); + for (const wt of worktreesOf(group)) { + wtChoices.appendChild(choiceButton(wt.path, wt.name, wt.path)); + } + wtStep.hidden = false; + } + + function choiceButton(choice: WorktreeChoice, label: string, sub: string): HTMLElement { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = `wt-choice${choice === "new" ? " wt-choice-new" : ""}`; + btn.dataset.kind = choice === "new" ? "new" : "existing"; + if (choice !== "new") btn.dataset.cwd = choice; + btn.innerHTML = ` + ${escapeHtml(label)} + ${escapeHtml(sub)}`; + btn.addEventListener("click", () => applyWorktreeChoice(choice)); + return btn; } function loadingRow(): HTMLElement { diff --git a/web/src/spawnFormWorktree.test.ts b/web/src/spawnFormWorktree.test.ts index b8012e1..40d8571 100644 --- a/web/src/spawnFormWorktree.test.ts +++ b/web/src/spawnFormWorktree.test.ts @@ -80,10 +80,10 @@ describe("spawnForm worktree defaults and branch picker (#61)", () => { }; function pickProject(form: HTMLFormElement): void { - form.querySelector(".project-row.project-other")!.click(); + form.querySelector(".project-other")!.click(); form - .querySelectorAll(".project-row") - .forEach((row) => row.dataset.cwd === "/home/user/maestro" && row.click()); + .querySelectorAll(".project-box") + .forEach((box) => box.dataset.cwd === "/home/user/maestro" && box.click()); } it("defaults the worktree toggle on for a picked project with no gitroot probe", async () => { @@ -118,8 +118,8 @@ describe("spawnForm worktree defaults and branch picker (#61)", () => { for (let i = 0; i < 40; i++) await Promise.resolve(); handle.el - .querySelectorAll(".project-row") - .forEach((row) => row.dataset.cwd === "/home/user/wt-91" && row.click()); + .querySelectorAll(".project-box") + .forEach((box) => box.dataset.cwd === "/home/user/wt-91" && box.click()); await drain(); expect(handle.el.querySelector("input[name=worktree_enabled]")!.checked).toBe( @@ -133,7 +133,7 @@ describe("spawnForm worktree defaults and branch picker (#61)", () => { const form = await mount(); // Escape hatch: switch to a free-text dir; only now does /api/gitroot fire. - form.querySelector(".project-row.project-other")!.click(); + form.querySelector(".project-other")!.click(); const raw = form.querySelector("input[name=cwd_raw]")!; raw.value = "/opt/somewhere"; raw.dispatchEvent(new Event("input")); diff --git a/web/src/styles.css b/web/src/styles.css index afe24c3..849aef9 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -727,10 +727,8 @@ textarea { } /* Earlier path segments collapse to their initial; the trailing segment (the - one that actually names the dir) stays full. Dim the collapsed prefix. Shared - by the sidebar tree and the spawn dialog's recent-directory picker. */ -.folder-name .folder-stem, -.recent-dir .folder-stem { + one that actually names the dir) stays full. Dim the collapsed prefix. */ +.folder-name .folder-stem { color: var(--faint); } @@ -1381,8 +1379,9 @@ textarea { color: var(--muted); } -/* Project picker rows (#59). Worktree rows are indented under their base - repository; rows stale beyond 7 days are dimmed but still selectable. */ +/* Working-directory picker (#95). Two steps: a grid of project boxes, then the + picked project's worktree choices. Stale beyond 7 days dims but stays + selectable. */ .picker { display: flex; flex-direction: column; @@ -1397,73 +1396,155 @@ textarea { width: 100%; } -.project-row { +/* `display` from `.field`/`.raw-wrap` outranks the UA sheet's `[hidden]`, so + step two and the raw-cwd hatch stayed visible while hidden. */ +.wt-step[hidden], +.raw-wrap[hidden] { + display: none; +} + +.picker.projects { + display: grid; + grid-template-columns: repeat(auto-fill, minmax(190px, 1fr)); + align-content: start; + gap: var(--sp-2); + max-height: 300px; + padding: 1px; +} + +.project-box { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; + padding: var(--sp-2) var(--sp-3); + background: var(--surface-2); + border: 1px solid var(--border); + border-radius: var(--r-2); + color: var(--ink); + font-family: inherit; + text-align: left; + cursor: pointer; + transition: + border-color var(--dur-fast) var(--ease-out), + background var(--dur-fast) var(--ease-out); +} + +.project-box:hover { + border-color: var(--border-strong); +} + +.project-box.is-active { + border-color: var(--accent); + background: var(--accent-tint); +} + +.project-box.is-stale { + opacity: 0.55; +} + +.project-name { + font-size: var(--fs-13); + font-weight: 600; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +/* The whole path, never abbreviated — it is what tells two projects sharing a + basename apart. Wraps rather than truncating so none of it is hidden. */ +.project-path { + font-family: var(--font-mono); + font-size: var(--fs-12); + line-height: 1.35; + color: var(--muted); + overflow-wrap: anywhere; +} + +.project-meta { + font-family: var(--font-mono); + font-size: var(--fs-12); + color: var(--faint); + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.project-meta:empty { + display: none; +} + +.project-other { + align-self: flex-start; + padding: 4px 0; + min-height: 30px; + background: none; + border: 0; + color: var(--muted); + font-family: inherit; + font-size: var(--fs-13); + font-style: italic; + text-align: left; + cursor: pointer; +} + +.project-other:hover { + color: var(--ink); +} + +/* Step two: "+ new" (create one) or an existing worktree (run in it). */ +.picker.wt-choices { + max-height: 200px; +} + +.wt-choice { display: flex; align-items: baseline; justify-content: space-between; gap: var(--sp-3); + min-height: 32px; padding: 6px var(--sp-3); background: var(--surface-2); border: 1px solid var(--border); border-radius: var(--r-2); color: var(--ink); font-family: inherit; - cursor: pointer; text-align: left; + cursor: pointer; transition: border-color var(--dur-fast) var(--ease-out), background var(--dur-fast) var(--ease-out); } -.project-row:hover { +.wt-choice:hover { border-color: var(--border-strong); } -.project-row.is-active { +.wt-choice.is-active { border-color: var(--accent); background: var(--accent-tint); } -.project-row.is-worktree { - margin-left: var(--sp-4); -} - -.project-row.is-stale { - opacity: 0.55; -} - -.project-info { - display: flex; - align-items: baseline; - gap: var(--sp-2); - min-width: 0; -} - -.project-name { +.wt-choice-name { + flex: 0 0 auto; font-size: var(--fs-13); font-weight: 600; white-space: nowrap; } -.recent-dir { +.wt-choice-new .wt-choice-name { font-family: var(--font-mono); - font-size: var(--fs-12); - color: var(--muted); - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; + color: var(--accent); } -.recent-cmd { - flex: 0 0 auto; +.wt-choice-sub { + min-width: 0; font-family: var(--font-mono); font-size: var(--fs-12); + line-height: 1.35; color: var(--muted); -} - -.project-other .recent-dir { - font-family: inherit; - font-style: italic; + text-align: right; + overflow-wrap: anywhere; } .picker-empty { diff --git a/web/src/worktreeStep.test.ts b/web/src/worktreeStep.test.ts new file mode 100644 index 0000000..c89a011 --- /dev/null +++ b/web/src/worktreeStep.test.ts @@ -0,0 +1,233 @@ +// @vitest-environment happy-dom +// Second step of the working-directory picker (#95). See the interaction rules +// block in spawnForm.ts for what each choice means. +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { createSpawnForm, type SpawnFormHandle, type SpawnFormValues } from "./spawnForm.js"; +import type { Project } from "./types.js"; + +const MAESTRO: Project = { + path: "/home/user/tea/maestro", + name: "maestro", + is_worktree: false, + last_activity: "2026-08-20T10:00:00Z", + last_command: "claude", +}; + +const WT_A: Project = { + path: "/home/user/tea/maestro/worktrees/bulgarians", + name: "bulgarians", + is_worktree: true, + base_repo: MAESTRO.path, + last_activity: "2026-08-22T10:00:00Z", +}; + +const WT_B: Project = { + path: "/home/user/tea/maestro/worktrees/spawn-v2", + name: "spawn-v2", + is_worktree: true, + base_repo: MAESTRO.path, + last_activity: "2026-08-21T10:00:00Z", +}; + +const OTHER: Project = { + path: "/home/user/tea/rev", + name: "rev", + is_worktree: false, + last_activity: "2026-08-19T10:00:00Z", +}; + +function stubFetch(projects: Project[]): string[] { + const urls: string[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + urls.push(url); + const body = url.includes("/api/projects") + ? projects + : url.includes("/api/names") + ? { names: ["amber-fox"] } + : url.includes("/api/branches") + ? [] + : { hostname: "h", version: "0", home_dir: "/home/user", presets: [] }; + return new Response(JSON.stringify(body), { status: 200 }); + }), + ); + return urls; +} + +describe("spawnForm worktree step (#95)", () => { + let handle: SpawnFormHandle | null = null; + let urls: string[] = []; + let changes: SpawnFormValues[] = []; + + beforeEach(() => { + vi.useFakeTimers({ now: Date.parse("2026-08-23T12:00:00Z") }); + changes = []; + }); + + afterEach(() => { + handle?.destroy(); + handle = null; + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + const drain = async () => { + for (let i = 0; i < 40; i++) await Promise.resolve(); + }; + + async function mount(projects: Project[]): Promise { + urls = stubFetch(projects); + handle = createSpawnForm({ onSuccess() {}, onChange: (v) => changes.push(v) }); + document.body.appendChild(handle.el); + await drain(); + return handle.el; + } + + const step = (form: HTMLFormElement) => form.querySelector(".wt-step")!; + const choices = (form: HTMLFormElement) => [ + ...form.querySelectorAll(".wt-choice"), + ]; + const activeChoice = (form: HTMLFormElement) => + form.querySelector(".wt-choice.is-active"); + const box = (form: HTMLFormElement, path: string) => + form.querySelector(`.project-box[data-cwd="${path}"]`)!; + const toggle = (form: HTMLFormElement) => + form.querySelector("input[name=worktree_enabled]")!; + + it("hides the worktree step until a project is picked", async () => { + const form = await mount([MAESTRO, WT_A]); + expect(step(form).hidden).toBe(true); + expect(choices(form)).toEqual([]); + + box(form, MAESTRO.path).click(); + await drain(); + + expect(step(form).hidden).toBe(false); + }); + + it("lists '+ new' first and selected by default, then the project's worktrees", async () => { + const form = await mount([MAESTRO, WT_A, WT_B]); + box(form, MAESTRO.path).click(); + await drain(); + + const labels = choices(form).map((c) => c.querySelector(".wt-choice-name")!.textContent); + expect(labels).toEqual(["+ new", "bulgarians", "spawn-v2"]); + expect(activeChoice(form)!.dataset.kind).toBe("new"); + }); + + it("taking the '+ new' default runs in the repo with the create toggle on", async () => { + const form = await mount([MAESTRO, WT_A]); + box(form, MAESTRO.path).click(); + await drain(); + + expect(changes.at(-1)!.cwd).toBe(MAESTRO.path); + expect(changes.at(-1)!.worktreeEnabled).toBe(true); + expect(toggle(form).checked).toBe(true); + expect(form.querySelector(".wt-repo")!.textContent).toContain(MAESTRO.path); + // The scan already knows the repo — no probe. + expect(urls.some((u) => u.includes("/api/gitroot"))).toBe(false); + }); + + it("picking an existing worktree runs in it with the create toggle off (#91)", async () => { + const form = await mount([MAESTRO, WT_A, WT_B]); + box(form, MAESTRO.path).click(); + await drain(); + + const existing = choices(form).find((c) => c.dataset.cwd === WT_A.path)!; + existing.click(); + await drain(); + + expect(changes.at(-1)!.cwd).toBe(WT_A.path); + expect(changes.at(-1)!.worktreeEnabled).toBe(false); + expect(toggle(form).checked).toBe(false); + expect(form.querySelector(".wt-body")!.hidden).toBe(true); + expect(existing.classList.contains("is-active")).toBe(true); + // Still no probe: the scan reported this worktree's repo membership. + expect(urls.some((u) => u.includes("/api/gitroot"))).toBe(false); + // The project box stays marked — it is what the worktree list belongs to. + expect(box(form, MAESTRO.path).classList.contains("is-active")).toBe(true); + }); + + it("switching back to '+ new' re-enables the create toggle", async () => { + const form = await mount([MAESTRO, WT_A]); + box(form, MAESTRO.path).click(); + await drain(); + choices(form) + .find((c) => c.dataset.cwd === WT_A.path)! + .click(); + await drain(); + + choices(form) + .find((c) => c.dataset.kind === "new")! + .click(); + await drain(); + + expect(changes.at(-1)!.cwd).toBe(MAESTRO.path); + expect(changes.at(-1)!.worktreeEnabled).toBe(true); + expect(toggle(form).checked).toBe(true); + }); + + it("repopulates and resets to '+ new' when another project is picked", async () => { + const form = await mount([MAESTRO, WT_A, OTHER]); + box(form, MAESTRO.path).click(); + await drain(); + choices(form) + .find((c) => c.dataset.cwd === WT_A.path)! + .click(); + await drain(); + + box(form, OTHER.path).click(); + await drain(); + + expect(choices(form).map((c) => c.dataset.kind)).toEqual(["new"]); + expect(activeChoice(form)!.dataset.kind).toBe("new"); + expect(changes.at(-1)!.cwd).toBe(OTHER.path); + expect(changes.at(-1)!.worktreeEnabled).toBe(true); + }); + + it("defaults to the worktree itself when the box is a linked worktree (#91)", async () => { + // No main checkout in the scan, so the orphan worktree is its own box. + const form = await mount([WT_A]); + box(form, WT_A.path).click(); + await drain(); + + expect(activeChoice(form)!.dataset.cwd).toBe(WT_A.path); + expect(changes.at(-1)!.cwd).toBe(WT_A.path); + expect(changes.at(-1)!.worktreeEnabled).toBe(false); + expect(toggle(form).checked).toBe(false); + expect(form.querySelector(".wt-body")!.hidden).toBe(true); + }); + + it("hides the worktree step again when the operator takes the raw-input hatch", async () => { + const form = await mount([MAESTRO, WT_A]); + box(form, MAESTRO.path).click(); + await drain(); + expect(step(form).hidden).toBe(false); + + form.querySelector(".project-other")!.click(); + await drain(); + + expect(step(form).hidden).toBe(true); + expect(choices(form)).toEqual([]); + expect(form.querySelector(".project-box.is-active")).toBeNull(); + }); + + it("restores the picked worktree from a draft seed", async () => { + urls = stubFetch([MAESTRO, WT_A, WT_B]); + handle = createSpawnForm({ + onSuccess() {}, + seed: { cwd: WT_A.path, cwdRawVisible: false, worktreeEnabled: false }, + }); + document.body.appendChild(handle.el); + await drain(); + const form = handle.el; + + expect(step(form).hidden).toBe(false); + expect(activeChoice(form)!.dataset.cwd).toBe(WT_A.path); + expect(box(form, MAESTRO.path).classList.contains("is-active")).toBe(true); + expect(toggle(form).checked).toBe(false); + }); +}); From 36cb03961c9706238f62069d974c112e8d35ab34 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 15:14:43 +0100 Subject: [PATCH 12/20] ci(maestro): build flake packages so a stale pnpmDeps hash fails loudly A web/pnpm-lock.yaml change invalidates the pinned pnpmDeps.hash in flake.nix silently; nix build .#maestro-web then stays broken until someone notices by hand (#98). Add a flake job that installs nix and builds the packaged outputs on every CI run. --- .gitea/workflows/ci.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 57b91fc..41f5c2e 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -97,3 +97,24 @@ jobs: - name: build run: pnpm -C web build + + # The flake pins pnpmDeps.hash against web/pnpm-lock.yaml (#98). A lockfile + # change invalidates the pin silently elsewhere, so the packaged build has to + # fail here too. + flake: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install nix + run: | + curl -fsSL https://nixos.org/nix/install | sh -s -- --daemon --yes + echo "/nix/var/nix/profiles/default/bin" >> "$GITHUB_PATH" + + - name: build flake packages + run: | + nix build \ + .#webFrontend \ + .#maestro \ + .#maestro-web \ + --no-link From 5de04af36acdc27ba6f1fca2ea0a7c424d91d882 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 15:15:47 +0100 Subject: [PATCH 13/20] feat(maestro): collapse the spawn form worktree section by default (#93) Closed by default behind a disclosure reusing the sidebar folder-header affordance (catalog.ts), since most spawns take the default target and the section was the largest block in the form. The collapsed header states whether a worktree will be created, so #91's off-by-default for a worktree row stays visible without expanding. Fixes a latent [hidden] + display:flex bug for the new .wt-collapsible element (author CSS beats the UA [hidden] rule at equal specificity); .wt-body has the same bug pre-existing and is out of scope here. --- web/src/spawnForm.ts | 64 +++++++++++++++++++++---------- web/src/spawnFormWorktree.test.ts | 53 +++++++++++++++++++++++++ web/src/styles.css | 27 ++++++++++--- 3 files changed, 118 insertions(+), 26 deletions(-) diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index c18b2db..714796b 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -147,33 +147,38 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
- + +
@@ -192,6 +197,9 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { const cwdRawWrap = form.querySelector(".cwd-raw-wrap") as HTMLElement; const cwdRaw = form.querySelector(".cwd-raw") as HTMLInputElement; const wtEnabled = form.querySelector("input[name=worktree_enabled]") as HTMLInputElement; + const wtToggle = form.querySelector(".wt-toggle") as HTMLButtonElement; + const wtCollapsible = form.querySelector(".wt-collapsible") as HTMLElement; + const wtSummary = form.querySelector(".wt-summary") as HTMLElement; const wtHint = form.querySelector(".wt-hint") as HTMLElement; const wtBody = form.querySelector(".wt-body") as HTMLElement; const wtRepo = form.querySelector(".wt-repo") as HTMLElement; @@ -464,6 +472,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { // off. Turning it back on by hand still tries (and fails with git's // own message), which is deliberate (#91 non-goal). wtEnabled.checked = !project.is_worktree; + updateWtSummary(); if (project.is_worktree) { inferredBaseRepo = null; wtBody.hidden = true; @@ -569,6 +578,18 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { return p; } + // ---- worktree: disclosure (#93), reuses the sidebar's folder-header --- + let advancedCollapsed = true; + function setAdvancedCollapsed(collapsed: boolean): void { + advancedCollapsed = collapsed; + wtToggle.setAttribute("aria-expanded", String(!collapsed)); + wtCollapsible.hidden = collapsed; + } + function updateWtSummary(): void { + wtSummary.textContent = wtEnabled.checked ? "creates a worktree" : "no worktree"; + } + wtToggle.addEventListener("click", () => setAdvancedCollapsed(!advancedCollapsed)); + // ---- worktree (branch-only; base_repo inferred via scan or /gitroot) -- // The project scan already reports repo membership, so a picked project // resolves its base repo locally (#61); only the free-text escape hatch @@ -649,6 +670,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { // Publish the untick too, or the next render seeds the box checked with // no inferred repo and submitting fails. wtEnabled.checked = false; + updateWtSummary(); emit(); wtBody.hidden = true; wtHint.hidden = false; @@ -722,6 +744,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { branchSelect.addEventListener("change", () => emit()); wtEnabled.addEventListener("change", () => { + updateWtSummary(); emit(); if (wtEnabled.checked) { resolveBaseRepo(); @@ -735,6 +758,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { // The toggle defaults ON (#61): the operator almost always wants a // worktree, and every non-repo target unticks it via the gitroot probe. wtEnabled.checked = prefill.worktreeEnabled ?? true; + updateWtSummary(); if (prefill.worktreeBranch) branchInput.value = prefill.worktreeBranch; setBranchMode(branchMode); diff --git a/web/src/spawnFormWorktree.test.ts b/web/src/spawnFormWorktree.test.ts index bb04b96..ff5ca1e 100644 --- a/web/src/spawnFormWorktree.test.ts +++ b/web/src/spawnFormWorktree.test.ts @@ -180,6 +180,59 @@ describe("spawnForm worktree defaults and branch picker (#61)", () => { expect(modes.at(-1)).toBe("existing"); }); + it("collapses the advanced (worktree) section by default (#93)", async () => { + const form = await mount(); + const toggle = form.querySelector(".wt-toggle")!; + expect(toggle.getAttribute("aria-expanded")).toBe("false"); + expect(form.querySelector(".wt-collapsible")!.hidden).toBe(true); + }); + + it("states in the collapsed header that a worktree will be created by default (#93)", async () => { + const form = await mount(); + expect(form.querySelector(".wt-summary")!.textContent).toMatch(/creates? a worktree/i); + }); + + it("states in the collapsed header that no worktree will be created for a worktree row (#91, #93)", async () => { + urls = stubFetch({ + projects: [ + { + path: "/home/user/wt-91", + name: "wt-91", + is_worktree: true, + base_repo: "/home/user/maestro", + last_activity: "2026-08-20T10:00:00Z", + }, + ], + }); + handle = createSpawnForm({ onSuccess() {} }); + document.body.appendChild(handle.el); + for (let i = 0; i < 40; i++) await Promise.resolve(); + + handle.el + .querySelectorAll(".project-row") + .forEach((row) => row.dataset.cwd === "/home/user/wt-91" && row.click()); + await drain(); + + expect(handle.el.querySelector(".wt-summary")!.textContent).toMatch(/no worktree/i); + // Still collapsed — picking a project must not force it open. + expect(handle.el.querySelector(".wt-toggle")!.getAttribute("aria-expanded")).toBe( + "false", + ); + }); + + it("expanding the advanced section does not reorder the fields above it (#93)", async () => { + const form = await mount(); + const before = [...form.querySelectorAll(".field")]; + + form.querySelector(".wt-toggle")!.click(); + + expect(form.querySelector(".wt-toggle")!.getAttribute("aria-expanded")).toBe( + "true", + ); + expect(form.querySelector(".wt-collapsible")!.hidden).toBe(false); + expect([...form.querySelectorAll(".field")]).toStrictEqual(before); + }); + it("does not repopulate a destroyed form when the branch list lands late", async () => { const gate: { release?: () => void } = {}; urls = stubFetch({ gateBranches: (r) => (gate.release = r) }); diff --git a/web/src/styles.css b/web/src/styles.css index afe24c3..5aef4a8 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1286,18 +1286,33 @@ textarea { .worktree { border: 1px solid var(--border); border-radius: var(--r-2); - padding: var(--sp-3); + padding: 0 var(--sp-3); margin: 0; display: flex; flex-direction: column; - gap: var(--sp-3); background: color-mix(in oklab, var(--surface-2) 50%, transparent); } -.worktree legend { - padding: 0 6px; - color: var(--ink); - font-size: var(--fs-13); +.worktree .wt-toggle { + margin: 0 calc(var(--sp-3) * -1); + padding-left: var(--sp-3); + width: calc(100% + var(--sp-3) * 2); +} + +.wt-summary { + color: var(--muted); + font-size: var(--fs-12); +} + +.wt-collapsible { + display: flex; + flex-direction: column; + gap: var(--sp-3); + padding-bottom: var(--sp-3); +} + +.wt-collapsible[hidden] { + display: none; } .form-error { From 6bb812f6bdd5155738eea507c3496bacc2162c94 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 15:20:35 +0100 Subject: [PATCH 14/20] feat(maestro): collapse the TUI draft's worktree section by default (#93) Mirrors the web spawn form: worktree/branch controls sit behind a new "Advanced options" disclosure line (bracket [+]/[-], toggled by Enter or Space, closed by default) instead of always showing the checkbox and branch rows. The header states whether a worktree will be created so #91's off-by-default for a worktree row is visible without expanding. Collapsed fields drop out of Tab order; expanding only adds rows below the disclosure, never moving the ones above it. --- crates/maestro-tui/src/app.rs | 4 +- crates/maestro-tui/src/draft.rs | 217 +++++++++++++++++++++++++++----- 2 files changed, 185 insertions(+), 36 deletions(-) diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index 1894082..3dc11af 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -1349,9 +1349,7 @@ mod tests { for c in "crimson-otter".chars() { app.handle_draft_input(&InputKey::char(c)); } - app.handle_draft_input(&InputKey::Tab); // → worktree toggle - app.handle_draft_input(&InputKey::Tab); // → branch mode - app.handle_draft_input(&InputKey::Tab); // → new-branch name + app.handle_draft_input(&InputKey::Tab); // → advanced disclosure (collapsed, #93) app.handle_draft_input(&InputKey::Tab); // → submit match app.handle_draft_input(&InputKey::Enter).as_slice() { diff --git a/crates/maestro-tui/src/draft.rs b/crates/maestro-tui/src/draft.rs index 0224662..34f7d66 100644 --- a/crates/maestro-tui/src/draft.rs +++ b/crates/maestro-tui/src/draft.rs @@ -57,6 +57,9 @@ pub enum DraftField { /// Free-text path, revealed by picking `Other directory…`. CwdRaw, Name, + /// Disclosure collapsing the worktree/branch controls below (#93). + /// Closed by default; Enter / Space toggles. + Advanced, /// Worktree on/off for this session (#65). Enter / Space toggles. WorktreeToggle, /// New-vs-existing branch source; Left / Right switches. @@ -148,6 +151,10 @@ pub struct DraftPane { /// Worktree on for this spawn (#65). Defaults on; a failed gitroot probe /// unticks it, like the web form. pub worktree_enabled: bool, + /// Whether the worktree/branch controls are collapsed behind the + /// "Advanced options" disclosure (#93). Closed by default — most spawns + /// take the default target. + pub advanced_collapsed: bool, /// Enclosing repository of the chosen target — from the project scan for /// picked rows, `/api/gitroot` for raw paths. `None` until known. pub base_repo: Option, @@ -204,6 +211,7 @@ impl Default for DraftPane { name_touched: false, needs_name_refill: false, worktree_enabled: true, + advanced_collapsed: true, base_repo: None, probed_cwd: None, probe_seq: 0, @@ -442,6 +450,21 @@ impl DraftPane { self.set_branch_mode(next); } + /// Enter / Space on the "Advanced options" header (#93). + pub fn toggle_advanced(&mut self) { + self.advanced_collapsed = !self.advanced_collapsed; + } + + /// What the collapsed header says about this spawn's default, so it stays + /// visible without expanding (#93). + fn advanced_summary(&self) -> &'static str { + if self.worktree_enabled { + "creates a worktree" + } else { + "no worktree" + } + } + /// Enter / Space on the toggle. Re-enabling reuses a still-valid repo /// answer instead of re-probing. pub fn toggle_worktree(&mut self) { @@ -602,13 +625,16 @@ impl DraftPane { order.push(DraftField::CwdRaw); } order.push(DraftField::Name); - order.push(DraftField::WorktreeToggle); - if self.worktree_body_visible() { - order.push(DraftField::BranchMode); - order.push(match self.branch_mode { - WorktreeBranchMode::New => DraftField::BranchNew, - WorktreeBranchMode::Existing => DraftField::BranchExisting, - }); + order.push(DraftField::Advanced); + if !self.advanced_collapsed { + order.push(DraftField::WorktreeToggle); + if self.worktree_body_visible() { + order.push(DraftField::BranchMode); + order.push(match self.branch_mode { + WorktreeBranchMode::New => DraftField::BranchNew, + WorktreeBranchMode::Existing => DraftField::BranchExisting, + }); + } } order.extend([DraftField::Submit]); order @@ -733,6 +759,10 @@ impl DraftPane { self.select_entry(); DraftInputResult::Consumed } + DraftField::Advanced => { + self.toggle_advanced(); + DraftInputResult::Consumed + } DraftField::WorktreeToggle => { self.toggle_worktree(); DraftInputResult::Consumed @@ -744,6 +774,14 @@ impl DraftPane { DraftInputResult::Consumed } }, + InputKey::Char { + c: ' ', + ctrl: false, + .. + } if self.focused_field == DraftField::Advanced => { + self.toggle_advanced(); + DraftInputResult::Consumed + } InputKey::Char { c: ' ', ctrl: false, @@ -832,13 +870,18 @@ impl DraftPane { let show_custom = self.is_custom(); let show_raw = self.cwd_raw_visible; - let show_wt_body = self.worktree_body_visible(); + // The worktree toggle and its body stay behind the "Advanced options" + // disclosure (#93): collapsed hides both, regardless of what they'd + // otherwise show. + let show_wt_toggle = !self.advanced_collapsed; + let show_wt_body = show_wt_toggle && self.worktree_body_visible(); // Up to three branch rows stay visible around the selection; held // branches need their reason on screen, so they aren't collapsed. let branch_rows = self.branches.len().clamp(1, 3) as u16; - let fixed: u16 = 3 /* tiles + dir + name */ + 1 /* worktree */ + 1 /* spawn */ + let fixed: u16 = 3 /* tiles + dir + name */ + 1 /* advanced header */ + 1 /* spawn */ + u16::from(show_custom) + u16::from(show_raw) + + u16::from(show_wt_toggle) + u16::from(show_wt_body) * (1 + if self.branch_mode == WorktreeBranchMode::New { 1 } else { branch_rows }) + u16::from(self.error.is_some()); let list_capacity = (inner.height.saturating_sub(fixed)).max(1) as usize; @@ -853,7 +896,10 @@ impl DraftPane { constraints.push(Constraint::Length(1)); } constraints.push(Constraint::Length(1)); // name - constraints.push(Constraint::Length(1)); // worktree toggle + constraints.push(Constraint::Length(1)); // advanced header + if show_wt_toggle { + constraints.push(Constraint::Length(1)); // worktree toggle + } if show_wt_body { constraints.push(Constraint::Length(1)); // branch mode match self.branch_mode { @@ -1030,33 +1076,55 @@ impl DraftPane { }, ); + // -- advanced disclosure (#93) ------------------------------------------ + let advanced_row = next_row(); + f.render_widget( + Paragraph::new(Line::from(vec![ + Span::raw(format!("{: