fix(maestro): second-review findings across daemon, tui and web
Ten findings from reviewing the accumulated blitz/spawn-v2 diff. The two that could wedge a repo: - The cleanup guard refused any worktree whose commits were on no other ref, which is every ordinary unpushed branch. Paths are deterministic now, so the leftover tree blocked the next spawn and each branch became a one-shot. `worktree remove` keeps refs/heads/<branch>, so committed work is never at risk: guard on an unclean working tree alone. - A worktree whose directory had vanished failed the git probe and was reported as unsaved work, stranding the registration forever. Prune it instead, and prune before `worktree add` tests the target path. Also: - The CLI hardcoded create_branch: true, so the daemon's existing-branch path was unreachable. Added --existing-branch. - Pastes were forwarded without their DECSET 2004 markers, so a remote shell ran a multi-line paste line by line. - spawnForm emitted before its seed was restored and after destroy(), so a rebuilt draft blanked its cwd and a dead form clobbered the live one. - showRawCwd and the probe's worktree untick never reached the leaf. - A late autocomplete fetch reopened the popup under a blurred input. - The "remove worktree when killed" checkbox promised something the daemon no longer honours; removed.
This commit is contained in:
+23
-19
@@ -14,7 +14,6 @@ export interface SpawnFormSeed {
|
||||
name?: string;
|
||||
worktreeEnabled?: boolean;
|
||||
worktreeBranch?: string;
|
||||
worktreeCleanup?: boolean;
|
||||
cwdRawVisible?: boolean;
|
||||
}
|
||||
|
||||
@@ -26,7 +25,6 @@ export interface SpawnFormValues {
|
||||
name: string;
|
||||
worktreeEnabled: boolean;
|
||||
worktreeBranch: string;
|
||||
worktreeCleanup: boolean;
|
||||
/** Whether the cwd lives in the raw text input (vs a picked recent row). */
|
||||
cwdRawVisible?: boolean;
|
||||
}
|
||||
@@ -55,7 +53,9 @@ export function buildSpawnRequest(
|
||||
req.worktree = {
|
||||
base_repo: baseRepo,
|
||||
branch,
|
||||
cleanup_on_kill: values.worktreeCleanup,
|
||||
// The daemon always cleans up, guarded against unsaved work (#56);
|
||||
// the wire field stays for older daemons.
|
||||
cleanup_on_kill: true,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -130,10 +130,6 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
Branch
|
||||
<input name="worktree_branch" type="text" placeholder="feat/foo" autocomplete="off" />
|
||||
</label>
|
||||
<label class="row-inline">
|
||||
<input type="checkbox" name="worktree_cleanup_on_kill" />
|
||||
Remove worktree when session is killed
|
||||
</label>
|
||||
</div>
|
||||
</fieldset>
|
||||
<p class="form-error" role="alert" aria-live="polite"></p>
|
||||
@@ -155,9 +151,6 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
const wtBody = form.querySelector(".wt-body") as HTMLElement;
|
||||
const wtRepo = form.querySelector(".wt-repo") as HTMLElement;
|
||||
const branchInput = form.querySelector("input[name=worktree_branch]") as HTMLInputElement;
|
||||
const cleanupInput = form.querySelector(
|
||||
"input[name=worktree_cleanup_on_kill]",
|
||||
) as HTMLInputElement;
|
||||
const nameInput = form.querySelector("input[name=name]") as HTMLInputElement;
|
||||
|
||||
// ---- form state ------------------------------------------------------
|
||||
@@ -172,14 +165,22 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
// settles — so the fetch can't race past and clobber the seeded directory.
|
||||
let seedCwd: string | null = prefill.cwd ?? null;
|
||||
|
||||
// `emit` stays silent until the initial load settles and goes silent again
|
||||
// once the form is destroyed. Construction runs with `cwd === ""` and the
|
||||
// seed is only restored two fetches later, so emitting in between would
|
||||
// blank the leaf's draft; and `multiview.render()` rebuilds forms while the
|
||||
// old one's fetches are still in flight, so a destroyed form emitting would
|
||||
// clobber the live one's leaf (#57 review).
|
||||
let ready = false;
|
||||
let destroyed = false;
|
||||
const emit = () => {
|
||||
if (!ready || destroyed) return;
|
||||
opts.onChange?.({
|
||||
command,
|
||||
cwd,
|
||||
name: nameInput.value,
|
||||
worktreeEnabled: wtEnabled.checked,
|
||||
worktreeBranch: branchInput.value,
|
||||
worktreeCleanup: cleanupInput.checked,
|
||||
cwdRawVisible: !cwdRawWrap.hidden,
|
||||
});
|
||||
};
|
||||
@@ -296,6 +297,9 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
cwdRaw.focus();
|
||||
cwdRaw.select();
|
||||
}
|
||||
// Both the visibility and the directory are draft state: without this a
|
||||
// render before the operator types snaps back to the recents picker.
|
||||
emit();
|
||||
}
|
||||
|
||||
cwdRaw.addEventListener("input", () => {
|
||||
@@ -357,10 +361,9 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
} else {
|
||||
cwdRawWrap.hidden = true;
|
||||
}
|
||||
// The seeded cwd is in place — a restored worktree can probe now.
|
||||
// Everything seeded: the form may publish from here on.
|
||||
ready = true;
|
||||
if (wtEnabled.checked) void probeGitRoot();
|
||||
// Publish it: construction ran `emit()` with an empty cwd, so without this
|
||||
// the leaf's draft keeps `cwd: ""` and the next render reseeds it blank.
|
||||
emit();
|
||||
}
|
||||
|
||||
@@ -423,7 +426,10 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
wtRepo.textContent = `repo: ${root} (inferred)`;
|
||||
} else {
|
||||
inferredBaseRepo = null;
|
||||
// Publish the untick too, or the next render seeds the box checked with
|
||||
// no inferred repo and submitting fails.
|
||||
wtEnabled.checked = false;
|
||||
emit();
|
||||
wtBody.hidden = true;
|
||||
wtHint.hidden = false;
|
||||
wtHint.textContent = "Not inside a git repository — pick a dir in a repo to use a worktree.";
|
||||
@@ -440,14 +446,12 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
wtHint.hidden = true;
|
||||
}
|
||||
});
|
||||
cleanupInput.addEventListener("change", emit);
|
||||
|
||||
// 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 || prefill.worktreeCleanup) {
|
||||
if (prefill.worktreeEnabled || prefill.worktreeBranch) {
|
||||
wtEnabled.checked = Boolean(prefill.worktreeEnabled);
|
||||
if (prefill.worktreeBranch) branchInput.value = prefill.worktreeBranch;
|
||||
cleanupInput.checked = Boolean(prefill.worktreeCleanup);
|
||||
wtBody.hidden = !wtEnabled.checked;
|
||||
wtHint.hidden = true;
|
||||
emit();
|
||||
@@ -471,8 +475,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
name: nameInput.value,
|
||||
worktreeEnabled: wtEnabled.checked,
|
||||
worktreeBranch: branchInput.value,
|
||||
worktreeCleanup: cleanupInput.checked,
|
||||
},
|
||||
},
|
||||
inferredBaseRepo,
|
||||
);
|
||||
if ("error" in built) {
|
||||
@@ -494,6 +497,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
|
||||
return {
|
||||
el: form,
|
||||
destroy() {
|
||||
destroyed = true;
|
||||
autocomplete.destroy();
|
||||
form.remove();
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user