From 7bc4082cb01493dce3fadd0f038ce1f02c87e3ce Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 19 Sep 2026 12:32:07 +1000 Subject: [PATCH 1/3] Add agentpr pr edit subcommand Update a PR's title and/or body as the agent user, sending only the fields supplied. --- AGENTS.md | 10 ++--- README.md | 8 +++- cmd/agentpr/main.go | 54 ++++++++++++++++++++++++- cmd/agentpr/main_test.go | 17 ++++++++ internal/agent/client_test.go | 74 +++++++++++++++++++++++++++++++++++ internal/agent/gitea.go | 16 ++++++++ 6 files changed, 171 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 46669a2..00f79fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,9 +8,9 @@ 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 pull requests and post PR comments as `unkin-agent` - (fixes the "tea posts as Ben" attribution problem). Subcommands: - `pr create`, `pr comment`, `whoami`. +- **`agentpr`** — create and edit pull requests, and post PR comments, as + `unkin-agent` (fixes the "tea posts as Ben" attribution problem). + Subcommands: `pr create`, `pr comment`, `pr edit`, `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 @@ -28,7 +28,7 @@ parsing, watch-state comparison, git worktree helpers). ## Structure ``` -cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / whoami) +cmd/agentpr/main.go # agentpr CLI (pr create / pr comment / pr edit / 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) @@ -36,7 +36,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 create/get, comments, status, whoami) + gitea.go # Gitea REST client (PR create/edit/get, 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) diff --git a/README.md b/README.md index 27f0cb5..c407371 100644 --- a/README.md +++ b/README.md @@ -6,7 +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 pull requests and post PR comments as the agent user. +- **`agentpr`** — create and edit pull requests, and post PR comments 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 @@ -50,6 +51,11 @@ agentpr pr create --repo unkin/argocd-apps \ # Comment on a PR agentpr pr comment --repo unkin/argocd-apps --pr 42 --body "Rebased, CI green." +# Edit a PR's title and/or body; an omitted flag is left unchanged +agentpr pr edit --repo unkin/argocd-apps --pr 42 --body "Adds the ServiceAccount ..." +agentpr pr edit --repo unkin/argocd-apps --pr 42 --title "Add woodpecker SA" +# prints: # + agentpr --version agentpr --help ``` diff --git a/cmd/agentpr/main.go b/cmd/agentpr/main.go index 5f12952..fe43423 100644 --- a/cmd/agentpr/main.go +++ b/cmd/agentpr/main.go @@ -5,6 +5,7 @@ // // agentpr pr create --repo owner/repo --base main --head feature --title T --body B // agentpr pr comment --repo owner/repo --pr 12 --body "..." +// agentpr pr edit --repo owner/repo --pr 12 --title T --body B // agentpr whoami package main @@ -56,9 +57,9 @@ func client() (*agent.GiteaClient, error) { func newPRCmd() *cobra.Command { cmd := &cobra.Command{ Use: "pr", - Short: "Create PRs and post PR comments", + Short: "Create and edit PRs, and post PR comments", } - cmd.AddCommand(newPRCreateCmd(), newPRCommentCmd()) + cmd.AddCommand(newPRCreateCmd(), newPRCommentCmd(), newPREditCmd()) return cmd } @@ -143,6 +144,55 @@ func newPRCommentCmd() *cobra.Command { return cmd } +func newPREditCmd() *cobra.Command { + var repo, title, body string + var pr int + cmd := &cobra.Command{ + Use: "edit", + Short: "Edit a pull request's title and/or body", + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + owner, name, err := agent.ParseRepo(repo) + if err != nil { + return err + } + if pr <= 0 { + return fmt.Errorf("--pr must be a positive PR number") + } + // Only the flags actually given are sent: omitting --title must + // leave the title as it is, not blank it. + var opts agent.EditPROptions + if cmd.Flags().Changed("title") { + opts.Title = &title + } + if cmd.Flags().Changed("body") { + opts.Body = &body + } + if opts.Title == nil && opts.Body == nil { + return fmt.Errorf("at least one of --title or --body is required") + } + c, err := client() + if err != nil { + return err + } + updated, err := c.EditPR(owner+"/"+name, pr, opts) + if err != nil { + return err + } + fmt.Printf("#%d %s\n", updated.Number, updated.HTMLURL) + return nil + }, + } + f := cmd.Flags() + f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)") + f.IntVar(&pr, "pr", 0, "PR number (required)") + f.StringVar(&title, "title", "", "New PR title (unchanged when omitted)") + f.StringVar(&body, "body", "", "New PR body (unchanged when omitted)") + _ = cmd.MarkFlagRequired("repo") + _ = cmd.MarkFlagRequired("pr") + return cmd +} + func newWhoamiCmd() *cobra.Command { return &cobra.Command{ Use: "whoami", diff --git a/cmd/agentpr/main_test.go b/cmd/agentpr/main_test.go index 899ff3a..b37f47f 100644 --- a/cmd/agentpr/main_test.go +++ b/cmd/agentpr/main_test.go @@ -2,6 +2,7 @@ package main import ( "io" + "strings" "testing" ) @@ -16,3 +17,19 @@ func TestExecuteBadRepoErrors(t *testing.T) { t.Fatal("Execute() = nil, want error for a malformed --repo") } } + +// `pr edit` with neither --title nor --body has nothing to send; it must fail +// with a usage error before any Vault/Gitea call, so this stays hermetic. +func TestPREditRequiresTitleOrBody(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"pr", "edit", "--repo", "unkin/repo", "--pr", "7"}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + if err == nil { + t.Fatal("Execute() = nil, want an error when neither --title nor --body is given") + } + if !strings.Contains(err.Error(), "--title or --body") { + t.Errorf("error = %q, want it to name the missing flags", err) + } +} diff --git a/internal/agent/client_test.go b/internal/agent/client_test.go index 47757b8..b13bc15 100644 --- a/internal/agent/client_test.go +++ b/internal/agent/client_test.go @@ -98,6 +98,80 @@ func TestCreatePRRequestBody(t *testing.T) { } } +// An edit must send only the fields it was given: Gitea overwrites whatever +// key it receives, so an omitted --title arriving as "" would blank the title. +func TestEditPRSendsOnlySuppliedFields(t *testing.T) { + title, body, empty := "new title", "new body", "" + tests := []struct { + name string + opts EditPROptions + want map[string]any + }{ + {"body only", EditPROptions{Body: &body}, map[string]any{"body": "new body"}}, + {"title only", EditPROptions{Title: &title}, map[string]any{"title": "new title"}}, + {"both", EditPROptions{Title: &title, Body: &body}, map[string]any{"title": "new title", "body": "new body"}}, + {"explicit empty body is sent", EditPROptions{Body: &empty}, map[string]any{"body": ""}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotBody map[string]any + var gotMethod, gotPath string + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) { + gotMethod, gotPath = r.Method, r.URL.Path + _ = json.NewDecoder(r.Body).Decode(&gotBody) + _, _ = io.WriteString(w, `{"number":7,"title":"new title","html_url":"https://git.unkin.net/unkin/repo/pulls/7"}`) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + c := &GiteaClient{BaseURL: srv.URL, Token: "gitea-abc", HTTP: srv.Client()} + pr, err := c.EditPR("unkin/repo", 7, tt.opts) + if err != nil { + t.Fatalf("EditPR: %v", err) + } + if gotMethod != http.MethodPatch { + t.Errorf("method = %s, want PATCH", gotMethod) + } + if gotPath != "/api/v1/repos/unkin/repo/pulls/7" { + t.Errorf("path = %q", gotPath) + } + if len(gotBody) != len(tt.want) { + t.Errorf("payload = %v, want exactly the supplied fields %v", gotBody, tt.want) + } + for k, v := range tt.want { + if gotBody[k] != v { + t.Errorf("payload[%q] = %v, want %v", k, gotBody[k], v) + } + } + if pr.Number != 7 || pr.HTMLURL == "" { + t.Errorf("parsed PR = %+v", pr) + } + }) + } +} + +// A non-2xx must surface the API's own message rather than a bare status. +func TestEditPRAPIError(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = io.WriteString(w, `{"message":"title cannot be empty"}`) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + title := "" + c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()} + _, err := c.EditPR("unkin/repo", 7, EditPROptions{Title: &title}) + if err == nil { + t.Fatal("expected error on 422") + } + if !strings.Contains(err.Error(), "title cannot be empty") { + t.Errorf("error %q should carry the API message", err) + } +} + func TestCreateComment(t *testing.T) { var gotBody map[string]string mux := http.NewServeMux() diff --git a/internal/agent/gitea.go b/internal/agent/gitea.go index 7a3be19..fa7c516 100644 --- a/internal/agent/gitea.go +++ b/internal/agent/gitea.go @@ -214,6 +214,22 @@ func (c *GiteaClient) CreatePR(repoPath string, opts CreatePROptions) (PullReque return pr, err } +// EditPROptions are the fields an edit may change. Pointers so an unset field +// is omitted from the payload entirely: Gitea overwrites whatever it is sent, +// so a nil Title leaves the title alone while a pointer to "" clears it. +type EditPROptions struct { + Title *string `json:"title,omitempty"` + Body *string `json:"body,omitempty"` +} + +// EditPR updates a pull request's title and/or body +// (PATCH /api/v1/repos/{owner}/{repo}/pulls/{index}). +func (c *GiteaClient) EditPR(repoPath string, number int, opts EditPROptions) (PullRequest, error) { + var pr PullRequest + err := c.do(http.MethodPatch, fmt.Sprintf("/api/v1/repos/%s/pulls/%d", repoPath, number), opts, &pr) + return pr, err +} + // GetPR fetches a single pull request. func (c *GiteaClient) GetPR(repoPath string, number int) (PullRequest, error) { var pr PullRequest -- 2.47.3 From 78d83b7a611bd67bea542b0a7ac8891b5d84f27b Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 19 Sep 2026 12:38:12 +1000 Subject: [PATCH 2/3] Reject an empty pr edit title Gitea would ignore --- cmd/agentpr/main.go | 5 +++++ cmd/agentpr/main_test.go | 16 ++++++++++++++++ internal/agent/client_test.go | 10 +++++----- internal/agent/gitea.go | 6 ++++-- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/cmd/agentpr/main.go b/cmd/agentpr/main.go index fe43423..f5faf40 100644 --- a/cmd/agentpr/main.go +++ b/cmd/agentpr/main.go @@ -163,6 +163,11 @@ func newPREditCmd() *cobra.Command { // leave the title as it is, not blank it. var opts agent.EditPROptions if cmd.Flags().Changed("title") { + // Gitea ignores an empty title, so sending one would report + // success while changing nothing. + if title == "" { + return fmt.Errorf("--title cannot be empty: a title can be set but not cleared") + } opts.Title = &title } if cmd.Flags().Changed("body") { diff --git a/cmd/agentpr/main_test.go b/cmd/agentpr/main_test.go index b37f47f..b3faecf 100644 --- a/cmd/agentpr/main_test.go +++ b/cmd/agentpr/main_test.go @@ -33,3 +33,19 @@ func TestPREditRequiresTitleOrBody(t *testing.T) { t.Errorf("error = %q, want it to name the missing flags", err) } } + +// Gitea silently ignores an empty title, so `pr edit --title ""` would report +// success while changing nothing; it must fail before any Vault/Gitea call. +func TestPREditRejectsEmptyTitle(t *testing.T) { + cmd := newRootCmd() + cmd.SetArgs([]string{"pr", "edit", "--repo", "unkin/repo", "--pr", "7", "--title", ""}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + err := cmd.Execute() + if err == nil { + t.Fatal("Execute() = nil, want an error for an empty --title") + } + if !strings.Contains(err.Error(), "--title cannot be empty") { + t.Errorf("error = %q, want it to reject the empty title", err) + } +} diff --git a/internal/agent/client_test.go b/internal/agent/client_test.go index b13bc15..e0cb8c8 100644 --- a/internal/agent/client_test.go +++ b/internal/agent/client_test.go @@ -155,19 +155,19 @@ func TestEditPRSendsOnlySuppliedFields(t *testing.T) { func TestEditPRAPIError(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusUnprocessableEntity) - _, _ = io.WriteString(w, `{"message":"title cannot be empty"}`) + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"message":"pull request does not exist"}`) }) srv := httptest.NewServer(mux) defer srv.Close() - title := "" + title := "new title" c := &GiteaClient{BaseURL: srv.URL, Token: "t", HTTP: srv.Client()} _, err := c.EditPR("unkin/repo", 7, EditPROptions{Title: &title}) if err == nil { - t.Fatal("expected error on 422") + t.Fatal("expected error on 404") } - if !strings.Contains(err.Error(), "title cannot be empty") { + if !strings.Contains(err.Error(), "pull request does not exist") { t.Errorf("error %q should carry the API message", err) } } diff --git a/internal/agent/gitea.go b/internal/agent/gitea.go index fa7c516..2f7fd74 100644 --- a/internal/agent/gitea.go +++ b/internal/agent/gitea.go @@ -215,8 +215,10 @@ func (c *GiteaClient) CreatePR(repoPath string, opts CreatePROptions) (PullReque } // EditPROptions are the fields an edit may change. Pointers so an unset field -// is omitted from the payload entirely: Gitea overwrites whatever it is sent, -// so a nil Title leaves the title alone while a pointer to "" clears it. +// is omitted from the payload entirely, leaving that field as it is. The two +// fields are not symmetric: Gitea only applies a title when it is non-empty, +// so Title can be set but never cleared and a "" title is a silent no-op, +// while a pointer to "" Body really does blank the body. type EditPROptions struct { Title *string `json:"title,omitempty"` Body *string `json:"body,omitempty"` -- 2.47.3 From 3f990c841d0ffd909fbb41dca6678c0add7412a1 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 19 Sep 2026 12:44:00 +1000 Subject: [PATCH 3/3] Mock the 404 body Gitea really returns --- internal/agent/client_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/agent/client_test.go b/internal/agent/client_test.go index e0cb8c8..c1aacfc 100644 --- a/internal/agent/client_test.go +++ b/internal/agent/client_test.go @@ -156,7 +156,7 @@ func TestEditPRAPIError(t *testing.T) { mux := http.NewServeMux() mux.HandleFunc("/api/v1/repos/unkin/repo/pulls/7", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) - _, _ = io.WriteString(w, `{"message":"pull request does not exist"}`) + _, _ = io.WriteString(w, `{"message":"not found","url":"https://git.unkin.net/api/swagger","errors":null}`) }) srv := httptest.NewServer(mux) defer srv.Close() @@ -167,7 +167,7 @@ func TestEditPRAPIError(t *testing.T) { if err == nil { t.Fatal("expected error on 404") } - if !strings.Contains(err.Error(), "pull request does not exist") { + if !strings.Contains(err.Error(), `"message":"not found"`) { t.Errorf("error %q should carry the API message", err) } } -- 2.47.3