Merge main into watchpr auth failure branch
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful

Keep both sides' additions to client_test.go and adopt main's
EditPROptions -> EditOptions rename.
This commit is contained in:
2026-09-19 16:40:44 +10:00
10 changed files with 760 additions and 66 deletions
+132 -29
View File
@@ -6,6 +6,9 @@
// 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 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 whoami
package main
@@ -34,14 +37,14 @@ func main() {
func newRootCmd() *cobra.Command {
root := &cobra.Command{
Use: "agentpr",
Short: "Manage Gitea PRs and comments as an agent user.",
Long: "agentpr manages Gitea pull requests and comments as an agent user, using a\nGitea token minted from Vault (AppRole login + gitea/creds/<AGENT_LOGIN>).\nSet AGENT_LOGIN to act as another agent identity, or GITEA_CREDS_PATH to name\nthe Vault creds path outright.",
Short: "Manage Gitea PRs, issues and comments as an agent user.",
Long: "agentpr manages Gitea pull requests, issues and comments as an agent user,\nusing a Gitea token minted from Vault (AppRole login + gitea/creds/<AGENT_LOGIN>).\nSet AGENT_LOGIN to act as another agent identity, or GITEA_CREDS_PATH to name\nthe Vault creds path outright.",
Version: version,
SilenceUsage: true,
}
root.SetVersionTemplate("{{.Version}}\n")
root.AddCommand(newPRCmd(), newWhoamiCmd(), newVersionCmd())
root.AddCommand(newPRCmd(), newIssueCmd(), newWhoamiCmd(), newVersionCmd())
return root
}
@@ -59,7 +62,16 @@ func newPRCmd() *cobra.Command {
Use: "pr",
Short: "Create and edit PRs, and post PR comments",
}
cmd.AddCommand(newPRCreateCmd(), newPRCommentCmd(), newPREditCmd())
cmd.AddCommand(newPRCreateCmd(), newCommentCmd("pr", "PR", "Post a comment on a pull request"), newPREditCmd())
return cmd
}
func newIssueCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "issue",
Short: "File and edit issues, and post issue comments",
}
cmd.AddCommand(newIssueCreateCmd(), newCommentCmd("issue", "issue", "Post a comment on an issue"), newIssueEditCmd())
return cmd
}
@@ -104,20 +116,24 @@ func newPRCreateCmd() *cobra.Command {
return cmd
}
func newPRCommentCmd() *cobra.Command {
// newCommentCmd builds a comment command whose number flag is named numFlag.
// Gitea backs every PR with an issue of the same number and serves comments
// from the issue endpoint, so `pr comment` and `issue comment` are one command
// under two flag names rather than two implementations that could drift.
func newCommentCmd(numFlag, noun, short string) *cobra.Command {
var repo, body string
var pr int
var number int
cmd := &cobra.Command{
Use: "comment",
Short: "Post a comment on a pull request",
Short: short,
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")
if number <= 0 {
return fmt.Errorf("--%s must be a positive %s number", numFlag, noun)
}
if body == "" {
return fmt.Errorf("--body is required")
@@ -126,20 +142,20 @@ func newPRCommentCmd() *cobra.Command {
if err != nil {
return err
}
cm, err := c.CreateComment(owner+"/"+name, pr, body)
cm, err := c.CreateComment(owner+"/"+name, number, body)
if err != nil {
return err
}
fmt.Printf("comment %d posted on %s/%s#%d\n", cm.ID, owner, name, pr)
fmt.Printf("comment %d posted on %s/%s#%d\n", cm.ID, owner, name, number)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.IntVar(&pr, "pr", 0, "PR number (required)")
f.IntVar(&number, numFlag, 0, noun+" number (required)")
f.StringVar(&body, "body", "", "Comment body (required)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("pr")
_ = cmd.MarkFlagRequired(numFlag)
_ = cmd.MarkFlagRequired("body")
return cmd
}
@@ -159,22 +175,9 @@ func newPREditCmd() *cobra.Command {
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") {
// 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") {
opts.Body = &body
}
if opts.Title == nil && opts.Body == nil {
return fmt.Errorf("at least one of --title or --body is required")
opts, err := editOptions(cmd, title, body)
if err != nil {
return err
}
c, err := client()
if err != nil {
@@ -198,6 +201,106 @@ func newPREditCmd() *cobra.Command {
return cmd
}
// editOptions turns the --title/--body flags actually given into an edit
// payload. Only the flags present are sent: omitting --title must leave the
// title as it is, not blank it.
func editOptions(cmd *cobra.Command, title, body string) (agent.EditOptions, error) {
var opts agent.EditOptions
if cmd.Flags().Changed("title") {
// Gitea ignores an empty title, so sending one would report success
// while changing nothing.
if title == "" {
return opts, fmt.Errorf("--title cannot be empty: a title can be set but not cleared")
}
opts.Title = &title
}
if cmd.Flags().Changed("body") {
opts.Body = &body
}
if opts.Title == nil && opts.Body == nil {
return opts, fmt.Errorf("at least one of --title or --body is required")
}
return opts, nil
}
func newIssueCreateCmd() *cobra.Command {
var repo, title, body string
cmd := &cobra.Command{
Use: "create",
Short: "File an issue",
SilenceUsage: true,
RunE: func(cmd *cobra.Command, args []string) error {
owner, name, err := agent.ParseRepo(repo)
if err != nil {
return err
}
if title == "" {
return fmt.Errorf("--title is required")
}
c, err := client()
if err != nil {
return err
}
issue, err := c.CreateIssue(owner+"/"+name, agent.CreateIssueOptions{
Title: title,
Body: body,
})
if err != nil {
return err
}
fmt.Printf("#%d %s\n", issue.Number, issue.HTMLURL)
return nil
},
}
f := cmd.Flags()
f.StringVar(&repo, "repo", "", "Repository as owner/repo (required)")
f.StringVar(&title, "title", "", "Issue title (required)")
f.StringVar(&body, "body", "", "Issue body")
_ = cmd.MarkFlagRequired("repo")
return cmd
}
func newIssueEditCmd() *cobra.Command {
var repo, title, body string
var issue int
cmd := &cobra.Command{
Use: "edit",
Short: "Edit an issue'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 issue <= 0 {
return fmt.Errorf("--issue must be a positive issue number")
}
opts, err := editOptions(cmd, title, body)
if err != nil {
return err
}
c, err := client()
if err != nil {
return err
}
updated, err := c.EditIssue(owner+"/"+name, issue, 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(&issue, "issue", 0, "Issue number (required)")
f.StringVar(&title, "title", "", "New issue title (unchanged when omitted)")
f.StringVar(&body, "body", "", "New issue body (unchanged when omitted)")
_ = cmd.MarkFlagRequired("repo")
_ = cmd.MarkFlagRequired("issue")
return cmd
}
func newWhoamiCmd() *cobra.Command {
return &cobra.Command{
Use: "whoami",
+100
View File
@@ -4,6 +4,8 @@ import (
"io"
"strings"
"testing"
"github.com/spf13/cobra"
)
// A malformed --repo must fail the command (so main exits non-zero). ParseRepo
@@ -49,3 +51,101 @@ func TestPREditRejectsEmptyTitle(t *testing.T) {
t.Errorf("error = %q, want it to reject the empty title", err)
}
}
// execute runs the command tree with args, discarding output, so tests assert
// on the error alone. Every case here fails before any Vault/Gitea call.
func execute(args ...string) error {
cmd := newRootCmd()
cmd.SetArgs(args)
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
return cmd.Execute()
}
// An issue needs a title; Gitea rejects an empty one, so the command must too.
func TestIssueCreateRequiresTitle(t *testing.T) {
err := execute("issue", "create", "--repo", "unkin/repo", "--body", "b")
if err == nil {
t.Fatal("Execute() = nil, want an error when --title is missing")
}
if !strings.Contains(err.Error(), "--title is required") {
t.Errorf("error = %q, want it to name the missing flag", err)
}
}
// --repo is required, and cobra must reject its absence before anything reaches
// for a token.
func TestIssueCreateRequiresRepo(t *testing.T) {
err := execute("issue", "create", "--title", "t")
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 flag", err)
}
}
func TestIssueCreateBadRepoErrors(t *testing.T) {
if err := execute("issue", "create", "--repo", "not-a-repo", "--title", "t"); err == nil {
t.Fatal("Execute() = nil, want error for a malformed --repo")
}
}
// `issue comment` addresses the issue by --issue, not --pr, and needs it.
func TestIssueCommentRequiresIssueNumber(t *testing.T) {
err := execute("issue", "comment", "--repo", "unkin/repo", "--body", "hi")
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)
}
}
func TestIssueEditRequiresTitleOrBody(t *testing.T) {
err := execute("issue", "edit", "--repo", "unkin/repo", "--issue", "12")
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)
}
}
func TestIssueEditRejectsEmptyTitle(t *testing.T) {
err := execute("issue", "edit", "--repo", "unkin/repo", "--issue", "12", "--title", "")
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)
}
}
// PRs and issues share Gitea's comment endpoint, so both comment commands are
// built from one constructor: they must stay identical apart from the flag
// naming the number.
func TestCommentCommandsStayInStep(t *testing.T) {
find := func(group string) *cobra.Command {
t.Helper()
cmd, _, err := newRootCmd().Find([]string{group, "comment"})
if err != nil || cmd.Name() != "comment" {
t.Fatalf("%s comment not found: %v", group, err)
}
return cmd
}
has := func(cmd *cobra.Command, name string) bool { return cmd.Flags().Lookup(name) != nil }
prCmd, issueCmd := find("pr"), find("issue")
for _, name := range []string{"repo", "body"} {
if !has(prCmd, name) || !has(issueCmd, name) {
t.Errorf("--%s missing: pr=%t issue=%t", name, has(prCmd, name), has(issueCmd, name))
}
}
if !has(prCmd, "pr") || has(prCmd, "issue") {
t.Error("pr comment must take --pr and only --pr")
}
if !has(issueCmd, "issue") || has(issueCmd, "pr") {
t.Error("issue comment must take --issue and only --issue")
}
}
+70 -8
View File
@@ -161,13 +161,24 @@ func newNewCmd() *cobra.Command {
return err
}
// c. Base branch: --from or the remote default.
// c. A branch origin already has is work in progress and must be
// checked out as it stands; only a branch origin does not have is
// forked from a base.
onRemote := agent.GitRemoteBranchExists(srcDir, "origin", branch)
startPoint := "origin/" + branch
base := from
if base == "" {
base, err = agent.GitRemoteDefaultBranch(srcDir, "origin")
if err != nil {
return err
if onRemote {
if from != "" {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "note: --from %s ignored, origin/%s already exists\n", from, branch)
}
} else {
if base == "" {
base, err = agent.GitRemoteDefaultBranch(srcDir, "origin")
if err != nil {
return err
}
}
startPoint = "origin/" + base
}
// d. Create the worktree FROM the source checkout so the branch is
@@ -176,7 +187,7 @@ func newNewCmd() *cobra.Command {
if _, statErr := os.Stat(wtPath); statErr == nil {
return fmt.Errorf("worktree already exists at %s", wtPath)
}
if err := agent.GitWorktreeAdd(srcDir, wtPath, branch, "origin/"+base); err != nil {
if err := agent.GitWorktreeAdd(srcDir, wtPath, branch, startPoint); err != nil {
return err
}
@@ -199,9 +210,19 @@ func newNewCmd() *cobra.Command {
return err
}
// f. Report the worktree path and branch.
// f. A local branch left from an earlier run may sit behind origin,
// so reusing it is not enough on its own.
summary := fmt.Sprintf("branch %s (new, from origin/%s)", branch, base)
if onRemote {
summary, err = alignToRemote(cmd.OutOrStdout(), wtPath, branch)
if err != nil {
return err
}
}
// g. Report the worktree path and which of the two paths was taken.
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", wtPath)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "branch %s (from origin/%s)\n", branch, base)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", summary)
return nil
},
}
@@ -212,6 +233,47 @@ func newNewCmd() *cobra.Command {
return cmd
}
// alignToRemote puts the worktree on origin/<branch> and reports what that took.
// A reused local branch can be stale, and a fast-forward is the only move that
// adds no commit and drops none; a local branch carrying commits origin does not
// have is left where it stands, because those commits exist nowhere else.
func alignToRemote(out io.Writer, wtPath, branch string) (string, error) {
remoteRef := "origin/" + branch
want, err := agent.GitRevParse(wtPath, remoteRef)
if err != nil {
return "", err
}
head, err := agent.GitRevParse(wtPath, "HEAD")
if err != nil {
return "", err
}
if head != want {
ahead, err := agent.GitAheadCount(wtPath, remoteRef, "HEAD")
if err != nil {
return "", err
}
if ahead > 0 {
return fmt.Sprintf("branch %s (local, %s not on %s, left at %s)",
branch, commitCount(ahead), remoteRef, shortSHA(head)), nil
}
if err := agent.GitMergeFFOnly(wtPath, remoteRef); err != nil {
return "", err
}
_, _ = fmt.Fprintf(out, "fast-forwarded stale %s to %s\n", branch, remoteRef)
}
if err := agent.GitSetUpstream(wtPath, branch, remoteRef); err != nil {
return "", err
}
return fmt.Sprintf("branch %s (tracking %s at %s)", branch, remoteRef, shortSHA(want)), nil
}
func shortSHA(sha string) string {
if len(sha) > 7 {
return sha[:7]
}
return sha
}
// --- list -----------------------------------------------------------------
func newListCmd() *cobra.Command {
+192
View File
@@ -0,0 +1,192 @@
package main
import (
"bytes"
"path/filepath"
"strings"
"testing"
"git.unkin.net/unkin/agent-tools/internal/agent"
)
// newFixtureOn builds the same origin/source/worktree-root layout as the prune
// fixture but with a chosen default branch, so `new` can be tested against a
// repo whose default is not "main".
func newFixtureOn(t *testing.T, defBranch string) *fixture {
t.Helper()
root := t.TempDir()
f := &fixture{
root: root,
bare: filepath.Join(root, "origin.git"),
srcDir: filepath.Join(root, "src", "repo"),
wtRoot: filepath.Join(root, "worktrees"),
}
git(t, root, "init", "--bare", "-b", defBranch, f.bare)
seed := filepath.Join(root, "seed")
git(t, root, "init", "-b", defBranch, seed)
identity(t, seed)
writeCommit(t, seed, "README.md", "hi\n", "init")
git(t, seed, "remote", "add", "origin", f.bare)
git(t, seed, "push", "-u", "origin", defBranch)
git(t, root, "clone", f.bare, f.srcDir)
identity(t, f.srcDir)
t.Setenv("AGENTWS_ROOT", f.wtRoot)
t.Setenv("AGENTWS_SRC_ROOT", filepath.Join(root, "src"))
t.Setenv("AGENTWS_OWNER", "unkin")
return f
}
// runNew invokes `agentws new repo --branch <branch>` and returns its output.
func runNew(t *testing.T, branch string, extra ...string) string {
t.Helper()
var out bytes.Buffer
cmd := newRootCmd()
cmd.SetArgs(append([]string{"new", "repo", "--branch", branch}, extra...))
cmd.SetOut(&out)
cmd.SetErr(&out)
if err := cmd.Execute(); err != nil {
t.Fatalf("new %s: %v (output %q)", branch, err, out.String())
}
return out.String()
}
// pushBranch creates branch on origin carrying one commit and returns its SHA.
func pushBranch(t *testing.T, f *fixture, branch, file, content string) string {
t.Helper()
seed := filepath.Join(f.root, "seed")
git(t, seed, "checkout", "-b", branch)
writeCommit(t, seed, file, content, "work on "+branch)
git(t, seed, "push", "origin", branch)
return git(t, seed, "rev-parse", "HEAD")
}
func wtPathFor(f *fixture, branch string) string {
return filepath.Join(f.wtRoot, agent.WorktreeDirName("repo", branch))
}
// The regression: a branch that already exists on origin must be checked out at
// origin's tip, not forked from the default branch.
func TestNewChecksOutExistingRemoteBranch(t *testing.T) {
f := newFixtureOn(t, "main")
want := pushBranch(t, f, "benvin/existing", "a.txt", "a\n")
out := runNew(t, "benvin/existing")
path := wtPathFor(f, "benvin/existing")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/existing %s", got, want)
}
if upstream := git(t, path, "rev-parse", "--abbrev-ref", "HEAD@{upstream}"); upstream != "origin/benvin/existing" {
t.Errorf("upstream = %q, want origin/benvin/existing", upstream)
}
if !contains(out, "tracking origin/benvin/existing") {
t.Errorf("output %q does not say the remote branch was checked out", out)
}
}
// A branch origin does not have is still forked from the default branch, and the
// output must say so rather than leaving the caller to guess.
func TestNewForksBranchMissingFromRemote(t *testing.T) {
f := newFixtureOn(t, "main")
want := git(t, f.srcDir, "rev-parse", "origin/main")
out := runNew(t, "benvin/fresh")
path := wtPathFor(f, "benvin/fresh")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/main %s", got, want)
}
if !contains(out, "branch benvin/fresh (new, from origin/main)") {
t.Errorf("output %q does not report a new branch", out)
}
}
// The base is the remote's own default branch, so a repo defaulting to master
// forks from master.
func TestNewForksFromMasterDefaultBranch(t *testing.T) {
f := newFixtureOn(t, "master")
want := git(t, f.srcDir, "rev-parse", "origin/master")
out := runNew(t, "benvin/on-master")
path := wtPathFor(f, "benvin/on-master")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/master %s", got, want)
}
if !contains(out, "from origin/master") {
t.Errorf("output %q does not name origin/master as the base", out)
}
}
// An existing remote branch beats --from: the flag is ignored and the caller is
// told, rather than the branch being silently re-forked.
func TestNewIgnoresFromWhenBranchIsOnRemote(t *testing.T) {
f := newFixtureOn(t, "main")
want := pushBranch(t, f, "benvin/with-from", "a.txt", "a\n")
out := runNew(t, "benvin/with-from", "--from", "main")
path := wtPathFor(f, "benvin/with-from")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/with-from %s", got, want)
}
if !contains(out, "--from main ignored") {
t.Errorf("output %q does not report the ignored --from", out)
}
}
// A local branch left behind by an earlier run must not pin the worktree to a
// commit origin has moved past.
func TestNewFastForwardsStaleLocalBranch(t *testing.T) {
f := newFixtureOn(t, "main")
stale := pushBranch(t, f, "benvin/stale", "a.txt", "a\n")
git(t, f.srcDir, "fetch", "origin")
git(t, f.srcDir, "branch", "benvin/stale", "origin/benvin/stale")
seed := filepath.Join(f.root, "seed")
writeCommit(t, seed, "b.txt", "b\n", "more work")
git(t, seed, "push", "origin", "benvin/stale")
want := git(t, seed, "rev-parse", "HEAD")
if want == stale {
t.Fatal("fixture did not move origin/benvin/stale on")
}
out := runNew(t, "benvin/stale")
path := wtPathFor(f, "benvin/stale")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want origin/benvin/stale %s", got, want)
}
if !contains(out, "fast-forwarded stale benvin/stale") {
t.Errorf("output %q does not report the fast-forward", out)
}
}
// A local branch carrying commits origin does not have keeps them: they exist
// nowhere else, so the worktree stays put and the output says so.
func TestNewKeepsLocalCommitsAheadOfRemote(t *testing.T) {
f := newFixtureOn(t, "main")
pushBranch(t, f, "benvin/ahead", "a.txt", "a\n")
git(t, f.srcDir, "fetch", "origin")
git(t, f.srcDir, "checkout", "-b", "benvin/ahead", "origin/benvin/ahead")
writeCommit(t, f.srcDir, "local.txt", "local\n", "local only")
want := git(t, f.srcDir, "rev-parse", "HEAD")
git(t, f.srcDir, "checkout", "main")
out := runNew(t, "benvin/ahead")
path := wtPathFor(f, "benvin/ahead")
if got := git(t, path, "rev-parse", "HEAD"); got != want {
t.Errorf("worktree HEAD = %s, want the local tip %s", got, want)
}
if !contains(out, "1 commit not on origin/benvin/ahead") {
t.Errorf("output %q does not report the unpushed commit", out)
}
}
func contains(haystack, needle string) bool {
return strings.Contains(haystack, needle)
}