From ca4f7cb1d57a448c1b693d3a9c2a7cb114d39a83 Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Sun, 23 Aug 2026 21:34:35 +0100 Subject: [PATCH] feat(maestro): worktree on by default with new-or-existing branch picker (#61) - toggle defaults on for any target the project scan reports (repo or worktree); only the free-text escape hatch pays for /api/gitroot - branch mode control switches between creating a new branch (named after the session name, as before) and picking an existing one from GET /api/branches; branches held by another worktree stay visible but disabled, labelled with that worktree's path - emits create_branch: false on the wire for existing-mode picks - branch-list load follows the form's ready/destroyed discipline so a late response cannot corrupt a rebuilt draft --- web/src/api.ts | 8 ++ web/src/routes.ts | 1 + web/src/spawnForm.test.ts | 42 ++++++- web/src/spawnForm.ts | 190 +++++++++++++++++++++++++----- web/src/spawnFormWorktree.test.ts | 172 +++++++++++++++++++++++++++ web/src/styles.css | 10 ++ web/src/types.ts | 17 +++ 7 files changed, 412 insertions(+), 28 deletions(-) create mode 100644 web/src/spawnFormWorktree.test.ts diff --git a/web/src/api.ts b/web/src/api.ts index d3867e9..89cec3f 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -1,6 +1,7 @@ import * as routes from "./routes.js"; import type { ApiError, + Branches, CreateSessionRequest, DaemonInfo, DirListing, @@ -89,4 +90,11 @@ export async function fetchNames(count: number): Promise { return jsonOrError(await fetch(url, { credentials: "same-origin" })); } +/** One repository's local branches, with worktree occupancy (#55). Empty for + * a path outside any repo. Drives the spawn form's existing-branch picker. */ +export async function fetchBranches(path: string): Promise { + const url = routes.withPathQuery(routes.BRANCHES, path); + return jsonOrError(await fetch(url, { credentials: "same-origin" })); +} + export { eventsWsUrl } from "./routes.js"; diff --git a/web/src/routes.ts b/web/src/routes.ts index c76bf8a..812160a 100644 --- a/web/src/routes.ts +++ b/web/src/routes.ts @@ -12,6 +12,7 @@ export const RECENT_DIRS = "/api/recent-dirs"; export const PROJECTS = "/api/projects"; export const GITROOT = "/api/gitroot"; export const NAMES = "/api/names"; +export const BRANCHES = "/api/branches"; export const EVENTS = "/api/events"; /** Name of the query parameter `DIRS` and `GITROOT` both take. */ diff --git a/web/src/spawnForm.test.ts b/web/src/spawnForm.test.ts index 4559526..3481628 100644 --- a/web/src/spawnForm.test.ts +++ b/web/src/spawnForm.test.ts @@ -8,6 +8,7 @@ const base: SpawnFormValues = { name: "amber-fox", worktreeEnabled: false, worktreeBranch: "", + worktreeBranchMode: "new", }; describe("buildSpawnRequest", () => { @@ -33,11 +34,50 @@ describe("buildSpawnRequest", () => { cwd: base.cwd, command: base.command, metadata: { name: "amber-fox" }, - worktree: { base_repo: "/home/user/project", branch: "feat/foo", cleanup_on_kill: true }, + worktree: { + base_repo: "/home/user/project", + branch: "feat/foo", + cleanup_on_kill: true, + create_branch: true, + }, }, }); }); + it("checks out an existing branch when the mode is existing (#61)", () => { + const res = buildSpawnRequest( + { + ...base, + worktreeEnabled: true, + worktreeBranch: "spawn-v2", + worktreeBranchMode: "existing", + }, + "/home/user/project", + ); + expect(res).toEqual({ + req: { + cwd: base.cwd, + command: base.command, + metadata: { name: "amber-fox" }, + worktree: { + base_repo: "/home/user/project", + branch: "spawn-v2", + cleanup_on_kill: true, + create_branch: false, + }, + }, + }); + }); + + it("rejects an existing-mode worktree with no branch picked", () => { + expect( + buildSpawnRequest( + { ...base, worktreeEnabled: true, worktreeBranch: "", worktreeBranchMode: "existing" }, + "/repo", + ), + ).toHaveProperty("error"); + }); + it("rejects a missing working directory", () => { expect(buildSpawnRequest({ ...base, cwd: "" }, null)).toHaveProperty("error"); }); diff --git a/web/src/spawnForm.ts b/web/src/spawnForm.ts index 3351949..2c11e34 100644 --- a/web/src/spawnForm.ts +++ b/web/src/spawnForm.ts @@ -1,10 +1,21 @@ import { escapeHtml } from "./html.js"; -import { createSession, fetchNames, getDaemonInfo, projects as fetchProjects, gitRoot } from "./api.js"; +import { + createSession, + fetchBranches, + fetchNames, + getDaemonInfo, + projects as fetchProjects, + gitRoot, +} from "./api.js"; import { attachDirAutocomplete } from "./dirAutocomplete.js"; import { filterProjects, groupProjects, isStale, sortProjects } from "./projectList.js"; -import type { CreateSessionRequest, Project, Session, SpawnPreset } from "./types.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). */ +export type WorktreeBranchMode = "new" | "existing"; + /** Seed values for the spawn form — used by "clone" to reopen with the same * cwd, command, and name as an existing session, and by draft panes to restore * their own half-filled state after a re-render. */ @@ -14,6 +25,7 @@ export interface SpawnFormSeed { name?: string; worktreeEnabled?: boolean; worktreeBranch?: string; + worktreeBranchMode?: WorktreeBranchMode; cwdRawVisible?: boolean; } @@ -25,6 +37,8 @@ export interface SpawnFormValues { name: string; worktreeEnabled: boolean; worktreeBranch: string; + /** "new" creates the branch; "existing" checks out a picked one (#61). */ + worktreeBranchMode: WorktreeBranchMode; /** Whether the cwd lives in the raw text input (vs a picked recent row). */ cwdRawVisible?: boolean; } @@ -47,15 +61,21 @@ export function buildSpawnRequest( }; if (values.worktreeEnabled) { + const mode = values.worktreeBranchMode ?? "new"; const branch = values.worktreeBranch.trim(); if (!baseRepo) return { error: "Worktree needs a directory inside a git repository." }; - if (!branch) return { error: "Worktree requires a branch name." }; + if (!branch) { + return { error: mode === "existing" ? "Pick an existing branch." : "Worktree requires a branch name." }; + } req.worktree = { base_repo: baseRepo, branch, // The daemon always cleans up, guarded against unsaved work (#56); // the wire field stays for older daemons. cleanup_on_kill: true, + // Existing-mode picks must check out as-is; the daemon creates + // anything else with `worktree add -b` (#56/#61). + create_branch: mode !== "existing", }; } @@ -128,10 +148,24 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { @@ -153,7 +187,13 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { const wtHint = form.querySelector(".wt-hint") as HTMLElement; const wtBody = form.querySelector(".wt-body") as HTMLElement; const wtRepo = form.querySelector(".wt-repo") as HTMLElement; + const modeRadios = [ + ...form.querySelectorAll("input[name=worktree_branch_mode]"), + ]; + const newWrap = form.querySelector(".wt-new-wrap") as HTMLElement; + const existWrap = form.querySelector(".wt-exist-wrap") as HTMLElement; const branchInput = form.querySelector("input[name=worktree_branch]") as HTMLInputElement; + const branchSelect = form.querySelector(".branch-select") as HTMLSelectElement; const nameInput = form.querySelector("input[name=name]") as HTMLInputElement; // ---- form state ------------------------------------------------------ @@ -162,11 +202,15 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { let selectedPresetId = ""; let command = ""; // resolved command (preset's, or the raw custom input) let cwd = ""; // chosen working directory - let inferredBaseRepo: string | null = null; // from the gitroot probe + let inferredBaseRepo: string | null = null; // from the gitroot probe or the project scan let probeToken = 0; // guards async gitroot probes against races // A seed's cwd, applied once the (async) projects load for the initial host // settles — so the fetch can't race past and clobber the seeded directory. let seedCwd: string | null = prefill.cwd ?? null; + let branchMode: WorktreeBranchMode = prefill.worktreeBranchMode ?? "new"; + let branchToken = 0; // guards async branch-list loads against races + // Branch list for the current cwd — refetched whenever the target changes. + let branchCache: { cwd: string; branches: BranchInfo[] } | null = null; // `emit` stays silent until the initial load settles and goes silent again // once the form is destroyed. Construction runs with `cwd === ""` and the @@ -176,6 +220,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { // clobber the live one's leaf (#57 review). let ready = false; let destroyed = false; + const branchValue = () => (branchMode === "existing" ? branchSelect.value : branchInput.value); const emit = () => { if (!ready || destroyed) return; opts.onChange?.({ @@ -183,7 +228,8 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { cwd, name: nameInput.value, worktreeEnabled: wtEnabled.checked, - worktreeBranch: branchInput.value, + worktreeBranch: branchValue(), + worktreeBranchMode: branchMode, cwdRawVisible: !cwdRawWrap.hidden, }); }; @@ -191,10 +237,10 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { // ---- name (also seeds the worktree branch) --------------------------- // The worktree branch defaults to the session name — including a clone's // prefilled name — and tracks it on reroll/edit until the user types their - // own branch. + // own branch. Only in "new" mode: an existing-branch pick is not a name. let branchEdited = Boolean(prefill.worktreeBranch); const syncBranchToName = () => { - if (!branchEdited) branchInput.value = nameInput.value; + if (!branchEdited && branchMode === "new") branchInput.value = nameInput.value; }; // Handles come from `GET /api/names` in batches (#53): the first one seeds // the field, rerolls consume the rest locally so they stay instant, and a @@ -357,6 +403,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { cwdRaw.addEventListener("input", () => { cwd = cwdRaw.value.trim(); + branchCache = null; emit(); if (wtEnabled.checked) probeGitRoot(); }); @@ -374,11 +421,14 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { cwd = project.path; 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); 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 !== ""); if (match) selectPreset(match.id); - if (wtEnabled.checked) probeGitRoot(); } let discovered: Project[] = []; // daemon snapshot, daemon-sorted @@ -432,7 +482,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { } // Everything seeded: the form may publish from here on. ready = true; - if (wtEnabled.checked) void probeGitRoot(); + resolveBaseRepo(); emit(); } @@ -473,7 +523,37 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { return p; } - // ---- worktree (branch-only; base_repo inferred via /gitroot) ---------- + // ---- 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 + // pays for a /api/gitroot probe. + function applyProjectRepo(repo: string): void { + inferredBaseRepo = repo; + branchCache = null; + wtHint.hidden = true; + wtBody.hidden = false; + wtRepo.textContent = `repo: ${repo}`; + if (branchMode === "existing") void loadBranches(); + } + + /** Decide how the worktree section describes the current target. Called + * once the initial load has settled `cwd`. */ + function resolveBaseRepo(): void { + if (!cwd) { + inferredBaseRepo = null; + wtBody.hidden = true; + wtHint.hidden = false; + wtHint.textContent = "Choose a working directory first."; + return; + } + const project = discovered.find((p) => p.path === cwd); + if (project) { + applyProjectRepo(project.base_repo ?? project.path); + return; + } + void probeGitRoot(); + } + async function probeGitRoot(): Promise { const token = ++probeToken; if (!cwd) { @@ -494,10 +574,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { } if (token !== probeToken) return; // a newer probe superseded us if (root) { - inferredBaseRepo = root; - wtHint.hidden = true; - wtBody.hidden = false; - wtRepo.textContent = `repo: ${root} (inferred)`; + applyProjectRepo(root); } else { inferredBaseRepo = null; // Publish the untick too, or the next render seeds the box checked with @@ -510,10 +587,72 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { } } + // ---- existing-branch picker (#61) ------------------------------------- + function setBranchMode(mode: WorktreeBranchMode): void { + branchMode = mode; + for (const radio of modeRadios) radio.checked = radio.value === mode; + newWrap.hidden = mode !== "new"; + existWrap.hidden = mode !== "existing"; + if (mode === "existing") void loadBranches(); + emit(); + } + + /** Fetch the target repo's branches once per cwd. Like every async load in + * this form: token-guarded, and nothing touches the DOM (or emits) after + * destroy() — a late response into a rebuilt form would corrupt that + * form's draft (#57 review). */ + async function loadBranches(): Promise { + if (!cwd || !inferredBaseRepo) return; + const token = ++branchToken; + if (branchCache?.cwd !== cwd) { + const target = cwd; + let branches: BranchInfo[]; + try { + branches = await fetchBranches(target); + } catch { + return; // picker just stays empty; "new" mode still works + } + if (destroyed || token !== branchToken || cwd !== target) return; + branchCache = { cwd: target, branches }; + } + renderBranchOptions(); + } + + function renderBranchOptions(): void { + if (destroyed) return; + const branches = branchCache?.cwd === cwd ? branchCache.branches : []; + branchSelect.replaceChildren(); + const opt = (label: string, value: string) => { + const o = document.createElement("option"); + o.textContent = label; + o.value = value; + return o; + }; + if (branches.length === 0) { + branchSelect.appendChild(opt("No local branches found.", "")); + return; + } + for (const b of branches) { + // A branch another worktree holds stays visible but unpickable — git + // would refuse to check it out twice; the path says who holds it. + const o = opt(b.worktree ? `${b.name} — held by ${b.worktree}` : b.name, b.name); + o.disabled = Boolean(b.worktree); + o.title = b.worktree ?? ""; + branchSelect.appendChild(o); + } + } + + for (const radio of modeRadios) { + radio.addEventListener("change", () => { + if (radio.checked) setBranchMode(radio.value as WorktreeBranchMode); + }); + } + branchSelect.addEventListener("change", () => emit()); + wtEnabled.addEventListener("change", () => { emit(); if (wtEnabled.checked) { - probeGitRoot(); + resolveBaseRepo(); } else { inferredBaseRepo = null; wtBody.hidden = true; @@ -521,15 +660,11 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { } }); - // A seeded worktree restores its checkbox/body up front; its gitroot probe - // waits for loadRecents to apply the seeded cwd. - if (prefill.worktreeEnabled || prefill.worktreeBranch) { - wtEnabled.checked = Boolean(prefill.worktreeEnabled); - if (prefill.worktreeBranch) branchInput.value = prefill.worktreeBranch; - wtBody.hidden = !wtEnabled.checked; - wtHint.hidden = true; - emit(); - } + // 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; + if (prefill.worktreeBranch) branchInput.value = prefill.worktreeBranch; + setBranchMode(branchMode); // Seed ordering: the raw-cwd application races the async info load (which // provides homeDir for the projects' abbreviations), so projects wait for it. @@ -548,8 +683,9 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle { cwd, name: nameInput.value, worktreeEnabled: wtEnabled.checked, - worktreeBranch: branchInput.value, - }, + worktreeBranch: branchValue(), + worktreeBranchMode: branchMode, + }, inferredBaseRepo, ); if ("error" in built) { diff --git a/web/src/spawnFormWorktree.test.ts b/web/src/spawnFormWorktree.test.ts new file mode 100644 index 0000000..76fd12c --- /dev/null +++ b/web/src/spawnFormWorktree.test.ts @@ -0,0 +1,172 @@ +// @vitest-environment happy-dom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { createSpawnForm, type SpawnFormHandle, type SpawnFormValues } from "./spawnForm.js"; + +const PROJECTS = [ + { path: "/home/user/maestro", name: "maestro", last_activity: "2026-08-20T10:00:00Z" }, +]; + +const BRANCHES = [ + { name: "main", is_head: true }, + { name: "spawn-v2", is_head: false, worktree: "/home/user/wt-spawn" }, + { name: "free-branch", is_head: false }, +]; + +/** Stub the fetches the form makes. `gateBranches` (when given) holds the + * /api/branches response until released, so tests can destroy the form while + * its branch list is still in flight. */ +function stubFetch(opts?: { + gateBranches?: (release: () => void) => void; +}): string[] { + const urls: string[] = []; + const gated = new Promise((resolve) => { + if (opts?.gateBranches) opts.gateBranches(resolve); + else resolve(); + }); + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + urls.push(url); + if (url.includes("/api/names")) { + return new Response(JSON.stringify({ names: ["amber-fox"] }), { status: 200 }); + } + if (url.includes("/api/branches")) { + await gated; + 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({ hostname: "h", version: "0", home_dir: "/home/user", presets: [] }), + { status: 200 }, + ); + }), + ); + return urls; +} + +describe("spawnForm worktree defaults and branch picker (#61)", () => { + let handle: SpawnFormHandle | null = null; + let urls: string[]; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + handle?.destroy(); + handle = null; + vi.unstubAllGlobals(); + vi.useRealTimers(); + }); + + async function mount( + seed?: Parameters[0]["seed"], + onChange?: (values: SpawnFormValues) => void, + ) { + urls = stubFetch(); + handle = createSpawnForm({ seed, onSuccess() {}, onChange }); + document.body.appendChild(handle.el); + for (let i = 0; i < 40; i++) await Promise.resolve(); + return handle.el; + } + + const drain = async () => { + for (let i = 0; i < 40; i++) await Promise.resolve(); + }; + + function pickProject(form: HTMLFormElement): void { + form.querySelector(".project-row.project-other")!.click(); + form + .querySelectorAll(".project-row") + .forEach((row) => row.dataset.cwd === "/home/user/maestro" && row.click()); + } + + it("defaults the worktree toggle on for a picked project with no gitroot probe", async () => { + const form = await mount(); + expect(form.querySelector("input[name=worktree_enabled]")!.checked).toBe( + true, + ); + + pickProject(form); + await drain(); + + expect(urls.some((u) => u.includes("/api/gitroot"))).toBe(false); + expect(form.querySelector(".wt-repo")!.textContent).toContain( + "/home/user/maestro", + ); + }); + + 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. + 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 drain(); + + expect(urls.some((u) => u.includes("/api/gitroot"))).toBe(true); + }); + + it("populates existing branches, disabling ones held by another worktree", async () => { + const form = await mount(); + pickProject(form); + + const radio = form.querySelector( + 'input[name=worktree_branch_mode][value="existing"]', + )!; + radio.click(); + await drain(); + + const select = form.querySelector(".branch-select")!; + expect(urls.some((u) => u.includes("/api/branches?path="))).toBe(true); + const options = [...select.options]; + expect(options.map((o) => o.value)).toEqual(["main", "spawn-v2", "free-branch"]); + const held = options.find((o) => o.value === "spawn-v2")!; + expect(held.disabled).toBe(true); + expect(held.textContent).toContain("/home/user/wt-spawn"); + expect(options.find((o) => o.value === "free-branch")!.disabled).toBe(false); + }); + + it("emits the branch mode as draft state when it changes", async () => { + const changes: SpawnFormValues[] = []; + const form = await mount(undefined, (v) => changes.push(v)); + await drain(); + + const radio = form.querySelector( + 'input[name=worktree_branch_mode][value="existing"]', + )!; + radio.click(); + await drain(); + + const modes = changes.map((c) => (c as { worktreeBranchMode?: string }).worktreeBranchMode); + expect(modes).toContain("new"); + expect(modes.at(-1)).toBe("existing"); + }); + + 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) }); + handle = createSpawnForm({ onSuccess() {} }); + document.body.appendChild(handle.el); + for (let i = 0; i < 40; i++) await Promise.resolve(); + + pickProject(handle.el); + handle.el + .querySelector('input[name=worktree_branch_mode][value="existing"]')! + .click(); + + handle.destroy(); + gate.release?.(); + await drain(); + + expect( + handle.el.querySelector(".branch-select")!.options.length, + ).toBe(0); + }); +}); diff --git a/web/src/styles.css b/web/src/styles.css index 8b6c38e..c1800ce 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1575,6 +1575,16 @@ textarea { color: var(--muted); } +/* New-vs-existing branch source (#61). */ +.wt-mode { + display: flex; + gap: var(--sp-4); +} + +.branch-select { + width: 100%; +} + /* Generated-name field with a reroll button. */ .name-row { display: flex; diff --git a/web/src/types.ts b/web/src/types.ts index 844a02a..11991c3 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -114,8 +114,25 @@ export interface WorktreeSpec { base_repo: string; branch: string; cleanup_on_kill?: boolean; + /** When false, `branch` already exists and is checked out as-is (#61). + * Defaults to true (create a new branch) on older daemons. */ + create_branch?: boolean; } +/** One local branch of the enclosing repository, from `GET /api/branches` + * (#55). Mirrors `maestro_protocol::BranchInfo`. */ +export interface BranchInfo { + /** Short branch name (`main`, `feature/foo`). */ + name: string; + /** Whether this branch is the main checkout's current HEAD. */ + is_head: boolean; + /** Toplevel path of the linked worktree holding this branch, if any. */ + worktree?: string | null; +} + +/** `GET /api/branches?path=` — the local branches of one repository (#55). */ +export type Branches = BranchInfo[]; + export interface CreateSessionRequest { cwd: string; command: string;