diff --git a/.specs/features/open-worktree-prompt/spec.md b/.specs/features/open-worktree-prompt/spec.md new file mode 100644 index 0000000..bd441e1 --- /dev/null +++ b/.specs/features/open-worktree-prompt/spec.md @@ -0,0 +1,12 @@ +# Open worktree prompt + +- OWP-01: For an interactive `open` without worktree flags or `--resume`, ask once after role resolution and before starting the daemon when cwd is in a git repository. +- OWP-02: A trimmed, case-insensitive `y` or `yes` enables `opts.worktree` and follows the existing `--worktree` launch path. +- OWP-03: An empty answer, `n`, or `no` continues without a worktree. +- OWP-04: Any other answer prints `Answer y or n.` to stderr and asks again. +- OWP-05: EOF rejects with `Worktree selection was interrupted; nothing was launched.` before the daemon starts or a session is adopted. +- OWP-06: `--worktree` skips the question and opens in a worktree. +- OWP-07: `--no-worktree` skips the question and opens in the current directory; omitting both flags leaves the option undefined. +- OWP-08: Non-interactive launches, including no-TTY and `-p`/`--print`, do not ask and do not use a worktree by default. +- OWP-09: A cwd outside a git repository does not trigger the question. +- OWP-10: `--resume` without `--worktree` does not trigger the question. diff --git a/README.md b/README.md index 8c38371..2a6ad2b 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,7 @@ Two things worth knowing before you edit an agent file. `--agent` layers on top npx codedeck run "review the diff on this branch" --agent codex --role reviewer ``` -`--no-bypass` drops the bypass flag, `--no-theme` keeps the status line but drops everything else the look changes, `--no-pty` opts out of the session naming itself, and `--model`/`--effort`/`--resume`/`--worktree` override the defaults. +`--no-bypass` drops the bypass flag, `--no-theme` keeps the status line but drops everything else the look changes, `--no-pty` opts out of the session naming itself, and `--model`/`--effort`/`--resume`/`--worktree` override the defaults. Interactive opens ask about a worktree when neither `--worktree` nor `--no-worktree` is set. ### What the session looks like diff --git a/scripts/pty-gate.sh b/scripts/pty-gate.sh index 91da827..166b56a 100755 --- a/scripts/pty-gate.sh +++ b/scripts/pty-gate.sh @@ -89,7 +89,7 @@ chmod +x "$WORK/bin/claude" sleep 2 ) | RUN_AGENT_CONFIG_DIR="$CONFIG_DIR" RUN_AGENT_DIR="$STATE_DIR" PATH="$WORK/bin:$PATH" FAKE_NAME="$NAME" \ timeout "$LIMIT" script -qec \ - "sh -c 'stty rows $ROWS cols $COLS; exec node \"$HERE/dist/cli/index.js\" open general --effort xhigh --no-theme'" \ + "sh -c 'stty rows $ROWS cols $COLS; exec node \"$HERE/dist/cli/index.js\" open general --effort xhigh --no-theme --no-worktree'" \ /dev/null > "$CAPTURE" 2>&1 || true failures=() diff --git a/scripts/rename-gate.sh b/scripts/rename-gate.sh index 65de614..b2b0690 100755 --- a/scripts/rename-gate.sh +++ b/scripts/rename-gate.sh @@ -54,7 +54,7 @@ echo "expecting the session to rename itself to the generated title" # --no-bypass keeps the gate runnable as root, where Claude Code refuses to # skip permission prompts. It changes nothing about the rename path. ( sleep "$SETTLE"; printf '%s\r' "$PROMPT"; sleep "$TURN"; printf '\003'; sleep 1; printf '\003'; sleep 2 ) \ - | timeout "$LIMIT" script -qec "node '$HERE/dist/cli/index.js' open general --no-bypass" /dev/null \ + | timeout "$LIMIT" script -qec "node '$HERE/dist/cli/index.js' open general --no-bypass --no-worktree" /dev/null \ > "$CAPTURE" 2>&1 || true sidecar="$(ls -t "$SESSIONS"/codedeck-session-*.name 2>/dev/null | head -1 || true)" diff --git a/scripts/theme-gate.sh b/scripts/theme-gate.sh index e0e2749..c4ac719 100755 --- a/scripts/theme-gate.sh +++ b/scripts/theme-gate.sh @@ -69,7 +69,7 @@ echo "expecting promptBorder $prompt_border and inactive $inactive" # A pty is required: claude renders nothing recognisable when stdout is a pipe. # `script` supplies one. Two Ctrl+C keys are how the TUI is asked to quit. ( sleep "$SETTLE"; printf '\003'; sleep 1; printf '\003'; sleep 2 ) \ - | timeout "$LIMIT" script -qec "node '$HERE/dist/cli/index.js' open general" /dev/null \ + | timeout "$LIMIT" script -qec "node '$HERE/dist/cli/index.js' open general --no-worktree" /dev/null \ > "$CAPTURE" 2>&1 || true missing=() diff --git a/src/cli/commands/open.ts b/src/cli/commands/open.ts index 6bd105b..6cc638d 100644 --- a/src/cli/commands/open.ts +++ b/src/cli/commands/open.ts @@ -367,6 +367,62 @@ function selectRole(): Promise { }); } +export function askWorktree(): Promise { + const rl = readline.createInterface({ + input: process.stdin, + output: process.stdout, + }); + + return new Promise((resolve, reject) => { + let settled = false; + const finish = (worktree: boolean) => { + if (settled) return; + settled = true; + rl.close(); + resolve(worktree); + }; + + rl.once("close", () => { + if (settled) return; + settled = true; + reject(new Error("Worktree selection was interrupted; nothing was launched.")); + }); + + const ask = () => { + rl.question("Open in a worktree? [y/N]: ", (answer) => { + const choice = answer.trim().toLowerCase(); + if (choice === "y" || choice === "yes") { + finish(true); + return; + } + if (choice === "" || choice === "n" || choice === "no") { + finish(false); + return; + } + + process.stderr.write("Answer y or n.\n"); + ask(); + }); + }; + + ask(); + }); +} + +export async function resolveOpenWorktree( + flag: boolean | undefined, + ctx: { interactive: boolean; resume?: string; cwd: string }, + ask: () => Promise = askWorktree, +): Promise { + if (flag !== undefined) return flag; + if (!ctx.interactive || ctx.resume !== undefined) return false; + + const gitInfo = await getGitInfo(ctx.cwd); + if (!gitInfo) return false; + + return ask(); +} + /** * `claude -p` answers one prompt and exits, so there is no session to choose a * role for. A terminal check alone does not catch this: under a pty (CI scripts, @@ -520,7 +576,8 @@ export function registerOpenCommand(program: Command): void { .option("--autocompact [value]", "native auto-compact: Claude window size is auto or 100000-1000000 tokens; OpenCode toggles its native setting") .option("--no-autocompact", "disable native auto-compaction in Claude and OpenCode") .option("--resume ", "resume an interactive session") - .option("--worktree", "ask Claude Code to create an isolated worktree") + .option("--worktree", "open the session in an isolated git worktree") + .option("--no-worktree", "open in the current directory without asking") .option("--profile ", "use a saved setup profile instead of the active one (see profile list)") .option("--no-bypass", "do not skip Claude Code permission prompts") .option("--no-theme", "keep only the CodeDeck status line, without the theme or the renderer") @@ -549,9 +606,14 @@ export function registerOpenCommand(program: Command): void { } const interactive = !isNonInteractiveLaunch(invocation.passthrough, hintLauncher); const role = await resolveRole(invocation.roleInput, interactive); + const cwd = currentWorkingDirectory(); + opts.worktree = await resolveOpenWorktree(opts.worktree, { + interactive: interactive && isInteractiveTerminal(), + resume: opts.resume, + cwd, + }); const pluginDir = resolvePluginDir(); assertPluginDirectory(pluginDir); - const cwd = currentWorkingDirectory(); const client = new IpcClient(); await client.ensureDaemonStarted(); diff --git a/tests/open-worktree-prompt.test.ts b/tests/open-worktree-prompt.test.ts new file mode 100644 index 0000000..ac35967 --- /dev/null +++ b/tests/open-worktree-prompt.test.ts @@ -0,0 +1,243 @@ +import { Command } from "commander"; +import * as readline from "node:readline"; +import { describe, expect, it, vi } from "vitest"; + +import { IpcClient } from "../src/daemon/ipc.js"; +import * as repository from "../src/git/repository.js"; +import * as worktree from "../src/git/worktree.js"; +import * as runtime from "../src/open/runtime.js"; +import * as setupCommands from "../src/cli/commands/setup.js"; +import { + askWorktree, + registerOpenCommand, + resolveOpenWorktree, + scanOptions, +} from "../src/cli/commands/open.js"; +import { setupOpenHarness } from "./helpers/open-harness.js"; + +const { createInterfaceMock } = vi.hoisted(() => ({ createInterfaceMock: vi.fn() })); + +vi.mock("node:readline", () => ({ createInterface: createInterfaceMock })); + +vi.mock("node:child_process", async (importOriginal) => { + const mod = await importOriginal(); + return { ...mod, execFileSync: vi.fn() }; +}); + +const { runOpen } = setupOpenHarness({ prefix: "codedeck-worktree-prompt-", restoreCwd: true }); + +interface FakeReadline { + instance: { + question: ReturnType; + once: ReturnType; + close: ReturnType; + }; + prompts: string[]; +} + +function mockReadline(...answerSets: Array>): FakeReadline[] { + let nextAnswerSet = 0; + const fakes: FakeReadline[] = answerSets.map((answers) => { + let closeHandler: (() => void) | undefined; + const prompts: string[] = []; + return { + prompts, + instance: { + question: vi.fn((prompt: string, callback: (answer: string) => void) => { + prompts.push(prompt); + const answer = answers.shift(); + if (answer === "close") { + closeHandler?.(); + return; + } + if (answer === undefined) throw new Error("Unexpected readline question"); + callback(answer); + }), + once: vi.fn((event: string, callback: () => void) => { + if (event === "close") closeHandler = callback; + }), + close: vi.fn(), + }, + }; + }); + + createInterfaceMock.mockClear(); + createInterfaceMock.mockImplementation(() => { + const fake = fakes[nextAnswerSet]; + if (!fake) throw new Error("Unexpected readline interface"); + nextAnswerSet += 1; + return fake.instance as unknown as readline.Interface; + }); + return fakes; +} + +describe("open worktree prompt", () => { + it("OWP-01 asks once for an interactive launch in a git repository", async () => { + vi.spyOn(repository, "getGitInfo").mockResolvedValue({ + root: "/repo", + head: "abc", + branch: "main", + isDirty: false, + }); + const ask = vi.fn().mockResolvedValue(false); + + await expect( + resolveOpenWorktree(undefined, { interactive: true, cwd: "/repo" }, ask), + ).resolves.toBe(false); + + expect(ask).toHaveBeenCalledTimes(1); + expect(repository.getGitInfo).toHaveBeenCalledWith("/repo"); + }); + + it("OWP-02 treats y and trimmed uppercase YES as yes and launches in the created worktree", async () => { + const choices = mockReadline(["y"], [" YES "]); + await expect(askWorktree()).resolves.toBe(true); + expect(choices[0].prompts).toEqual(["Open in a worktree? [y/N]: "]); + + vi.spyOn(setupCommands, "isInteractiveTerminal").mockReturnValue(true); + vi.spyOn(repository, "getGitInfo").mockResolvedValue({ + root: "/repo", + head: "abc", + branch: "main", + isDirty: false, + }); + const create = vi + .spyOn(worktree, "createWorktree") + .mockResolvedValue({ path: "/wt/yes", branch: "ra/reviewer-yes", baseCommit: "abc" }); + + await runOpen(["reviewer", "--no-theme"]); + + expect(choices[1].prompts).toEqual(["Open in a worktree? [y/N]: "]); + expect(create).toHaveBeenCalledWith(expect.objectContaining({ repoRoot: "/repo", name: "reviewer" })); + const [, , spawnOptions] = vi.mocked(runtime.spawnHarness).mock.calls[0]; + expect(spawnOptions.cwd).toBe("/wt/yes"); + expect(choices[1].instance.question.mock.invocationCallOrder[0]).toBeLessThan( + vi.mocked(IpcClient.prototype.ensureDaemonStarted).mock.invocationCallOrder[0], + ); + }); + + it("OWP-03 treats an empty answer, n, and no as no", async () => { + const choices = mockReadline([""], ["n"], [" no "]); + + await expect(askWorktree()).resolves.toBe(false); + await expect(askWorktree()).resolves.toBe(false); + await expect(askWorktree()).resolves.toBe(false); + + expect(choices.map((choice) => choice.prompts)).toEqual([ + ["Open in a worktree? [y/N]: "], + ["Open in a worktree? [y/N]: "], + ["Open in a worktree? [y/N]: "], + ]); + }); + + it("OWP-04 reports invalid answers to stderr and asks again", async () => { + const [choice] = mockReadline(["maybe", "YeS"]); + const stderrWrite = vi.spyOn(process.stderr, "write").mockReturnValue(true); + + await expect(askWorktree()).resolves.toBe(true); + + expect(choice.prompts).toEqual([ + "Open in a worktree? [y/N]: ", + "Open in a worktree? [y/N]: ", + ]); + expect(stderrWrite).toHaveBeenCalledWith("Answer y or n.\n"); + }); + + it("OWP-05 rejects on EOF before starting the daemon or adopting a session", async () => { + mockReadline(["close"]); + vi.spyOn(setupCommands, "isInteractiveTerminal").mockReturnValue(true); + vi.spyOn(repository, "getGitInfo").mockResolvedValue({ + root: "/repo", + head: "abc", + branch: "main", + isDirty: false, + }); + + await expect(runOpen(["reviewer", "--no-theme"])).rejects.toThrow( + "Worktree selection was interrupted; nothing was launched.", + ); + expect(IpcClient.prototype.ensureDaemonStarted).not.toHaveBeenCalled(); + expect(IpcClient.prototype.request).not.toHaveBeenCalled(); + }); + + it("OWP-06 skips the question and enables an explicit worktree flag", async () => { + vi.spyOn(repository, "getGitInfo"); + const ask = vi.fn(); + + await expect( + resolveOpenWorktree(true, { interactive: true, cwd: "/repo" }, ask), + ).resolves.toBe(true); + + expect(ask).not.toHaveBeenCalled(); + expect(repository.getGitInfo).not.toHaveBeenCalled(); + }); + + it("OWP-07 accepts --no-worktree and skips the question", async () => { + const program = new Command(); + program.exitOverride(); + registerOpenCommand(program); + const command = program.commands.find((candidate) => candidate.name() === "open"); + if (!command) throw new Error("open command was not registered"); + expect(scanOptions(["--no-worktree"], command)).toEqual([]); + + vi.spyOn(setupCommands, "isInteractiveTerminal").mockReturnValue(true); + createInterfaceMock.mockClear(); + const create = vi.spyOn(worktree, "createWorktree"); + const cwd = process.cwd(); + + await runOpen(["reviewer", "--no-theme", "--no-worktree"]); + + expect(createInterfaceMock).not.toHaveBeenCalled(); + expect(create).not.toHaveBeenCalled(); + const [, , spawnOptions] = vi.mocked(runtime.spawnHarness).mock.calls[0]; + expect(spawnOptions.cwd).toBe(cwd); + }); + + it("OWP-08 skips the question without a TTY or for a print launch", async () => { + const choices = mockReadline(); + const terminal = vi.spyOn(setupCommands, "isInteractiveTerminal").mockReturnValue(false); + const create = vi.spyOn(worktree, "createWorktree").mockResolvedValue({ path: "/wt" }); + await runOpen(["reviewer", "--no-theme"]); + expect(createInterfaceMock).not.toHaveBeenCalled(); + + terminal.mockReturnValue(true); + await runOpen(["reviewer", "--no-theme", "--", "--print"]); + + expect(createInterfaceMock).not.toHaveBeenCalled(); + expect(choices).toEqual([]); + expect(create).not.toHaveBeenCalled(); + }); + + it("OWP-09 skips the question outside a git repository", async () => { + vi.spyOn(repository, "getGitInfo").mockResolvedValue(null); + const ask = vi.fn(); + + await expect( + resolveOpenWorktree(undefined, { interactive: true, cwd: "/outside" }, ask), + ).resolves.toBe(false); + + expect(repository.getGitInfo).toHaveBeenCalledWith("/outside"); + expect(ask).not.toHaveBeenCalled(); + }); + + it("OWP-10 skips the question for a resumed session without a worktree flag", async () => { + vi.spyOn(repository, "getGitInfo").mockResolvedValue({ + root: "/repo", + head: "abc", + branch: "main", + isDirty: false, + }); + const ask = vi.fn().mockResolvedValue(true); + + await expect( + resolveOpenWorktree(undefined, { + interactive: true, + resume: "native-session-id", + cwd: "/repo", + }, ask), + ).resolves.toBe(false); + + expect(ask).not.toHaveBeenCalled(); + expect(repository.getGitInfo).not.toHaveBeenCalled(); + }); +});