agentpr: close and reopen issues #19

Merged
benvin merged 1 commits from benvin/agentpr-issue-state into main 2026-09-20 23:08:35 +10:00
6 changed files with 303 additions and 11 deletions
+13 -7
View File
@@ -8,10 +8,11 @@ from Vault, so actions are attributed to the agent rather than to whoever runs
the tool. Setting `AGENT_LOGIN` selects a different agent identity, so a service
like repospawner can run these tools as itself.
- **`agentpr`** — create and edit pull requests and issues, and post comments on
either, as `unkin-agent` (fixes the "tea posts as Ben" attribution problem).
Subcommands: `pr create`, `pr comment`, `pr edit`, `issue create`,
`issue comment`, `issue edit`, `whoami`.
- **`agentpr`** — create and edit pull requests and issues, close and reopen
issues, and post comments on either, as `unkin-agent` (fixes the "tea posts
as Ben" attribution problem). Subcommands: `pr create`, `pr comment`,
`pr edit`, `issue create`, `issue comment`, `issue edit`, `issue close`,
`issue reopen`, `whoami`.
- **`watchpr`** — poll one or more PRs and exit when a tracked PR changes
meaningfully: it merges/closes, gets a new non-agent comment, its CI fails,
or it loses mergeability. Benign transitions (CI pending→success, the agent's
@@ -29,7 +30,7 @@ parsing, watch-state comparison, git worktree helpers).
## Structure
```
cmd/agentpr/main.go # agentpr CLI (pr + issue create/comment/edit, whoami)
cmd/agentpr/main.go # agentpr CLI (pr + issue create/comment/edit, issue close/reopen, whoami)
cmd/watchpr/main.go # watchpr CLI (poll + meaningful-change exit)
cmd/agentws/main.go # agentws CLI (new / list / rm / clean / token / credential)
cmd/agentws/prune.go # agentws prune (classify worktrees, remove the safe ones)
@@ -37,7 +38,7 @@ cmd/agentvault/main.go # agentvault CLI (seed-outpost / seed-oauth)
internal/agent/ # shared plumbing:
token.go # env config + in-process Gitea-token cache
vault.go # AppRole login + read the gitea creds path
gitea.go # Gitea REST client (PR/issue create/edit/get, comments, status, whoami)
gitea.go # Gitea REST client (PR/issue create/edit/get, issue state, comments, status, whoami)
parse.go # owner/repo#N and owner/repo parsing
watch.go # PRState snapshot + MeaningfulChange comparison
git.go # git worktree/clone/fetch helpers (os/exec, no go-git)
@@ -131,7 +132,8 @@ make test # go test -v -race ./...
`internal/agent` covers PR-ref parsing, the `MeaningfulChange` table (benign vs
alerting transitions), request-body construction, and the Vault+Gitea client
against `httptest` servers (fake AppRole login + gitea creds + PR/issue create
+ edit / comment / whoami / status). No live Vault/Gitea access is required for tests.
+ edit / close / reopen / comment / whoami / status). No live Vault/Gitea access
is required for tests.
## agentvault seed-outpost
@@ -223,3 +225,7 @@ wrapped per stage (login / read denied / write denied) via `ErrVaultDenied`.
- Gitea backs every PR with an issue of the same number and serves comments from
`/issues/{n}/comments`, so `agentpr pr comment` and `agentpr issue comment`
are one implementation under two flag names (`--pr` / `--issue`).
- `issue close`/`issue reopen` read the issue before the PATCH: Gitea answers a
no-op state change with 200, so without the read an already-closed issue would
report success. There is no `pr close`: closing a pull request is a human's
call, not an agent's.
+8 -2
View File
@@ -6,8 +6,8 @@ token from Vault, so automated PRs, comments and pushes are attributed to the
agent — not to whoever happens to run the command. Set `AGENT_LOGIN` to act as a
different agent identity.
- **`agentpr`** — create and edit pull requests and issues, and post comments on
either, as the agent user.
- **`agentpr`** — create and edit pull requests and issues, close and reopen
issues, and post comments on either, as the agent user.
- **`watchpr`** — poll one or more PRs and exit when one changes in a way worth
acting on.
- **`agentws`** — manage per-branch git worktrees for `unkin-agent`, cloning
@@ -68,6 +68,12 @@ agentpr issue comment --repo unkin/argocd-apps --issue 43 --body "Fixed in #44."
agentpr issue edit --repo unkin/argocd-apps --issue 43 --body "The pipeline fails with ..."
# prints: #<number> <html_url>
# Close or reopen an issue; an issue already in that state is an error, not a
# silent success
agentpr issue close --repo unkin/argocd-apps --issue 43
agentpr issue reopen --repo unkin/argocd-apps --issue 43
# prints: #<number> <state> <html_url>
agentpr --version
agentpr --help
```
+48 -2
View File
@@ -9,6 +9,8 @@
// agentpr issue create --repo owner/repo --title T --body B
// agentpr issue comment --repo owner/repo --issue 12 --body "..."
// agentpr issue edit --repo owner/repo --issue 12 --title T --body B
// agentpr issue close --repo owner/repo --issue 12
// agentpr issue reopen --repo owner/repo --issue 12
// agentpr whoami
package main
@@ -69,9 +71,15 @@ func newPRCmd() *cobra.Command {
func newIssueCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "issue",
Short: "File and edit issues, and post issue comments",
Short: "File, edit, close and reopen issues, and post issue comments",
}
cmd.AddCommand(newIssueCreateCmd(), newCommentCmd("issue", "issue", "Post a comment on an issue"), newIssueEditCmd())
cmd.AddCommand(
newIssueCreateCmd(),
newCommentCmd("issue", "issue", "Post a comment on an issue"),
newIssueEditCmd(),
newIssueStateCmd("close", "Close an issue", agent.IssueStateClosed),
newIssueStateCmd("reopen", "Reopen a closed issue", agent.IssueStateOpen),
)
return cmd
}
@@ -301,6 +309,44 @@ func newIssueEditCmd() *cobra.Command {
return cmd
}
// newIssueStateCmd builds `issue close` and `issue reopen`, which differ only
// in the state they ask for. An issue already in that state is an error, not a
// silent success: Gitea answers the PATCH with 200 either way.
func newIssueStateCmd(use, short, state string) *cobra.Command {
var repo string
var issue int
cmd := &cobra.Command{
Use: use,
Short: short,
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if issue <= 0 {
return fmt.Errorf("--issue must be a positive issue number")
}
c, err := client()
if err != nil {
return err
}
updated, err := c.SetIssueState(owner+"/"+name, issue, state)
if err != nil {
return err
}
fmt.Printf("#%d %s %s\n", updated.Number, updated.State, updated.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&issue, "issue", 0, "Issue number (required)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("issue")
return cmd
}
func newWhoamiCmd() *cobra.Command {
return &cobra.Command{
Use: "whoami",
+53
View File
@@ -149,3 +149,56 @@ func TestCommentCommandsStayInStep(t *testing.T) {
t.Error("issue comment must take --issue and only --issue")
}
}
// Both state commands need a repo and an issue number; cobra and the RunE
// guard must reject their absence before anything reaches for a token.
func TestIssueStateCommandsRequireFlags(t *testing.T) {
for _, verb := range []string{"close", "reopen"} {
t.Run(verb+" without --issue", func(t *testing.T) {
err := execute("issue", verb, "--repo", "unkin/repo")
if err == nil {
t.Fatal("Execute() = nil, want an error when --issue is missing")
}
if !strings.Contains(err.Error(), "issue") {
t.Errorf("error = %q, want it to name the missing --issue flag", err)
}
})
t.Run(verb+" without --repo", func(t *testing.T) {
err := execute("issue", verb, "--issue", "12")
if err == nil {
t.Fatal("Execute() = nil, want an error when --repo is missing")
}
if !strings.Contains(err.Error(), "repo") {
t.Errorf("error = %q, want it to name the missing --repo flag", err)
}
})
t.Run(verb+" with a zero --issue", func(t *testing.T) {
err := execute("issue", verb, "--repo", "unkin/repo", "--issue", "0")
if err == nil {
t.Fatal("Execute() = nil, want an error for a non-positive issue number")
}
if !strings.Contains(err.Error(), "--issue must be a positive") {
t.Errorf("error = %q, want it to reject the issue number", err)
}
})
t.Run(verb+" with a malformed --repo", func(t *testing.T) {
if err := execute("issue", verb, "--repo", "not-a-repo", "--issue", "12"); err == nil {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
})
}
}
// Closing and reopening are issue-only: a PR is closed by a human, so the pr
// group must not grow these verbs by accident.
func TestPRHasNoStateCommands(t *testing.T) {
for _, verb := range []string{"close", "reopen"} {
if cmd, _, err := newRootCmd().Find([]string{"pr", verb}); err == nil && cmd.Name() == verb {
t.Errorf("pr %s exists; closing a PR is not agentpr's to do", verb)
}
cmd, _, err := newRootCmd().Find([]string{"issue", verb})
if err != nil || cmd.Name() != verb {
t.Fatalf("issue %s not found: %v", verb, err)
}
}
}
+141
View File
@@ -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)
}
}
+40
View File
@@ -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"`