From 16609eff9e94c5bdaad9d14fec4c993ec0eef64e Mon Sep 17 00:00:00 2001 From: Miguel Palhas Date: Mon, 24 Aug 2026 14:59:56 +0100 Subject: [PATCH] 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);