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
192 changes: 190 additions & 2 deletions src/cli/commands/remove.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand All @@ -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(
Expand Down Expand Up @@ -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 <branch> --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
Expand Down
6 changes: 3 additions & 3 deletions src/cli/commands/remove.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -80,7 +80,7 @@ export function removeCommand(container: Container) {
? await resolveDeleteBranch(
parsed["delete-branch"],
config?.remove.deleteBranch,
{ ui },
{ ui, yes, dryRun },
{
branches: branchCapableNames,
},
Expand All @@ -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;
Expand Down
21 changes: 15 additions & 6 deletions src/cli/resolve-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
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
Expand All @@ -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<boolean> {
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
Expand Down