Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion apps/web/src/components/app/create-agent-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ function CreateAgentDialogContent({
<WorktreeSection
cwd={form.createCwd}
worktreeAvailable={form.worktreeAvailable}
worktreeChecked={form.worktreeChecked}
useWorktree={form.createUseWorktree}
onUseWorktreeChange={form.setCreateUseWorktree}
baseBranch={form.createBaseBranch}
Expand Down
86 changes: 86 additions & 0 deletions apps/web/src/components/app/create-agent-worktree-section.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// @vitest-environment jsdom
import { cleanup, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";

import { WorktreeSection } from "./create-agent-worktree-section";

const defaultProps = {
cwd: "/repo/app",
worktreeAvailable: true,
useWorktree: true,
onUseWorktreeChange: vi.fn(),
baseBranch: "main",
onBaseBranchChange: vi.fn(),
worktreeBranch: "feature/saved",
onWorktreeBranchChange: vi.fn(),
createNewBranch: true,
onCreateNewBranchChange: vi.fn(),
};

afterEach(cleanup);

function checkbox(testId: string): HTMLButtonElement {
return screen.getByTestId(testId) as HTMLButtonElement;
}

describe("WorktreeSection", () => {
it("renders saved true values unchecked and disabled outside a git repo", () => {
render(<WorktreeSection {...defaultProps} worktreeAvailable={false} />);

const worktree = checkbox("create-agent-worktree");
const newBranch = checkbox("create-agent-new-branch");

expect(worktree.getAttribute("aria-checked")).toBe("false");
expect(worktree.disabled).toBe(true);
expect(newBranch.getAttribute("aria-checked")).toBe("false");
expect(newBranch.disabled).toBe(true);
expect(
(screen.getByTestId("create-agent-base-branch") as HTMLButtonElement)
.disabled
).toBe(true);
expect(
(screen.getByTestId("create-agent-worktree-branch") as HTMLInputElement)
.disabled
).toBe(true);
});

it("uses saved values when the cwd is a git repo", () => {
render(<WorktreeSection {...defaultProps} />);

const worktree = checkbox("create-agent-worktree");
const newBranch = checkbox("create-agent-new-branch");

expect(worktree.getAttribute("aria-checked")).toBe("true");
expect(worktree.disabled).toBe(false);
expect(newBranch.getAttribute("aria-checked")).toBe("true");
expect(newBranch.disabled).toBe(false);
expect(
(screen.getByTestId("create-agent-base-branch") as HTMLButtonElement)
.disabled
).toBe(false);
expect(
(screen.getByTestId("create-agent-worktree-branch") as HTMLInputElement)
.disabled
).toBe(false);
});

it("renders branch controls unchecked and disabled when worktrees are off", () => {
render(<WorktreeSection {...defaultProps} useWorktree={false} />);

const worktree = checkbox("create-agent-worktree");
const newBranch = checkbox("create-agent-new-branch");

expect(worktree.getAttribute("aria-checked")).toBe("false");
expect(worktree.disabled).toBe(false);
expect(newBranch.getAttribute("aria-checked")).toBe("false");
expect(newBranch.disabled).toBe(true);
expect(
(screen.getByTestId("create-agent-base-branch") as HTMLButtonElement)
.disabled
).toBe(true);
expect(
(screen.getByTestId("create-agent-worktree-branch") as HTMLInputElement)
.disabled
).toBe(true);
});
});
35 changes: 16 additions & 19 deletions apps/web/src/components/app/create-agent-worktree-section.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { cn } from "@/lib/utils";
type WorktreeSectionProps = {
cwd: string;
worktreeAvailable: boolean;
worktreeChecked: boolean;
useWorktree: boolean;
onUseWorktreeChange: (value: boolean) => void;
baseBranch: string;
Expand All @@ -22,7 +21,6 @@ type WorktreeSectionProps = {
export function WorktreeSection({
cwd,
worktreeAvailable,
worktreeChecked,
useWorktree,
onUseWorktreeChange,
baseBranch,
Expand All @@ -32,9 +30,9 @@ export function WorktreeSection({
createNewBranch,
onCreateNewBranchChange,
}: WorktreeSectionProps): JSX.Element {
const controlsDisabled = !worktreeAvailable || !worktreeChecked;
const branchOptionsEnabled = worktreeAvailable && worktreeChecked;
const newBranchChecked = branchOptionsEnabled && createNewBranch;
const worktreeChecked = worktreeAvailable && useWorktree;
const branchControlsEnabled = worktreeChecked;
const newBranchChecked = branchControlsEnabled && createNewBranch;

return (
<div
Expand All @@ -52,10 +50,7 @@ export function WorktreeSection({
>
<Checkbox
checked={worktreeChecked}
onCheckedChange={() => {
const nextUseWorktree = !useWorktree;
onUseWorktreeChange(nextUseWorktree);
}}
onCheckedChange={(checked) => onUseWorktreeChange(checked === true)}
disabled={!worktreeAvailable}
className="mt-0.5"
title={
Expand All @@ -78,10 +73,10 @@ export function WorktreeSection({
</span>
</label>
<div
aria-disabled={controlsDisabled}
aria-disabled={!branchControlsEnabled}
className={cn(
"grid transition-[grid-template-rows,opacity] duration-200 ease-out",
branchOptionsEnabled
branchControlsEnabled
? "grid-rows-[1fr] opacity-100"
: "grid-rows-[1fr] opacity-60"
)}
Expand All @@ -98,27 +93,29 @@ export function WorktreeSection({
baseBranchHelper="The branch to check out in the worktree."
showNewBranchInput={false}
testIdPrefix="create-agent"
disabled={controlsDisabled}
disabled={!branchControlsEnabled}
/>
<div
aria-disabled={controlsDisabled}
aria-disabled={!branchControlsEnabled}
className="space-y-2 rounded-md border border-border/60 bg-background/40 px-3 py-3"
>
<label
className={cn(
"flex items-start gap-3",
controlsDisabled ? "cursor-not-allowed" : "cursor-pointer"
branchControlsEnabled
? "cursor-pointer"
: "cursor-not-allowed"
)}
>
<Checkbox
checked={newBranchChecked}
onCheckedChange={() =>
onCreateNewBranchChange(!createNewBranch)
onCheckedChange={(checked) =>
onCreateNewBranchChange(checked === true)
}
className="mt-0.5"
title="Toggle new branch creation"
data-testid="create-agent-new-branch"
disabled={controlsDisabled}
disabled={!branchControlsEnabled}
/>
<span className="space-y-1">
<span className="block text-sm font-medium text-foreground">
Expand All @@ -131,7 +128,7 @@ export function WorktreeSection({
</span>
</label>
<div
aria-disabled={controlsDisabled || !newBranchChecked}
aria-disabled={!newBranchChecked}
className={cn(
"grid transition-[grid-template-rows,opacity] duration-200 ease-out",
newBranchChecked
Expand All @@ -151,7 +148,7 @@ export function WorktreeSection({
}
placeholder="auto-generated if empty"
data-testid="create-agent-worktree-branch"
disabled={controlsDisabled || !newBranchChecked}
disabled={!newBranchChecked}
/>
</div>
</div>
Expand Down
103 changes: 37 additions & 66 deletions apps/web/src/components/app/use-create-agent-form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,6 @@ function submitEvent(): FormEvent<HTMLFormElement> {
// (apps/v1/system/path-info's {exists, isDirectory, isGitRepo} shape).
const REPO_INFO = { exists: true, isDirectory: true, isGitRepo: true };
const NON_REPO_DIR_INFO = { exists: true, isDirectory: true, isGitRepo: false };
// A path that doesn't exist yet also resolves isGitRepo: false — distinct
// from NON_REPO_DIR_INFO because a half-typed path shouldn't be treated as
// a deliberate "this is not a repo" signal.
const NOT_FOUND_INFO = { exists: false, isDirectory: false, isGitRepo: false };

/** The POST /api/v1/agents call, or undefined if none was made. */
function agentsPost(): [string, RequestInit] | undefined {
Expand Down Expand Up @@ -360,18 +356,20 @@ describe("handleSubmit", () => {
expect(result.current.creating).toBe(false);
});

it("clears the new branch preference when worktree creation is disabled", async () => {
it("keeps the new branch preference when worktree creation is disabled", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));

expect(result.current.createNewBranch).toBe(true);
act(() => result.current.setCreateUseWorktree(false));

expect(result.current.createUseWorktree).toBe(false);
expect(result.current.createNewBranch).toBe(false);
expect(result.current.createNewBranch).toBe(true);
});

it("sends the default JSON payload and omits context-only fields on the config step", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.setCreateName(" my agent "));
// Prompt typed but still on the config step — must not be sent.
act(() => result.current.setInitialPrompt("draft prompt"));
Expand Down Expand Up @@ -418,6 +416,7 @@ describe("handleSubmit", () => {

it("drops the worktree branch when not creating a new branch", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.setCreateNewBranch(false));
act(() => result.current.setCreateWorktreeBranch("feat/x"));

Expand Down Expand Up @@ -445,6 +444,7 @@ describe("handleSubmit", () => {
it("switches to FormData when context files or links exist, skipping empty fields", async () => {
const img = file("shot.png", "image/png");
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.enterContextStep());
act(() => result.current.appendStartupFiles([img]));
act(() => result.current.handleAddLink("https://example.com"));
Expand Down Expand Up @@ -538,103 +538,74 @@ describe("handleSubmit", () => {
});

describe("worktree checkbox state vs. cwd repo-ness", () => {
it("forces the worktree checkbox off once a previously-available repo becomes a confirmed non-repo directory", async () => {
it("keeps saved option state untouched while repo availability is unknown", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));

expect(result.current.worktreeAvailable).toBe(false);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);

act(() => result.current.handlePathInfoChange(NON_REPO_DIR_INFO));
act(() => result.current.handlePathInfoChange(null));

expect(result.current.createUseWorktree).toBe(false);
expect(result.current.worktreeChecked).toBe(false);
expect(result.current.worktreeAvailable).toBe(false);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);
});

it("does not spuriously re-check once the cwd becomes a repo again", async () => {
it("marks worktrees available only after the current cwd is confirmed as a repo", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.handlePathInfoChange(NON_REPO_DIR_INFO));
expect(result.current.createUseWorktree).toBe(false);

act(() => result.current.handlePathInfoChange(REPO_INFO));

expect(result.current.worktreeAvailable).toBe(true);
expect(result.current.createUseWorktree).toBe(false);
expect(result.current.worktreeChecked).toBe(false);
});

it("leaves the preference untouched while repo-ness is still unknown", async () => {
const { result } = await setup();

// handlePathInfoChange(null) is what PathInput sends while a debounced
// validation is in flight — must not be treated as "confirmed not a repo".
act(() => result.current.handlePathInfoChange(null));

expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);
});

it("leaves the untouched default alone when the dialog opens on a non-repo cwd", async () => {
// No repo has ever been available yet (e.g. the dialog's default cwd is
// the user's home directory) — the useState(true) default must survive,
// not read as though the user explicitly unchecked it.
it("does not change either saved option when the current cwd is not a repo", async () => {
const { result } = await setup();

act(() => result.current.handlePathInfoChange(NON_REPO_DIR_INFO));

expect(result.current.worktreeAvailable).toBe(false);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);
expect(
window.localStorage.getItem("dispatch:createNewBranch:/repo/app")
).toBeNull();
});

it("does not reset for a path that merely doesn't exist yet, even after a prior available repo", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));

// A half-typed path (mid-Tab-completion, or a pause between keystrokes)
// resolves isGitRepo: false too, via exists: false — must not clobber
// the user's choice while they're still typing toward a real path.
act(() => result.current.handlePathInfoChange(NOT_FOUND_INFO));

expect(result.current.createUseWorktree).toBe(true);
});

it("still forces off on a confirmed non-repo dir reached via an intermediate not-found path", async () => {
it("keeps option state through repo availability changes", async () => {
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.handlePathInfoChange(NOT_FOUND_INFO));
expect(result.current.createUseWorktree).toBe(true);

act(() => result.current.handlePathInfoChange(NON_REPO_DIR_INFO));
expect(result.current.worktreeAvailable).toBe(false);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);

expect(result.current.createUseWorktree).toBe(false);
});

it("never touches the per-cwd createNewBranch preference directly", async () => {
// createNewBranch's checked state cascades from worktreeChecked in the
// UI (create-agent-worktree-section.tsx), so the reset effect doesn't
// need to — and must not — write through to its own per-cwd atom.
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.handlePathInfoChange(NON_REPO_DIR_INFO));

expect(result.current.worktreeAvailable).toBe(true);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);
expect(
window.localStorage.getItem("dispatch:createNewBranch:/repo/app")
).toBeNull();
});

it("doesn't clobber a different cwd's saved createNewBranch pref on switch", async () => {
// A previously-visited repo with its own saved "create new branch" pref.
it("does not apply stale validation to a new cwd", async () => {
window.localStorage.setItem("dispatch:createNewBranch:/repo/other", "true");
const { result } = await setup();
act(() => result.current.handlePathInfoChange(REPO_INFO));
act(() => result.current.handlePathInfoChange(NON_REPO_DIR_INFO));

// Switching to the other repo must not stomp its independently-saved
// preference — a standing guard against a cwd-keyed setter creeping
// back into the reset effect's deps.
act(() => result.current.setCreateCwd("/repo/other"));

expect(result.current.worktreeAvailable).toBe(false);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);
expect(
window.localStorage.getItem("dispatch:createNewBranch:/repo/other")
).toBe("true");

act(() => result.current.handlePathInfoChange(REPO_INFO));

expect(result.current.worktreeAvailable).toBe(true);
expect(result.current.createUseWorktree).toBe(true);
expect(result.current.createNewBranch).toBe(true);
});
});
Loading
Loading