feat(deliver): baton deliver pushes a ready feature and opens its pull request

README.md

@@ -10,7 +10,7 @@ You talk to one agent, the orchestrator. It reads the ticket, plans the work as
 
 - macOS, git 2.36 or newer, and tmux 3.5 or newer.
 - [GitHub Copilot CLI](https://docs.github.com/copilot/how-tos/copilot-cli), logged in with `copilot login`, or with a token in `COPILOT_GITHUB_TOKEN`, `GH_TOKEN` or `GITHUB_TOKEN`: `gh` doesn't work in the roles' panes, so their `copilot` can't take its login from `gh`.
-- `gh` for GitHub pull requests; a Bitbucket Cloud API token for Bitbucket pull requests; a read-only Jira API token for Jira tickets.
+- `gh`, logged in with `gh auth login`, for GitHub pull requests; a Bitbucket Cloud API token for Bitbucket pull requests; a read-only Jira API token for Jira tickets.
 
 ## Config
 
@@ -35,6 +35,12 @@ For example, one gate and QA on another model with a cap, for every repo, and tw
 
 baton checks the whole file each time it reads it, and refuses a file with a mistake, naming the file and the mistake.
 
+## Pull requests
+
+When a feature is ready, the orchestrator reports it to you, with a title for its pull request. On your word, it delivers the feature with `baton deliver <slug> --title "<title>"`, which baton's guard asks you about first; you can run it too. baton pushes the feature's branch to `origin`, with your git, then opens its pull request with your `gh`, into the GitHub repo's default branch, with the lead's final report as its description, as it is: read the report first, at the path that the orchestrator gives, if you want to see what goes out. If you hand the feature back for changes, deliver it again once it's ready, without `--title`: baton pushes the new commits and comments the lead's new report on the same pull request. baton never merges.
+
+gh picks the GitHub repo from the repo's remotes, as `gh pr create` does, so baton doesn't handle a fork with an `upstream` remote yet.
+
 ## Jira
 
 When you name a Jira Cloud ticket, the orchestrator reads it with `baton jira <KEY>`, such as `baton jira DEMO-1`, which prints the ticket and its comments as Markdown; you can run it too. baton only reads Jira, as you, with an API token that can only read:

internal/cli/deliver.go

@@ -0,0 +1,132 @@
+package cli
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/codedogapp/baton/internal/flow"
+	"github.com/codedogapp/baton/internal/github"
+	"github.com/codedogapp/baton/internal/repo"
+	"github.com/codedogapp/baton/internal/state"
+)
+
+// deliver delivers a ready feature, on the user's word: it pushes the feature's branch to origin,
+// then opens the branch's pull request on GitHub, with the title and the lead's final report as
+// its body, or comments the report on the pull request that an earlier delivery opened, and
+// records it (decision 61). Only the orchestrator delivers features, so it refuses a role's pane.
+func deliver(ctx context.Context, env Env, args []string) error {
+	slug, title, err := parseDeliver(env, args)
+	if err != nil {
+		return err
+	}
+
+	if role := flow.Role(env.Getenv(envRole)); role != "" && role != flow.RoleOrchestrator {
+		return fmt.Errorf("only the orchestrator delivers a feature, and %s names the %s",
+			envRole, role)
+	}
+
+	store, err := openStore(ctx, env.Dir)
+	if err != nil {
+		return err
+	}
+
+	feature, err := store.Read(slug)
+	if err == nil {
+		err = deliverable(feature, title)
+	}
+
+	if err == nil {
+		err = repo.Push(ctx, env.Dir, feature.Branch)
+	}
+
+	if err != nil {
+		return err
+	}
+
+	pr := github.PullRequest{
+		Branch: feature.Branch, Title: title, BodyFile: store.NotePath(slug, feature.Work.Note),
+	}
+
+	url, did, err := publish(ctx, env.Dir, feature.PullRequest, pr)
+	if err != nil {
+		return err
+	}
+
+	entry := state.Entry{Event: state.Event{
+		Kind: state.EventDeliver, Role: flow.RoleOrchestrator, Text: url,
+	}}
+	if _, err := store.Update(slug, entry, delivered(url)); err != nil {
+		return fmt.Errorf("record the pull request %s: %w", url, err)
+	}
+
+	_, err = fmt.Fprintf(env.Stdout, "Delivered %s: pushed %s to origin and %s its pull request, "+
+		"%s.\n", slug, feature.Branch, did, url)
+
+	return err
+}
+
+// parseDeliver parses baton deliver's command line: the feature's slug, and the pull request's
+// title, trimmed, if given.
+func parseDeliver(env Env, args []string) (slug, title string, err error) {
+	flags := newFlags(env, "deliver")
+	titled := flags.String("title", "", "the pull request's `title`, for the first delivery")
+
+	slug, err = parseSlug(flags, args)
+	if err == nil && slug == "" {
+		err = errors.New("want the feature's slug: baton deliver <slug> [--title <title>]")
+	}
+
+	return slug, strings.TrimSpace(*titled), err
+}
+
+// deliverable returns an error if the feature isn't ready, or if the title doesn't suit the
+// delivery: the first opens the feature's pull request, with the title, and the later ones
+// comment on it, which keeps its title.
+func deliverable(feature state.Feature, title string) error {
+	if _, err := feature.Flow.Deliver(); err != nil {
+		return fmt.Errorf("%s: %w", feature.Slug, err)
+	}
+
+	switch {
+	case feature.PullRequest == "" && title == "":
+		return fmt.Errorf("want the pull request's title for the first delivery of %s: "+
+			"baton deliver %[1]s --title <title>", feature.Slug)
+	case feature.PullRequest != "" && title != "":
+		return fmt.Errorf("%s has its pull request already, %s, which keeps its title: "+
+			"deliver it again without --title", feature.Slug, feature.PullRequest)
+	}
+
+	return nil
+}
+
+// publish opens the pull request, or comments its body on the one at the URL, if any, in the
+// folder's repo. It returns the pull request's URL, and what it did.
+func publish(ctx context.Context, dir, url string, request github.PullRequest) (
+	link, did string, err error,
+) {
+	if url != "" {
+		return url, "commented the lead's report on",
+			github.Comment(ctx, dir, url, request.BodyFile)
+	}
+
+	link, err = github.Open(ctx, dir, request)
+
+	return link, "opened", err
+}
+
+// delivered returns the change that records the feature's delivery to its pull request at the
+// URL.
+func delivered(url string) func(*state.Feature) error {
+	return func(feature *state.Feature) error {
+		next, err := feature.Flow.Deliver()
+		if err != nil {
+			return fmt.Errorf("%s: %w", feature.Slug, err)
+		}
+
+		feature.Flow, feature.PullRequest = next, url
+
+		return nil
+	}
+}

internal/cli/deliver_refusals_test.go

@@ -0,0 +1,193 @@
+package cli_test
+
+import (
+	"fmt"
+	"maps"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/codedogapp/baton/internal/flow"
+	"github.com/codedogapp/baton/internal/state"
+)
+
+// TestDeliverRefusesAndChangesNothing checks that a refusal pushes nothing, runs no gh, and leaves
+// baton's folder as it was.
+func TestDeliverRefusesAndChangesNothing(t *testing.T) {
+	ready := readyToDeliver(t)
+
+	cases := []struct {
+		name  string
+		args  []string          // After baton deliver.
+		env   map[string]string // The pane's environment.
+		setup func(t *testing.T)
+		want  string // The reason, after "baton deliver: ".
+	}{
+		{
+			name: "without a slug", args: []string{"--title", "Add the demo"},
+			want: "want the feature's slug: baton deliver <slug> [--title <title>]",
+		},
+		{
+			name: "with an argument after the slug", args: []string{slug, "now"},
+			want: `unexpected argument "now"`,
+		},
+		{
+			name: "in a role's pane", args: []string{slug, "--title", "Add it"},
+			env:  paneOf(flow.RoleLead),
+			want: "only the orchestrator delivers a feature, and BATON_ROLE names the lead",
+		},
+		{
+			name: "for a feature that isn't there", args: []string{"demo-9", "--title", "Add it"},
+			want: "no feature demo-9",
+		},
+		{
+			name: "for a feature that runs", args: []string{"demo-2", "--title", "Add it"},
+			setup: func(t *testing.T) {
+				add(t, ready.store, state.Feature{Slug: "demo-2", Branch: "linked", Flow: fresh()})
+			},
+			want: "demo-2: a feature that is running isn't ready to deliver: it is once the " +
+				"lead's final report comes",
+		},
+		{
+			name: "without a title", args: []string{slug, "--title", " "},
+			want: "want the pull request's title for the first delivery of demo-1: " +
+				"baton deliver demo-1 --title <title>",
+		},
+		{
+			name: "with a title for a later delivery", args: []string{slug, "--title", "Add it"},
+			setup: func(t *testing.T) {
+				edit(t, ready.store, func(feature *state.Feature) {
+					feature.PullRequest = pullRequest
+				})
+			},
+			want: "demo-1 has its pull request already, " + pullRequest + ", which keeps its " +
+				"title: deliver it again without --title",
+		},
+		{
+			name: "once delivered", args: []string{slug},
+			setup: func(t *testing.T) {
+				edit(t, ready.store, func(feature *state.Feature) {
+					feature.Flow.Status = flow.StatusDelivered
+				})
+			},
+			want: "demo-1: a feature that is delivered isn't ready to deliver: it is once the " +
+				"lead's final report comes",
+		},
+	}
+
+	for _, test := range cases {
+		t.Run(test.name, func(t *testing.T) {
+			if test.setup != nil {
+				test.setup(t)
+			}
+
+			calls, before := standInGh(t, "echo "+pullRequest+"\n"), snapshot(t, ready.store.Dir)
+
+			got := run(t, ready.mainDir, test.env, append([]string{"deliver"}, test.args...)...)
+			if want := (result{code: 1, stderr: "baton deliver: " + test.want + "\n"}); got != want {
+				t.Errorf("got %+v; want %+v", got, want)
+			}
+
+			wantPushed(t, ready.origin, "")
+			wantGh(t, calls, "")
+
+			if after := snapshot(t, ready.store.Dir); !maps.Equal(after, before) {
+				t.Errorf("baton's folder changed:\n%v\nwant:\n%v", after, before)
+			}
+		})
+	}
+}
+
+// TestDeliverRefusesOutsideARepo refuses a delivery that has no git repo, and so no feature.
+func TestDeliverRefusesOutsideARepo(t *testing.T) {
+	outside := t.TempDir()
+	t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(outside))
+	calls := standInGh(t, "")
+
+	got := run(t, outside, orchestratorPane(), "deliver", slug, "--title", "Add it")
+
+	want := "baton deliver: find the git repo: fatal: not a git repository"
+	if got.code != 1 || got.stdout != "" || !strings.HasPrefix(got.stderr, want) {
+		t.Errorf("got %+v; want exit code 1 and %q", got, want)
+	}
+
+	wantGh(t, calls, "")
+}
+
+// TestDeliverReportsWhatFails checks that a delivery that fails leaves the feature ready, without
+// a pull request, so that a later delivery tries again, and says what it did.
+func TestDeliverReportsWhatFails(t *testing.T) {
+	cases := []struct {
+		name   string
+		gh     string // The stand-in gh's script.
+		pushed bool   // Whether origin has the branch afterwards.
+		ran    bool   // Whether gh ran.
+		want   string // The reason's start, after "baton deliver: ".
+		// setup changes the repo first, if it isn't nil.
+		setup func(t *testing.T, ready delivery)
+	}{
+		{
+			name: "a push that git refuses", gh: "echo " + pullRequest + "\n",
+			setup: func(t *testing.T, ready delivery) {
+				gitIn(t, ready.mainDir, exec.CommandContext(t.Context(), "git", "remote", "remove",
+					"origin"))
+			},
+			want: "push linked to origin: fatal: 'origin' does not appear to be a git repository\n",
+		},
+		{
+			name: "a gh that fails", gh: "echo 'HTTP 401: Bad credentials' >&2\nexit 1\n",
+			pushed: true, ran: true,
+			want: "open the pull request: HTTP 401: Bad credentials (exit status 1)\n",
+		},
+	}
+
+	for _, test := range cases {
+		t.Run(test.name, func(t *testing.T) {
+			ready := readyToDeliver(t)
+			if test.setup != nil {
+				test.setup(t, ready)
+			}
+
+			calls := standInGh(t, test.gh)
+
+			got := run(t, ready.mainDir, orchestratorPane(), "deliver", slug, "--title", "Add it")
+			if got.code != 1 || got.stdout != "" ||
+				!strings.HasPrefix(got.stderr, "baton deliver: "+test.want) {
+				t.Errorf("got %+v; want exit code 1 and a reason that starts with %q", got,
+					test.want)
+			}
+
+			wantPushed(t, ready.origin, map[bool]string{true: head(t, ready.mainDir)}[test.pushed])
+			wantGh(t, calls, map[bool]string{true: "pr\ncreate\n--head=linked\n--title=Add it\n" +
+				"--body-file=" + ready.report + "\n"}[test.ran])
+
+			if feature, err := ready.store.Read(slug); err != nil ||
+				feature.Flow.Status != flow.StatusReady || feature.PullRequest != "" {
+				t.Errorf("Read = %+v, %v; want it ready, without a pull request", feature, err)
+			}
+		})
+	}
+}
+
+// TestDeliverReportsAPullRequestThatItCannotRecord has the feature change while gh opens its pull
+// request, as if the lead held it again: the delivery then names the pull request, which the
+// feature doesn't keep.
+func TestDeliverReportsAPullRequestThatItCannotRecord(t *testing.T) {
+	ready := readyToDeliver(t)
+	file := filepath.Join(ready.store.Dir, "features", slug, "feature.json")
+	edits := `-e 's/"holder": "orchestrator"/"holder": "lead"/' ` +
+		`-e 's/"status": "ready"/"status": "running"/'`
+	standInGh(t, fmt.Sprintf("sed -i.old %s %q\necho %s\n", edits, file, pullRequest))
+
+	got := run(t, ready.mainDir, orchestratorPane(), "deliver", slug, "--title", "Add it")
+	if want := (result{code: 1, stderr: "baton deliver: record the pull request " + pullRequest +
+		": demo-1: a feature that is running isn't ready to deliver: it is once the lead's " +
+		"final report comes\n"}); got != want {
+		t.Errorf("got %+v; want %+v", got, want)
+	}
+
+	if feature, err := ready.store.Read(slug); err != nil || feature.PullRequest != "" {
+		t.Errorf("Read = %+v, %v; want it without a pull request", feature, err)
+	}
+}