diff --git a/AGENTS.md b/AGENTS.md index cb64a3e..8fa0af6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,6 +25,7 @@ Do not apply both to the same text. - Phase 10 Plan: documents/phase-10-plan.adoc - Phase 11 Plan: documents/phase-11-plan.adoc - Phase 12 Plan: documents/phase-12-plan.adoc +- Phase 12 confirmation decision: documents/phase-12-confirmation-decision.adoc - Phase 13 Plan: documents/phase-13-plan.adoc - Phase 14 Plan: documents/phase-14-plan.adoc - Phase 15 Plan: documents/phase-15-plan.adoc diff --git a/Readme.adoc b/Readme.adoc index c3de13d..82911c9 100644 --- a/Readme.adoc +++ b/Readme.adoc @@ -313,8 +313,9 @@ issues assigned to other users, or `--assignee` to select a named user. Already unassigned issues are skipped. Exact project and assignee matches are used without a prompt. Partial matches prompt you to choose a project or assignee. Before any filtered mutation, the command -asks for confirmation; `--yes` skips that prompt and `--dry-run` prints the -matching issues without changing them. +asks for confirmation. An explicit `y` or `yes`, or `--yes`, is required. +EOF, blank input, and exhausted invalid input cancel without +changing issues. `--dry-run` prints the matching issues without changing them. The command processes at most 100 matches at a time. If more than 100 issues match, it prints a warning and processes only the first 100. Updates remain independent API calls, so a failed request can leave earlier matches already @@ -437,7 +438,7 @@ $ lc issue move --project Manhattan --dry-run CRY-1 <3> $ lc issue move --project Manhattan --yes CRY-1 <4> $ lc issue move --project Manhattan --team ENG CRY-1 <5> ---- -<1> Move a single issue; prompts for confirmation before executing +<1> Move a single issue; an explicit `y` or `yes` is required before execution <2> Move multiple issues at once — updates run concurrently <3> Preview the planned moves without executing any mutations <4> Skip the confirmation prompt @@ -454,7 +455,7 @@ $ lc issue move --from Retired --to Active --all <2> $ lc issue move --from p-uuid-1 --to p-uuid-2 <3> $ lc issue move --from Retired --to Active --dry-run <4> ---- -<1> Moves all open issues from "Retired" to "Active", prompts for confirmation +<1> Moves all open issues from "Retired" to "Active". An explicit `y` or `yes` is required <2> Includes completed and cancelled issues too <3> Move by project UUID — the way to target a project in another team <4> Preview without mutating diff --git a/app/lib/linear_cli/cli/commands/issues/move.ex b/app/lib/linear_cli/cli/commands/issues/move.ex index 01e89d2..d0f6c5f 100644 --- a/app/lib/linear_cli/cli/commands/issues/move.ex +++ b/app/lib/linear_cli/cli/commands/issues/move.ex @@ -73,7 +73,7 @@ defmodule LinearCli.CLI.Commands.Issues.Move do do: apply_moves(issues, project, output) defp execute_moves_if_confirmed(issues, project, _flags, output) do - if Prompt.yes?("Proceed with move?"), + if Prompt.confirm_destructive?("Proceed with move?"), do: apply_moves(issues, project, output), else: Prompt.warn("Move cancelled") end @@ -145,7 +145,7 @@ defmodule LinearCli.CLI.Commands.Issues.Move do :ok not flags.yes and - not Prompt.yes?( + not Prompt.confirm_destructive?( "Move #{length(issues)} issue(s) from #{source.name} to #{target.name}?" ) -> Prompt.warn("Move cancelled") diff --git a/app/lib/linear_cli/cli/commands/issues/mutations.ex b/app/lib/linear_cli/cli/commands/issues/mutations.ex index 86913ca..0b81c69 100644 --- a/app/lib/linear_cli/cli/commands/issues/mutations.ex +++ b/app/lib/linear_cli/cli/commands/issues/mutations.ex @@ -212,7 +212,7 @@ defmodule LinearCli.CLI.Commands.Issues.Mutations do end defp unassign_filtered_issues(issues, _flags, options) do - if Prompt.yes?("Unassign #{length(issues)} issue(s)?") do + if Prompt.confirm_destructive?("Unassign #{length(issues)} issue(s)?") do unassign_and_show(issues, options) else cancel_unassign(options) diff --git a/app/lib/linear_cli/cli/prompt.ex b/app/lib/linear_cli/cli/prompt.ex index ffa3ec8..0b60540 100644 --- a/app/lib/linear_cli/cli/prompt.ex +++ b/app/lib/linear_cli/cli/prompt.ex @@ -126,6 +126,16 @@ defmodule LinearCli.CLI.Prompt do @spec yes?(Owl.Data.t()) :: boolean() def yes?(message), do: Owl.IO.confirm(message: message, default: true) + @doc """ + Asks a yes/no question for a destructive operation, defaulting to `false`. + + A blank answer or end of input cancels the operation. Use this helper for + destructive confirmations. Keep `yes?/1` for prompts where the affirmative + default is part of the existing behavior. + """ + @spec confirm_destructive?(Owl.Data.t()) :: boolean() + def confirm_destructive?(message), do: Owl.IO.confirm(message: message, default: false) + @doc """ Prompts for a single choice from an ordered `[{label, value}]` list, returning the chosen `value`. diff --git a/app/test/linear_cli/cli/commands/issues/move_test.exs b/app/test/linear_cli/cli/commands/issues/move_test.exs index d0cf8b3..6a9fcfd 100644 --- a/app/test/linear_cli/cli/commands/issues/move_test.exs +++ b/app/test/linear_cli/cli/commands/issues/move_test.exs @@ -150,6 +150,46 @@ defmodule LinearCli.CLI.Commands.Issues.MoveTest do assert output =~ "Move cancelled" end + test "closed stdin cancels explicit-ID move without mutation" do + test_pid = self() + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + query = decoded["query"] + + cond do + String.contains?(query, "issue(id: $id)") -> + Req.Test.json(conn, %{"data" => %{"issue" => issue_map()}}) + + String.contains?(query, "projects(first: 100") -> + Req.Test.json(conn, move_team_projects()) + + String.contains?(query, "issueUpdate") -> + send(test_pid, :mutation_called) + raise "closed stdin must not send issueUpdate" + + true -> + raise "no stub matched query: #{query}" + end + end) + + output = + capture_io([input: ""], fn -> + assert :ok = + LinearCli.CLI.main([ + "issue", + "move", + "--project", + "Manhattan", + "CRY-1" + ]) + end) + + refute_received :mutation_called + assert output =~ "Move cancelled" + end + test "user confirms, mutation is called" do test_pid = self() @@ -473,6 +513,43 @@ defmodule LinearCli.CLI.Commands.Issues.MoveTest do assert vars3["input"]["projectId"] == "p-tgt" end + test "--from/--to proceeds with an explicit yes answer" do + test_pid = self() + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + %{"query" => query} = decoded + + if String.contains?(query, "issueUpdate") do + send(test_pid, :update) + end + + case Enum.find(bulk_stub_pairs(), fn {match, _} -> String.contains?(query, match) end) do + {_match, response} -> Req.Test.json(conn, response) + nil -> raise "no stub matched query: #{query}" + end + end) + + capture_io([input: "y\n"], fn -> + assert :ok = + LinearCli.CLI.main([ + "issue", + "move", + "--from", + "Source Project", + "--to", + "Target Project", + "--team", + "ENG" + ]) + end) + + assert_received :update + assert_received :update + assert_received :update + end + test "--from/--to --all sends list query without completedAt/canceledAt guards" do test_pid = self() @@ -726,6 +803,43 @@ defmodule LinearCli.CLI.Commands.Issues.MoveTest do assert output =~ "Move cancelled" end + test "--from/--to closed stdin cancels without mutation" do + test_pid = self() + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + %{"query" => query} = Jason.decode!(body) + + if String.contains?(query, "issueUpdate") do + send(test_pid, :mutation_called) + raise "closed stdin must not send issueUpdate" + end + + case Enum.find(bulk_stub_pairs(), fn {match, _} -> String.contains?(query, match) end) do + {_match, response} -> Req.Test.json(conn, response) + nil -> raise "no stub matched query: #{query}" + end + end) + + output = + capture_io([input: ""], fn -> + assert :ok = + LinearCli.CLI.main([ + "issue", + "move", + "--from", + "Source Project", + "--to", + "Target Project", + "--team", + "ENG" + ]) + end) + + refute_received :mutation_called + assert output =~ "Move cancelled" + end + test "--from without --to exits 22 with a clear error" do test_pid = self() halt = fn code -> send(test_pid, {:halted, code}) end diff --git a/app/test/linear_cli/cli/commands/issues/mutations_test.exs b/app/test/linear_cli/cli/commands/issues/mutations_test.exs index 53e717c..915611f 100644 --- a/app/test/linear_cli/cli/commands/issues/mutations_test.exs +++ b/app/test/linear_cli/cli/commands/issues/mutations_test.exs @@ -315,6 +315,80 @@ defmodule LinearCli.CLI.Commands.Issues.MutationsTest do assert output =~ "Unassign cancelled" end + test "confirms the filtered batch with an explicit yes answer" do + test_pid = self() + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + %{"query" => query} = Jason.decode!(body) + + cond do + String.contains?(query, "issues(filter:") -> + Req.Test.json(conn, issues_response([issue_map()])) + + String.contains?(query, "issueUpdate") -> + send(test_pid, :mutated) + Req.Test.json(conn, issue_updated(%{"assignee" => nil})) + + true -> + raise "no stub matched query: #{query}" + end + end) + + capture_io([input: "yes\n"], fn -> + assert :ok = + LinearCli.CLI.main([ + "issue", + "unassign", + "--no-profile", + "--team", + "ENG", + "--state", + "started" + ]) + end) + + assert_received :mutated + end + + test "closed stdin cancels the filtered batch without mutating" do + test_pid = self() + + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + %{"query" => query} = Jason.decode!(body) + + cond do + String.contains?(query, "issues(filter:") -> + Req.Test.json(conn, issues_response([issue_map()])) + + String.contains?(query, "issueUpdate") -> + send(test_pid, :mutated) + raise "closed stdin must not send issueUpdate" + + true -> + raise "no stub matched query: #{query}" + end + end) + + output = + capture_io([input: ""], fn -> + assert :ok = + LinearCli.CLI.main([ + "issue", + "unassign", + "--no-profile", + "--team", + "ENG", + "--state", + "started" + ]) + end) + + refute_received :mutated + assert output =~ "Unassign cancelled" + end + test "--dry-run lists filtered matches without mutation" do test_pid = self() diff --git a/app/test/linear_cli/cli/prompt_test.exs b/app/test/linear_cli/cli/prompt_test.exs index 0bbff21..7d63af0 100644 --- a/app/test/linear_cli/cli/prompt_test.exs +++ b/app/test/linear_cli/cli/prompt_test.exs @@ -74,6 +74,42 @@ defmodule LinearCli.CLI.PromptTest do end end + describe "confirm_destructive?/1" do + test "defaults to false on EOF" do + assert capture_io([input: ""], fn -> + refute Prompt.confirm_destructive?("Proceed?") + end) =~ "[yN]" + end + + test "defaults to false on a blank answer" do + assert capture_io([input: "\n"], fn -> + refute Prompt.confirm_destructive?("Proceed?") + end) =~ "[yN]" + end + + test "returns false when invalid input is followed by EOF" do + output = + capture_io([input: "invalid\n"], fn -> + refute Prompt.confirm_destructive?("Proceed?") + end) + + assert output =~ "unknown answer" + assert output =~ "[yN]" + end + + test "an explicit yes answer returns true" do + assert capture_io([input: "yes\n"], fn -> + assert Prompt.confirm_destructive?("Proceed?") + end) =~ "[yN]" + end + + test "an explicit no answer returns false" do + assert capture_io([input: "n\n"], fn -> + refute Prompt.confirm_destructive?("Proceed?") + end) =~ "[yN]" + end + end + describe "select/2" do test "returns the value paired with the chosen label" do choices = [{"Bug fixes", :fix}, {"New feature work", :feat}] diff --git a/documents/phase-12-confirmation-decision.adoc b/documents/phase-12-confirmation-decision.adoc new file mode 100644 index 0000000..53f3b1f --- /dev/null +++ b/documents/phase-12-confirmation-decision.adoc @@ -0,0 +1,38 @@ += Phase 12 confirmation behavior: fail closed for destructive commands +:revdate: Sep 27, 2026 +:icons: font +:toc: + +== Status + +This note supersedes the confirmation rule in Phase 12, decision 7, only. +The accepted Phase 12 plan remains unchanged. + +== Decision + +Destructive bulk commands require an explicit affirmative answer unless the +caller passes `--yes`. + +* Filter-only `issue unassign` uses `Prompt.confirm_destructive?/1`. +* Both `issue move` confirmation paths use `Prompt.confirm_destructive?/1`. +* The helper calls `Owl.IO.confirm/1` with `default: false`. +* EOF, blank input, and an exhausted stream after invalid input return `false` and send no mutation. +* `y` and `yes` return `true` from a terminal or a usable pipe. +* `--yes` bypasses the prompt. + +`Prompt.yes?/1` keeps its affirmative default. Non-destructive prompts, such as +the issue-create self-assignment prompt, keep their existing behavior. + +== Reason + +Owl maps EOF and blank input to its configured default. A destructive prompt +with `default: true` can therefore authorize a mutation without an explicit +affirmative answer. A separate helper limits the fail-closed behavior to the +three destructive confirmation paths. + +== Verification + +Prompt tests cover EOF, blank input, `y`, `yes`, and `n`. Command tests prove +that closed stdin sends no `issueUpdate` request for filter-only unassign, +explicit-ID move, and project-to-project move. Existing `--yes` and explicit +answer tests remain in place.