From 8c165520f9f0ffa381caada54309555da5c506f7 Mon Sep 17 00:00:00 2001 From: "Tj (bougyman) Vanderpoel" Date: Fri, 14 Aug 2026 14:38:58 -0400 Subject: [PATCH] fix(close): make issue close/cancel idempotent when already in terminal state - Add `state` attribute to Issue resource; include `state { id name type }` in both base_fields and full_fields GraphQL queries so the CLI can inspect an issue's current workflow-state type. - Short-circuit `close_issue/2` and `cancel_issue/2` in IssueHelpers: when `issue.state.type` is already the target type (completed / cancelled), skip the comment and mutation, print " is already ", and return `{:ok, issue}`. - Fix `Issue.Update.Close` mutation payload: omit `trashed` entirely when false instead of sending `"trashed" => false`, which the Linear API rejects (only `true` or null are valid). - Tests: add idempotency cases for close, cancel, and close-with-cancel-flag; verify state is parsed from API responses; update trashed-omission assertion. Co-Authored-By: Claude Sonnet 4.6 --- app/lib/linear_cli/cli/issue_helpers.ex | 45 ++++++++++++------- app/lib/linear_cli/linear/issue.ex | 12 +++-- .../linear_cli/cli/issue_helpers_test.exs | 40 +++++++++++++++++ app/test/linear_cli/linear/issue_test.exs | 37 +++++++++++++-- 4 files changed, 111 insertions(+), 23 deletions(-) diff --git a/app/lib/linear_cli/cli/issue_helpers.ex b/app/lib/linear_cli/cli/issue_helpers.ex index 3e6ca8b..a173641 100644 --- a/app/lib/linear_cli/cli/issue_helpers.ex +++ b/app/lib/linear_cli/cli/issue_helpers.ex @@ -121,14 +121,19 @@ defmodule LinearCli.CLI.IssueHelpers do """ @spec cancel_issue(%Linear.Issue{}, keyword()) :: {:ok, %Linear.Issue{}} | {:error, term()} def cancel_issue(issue, opts \\ []) do - reason = - WhatFor.reason_for(opts[:reason], four: "cancelling #{issue.identifier} - #{issue.title}") - - with {:ok, _comment} <- issue_comment(issue, reason), - {:ok, cancel_state} <- cancelled_state_for(issue), - {:ok, updated} <- Linear.close_issue(issue, cancel_state.id, %{trash: !!opts[:trash]}) do - Prompt.ok("#{issue.identifier} was cancelled") - {:ok, updated} + if issue.state && issue.state.type in ["cancelled", "canceled"] do + Prompt.ok("#{issue.identifier} is already #{issue.state.name}") + {:ok, issue} + else + reason = + WhatFor.reason_for(opts[:reason], four: "cancelling #{issue.identifier} - #{issue.title}") + + with {:ok, _comment} <- issue_comment(issue, reason), + {:ok, cancel_state} <- cancelled_state_for(issue), + {:ok, updated} <- Linear.close_issue(issue, cancel_state.id, %{trash: !!opts[:trash]}) do + Prompt.ok("#{issue.identifier} was cancelled") + {:ok, updated} + end end end @@ -148,17 +153,25 @@ defmodule LinearCli.CLI.IssueHelpers do @spec close_issue(%Linear.Issue{}, keyword()) :: {:ok, %Linear.Issue{}} | {:error, term()} def close_issue(issue, opts \\ []) do cancelled = opts[:cancel] - doing = if cancelled, do: "cancelling", else: "closing" + target_types = if cancelled, do: ["cancelled", "canceled"], else: ["completed"] done = if cancelled, do: "cancelled", else: "closed" - reason = - WhatFor.reason_for(opts[:reason], four: "#{doing} *#{issue.identifier} - #{issue.title}*") + if issue.state && issue.state.type in target_types do + Prompt.ok("#{issue.identifier} is already #{issue.state.name}") + {:ok, issue} + else + doing = if cancelled, do: "cancelling", else: "closing" - with {:ok, _comment} <- issue_comment(issue, reason), - {:ok, workflow_state} <- state_for(cancelled, issue), - {:ok, updated} <- Linear.close_issue(issue, workflow_state.id, %{trash: !!opts[:trash]}) do - Prompt.ok("#{issue.identifier} was #{done}") - {:ok, updated} + reason = + WhatFor.reason_for(opts[:reason], four: "#{doing} *#{issue.identifier} - #{issue.title}*") + + with {:ok, _comment} <- issue_comment(issue, reason), + {:ok, workflow_state} <- state_for(cancelled, issue), + {:ok, updated} <- + Linear.close_issue(issue, workflow_state.id, %{trash: !!opts[:trash]}) do + Prompt.ok("#{issue.identifier} was #{done}") + {:ok, updated} + end end end diff --git a/app/lib/linear_cli/linear/issue.ex b/app/lib/linear_cli/linear/issue.ex index 8a6d1cf..5d7d615 100644 --- a/app/lib/linear_cli/linear/issue.ex +++ b/app/lib/linear_cli/linear/issue.ex @@ -55,15 +55,18 @@ defmodule LinearCli.Linear.Issue do attribute :branch_name, :string, public?: true attribute :description, :string, public?: true attribute :assignee, :term, public?: true + attribute :state, :term, public?: true attribute :team, :term, public?: true attribute :comments, {:array, :term}, public?: true, default: [] end @issue_fields "id identifier title branchName description createdAt updatedAt" + @state_fields "id name type" @doc "GraphQL field selection for an issue plus its assignee/team (Ruby: Issue.base_fragment)." def base_fields do "#{@issue_fields} " <> + "state { #{@state_fields} } " <> "assignee { #{LinearCli.Linear.User.fields_with_teams()} } " <> "team { #{LinearCli.Linear.Team.base_fields()} }" end @@ -71,6 +74,7 @@ defmodule LinearCli.Linear.Issue do @doc "GraphQL field selection for a fully detailed issue, incl. comments (Ruby: Issue.full_fragment)." def full_fields do "#{@issue_fields} " <> + "state { #{@state_fields} } " <> "assignee { #{LinearCli.Linear.User.fields_with_teams()} } " <> "team { #{LinearCli.Linear.Team.full_fields()} } " <> "comments { nodes { #{LinearCli.Linear.Comment.base_fields()} } }" @@ -85,6 +89,7 @@ defmodule LinearCli.Linear.Issue do branch_name: map["branchName"], description: map["description"], assignee: map["assignee"] && LinearCli.Linear.User.from_map(map["assignee"]), + state: map["state"] && LinearCli.Linear.WorkflowState.from_map(map["state"]), team: map["team"] && LinearCli.Linear.Team.from_map(map["team"]), comments: Enum.map( @@ -306,10 +311,9 @@ defmodule LinearCli.Linear.Issue.Update.Close do def update(changeset, _opts, _context) do args = changeset.arguments + input = %{"stateId" => args.state_id} + input = if args.trash, do: Map.put(input, "trashed", true), else: input - Issue.Update.run(changeset.data.identifier, %{ - "stateId" => args.state_id, - "trashed" => args.trash - }) + Issue.Update.run(changeset.data.identifier, input) end end diff --git a/app/test/linear_cli/cli/issue_helpers_test.exs b/app/test/linear_cli/cli/issue_helpers_test.exs index 0fc4e50..54a7226 100644 --- a/app/test/linear_cli/cli/issue_helpers_test.exs +++ b/app/test/linear_cli/cli/issue_helpers_test.exs @@ -182,6 +182,20 @@ defmodule LinearCli.CLI.IssueHelpersTest do IssueHelpers.cancel_issue(issue(), reason: "no longer needed") end) =~ "Comment added to CRY-1" end + + test "is a no-op when the issue is already in a cancelled state" do + already_cancelled = + issue(%{state: %WorkflowState{id: "s1", name: "Cancelled", type: "cancelled"}}) + + output = + capture_io(fn -> + assert {:ok, ^already_cancelled} = + IssueHelpers.cancel_issue(already_cancelled, reason: "no longer needed") + end) + + assert output =~ "CRY-1 is already Cancelled" + refute output =~ "Comment added" + end end describe "close_issue/2 (Ruby: CLI::Issue#close_issue)" do @@ -221,6 +235,32 @@ defmodule LinearCli.CLI.IssueHelpersTest do assert output =~ "CRY-1 was cancelled" end + + test "is a no-op when the issue is already in a completed state" do + already_done = issue(%{state: %WorkflowState{id: "s1", name: "Done", type: "completed"}}) + + output = + capture_io(fn -> + assert {:ok, ^already_done} = IssueHelpers.close_issue(already_done, reason: "shipped") + end) + + assert output =~ "CRY-1 is already Done" + refute output =~ "Comment added" + end + + test "is a no-op when cancel: true and issue is already in a cancelled state" do + already_cancelled = + issue(%{state: %WorkflowState{id: "s1", name: "Cancelled", type: "cancelled"}}) + + output = + capture_io(fn -> + assert {:ok, ^already_cancelled} = + IssueHelpers.close_issue(already_cancelled, cancel: true, reason: "nope") + end) + + assert output =~ "CRY-1 is already Cancelled" + refute output =~ "Comment added" + end end describe "attach_project/2 (Ruby: CLI::Issue#attach_project)" do diff --git a/app/test/linear_cli/linear/issue_test.exs b/app/test/linear_cli/linear/issue_test.exs index 99c5518..f1d7fab 100644 --- a/app/test/linear_cli/linear/issue_test.exs +++ b/app/test/linear_cli/linear/issue_test.exs @@ -91,6 +91,34 @@ defmodule LinearCli.Linear.IssueTest do assert issue.identifier == "CRY-2" end + test "issues/1 parses the issue's current state when present in the response" do + Req.Test.stub(LinearCli.Api, fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + %{"query" => query} = Jason.decode!(body) + assert query =~ "state {" + + Req.Test.json(conn, %{ + "data" => %{ + "issue" => %{ + "id" => "i2", + "identifier" => "CRY-2", + "title" => "Ship it", + "branchName" => "cry-2-ship-it", + "description" => nil, + "assignee" => nil, + "state" => %{"id" => "s1", "name" => "Done", "type" => "completed"}, + "team" => %{"id" => "t1", "key" => "ENG", "name" => "Engineering"}, + "comments" => %{"nodes" => []} + } + } + }) + end) + + assert {:ok, [issue]} = Linear.issues(%{ids: ["cry-2"]}) + assert issue.state.type == "completed" + assert issue.state.name == "Done" + end + test "issues/1 with an unknown id returns a not_found error" do Req.Test.stub(LinearCli.Api, fn conn -> Req.Test.json(conn, %{"data" => %{"issue" => nil}}) @@ -274,7 +302,7 @@ defmodule LinearCli.Linear.IssueTest do end describe "close_issue/2+" do - test "defaults trashed to false when not given" do + test "omits trashed from the mutation input when not given (Linear API rejects trashed: false)" do issue = struct!(LinearCli.Linear.Issue, id: "i1", identifier: "CRY-1") Req.Test.stub(LinearCli.Api, fn conn -> @@ -282,7 +310,7 @@ defmodule LinearCli.Linear.IssueTest do %{"variables" => %{"id" => id, "input" => input}} = Jason.decode!(body) assert id == "CRY-1" - assert input == %{"stateId" => "s1", "trashed" => false} + assert input == %{"stateId" => "s1"} Req.Test.json(conn, %{ "data" => %{ @@ -294,6 +322,7 @@ defmodule LinearCli.Linear.IssueTest do "branchName" => "cry-1-fix-it", "description" => nil, "assignee" => nil, + "state" => %{"id" => "s1", "name" => "Done", "type" => "completed"}, "team" => %{"id" => "t1", "key" => "ENG", "name" => "Engineering"}, "comments" => %{"nodes" => []} } @@ -302,7 +331,9 @@ defmodule LinearCli.Linear.IssueTest do }) end) - assert {:ok, _updated} = Linear.close_issue(issue, "s1") + assert {:ok, updated} = Linear.close_issue(issue, "s1") + assert updated.state.type == "completed" + assert updated.state.name == "Done" end test "sends trashed: true when given via opts" do