From 0b76faf4c4d51894c9aac7cdb5bd886e0f67575f Mon Sep 17 00:00:00 2001 From: Matthias Wenz Date: Thu, 18 Jun 2026 14:30:57 +0200 Subject: [PATCH] Add `entire trail merge` subcommand Merge a trail's branch into its base, gated on mergeability. The trail is resolved from --trail (number, id, or branch) or the current branch. - Checks mergeability first via GET .../{number}/mergeability and prints a readiness summary (approvals, CI checks, up-to-date with base). - Refuses to merge when not mergeable, listing the blockers in the same order the server evaluates its gates. - --dry-run reports mergeability without merging; exits non-zero when not mergeable so it can gate CI. - Performs the merge via POST .../{number}/merge and verifies ok:true before reporting the merge commit. Extracts a shared resolveTrailBySelector helper and refactors runTrailShow to use it. Co-Authored-By: Claude Opus 4.8 (1M context) Entire-Checkpoint: 423f73da96c6 --- cmd/entire/cli/api/trail_types.go | 24 +++ cmd/entire/cli/trail_cmd.go | 222 ++++++++++++++++++++++--- cmd/entire/cli/trail_merge_cmd_test.go | 211 +++++++++++++++++++++++ 3 files changed, 435 insertions(+), 22 deletions(-) create mode 100644 cmd/entire/cli/trail_merge_cmd_test.go diff --git a/cmd/entire/cli/api/trail_types.go b/cmd/entire/cli/api/trail_types.go index c4565623af..c2194ca4d1 100644 --- a/cmd/entire/cli/api/trail_types.go +++ b/cmd/entire/cli/api/trail_types.go @@ -126,3 +126,27 @@ type TrailUpdateResponse struct { type TrailDeleteResponse struct { OK bool `json:"ok"` } + +// TrailMergeabilityResponse is the response from +// GET /api/v1/trails/:host/:owner/:repo/:number/mergeability. Mergeable is the +// server's combined gate (approvals + checks + up-to-date with base); the +// scalar fields explain why when it is false. +type TrailMergeabilityResponse struct { + ApprovalGatePassed bool `json:"approval_gate_passed"` + ChecksPassed bool `json:"checks_passed"` + // ChecksStatus is one of "success", "failure", "pending", or "none". + ChecksStatus string `json:"checks_status"` + BehindBy int `json:"behind_by"` + // ComparisonStatus is "available" when the base/branch comparison + // succeeded, or "unknown" when the server could not compute it. + ComparisonStatus string `json:"comparison_status"` + Mergeable bool `json:"mergeable"` +} + +// TrailMergeResponse is the response from +// POST /api/v1/trails/:host/:owner/:repo/:number/merge. OK is the server's +// explicit success signal; MergeCommitSHA is the resulting merge commit. +type TrailMergeResponse struct { + OK bool `json:"ok"` + MergeCommitSHA string `json:"merge_commit_sha"` +} diff --git a/cmd/entire/cli/trail_cmd.go b/cmd/entire/cli/trail_cmd.go index b0ea938a78..66d822a3d8 100644 --- a/cmd/entire/cli/trail_cmd.go +++ b/cmd/entire/cli/trail_cmd.go @@ -63,6 +63,7 @@ func newTrailCmd() *cobra.Command { cmd.AddCommand(newTrailListCmd()) cmd.AddCommand(newTrailCreateCmd()) cmd.AddCommand(newTrailUpdateCmd()) + cmd.AddCommand(newTrailMergeCmd()) cmd.AddCommand(newTrailDeleteCmd()) cmd.AddCommand(newTrailFindingCmd()) cmd.AddCommand(newTrailWatchCmd()) @@ -122,28 +123,9 @@ func runTrailShow(ctx context.Context, w, errW io.Writer, insecureHTTP bool, sel return err } - selector = strings.TrimSpace(selector) - var found *api.TrailResource - if selector == "" { - branch, err := GetCurrentBranch(ctx) - if err != nil { - return fmt.Errorf("no trail selector given and current branch is unknown: %w\nhint: run 'entire trail list --status any' or pass a trail number, id, or branch", err) - } - found, err = findTrailByBranch(ctx, client, forge, owner, repo, branch) - if err != nil { - return err - } - if found == nil { - return fmt.Errorf("no trail found for current branch %q\nhint: run 'entire trail create' or 'entire trail list --status any'", branch) - } - } else { - found, err = findTrailBySelector(ctx, client, forge, owner, repo, selector) - if err != nil { - return err - } - if found == nil { - return fmt.Errorf("no trail %q found in %s/%s/%s (run 'entire trail list --status any')", selector, forge, owner, repo) - } + found, err := resolveTrailBySelector(ctx, client, forge, owner, repo, selector) + if err != nil { + return err } printTrailDetails(w, found.ToMetadata()) @@ -151,6 +133,36 @@ func runTrailShow(ctx context.Context, w, errW io.Writer, insecureHTTP bool, sel }) } +// resolveTrailBySelector resolves a trail by an optional selector (trail +// number, id, or branch). An empty selector falls back to the current branch's +// trail. It returns an actionable error (never a nil trail with a nil error) +// when nothing matches, so callers can rely on a non-nil result. +func resolveTrailBySelector(ctx context.Context, client *api.Client, forge, owner, repo, selector string) (*api.TrailResource, error) { + selector = strings.TrimSpace(selector) + if selector == "" { + branch, err := GetCurrentBranch(ctx) + if err != nil { + return nil, fmt.Errorf("no trail selector given and current branch is unknown: %w\nhint: run 'entire trail list --status any' or pass a trail number, id, or branch", err) + } + found, err := findTrailByBranch(ctx, client, forge, owner, repo, branch) + if err != nil { + return nil, err + } + if found == nil { + return nil, fmt.Errorf("no trail found for current branch %q\nhint: run 'entire trail create' or 'entire trail list --status any'", branch) + } + return found, nil + } + found, err := findTrailBySelector(ctx, client, forge, owner, repo, selector) + if err != nil { + return nil, err + } + if found == nil { + return nil, fmt.Errorf("no trail %q found in %s/%s/%s (run 'entire trail list --status any')", selector, forge, owner, repo) + } + return found, nil +} + func printTrailDetails(w io.Writer, m *trail.Metadata) { fmt.Fprintf(w, "Trail: %s\n", m.Title) if m.Number > 0 { @@ -934,6 +946,172 @@ func buildTrailUpdateRequest(current *api.TrailResource, inputs trailUpdateInput return req } +func newTrailMergeCmd() *cobra.Command { + var trailSelector string + var dryRun bool + + cmd := &cobra.Command{ + Use: "merge", + Short: "Merge a trail's branch into its base", + Long: `Merge a trail's branch into its base branch. + +The trail is checked for mergeability first (required approvals, passing CI +checks, and being up to date with the base branch); the merge is only attempted +when those gates pass. + +With --trail, the trail may be given as a number, id, or branch. Without it, the +trail for the current branch is used. Pass --dry-run to only report whether the +trail is mergeable without performing the merge.`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return runTrailMerge(cmd.Context(), cmd.OutOrStdout(), cmd.ErrOrStderr(), trailInsecureHTTP(cmd), trailSelector, dryRun) + }, + } + + cmd.Flags().StringVar(&trailSelector, "trail", "", "Trail to merge (number, id, or branch; defaults to the current branch's trail)") + cmd.Flags().BoolVar(&dryRun, "dry-run", false, "Only check whether the trail is mergeable; do not merge") + + return cmd +} + +func runTrailMerge(ctx context.Context, w, errW io.Writer, insecureHTTP bool, selector string, dryRun bool) error { + return runAuthenticatedDataAPI(ctx, errW, insecureHTTP, func(ctx context.Context, client *api.Client) error { + forge, owner, repo, err := resolveTrailRemote(ctx) + if err != nil { + return err + } + + found, err := resolveTrailBySelector(ctx, client, forge, owner, repo, selector) + if err != nil { + return err + } + // The merge and mergeability endpoints are keyed by trail number, not id. + if found.Number <= 0 { + return fmt.Errorf("trail for branch %q has no number yet; cannot merge", found.Branch) + } + + // Check mergeability before attempting the merge so an un-mergeable + // trail is reported with reasons instead of a raw 422 from the merge call. + mergeability, err := fetchTrailMergeability(ctx, client, forge, owner, repo, found.Number) + if err != nil { + return err + } + printTrailMergeability(w, found, mergeability) + + if !mergeability.Mergeable { + reasons := describeMergeBlockers(mergeability) + return fmt.Errorf("trail #%d is not mergeable: %s", found.Number, strings.Join(reasons, "; ")) + } + + if dryRun { + fmt.Fprintf(w, "Trail #%d is mergeable (dry run; no merge performed).\n", found.Number) + return nil + } + + res, err := mergeTrailByNumber(ctx, client, forge, owner, repo, found.Number) + if err != nil { + return err + } + fmt.Fprintf(w, "Merged trail #%d into %s (%s)\n", found.Number, found.Base, res.MergeCommitSHA) + return nil + }) +} + +// fetchTrailMergeability reads merge readiness for a trail from the API. +func fetchTrailMergeability(ctx context.Context, client *api.Client, forge, owner, repo string, number int) (*api.TrailMergeabilityResponse, error) { + resp, err := client.Get(ctx, trailNumberPath(forge, owner, repo, number)+"/mergeability") + if err != nil { + return nil, fmt.Errorf("failed to check mergeability: %w", err) + } + defer resp.Body.Close() + if err := checkTrailResponse(resp); err != nil { + return nil, err + } + var m api.TrailMergeabilityResponse + if err := api.DecodeJSON(resp, &m); err != nil { + return nil, fmt.Errorf("failed to decode mergeability response: %w", err) + } + return &m, nil +} + +// mergeTrailByNumber merges a trail's branch into its base and verifies the +// server's {ok:true} signal. Like deleteTrailByNumber, a 2xx alone is not +// enough: the merge must be confirmed before reporting success. +func mergeTrailByNumber(ctx context.Context, client *api.Client, forge, owner, repo string, number int) (*api.TrailMergeResponse, error) { + resp, err := client.Post(ctx, trailNumberPath(forge, owner, repo, number)+"/merge", nil) + if err != nil { + return nil, fmt.Errorf("failed to merge trail: %w", err) + } + defer resp.Body.Close() + if err := checkTrailResponse(resp); err != nil { + return nil, err + } + var res api.TrailMergeResponse + if err := api.DecodeJSON(resp, &res); err != nil { + return nil, fmt.Errorf("failed to decode merge response: %w", err) + } + if !res.OK { + return nil, fmt.Errorf("trail API did not confirm merge of trail #%d", number) + } + return &res, nil +} + +// printTrailMergeability renders the merge-readiness summary for a trail. +func printTrailMergeability(w io.Writer, t *api.TrailResource, m *api.TrailMergeabilityResponse) { + fmt.Fprintf(w, "Trail #%d (%s → %s)\n", t.Number, t.Branch, t.Base) + fmt.Fprintf(w, " Approvals: %s\n", checkmark(m.ApprovalGatePassed)) + fmt.Fprintf(w, " Checks: %s (%s)\n", checkmark(m.ChecksPassed), trailChecksStatusDisplay(m.ChecksStatus)) + fmt.Fprintf(w, " Up to date: %s\n", checkmark(m.ComparisonStatus == "available" && m.BehindBy == 0)) + fmt.Fprintf(w, " Mergeable: %s\n", checkmark(m.Mergeable)) +} + +// checkmark renders a boolean gate as a ✓/✗ glyph, matching the status output +// style used elsewhere in the CLI. +func checkmark(ok bool) string { + if ok { + return "✓" + } + return "✗" +} + +func trailChecksStatusDisplay(status string) string { + if strings.TrimSpace(status) == "" { + return "none" + } + return status +} + +// describeMergeBlockers explains why a trail is not mergeable, in the same +// order the server evaluates its gates. It always returns at least one reason +// for an un-mergeable trail. +func describeMergeBlockers(m *api.TrailMergeabilityResponse) []string { + var reasons []string + if !m.ApprovalGatePassed { + reasons = append(reasons, "required approvals are missing") + } + if !m.ChecksPassed { + switch m.ChecksStatus { + case "failure": + reasons = append(reasons, "CI checks failed") + case "pending": + reasons = append(reasons, "CI checks are still running") + default: + reasons = append(reasons, "CI checks have not passed") + } + } + if m.ComparisonStatus != "available" { + reasons = append(reasons, "could not compare the branch with its base") + } else if m.BehindBy > 0 { + reasons = append(reasons, fmt.Sprintf("branch is %d %s behind the base branch", m.BehindBy, pluralize("commit", m.BehindBy))) + } + if len(reasons) == 0 { + // Defensive: the server reported not-mergeable without a recognized + // blocker. Surface a generic reason rather than an empty list. + reasons = append(reasons, "the trail is not in a mergeable state") + } + return reasons +} + // parseTrailNumberArg parses an optional positional trail-number argument. // It returns 0 when no argument is supplied; a supplied value must be a // positive integer (the server keys single-trail endpoints by number). diff --git a/cmd/entire/cli/trail_merge_cmd_test.go b/cmd/entire/cli/trail_merge_cmd_test.go new file mode 100644 index 0000000000..5f3460c451 --- /dev/null +++ b/cmd/entire/cli/trail_merge_cmd_test.go @@ -0,0 +1,211 @@ +package cli + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/api" +) + +func TestFetchTrailMergeability(t *testing.T) { + t.Parallel() + + var gotMethod, gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod, gotPath = r.Method, r.URL.Path + if err := json.NewEncoder(w).Encode(api.TrailMergeabilityResponse{ + ApprovalGatePassed: true, + ChecksPassed: true, + ChecksStatus: "success", + ComparisonStatus: "available", + BehindBy: 0, + Mergeable: true, + }); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + client := api.NewClientWithBaseURL("tok", srv.URL) + m, err := fetchTrailMergeability(t.Context(), client, "gh", "acme", "repo", 575) + if err != nil { + t.Fatalf("fetchTrailMergeability: %v", err) + } + if gotMethod != http.MethodGet { + t.Fatalf("method = %q, want GET", gotMethod) + } + if want := "/api/v1/trails/gh/acme/repo/575/mergeability"; gotPath != want { + t.Fatalf("path = %q, want %q", gotPath, want) + } + if !m.Mergeable { + t.Fatalf("mergeable = false, want true") + } +} + +func TestFetchTrailMergeability_SurfacesServerError(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + if err := json.NewEncoder(w).Encode(map[string]string{"error": "Trail not found"}); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + client := api.NewClientWithBaseURL("tok", srv.URL) + if _, err := fetchTrailMergeability(t.Context(), client, "gh", "acme", "repo", 999); err == nil { + t.Fatal("expected error for 404, got nil") + } +} + +func TestMergeTrailByNumber(t *testing.T) { + t.Parallel() + + t.Run("merges via the integer number path and accepts ok:true", func(t *testing.T) { + t.Parallel() + var gotMethod, gotPath string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod, gotPath = r.Method, r.URL.Path + if err := json.NewEncoder(w).Encode(api.TrailMergeResponse{OK: true, MergeCommitSHA: "abc1234"}); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + client := api.NewClientWithBaseURL("tok", srv.URL) + res, err := mergeTrailByNumber(t.Context(), client, "gh", "acme", "repo", 575) + if err != nil { + t.Fatalf("mergeTrailByNumber: %v", err) + } + if gotMethod != http.MethodPost { + t.Fatalf("method = %q, want POST", gotMethod) + } + if want := "/api/v1/trails/gh/acme/repo/575/merge"; gotPath != want { + t.Fatalf("path = %q, want %q", gotPath, want) + } + if res.MergeCommitSHA != "abc1234" { + t.Fatalf("merge_commit_sha = %q, want abc1234", res.MergeCommitSHA) + } + }) + + t.Run("treats a 2xx without ok:true as failure", func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if err := json.NewEncoder(w).Encode(api.TrailMergeResponse{OK: false}); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + client := api.NewClientWithBaseURL("tok", srv.URL) + if _, err := mergeTrailByNumber(t.Context(), client, "gh", "acme", "repo", 575); err == nil { + t.Fatal("expected error for 2xx without ok:true, got nil") + } + }) + + t.Run("surfaces the server's 422 gate-failure message", func(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + if err := json.NewEncoder(w).Encode(map[string]string{"error": "Branch is out of date with the base branch"}); err != nil { + t.Errorf("encode response: %v", err) + } + })) + defer srv.Close() + + client := api.NewClientWithBaseURL("tok", srv.URL) + _, err := mergeTrailByNumber(t.Context(), client, "gh", "acme", "repo", 575) + if err == nil { + t.Fatal("expected error for 422, got nil") + } + if !strings.Contains(err.Error(), "out of date") { + t.Fatalf("error = %v, want it to surface the server message", err) + } + }) +} + +func TestDescribeMergeBlockers(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in api.TrailMergeabilityResponse + want []string + }{ + { + name: "missing approvals", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: false, ChecksPassed: true, ComparisonStatus: "available"}, + want: []string{"required approvals are missing"}, + }, + { + name: "failed checks", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: true, ChecksPassed: false, ChecksStatus: "failure", ComparisonStatus: "available"}, + want: []string{"CI checks failed"}, + }, + { + name: "pending checks", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: true, ChecksPassed: false, ChecksStatus: "pending", ComparisonStatus: "available"}, + want: []string{"CI checks are still running"}, + }, + { + name: "behind base", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: true, ChecksPassed: true, ComparisonStatus: "available", BehindBy: 3}, + want: []string{"branch is 3 commits behind the base branch"}, + }, + { + name: "behind base by one", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: true, ChecksPassed: true, ComparisonStatus: "available", BehindBy: 1}, + want: []string{"branch is 1 commit behind the base branch"}, + }, + { + name: "unknown comparison", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: true, ChecksPassed: true, ComparisonStatus: "unknown"}, + want: []string{"could not compare the branch with its base"}, + }, + { + name: "multiple blockers in gate order", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: false, ChecksPassed: false, ChecksStatus: "failure", ComparisonStatus: "available", BehindBy: 2}, + want: []string{"required approvals are missing", "CI checks failed", "branch is 2 commits behind the base branch"}, + }, + { + name: "defensive generic reason when nothing recognized", + in: api.TrailMergeabilityResponse{ApprovalGatePassed: true, ChecksPassed: true, ComparisonStatus: "available", BehindBy: 0}, + want: []string{"the trail is not in a mergeable state"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := describeMergeBlockers(&tt.in) + if strings.Join(got, "|") != strings.Join(tt.want, "|") { + t.Fatalf("describeMergeBlockers() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPrintTrailMergeability(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + printTrailMergeability(&buf, &api.TrailResource{Number: 7, Branch: "feature", Base: "main"}, &api.TrailMergeabilityResponse{ + ApprovalGatePassed: true, + ChecksPassed: false, + ChecksStatus: "pending", + ComparisonStatus: "available", + BehindBy: 0, + Mergeable: false, + }) + out := buf.String() + for _, want := range []string{"Trail #7 (feature → main)", "Approvals: ✓", "Checks: ✗ (pending)", "Mergeable: ✗"} { + if !strings.Contains(out, want) { + t.Errorf("output missing %q:\n%s", want, out) + } + } +}