From 9de62455105a82a42c3c55ccd7a0f1dbe46f3d50 Mon Sep 17 00:00:00 2001 From: Maxime Bourret Date: Tue, 21 Jul 2026 14:54:00 -0400 Subject: [PATCH] Print a Run-complete message that links the opened pull request. After Final succeeds, operators only saw tools/tokens; surface that the agents finished and include the PR URL from the open-PR check. Co-authored-by: Cursor --- internal/run/orchestrator.go | 9 +++++--- internal/run/orchestrator_test.go | 37 ++++++++++++++++++++----------- internal/ticket/github.go | 20 ++++++++++------- internal/ticket/github_test.go | 25 +++++++++++---------- 4 files changed, 55 insertions(+), 36 deletions(-) diff --git a/internal/run/orchestrator.go b/internal/run/orchestrator.go index 52558b9..408658e 100644 --- a/internal/run/orchestrator.go +++ b/internal/run/orchestrator.go @@ -36,7 +36,9 @@ type Orchestrator struct { // PullRequests is the side-effect seam Final success is checked against: an // open pull request for the Run branch after the Final Agent exits 0. type PullRequests interface { - HasOpenPR(ctx context.Context, branch string) (bool, error) + // OpenPRURL returns the URL of an open pull request for branch, or "" if + // none exists. + OpenPRURL(ctx context.Context, branch string) (string, error) } // Run executes one Ship Run in the current checkout. @@ -180,13 +182,14 @@ func (r Orchestrator) final(ctx context.Context, branch string, done []ticket.Ti if err := r.runPhase(ctx, throbber.Status{Phase: "Final"}, finalPrompt); err != nil { return fmt.Errorf("Abort: Final Phase: %w", err) } - open, err := r.PRs.HasOpenPR(ctx, branch) + prURL, err := r.PRs.OpenPRURL(ctx, branch) if err != nil { return fmt.Errorf("Abort: check Final pull request for %s: %w", branch, err) } - if !open { + if prURL == "" { return fmt.Errorf("Abort: Final Phase produced no open pull request for branch %s", branch) } + fmt.Fprintf(r.stdout(), "Run complete. Agents finished and opened a pull request:\n%s\n", prURL) return nil } diff --git a/internal/run/orchestrator_test.go b/internal/run/orchestrator_test.go index 316d0b1..eaa2d7b 100644 --- a/internal/run/orchestrator_test.go +++ b/internal/run/orchestrator_test.go @@ -555,8 +555,8 @@ func TestRun_skipsFinalWhenNoSuccessfulIteration(t *testing.T) { if len(tickets.doneList) != 0 { t.Errorf("done = %v, want none", tickets.doneList) } - open, _ := prs.HasOpenPR(context.Background(), "ship/run") - if open { + url, _ := prs.OpenPRURL(context.Background(), "ship/run") + if url != "" { t.Error("no PR should be recorded when Final never runs") } } @@ -597,6 +597,13 @@ func TestRun_finalPhaseRunsAfterQueueDrains(t *testing.T) { if !strings.Contains(out.String(), "Queue drained") { t.Errorf("stdout = %q, want drain summary", out.String()) } + wantPR := "https://example.com/ship/run/pull/1" + if !strings.Contains(out.String(), "Run complete. Agents finished and opened a pull request:") { + t.Errorf("stdout = %q, want Run complete message", out.String()) + } + if !strings.Contains(out.String(), wantPR) { + t.Errorf("stdout = %q, want linked pull request %q", out.String(), wantPR) + } } func TestRun_finalWithoutOpenedPRFails(t *testing.T) { @@ -665,8 +672,8 @@ func TestRun_finalPhaseFailureAbortsRun(t *testing.T) { if got := tickets.doneList; !equalInts(got, []int{7}) { t.Errorf("done = %v, want [7]", got) } - open, _ := prs.HasOpenPR(context.Background(), "ship/run") - if open { + url, _ := prs.OpenPRURL(context.Background(), "ship/run") + if url != "" { t.Error("no PR should be recorded when Final Agent fails") } } @@ -710,6 +717,9 @@ func TestRun_stopsAtMaxIterationsWithPartialProgress(t *testing.T) { if !strings.Contains(finalPrompt, "2") { t.Errorf("Final prompt must include max iterations (2):\n%s", finalPrompt) } + if !strings.Contains(out.String(), "https://example.com/ship/run/pull/1") { + t.Errorf("stdout = %q, want linked pull request after Partial Progress Final", out.String()) + } } func TestRun_implementWithoutCommitFailsAndDoesNotMarkDone(t *testing.T) { @@ -1399,23 +1409,24 @@ func (f *fakeTickets) Done(_ context.Context, t ticket.Ticket) error { } // fakePRs reports which branches have an open pull request. Final success -// requires HasOpenPR to return true for the Run branch after the Agent exits. +// requires OpenPRURL to return a non-empty URL for the Run branch after the +// Agent exits. type fakePRs struct { - open map[string]bool + urls map[string]string } -func (f *fakePRs) HasOpenPR(_ context.Context, branch string) (bool, error) { - if f.open == nil { - return false, nil +func (f *fakePRs) OpenPRURL(_ context.Context, branch string) (string, error) { + if f.urls == nil { + return "", nil } - return f.open[branch], nil + return f.urls[branch], nil } func (f *fakePRs) openPR(branch string) { - if f.open == nil { - f.open = map[string]bool{} + if f.urls == nil { + f.urls = map[string]string{} } - f.open[branch] = true + f.urls[branch] = "https://example.com/" + branch + "/pull/1" } // fakeAgent records Phase requests and defers behavior to handler. The 1-based diff --git a/internal/ticket/github.go b/internal/ticket/github.go index 8458b9c..a4c0dc1 100644 --- a/internal/ticket/github.go +++ b/internal/ticket/github.go @@ -220,24 +220,28 @@ func (g *GitHub) Done(ctx context.Context, t Ticket) error { return err } -// HasOpenPR reports whether an open pull request exists for branch. -// Ship uses this to verify the Final Phase opened a PR. -func (g *GitHub) HasOpenPR(ctx context.Context, branch string) (bool, error) { +// OpenPRURL returns the URL of an open pull request for branch, or "" if none. +// Ship uses this to verify the Final Phase opened a PR and to link it in the +// Run completion message. +func (g *GitHub) OpenPRURL(ctx context.Context, branch string) (string, error) { out, err := g.exec()(ctx, "pr", "list", "--head", branch, "--state", "open", - "--json", "number", + "--json", "url", ) if err != nil { - return false, err + return "", err } var prs []struct { - Number int `json:"number"` + URL string `json:"url"` } if err := json.Unmarshal(out, &prs); err != nil { - return false, fmt.Errorf("parse pr list: %w", err) + return "", fmt.Errorf("parse pr list: %w", err) } - return len(prs) > 0, nil + if len(prs) == 0 { + return "", nil + } + return prs[0].URL, nil } var _ Port = (*GitHub)(nil) diff --git a/internal/ticket/github_test.go b/internal/ticket/github_test.go index 6fb33c4..e29870f 100644 --- a/internal/ticket/github_test.go +++ b/internal/ticket/github_test.go @@ -241,31 +241,32 @@ func TestDone_removesShipAndClosesIssue(t *testing.T) { } } -func TestHasOpenPR_trueWhenOpenPRExistsForBranch(t *testing.T) { +func TestOpenPRURL_returnsURLWhenOpenPRExistsForBranch(t *testing.T) { gh := &ticket.GitHub{Exec: scriptedExec(t, map[string]string{ - "pr list --head ship/run --state open --json number": `[{"number":42}]`, + "pr list --head ship/run --state open --json url": `[{"url":"https://github.com/acme/ship/pull/42"}]`, })} - open, err := gh.HasOpenPR(context.Background(), "ship/run") + got, err := gh.OpenPRURL(context.Background(), "ship/run") if err != nil { - t.Fatalf("HasOpenPR: %v", err) + t.Fatalf("OpenPRURL: %v", err) } - if !open { - t.Fatal("HasOpenPR = false, want true when an open PR exists") + want := "https://github.com/acme/ship/pull/42" + if got != want { + t.Fatalf("OpenPRURL = %q, want %q", got, want) } } -func TestHasOpenPR_falseWhenNone(t *testing.T) { +func TestOpenPRURL_emptyWhenNone(t *testing.T) { gh := &ticket.GitHub{Exec: scriptedExec(t, map[string]string{ - "pr list --head ship/run --state open --json number": `[]`, + "pr list --head ship/run --state open --json url": `[]`, })} - open, err := gh.HasOpenPR(context.Background(), "ship/run") + got, err := gh.OpenPRURL(context.Background(), "ship/run") if err != nil { - t.Fatalf("HasOpenPR: %v", err) + t.Fatalf("OpenPRURL: %v", err) } - if open { - t.Fatal("HasOpenPR = true, want false when no open PR") + if got != "" { + t.Fatalf("OpenPRURL = %q, want empty when no open PR", got) } }