diff --git a/src/cli/commands/remove.test.ts b/src/cli/commands/remove.test.ts index 91da1ab..2742544 100644 --- a/src/cli/commands/remove.test.ts +++ b/src/cli/commands/remove.test.ts @@ -38,10 +38,12 @@ function createFakeUi(opts: { nonInteractive?: boolean; confirm?: boolean; multi log: FakeUiLog; multiSpinnerLog: FakeMultiSpinnerLog; spinnerLog: FakeSpinnerLog; + confirmMessages: string[]; } { const log: FakeUiLog = { info: [], success: [], warn: [], error: [], outro: [] }; const multiSpinnerLog: FakeMultiSpinnerLog = { complete: [], fail: [] }; const spinnerLog: FakeSpinnerLog = { start: [], message: [], stop: [] }; + const confirmMessages: string[] = []; const ui = { nonInteractive: opts.nonInteractive ?? false, @@ -92,7 +94,8 @@ function createFakeUi(opts: { nonInteractive?: boolean; confirm?: boolean; multi async text() { return ""; }, - async confirm() { + async confirm(options: { message: string }) { + confirmMessages.push(options.message); return opts.confirm ?? true; }, async select() { @@ -107,7 +110,7 @@ function createFakeUi(opts: { nonInteractive?: boolean; confirm?: boolean; multi cancel() {}, } satisfies UiPort; - return { ui, log, multiSpinnerLog, spinnerLog }; + return { ui, log, multiSpinnerLog, spinnerLog, confirmMessages }; } function buildContainer( @@ -270,6 +273,191 @@ describe("remove — multi path branch delete failures", () => { }); }); +describe("remove — non-interactive suppression of remote-delete prompt", () => { + // Regression for Vikunja #49: `wt remove --delete-branch --force --yes` + // must NOT prompt "Also delete remote branch?". The same applies to global + // --non-interactive and to --dry-run. Default decision in all these cases: + // do NOT delete the remote branch (explicit opt-in required via flag or config). + + function singleRemoveFs() { + return createFakeFilesystem({ + files: { [`${ROOT}/${CONFIG_FILENAME}`]: JSON.stringify({ rootDir: ".worktrees" }) }, + directories: [ROOT, `${ROOT}/.worktrees`, featureWt.path], + }); + } + + function singleRemoveGit() { + return createFakeGit({ + root: ROOT, + mainRoot: ROOT, + worktrees: [mainWt, featureWt], + branches: ["main", "feature"], + mergedBranches: ["feature"], + }); + } + + test("--yes with --delete-branch but no --delete-remote-branch → no prompt, remote NOT deleted", async () => { + const fs = singleRemoveFs(); + const git = singleRemoveGit(); + const { ui, spinnerLog, confirmMessages } = createFakeUi(); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + "delete-branch": true, + // "delete-remote-branch" intentionally omitted + yes: true, + force: false, + "dry-run": false, + }); + + // No prompt of any kind. + expect(confirmMessages.length).toBe(0); + // Local branch deleted, remote NOT touched. + expect(spinnerLog.stop.some((m) => m.includes("deleted (local)"))).toBe(true); + expect(spinnerLog.stop.some((m) => m.includes("local & remote"))).toBe(false); + expect(code).toBe(0); + }); + + test("--non-interactive with --delete-branch but no --delete-remote-branch → no prompt, remote NOT deleted", async () => { + const fs = singleRemoveFs(); + const git = singleRemoveGit(); + const { ui, spinnerLog, confirmMessages } = createFakeUi({ nonInteractive: true }); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + "delete-branch": true, + yes: false, + force: false, + "dry-run": false, + }); + + expect(confirmMessages.length).toBe(0); + expect(spinnerLog.stop.some((m) => m.includes("deleted (local)"))).toBe(true); + expect(spinnerLog.stop.some((m) => m.includes("local & remote"))).toBe(false); + expect(code).toBe(0); + }); + + test("--dry-run with --delete-branch (no --delete-remote-branch) → no prompt, plan reflects local-only", async () => { + const fs = singleRemoveFs(); + const git = singleRemoveGit(); + const { ui, log, confirmMessages } = createFakeUi(); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + "delete-branch": true, + yes: false, + force: false, + "dry-run": true, + }); + + expect(confirmMessages.length).toBe(0); + // Plan: local-only, not "local & remote". + const planLine = log.info.find((m) => m.includes("Would delete branch")); + expect(planLine).toBeDefined(); + expect(planLine).toContain("local"); + expect(planLine).not.toContain("local & remote"); + expect(log.outro.some((m) => m.includes("Dry run"))).toBe(true); + expect(code).toBe(0); + }); + + test("--yes with explicit --delete-remote-branch=true → no prompt, remote DELETED", async () => { + const fs = singleRemoveFs(); + const git = singleRemoveGit(); + const { ui, spinnerLog, confirmMessages } = createFakeUi(); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + "delete-branch": true, + "delete-remote-branch": true, + yes: true, + force: false, + "dry-run": false, + }); + + expect(confirmMessages.length).toBe(0); + expect(spinnerLog.stop.some((m) => m.includes("local & remote"))).toBe(true); + expect(code).toBe(0); + }); + + test("--yes alone (no --delete-branch flag, no config) → no prompt, branch NOT deleted", async () => { + // Exercises the new yes-gate in resolveDeleteBranch: with neither flag nor config, + // --yes must default to false instead of prompting "Also delete branch?". + const fs = singleRemoveFs(); + const git = singleRemoveGit(); + const { ui, spinnerLog, confirmMessages } = createFakeUi(); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + // "delete-branch" omitted on purpose + yes: true, + force: false, + "dry-run": false, + }); + + expect(confirmMessages.length).toBe(0); + // Worktree removed, but no branch-delete spinner fired. + expect(spinnerLog.stop.some((m) => m.includes("removed"))).toBe(true); + expect(spinnerLog.start.some((m) => m.includes("Deleting branch"))).toBe(false); + expect(code).toBe(0); + }); + + test("--yes with explicit --delete-remote-branch=false → no prompt, remote NOT deleted", async () => { + // Explicit opt-out must beat the yes-gate (step 1 of resolution order). + const fs = singleRemoveFs(); + const git = singleRemoveGit(); + const { ui, spinnerLog, confirmMessages } = createFakeUi(); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + "delete-branch": true, + "delete-remote-branch": false, + yes: true, + force: false, + "dry-run": false, + }); + + expect(confirmMessages.length).toBe(0); + expect(spinnerLog.stop.some((m) => m.includes("deleted (local)"))).toBe(true); + expect(spinnerLog.stop.some((m) => m.includes("local & remote"))).toBe(false); + expect(code).toBe(0); + }); + + test("--yes with config remove.deleteRemoteBranch=true → no prompt, remote DELETED", async () => { + // Config opt-in (step 2) must beat the yes-gate (step 3). + const fs = createFakeFilesystem({ + files: { + [`${ROOT}/${CONFIG_FILENAME}`]: JSON.stringify({ + rootDir: ".worktrees", + remove: { deleteRemoteBranch: true }, + }), + }, + directories: [ROOT, `${ROOT}/.worktrees`, featureWt.path], + }); + const git = singleRemoveGit(); + const { ui, spinnerLog, confirmMessages } = createFakeUi(); + const container = buildContainer(ui, git, fs); + + const code = await runRemove(container, { + branch: "feature", + "delete-branch": true, + // "delete-remote-branch" omitted — config decides + yes: true, + force: false, + "dry-run": false, + }); + + expect(confirmMessages.length).toBe(0); + expect(spinnerLog.stop.some((m) => m.includes("local & remote"))).toBe(true); + expect(code).toBe(0); + }); +}); + describe("remove — single path force on unmerged branch", () => { // Regression for the branch-deletion-policy extraction: with --force/--yes on an // unmerged branch the CLI must still surface the "not merged" warning and the diff --git a/src/cli/commands/remove.ts b/src/cli/commands/remove.ts index 86f265e..7de8f4d 100644 --- a/src/cli/commands/remove.ts +++ b/src/cli/commands/remove.ts @@ -42,7 +42,7 @@ export function removeCommand(container: Container) { }, yes: { type: "boolean", - description: "Confirm destructive operations without prompting", + description: "Skip confirmation prompts (branch deletion stays off unless opted in via flag or config)", required: false, }, "dry-run": { @@ -80,7 +80,7 @@ export function removeCommand(container: Container) { ? await resolveDeleteBranch( parsed["delete-branch"], config?.remove.deleteBranch, - { ui }, + { ui, yes, dryRun }, { branches: branchCapableNames, }, @@ -92,7 +92,7 @@ export function removeCommand(container: Container) { ? await resolveDeleteRemoteBranch( parsed["delete-remote-branch"], config?.remove.deleteRemoteBranch, - { ui }, + { ui, yes, dryRun }, { branches: branchCapableNames }, ) : false; diff --git a/src/cli/resolve-params.ts b/src/cli/resolve-params.ts index 328a92c..e826907 100644 --- a/src/cli/resolve-params.ts +++ b/src/cli/resolve-params.ts @@ -243,19 +243,28 @@ export async function resolveWorktreesToRemove( }); } +// Resolution order for both resolvers below: +// 1. explicit CLI flag (--delete-branch / --delete-remote-branch) wins +// 2. config value (remove.deleteBranch / remove.deleteRemoteBranch) +// 3. non-interactive contexts (`--yes`, global `--non-interactive`, `--dry-run`) +// → return `false` and skip the prompt. Destructive defaults must require +// explicit opt-in, so agents and CI never accidentally delete branches or +// remote refs. +// 4. interactive fallback: prompt the user. + export async function resolveDeleteRemoteBranch( flag: boolean | undefined, configValue: boolean | undefined, - deps: { ui: UiPort }, + deps: { ui: UiPort; yes: boolean; dryRun: boolean }, context: { branches: string[] }, ): Promise { if (flag !== undefined) return flag; if (configValue !== undefined) return configValue; - const { ui } = deps; + const { ui, yes, dryRun } = deps; - if (ui.nonInteractive) return false; + if (ui.nonInteractive || yes || dryRun) return false; const message = context.branches.length > 1 @@ -268,16 +277,16 @@ export async function resolveDeleteRemoteBranch( export async function resolveDeleteBranch( flag: boolean | undefined, configValue: boolean | undefined, - deps: { ui: UiPort }, + deps: { ui: UiPort; yes: boolean; dryRun: boolean }, context: { branches: string[] }, ): Promise { if (flag !== undefined) return flag; if (configValue !== undefined) return configValue; - const { ui } = deps; + const { ui, yes, dryRun } = deps; - if (ui.nonInteractive) return false; + if (ui.nonInteractive || yes || dryRun) return false; const message = context.branches.length > 1