agentpr: close and reopen issues

- add issue close and issue reopen subcommands - add GetIssue and SetIssueState to the Gitea client - fail when the issue is already in the requested state - document the new subcommands in README.md and AGENTS.md
2026-09-20 23:06:15 +10:00
parent d77607c4f0
commit 9de9dffab1
6 changed files with 303 additions and 11 deletions
@@ -946,3 +946,144 @@ func TestEditIssueSendsOnlySuppliedFields(t *testing.T) {
 		})
 	}
 }
+
+// Closing and reopening differ only in the state sent; both must read the
+// issue first and then PATCH the issue endpoint with that state alone.
+func TestSetIssueStateRequest(t *testing.T) {
+	tests := []struct {
+		name    string
+		current string
+		state   string
+	}{
+		{"close an open issue", IssueStateOpen, IssueStateClosed},
+		{"reopen a closed issue", IssueStateClosed, IssueStateOpen},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var methods []string
+			var gotBody map[string]any
+			var gotPath string
+			mux := http.NewServeMux()
+			mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
+				methods = append(methods, r.Method)
+				gotPath = r.URL.Path
+				if r.Method == http.MethodGet {
+					_, _ = io.WriteString(w, `{"number":12,"state":"`+tt.current+`","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
+					return
+				}
+				_ = json.NewDecoder(r.Body).Decode(&gotBody)
+				_, _ = io.WriteString(w, `{"number":12,"state":"`+tt.state+`","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
+			})
+			srv := httptest.NewServer(mux)
+			defer srv.Close()
+
+			c := &GiteaClient{BaseURL: srv.URL, Token: "gitea-abc", HTTP: srv.Client()}
+			issue, err := c.SetIssueState("unkin/repo", 12, tt.state)
+			if err != nil {
+				t.Fatalf("SetIssueState: %v", err)
+			}
+			if len(methods) != 2 || methods[0] != http.MethodGet || methods[1] != http.MethodPatch {
+				t.Errorf("requests = %v, want a GET then a PATCH", methods)
+			}
+			if gotPath != "/api/v1/repos/unkin/repo/issues/12" {
+				t.Errorf("path = %q", gotPath)
+			}
+			if len(gotBody) != 1 || gotBody["state"] != tt.state {
+				t.Errorf("payload = %v, want only {\"state\":%q}", gotBody, tt.state)
+			}
+			if issue.State != tt.state || issue.Number != 12 {
+				t.Errorf("parsed issue = %+v", issue)
+			}
+		})
+	}
+}
+
+// Gitea answers a no-op state change with 200, so an issue already in the
+// requested state must fail rather than report a change that never happened —
+// and no PATCH may be sent.
+func TestSetIssueStateAlreadyInState(t *testing.T) {
+	patches := 0
+	mux := http.NewServeMux()
+	mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodPatch {
+			patches++
+		}
+		_, _ = io.WriteString(w, `{"number":12,"state":"closed","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
+	})
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
+	_, err := c.SetIssueState("unkin/repo", 12, IssueStateClosed)
+	if err == nil {
+		t.Fatal("expected an error closing an already-closed issue")
+	}
+	if !errors.Is(err, ErrIssueStateUnchanged) {
+		t.Errorf("errors.Is(%v, ErrIssueStateUnchanged) = false", err)
+	}
+	if !strings.Contains(err.Error(), "unkin/repo#12") {
+		t.Errorf("error %q should name the issue", err)
+	}
+	if patches != 0 {
+		t.Errorf("PATCH requests = %d, want 0", patches)
+	}
+}
+
+// A non-2xx on either leg must surface the API's own message, not a bare
+// status, and must not be mistaken for a successful change.
+func TestSetIssueStateAPIError(t *testing.T) {
+	tests := []struct {
+		name   string
+		failOn string
+		status int
+	}{
+		{"read fails", http.MethodGet, http.StatusNotFound},
+		{"write fails", http.MethodPatch, http.StatusForbidden},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mux := http.NewServeMux()
+			mux.HandleFunc("/api/v1/repos/unkin/repo/issues/12", func(w http.ResponseWriter, r *http.Request) {
+				if r.Method == tt.failOn {
+					w.WriteHeader(tt.status)
+					_, _ = io.WriteString(w, `{"message":"no dice","url":"https://git.unkin.net/api/swagger","errors":null}`)
+					return
+				}
+				_, _ = io.WriteString(w, `{"number":12,"state":"open","title":"T","html_url":"https://git.unkin.net/unkin/repo/issues/12"}`)
+			})
+			srv := httptest.NewServer(mux)
+			defer srv.Close()
+
+			c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
+			_, err := c.SetIssueState("unkin/repo", 12, IssueStateClosed)
+			if err == nil {
+				t.Fatalf("expected an error when %s returns %d", tt.failOn, tt.status)
+			}
+			if !strings.Contains(err.Error(), `"message":"no dice"`) {
+				t.Errorf("error %q should carry the API message", err)
+			}
+		})
+	}
+}
+
+// Only Gitea's two states are accepted, and a bad one is rejected before any
+// request goes out.
+func TestSetIssueStateRejectsUnknownState(t *testing.T) {
+	requests := 0
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		requests++
+	}))
+	defer srv.Close()
+
+	c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()}
+	_, err := c.SetIssueState("unkin/repo", 12, "merged")
+	if err == nil {
+		t.Fatal("expected an error for an unknown state")
+	}
+	if !strings.Contains(err.Error(), `invalid issue state "merged"`) {
+		t.Errorf("error = %q, want it to name the invalid state", err)
+	}
+	if requests != 0 {
+		t.Errorf("requests = %d, want 0", requests)
+	}
+}
@@ -329,6 +329,46 @@ func (c *GiteaClient) EditIssue(repoPath string, number int, opts EditOptions) (
 	return issue, err
 }

+// Issue states Gitea accepts on a state change. Gitea has no third state: an
+// issue is open or closed.
+const (
+	IssueStateOpen   = "open"
+	IssueStateClosed = "closed"
+)
+
+// ErrIssueStateUnchanged reports a state change asked for the state the issue
+// is already in. Gitea answers such a PATCH with 200 and changes nothing, so
+// without this check closing an already-closed issue would look like it worked.
+var ErrIssueStateUnchanged = errors.New("issue is already in that state")
+
+// GetIssue fetches a single issue
+// (GET /api/v1/repos/{owner}/{repo}/issues/{index}).
+func (c *GiteaClient) GetIssue(repoPath string, number int) (Issue, error) {
+	var issue Issue
+	err := c.do(http.MethodGet, fmt.Sprintf("/api/v1/repos/%s/issues/%d", repoPath, number), nil, &issue)
+	return issue, err
+}
+
+// SetIssueState closes or reopens an issue. It reads the issue first so an
+// issue already in the requested state fails with ErrIssueStateUnchanged
+// instead of reporting a change that never happened.
+func (c *GiteaClient) SetIssueState(repoPath string, number int, state string) (Issue, error) {
+	if state != IssueStateOpen && state != IssueStateClosed {
+		return Issue{}, fmt.Errorf("invalid issue state %q: want %q or %q", state, IssueStateOpen, IssueStateClosed)
+	}
+	current, err := c.GetIssue(repoPath, number)
+	if err != nil {
+		return Issue{}, err
+	}
+	if current.State == state {
+		return current, fmt.Errorf("%s#%d: %w (%s)", repoPath, number, ErrIssueStateUnchanged, state)
+	}
+	var issue Issue
+	payload := map[string]string{"state": state}
+	err = c.do(http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/issues/%d", repoPath, number), payload, &issue)
+	return issue, err
+}
+
 // Comment is the subset of an issue comment we track.
 type Comment struct {
 	ID   int64  `json:"id"`