diff --git a/docs/adopting-existing-prs.md b/docs/adopting-existing-prs.md index bab5312..f5f4201 100644 --- a/docs/adopting-existing-prs.md +++ b/docs/adopting-existing-prs.md @@ -93,6 +93,7 @@ This tells you: - which tracked branches are healthy - which branches already have linked PRs - which PR bases or heads disagree with your intended stack +- whether a branch needs `stack submit`, `stack restack`, or manual metadata repair next ### 5. Fix the shape @@ -117,6 +118,11 @@ stack submit --all That is the step that turns your intended parent graph into updated PR bases and branch tips on GitHub. +When `stack submit` creates a PR during adoption, it uses the tip commit subject +and body by default. If the branch tip has no commit body yet, `stack` uses a +deterministic fallback body so the first adoption submit stays non-interactive +and reviewable. + ## Large sets: how to keep them manageable For a larger PR set, the safest pattern is: @@ -131,6 +137,10 @@ That gives you tighter feedback loops and makes conflicts easier to localize. If you try to adopt everything at once, you can still do it, but debugging parent mistakes gets slower and noisier. +If `stack submit` reports multiple open PRs for one head branch, stop and clean +that up before continuing. `stack` refuses to guess which open PR owns a reused +head name. + ## Testing a composed set Once the branches are tracked, the stack graph gives you a clearer local test diff --git a/docs/landing-workflow-followups.md b/docs/landing-workflow-followups.md new file mode 100644 index 0000000..d6a0d63 --- /dev/null +++ b/docs/landing-workflow-followups.md @@ -0,0 +1,261 @@ +# Landing workflow follow-ups + +This document captures the next layer of work that became obvious while using +`stack` to turn a pile of existing PRs into one verified landing PR in +`TeamSidewinder/event-agent`. + +The current tool is already useful for: + +- adopting existing branches into an explicit parent graph +- keeping parent/child intent legible +- restacking after lower branches move +- syncing PR base drift back to GitHub + +What it does not yet handle well is the operator workflow after the graph is +known: + +- grouping already-open PRs into one strict landing batch +- excluding later follow-up commits from that landing batch +- marking original PRs as superseded by the landing PR +- carrying verification evidence alongside the stack +- telling the operator which tickets and original PRs are safe to close after + merge and deploy + +Those are not side concerns. For a team that produces many runner PRs in +parallel, they are the difference between a stack tool and a landing workflow. + +## What we learned from a real workflow + +The `event-agent` discovery batch looked like this: + +- original PRs: `#353`, `#354`, `#363`, `#364` +- intended outcome: one combined landing PR with the verified set +- complication: the working composed branch later picked up an extra follow-up + commit that we did not want to include in the first merge + +`stack` was still useful: + +- it helped make the intended grouping explicit +- it helped us reason about parent order and adoption + +But the final landing workflow was manual: + +1. identify the exact commit that represented the strict verified scope +2. cut a fresh landing branch at that commit +3. push it manually +4. open a combined PR manually +5. comment on the original PRs manually +6. manually decide which Linear tickets were safe to close after deploy + +That is the gap this document is about. + +## Recommendation + +Keep `stack` and keep using it. + +Do not turn it into a hosted system or a merge queue reimplementation. + +Do extend it from: + +- branch graph management + +to: + +- branch graph management plus landing orchestration + +The shape should stay explicit, local-first, and Git/GitHub-native. + +## Priority follow-ups + +### 1. First-class landing branch composition + +The tool should help create a strict landing branch from a selected portion of +the stack. + +Example operator need: + +```bash +stack compose discovery-core \ + --branches hack-agent/lnhack-66-... \ + --branches hack-agent/lnhack-74-... \ + --branches hack-agent/lnhack-68-... \ + --branches hack-agent/lnhack-69-... +``` + +Or, when the stack already exists: + +```bash +stack compose discovery-core --from --to +``` + +Expected behavior: + +- create a new ordinary branch, for example `stack/discovery-core` +- base it on trunk +- replay only the selected stack commits in order +- exclude later unrelated or follow-up commits unless explicitly requested +- show the exact commit set before mutating anything + +This is the biggest missing piece from the real workflow. + +### 2. Superseded PR support + +Once a composed landing PR exists, the original PRs should not remain ambiguous. + +Example: + +```bash +stack supersede --landing stack/discovery-core --prs 353,354,363,364 +``` + +Expected behavior: + +- add a comment to each original PR saying it is superseded by the landing PR +- optionally add a local metadata link from original PRs to the landing PR +- optionally close the originals after the landing PR merges +- refuse to guess if more than one open PR appears to own the same branch + +This should be explicit and reversible. + +### 3. Verification metadata + +Right now verification lives in the PR body or in chat. + +That is too fragile for a stack tool that is supposed to support landing order +and closeout decisions. + +Add a lightweight verification record per branch or landing branch. + +Example: + +```bash +stack verify add stack/discovery-core \ + --note "AA General Festival Discovery Deeplink" \ + --run-id b2f34b20-... \ + --score 100 \ + --passed +``` + +Expected stored fields: + +- branch or landing branch +- check type: sim, unit, integration, manual, deploy, smoke +- identifier: run id, check URL, commit SHA +- pass/fail +- optional score +- optional note +- timestamp + +This should feed status and closeout views. + +### 4. Closeout planning + +After merge, the operator needs one command that answers: + +- which original PRs should now be closed as superseded +- which tickets are safe to close immediately +- which tickets remain pending post-deploy checks +- what post-deploy checks are still outstanding + +Example: + +```bash +stack closeout stack/discovery-core +``` + +Expected output: + +- landing PR +- superseded PRs +- tickets safe to close now +- tickets blocked on deploy verification +- required follow-up checks + +This should be read-only by default and optionally able to post comments or +write local notes. + +### 5. Better status for operators + +`stack status` should grow from “stack health” into “what should I do next”. + +Useful additions: + +- landing branch detection +- original PR to landing PR relationships +- verification summary +- unresolved post-deploy work +- clear ready/blocking reasons + +Example questions it should answer directly: + +- which branch is the real merge target? +- which original PRs are now traceability-only? +- which stack item is blocked, and on what? +- is this safe to queue? + +### 6. Better adoption ergonomics for existing PR piles + +The adoption fix for stale branches was necessary, but the operator experience +is still too manual for a large pile of existing PRs. + +Improvements worth adding: + +- `stack adopt pr 353 --parent main` +- `stack adopt pr 354 --parent pr/353` +- optional helpers to fetch the PR head locally if missing +- clearer warnings when a branch has drift that suggests “compose instead of + direct stack submit” + +The tool should not infer the whole dependency graph. It should make the +adoption path faster and safer once the operator already knows the shape. + +## Commands worth adding + +Suggested command surface: + +- `stack compose` +- `stack supersede` +- `stack verify add` +- `stack verify list` +- `stack closeout` +- `stack adopt pr` + +Commands that probably should not exist yet: + +- automatic ticket closure +- automatic deployment polling +- merge queue orchestration beyond normal `stack queue` handoff + +Those belong later, if at all. + +## Recommended v1 order + +1. `stack compose` +2. `stack supersede` +3. `stack verify add` and `stack verify list` +4. `stack closeout` +5. `stack status` improvements +6. `stack adopt pr` + +This order matches the highest-friction steps from the real discovery landing. + +## Design constraints + +Keep the following properties: + +- ordinary local branches +- ordinary GitHub PRs +- explicit operator choices +- deterministic repair flows +- no hidden hosted state +- no attempt to replace GitHub merge queue + +The tool should help an operator say: + +- “these are the changes I want to land together” +- “these are the original PRs this replaces” +- “this is the evidence that the landing branch is good” +- “these are the tickets safe to close after deploy” + +without requiring a separate spreadsheet or a memory-heavy manual ritual. + diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1bebec6..38f3fc6 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -67,6 +67,12 @@ stack sync Then choose a repair path deliberately. +Common outcomes: + +- `run \`stack submit \`` when the remote branch or PR base/head is simply stale +- relink or clear local PR metadata when the tracked PR was closed, deleted, or points at the wrong head +- inspect the merged parent manually when GitHub-side history drift means `sync --apply` would have to guess + ## `stack submit` or `stack queue` fails against GitHub Check: @@ -88,6 +94,10 @@ If the CLI reports multiple open PRs for one head branch, it is refusing to guess which live PR owns that branch. Close or retarget the duplicate until one open PR remains for that head name, then rerun `stack submit`. +When `stack submit` creates a new PR, it uses the tip commit subject and body by +default. If the commit body is empty, the preview will show that `stack` is +using its generated fallback body instead. + ## Release automation does not update the tap The release workflow needs: diff --git a/docs/usage.md b/docs/usage.md index 198fb8b..f3931d7 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -49,6 +49,13 @@ and cached PR state. 4. Run `stack queue ` only when the bottom branch targets trunk and is healthy. 5. Run `stack sync` after merges or GitHub-side base changes. +When `stack submit` creates a new PR, it stays non-interactive by default: + +- the PR title comes from the tip commit subject +- the PR body comes from the tip commit body +- if the tip commit body is empty, `stack` generates a deterministic fallback body that records the branch and parent +- if the tip commit subject is empty, `stack` falls back to the branch name + For `stack queue`, GitHub repository auto-merge must be enabled. `stack` hands off through `gh pr merge --auto`, then GitHub applies the repo's normal auto-merge or merge-queue policy. @@ -58,7 +65,9 @@ auto-merge or merge-queue policy. Use `stack sync` first when local metadata and GitHub disagree. Use `stack sync --apply` only for clean repairs. If the CLI reports a -manual-review case, keep it manual. +manual-review case, keep it manual. `stack status` and `stack sync` should tell +you whether the next step is `submit`, metadata repair, or deliberate manual +inspection. If a rebase or restack stops for conflicts: diff --git a/internal/cmd/root.go b/internal/cmd/root.go index ba83070..6da2eac 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -612,15 +612,7 @@ stack submit --all --yes } if record.PR.Number == 0 { - title, body, err := runtime.Git.CommitMessage(runtime.Context, plan.Branch) - if err != nil { - return err - } - if title == "" { - title = plan.Branch - } - body = chooseString(body, fmt.Sprintf("Stack branch `%s` targeting `%s`.", plan.Branch, record.ParentBranch)) - pr, err := runtime.GitHub.CreatePR(runtime.Context, record.ParentBranch, plan.Branch, title, body, draft) + pr, err := runtime.GitHub.CreatePR(runtime.Context, record.ParentBranch, plan.Branch, plan.Metadata.Title, plan.Metadata.Body, draft) if err != nil { return err } @@ -677,24 +669,24 @@ stack sync --apply record := state.Branches[branch] if record.PR.Number == 0 { if runtime.Git.RemoteBranchExists(runtime.Context, state.DefaultRemote, branch) { - repairs = append(repairs, fmt.Sprintf("%s: remote branch exists but no PR is linked in local metadata", branch)) + repairs = append(repairs, fmt.Sprintf("%s: remote branch exists but no PR is linked in local metadata; run `stack submit %s` to relink or create the PR", branch, branch)) } continue } pr, err := runtime.GitHub.ViewPR(runtime.Context, record.PR.Number) if err != nil { - repairs = append(repairs, fmt.Sprintf("%s: tracked PR #%d could not be loaded; repair required", branch, record.PR.Number)) + repairs = append(repairs, fmt.Sprintf("%s: tracked PR #%d could not be loaded; run `stack status` and inspect the PR before resubmitting", branch, record.PR.Number)) continue } record.PR = pr state.Branches[branch] = record if pr.HeadRefName != "" && pr.HeadRefName != branch { - repairs = append(repairs, fmt.Sprintf("%s: PR head is %s, expected %s", branch, pr.HeadRefName, branch)) + repairs = append(repairs, fmt.Sprintf("%s: PR head is %s, expected %s; repair or relink the PR before submitting again", branch, pr.HeadRefName, branch)) } if pr.BaseRefName != "" && pr.BaseRefName != record.ParentBranch { - repairs = append(repairs, fmt.Sprintf("%s: PR base is %s, expected %s", branch, pr.BaseRefName, record.ParentBranch)) + repairs = append(repairs, fmt.Sprintf("%s: PR base is %s, expected %s; run `stack submit %s` to retarget it", branch, pr.BaseRefName, record.ParentBranch, branch)) if apply && pr.State == "OPEN" && pr.HeadRefName == branch { if err := runtime.GitHub.EditPRBase(runtime.Context, pr.Number, record.ParentBranch); err != nil { return err @@ -708,7 +700,7 @@ stack sync --apply } } if !runtime.Git.RemoteBranchExists(runtime.Context, state.DefaultRemote, branch) { - repairs = append(repairs, fmt.Sprintf("%s: remote branch is missing", branch)) + repairs = append(repairs, fmt.Sprintf("%s: remote branch is missing; run `stack submit %s` to republish it", branch, branch)) } cleanMergedParent := pr.State == "MERGED" && @@ -716,7 +708,7 @@ stack sync --apply (pr.BaseRefName == "" || pr.BaseRefName == record.ParentBranch) if pr.State == "MERGED" && !cleanMergedParent { - repairs = append(repairs, fmt.Sprintf("%s: merged parent has drifted PR metadata; inspect manually before reparenting children", branch)) + repairs = append(repairs, fmt.Sprintf("%s: merged parent has drifted PR metadata; run `stack status` and inspect the merged PR before reparenting children", branch)) } if cleanMergedParent { @@ -726,7 +718,7 @@ stack sync --apply if parentHeadOID != "" && childRecord.Restack.LastParentHeadOID == parentHeadOID { repairs = append(repairs, fmt.Sprintf("%s: clean reparent %s -> %s", child, branch, record.ParentBranch)) } else { - repairs = append(repairs, fmt.Sprintf("%s: merged parent %s needs manual review before reparenting", child, branch)) + repairs = append(repairs, fmt.Sprintf("%s: merged parent %s needs manual review before reparenting; repair the branch graph, then rerun `stack sync --apply`", child, branch)) continue } if apply { @@ -735,7 +727,7 @@ stack sync --apply } } } else if pr.State == "CLOSED" { - repairs = append(repairs, fmt.Sprintf("%s: PR is closed without merge; manual repair required", branch)) + repairs = append(repairs, fmt.Sprintf("%s: PR is closed without merge; relink or clear local metadata before continuing", branch)) } } @@ -908,10 +900,18 @@ type submitPlan struct { LocalHead string RemoteHead string RemoteExists bool + Metadata submitPRMetadata Record store.BranchRecord Preview []string } +type submitPRMetadata struct { + Title string + Body string + TitleSource string + BodySource string +} + func buildSubmitPlan(runtime *stackruntime.Runtime, state store.RepoState, branch string) (submitPlan, error) { record, ok := state.Branches[branch] if !ok { @@ -940,6 +940,7 @@ func buildSubmitPlan(runtime *stackruntime.Runtime, state store.RepoState, branc } preview := []string{fmt.Sprintf("%s -> %s", branch, record.ParentBranch)} + metadata := submitPRMetadata{} if !remoteExists { preview = append(preview, "push new remote branch") } else if remoteHeadOID != localHeadOID { @@ -949,7 +950,13 @@ func buildSubmitPlan(runtime *stackruntime.Runtime, state store.RepoState, branc } if record.PR.Number == 0 { + metadata, err = resolveSubmitPRMetadata(runtime, branch, record.ParentBranch) + if err != nil { + return submitPlan{}, err + } preview = append(preview, "create GitHub pull request") + preview = append(preview, fmt.Sprintf("PR title: %q (%s)", metadata.Title, metadata.TitleSource)) + preview = append(preview, fmt.Sprintf("PR body: %s", metadata.BodySource)) } else if record.PR.BaseRefName != "" && record.PR.BaseRefName != record.ParentBranch { preview = append(preview, fmt.Sprintf("retarget PR #%d base to %s", record.PR.Number, record.ParentBranch)) } else { @@ -961,11 +968,38 @@ func buildSubmitPlan(runtime *stackruntime.Runtime, state store.RepoState, branc LocalHead: localHeadOID, RemoteHead: remoteHeadOID, RemoteExists: remoteExists, + Metadata: metadata, Record: record, Preview: preview, }, nil } +func resolveSubmitPRMetadata(runtime *stackruntime.Runtime, branch string, parent string) (submitPRMetadata, error) { + title, body, err := runtime.Git.CommitMessage(runtime.Context, branch) + if err != nil { + return submitPRMetadata{}, err + } + + metadata := submitPRMetadata{ + Title: strings.TrimSpace(title), + Body: strings.TrimSpace(body), + TitleSource: "commit subject", + BodySource: "commit body", + } + + if metadata.Title == "" { + metadata.Title = branch + metadata.TitleSource = "branch name fallback" + } + + if metadata.Body == "" { + metadata.Body = fmt.Sprintf("Stack branch `%s` targeting `%s`.\n\nGenerated by `stack submit` because the tip commit body was empty.", branch, parent) + metadata.BodySource = "generated default" + } + + return metadata, nil +} + func restackSteps(runtime *stackruntime.Runtime, state store.RepoState, args []string, all bool) ([]store.RestackStep, error) { targets, err := selectTargets(runtime, state, args, all) if err != nil { @@ -1318,13 +1352,13 @@ func validateTrackedPR(branch string, record store.BranchRecord) error { return nil } if record.PR.State == "CLOSED" { - return fmt.Errorf("tracked PR for %q is closed; repair local metadata before submitting again", branch) + return fmt.Errorf("tracked PR for %q is closed; run `stack status` and repair or relink local metadata before submitting again", branch) } if record.PR.State == "MERGED" { return fmt.Errorf("tracked PR for %q is already merged; run `stack sync` before submitting again", branch) } if record.PR.HeadRefName != "" && record.PR.HeadRefName != branch { - return fmt.Errorf("tracked PR for %q points at head %q", branch, record.PR.HeadRefName) + return fmt.Errorf("tracked PR for %q points at head %q; inspect with `stack status` and relink the correct PR before submitting again", branch, record.PR.HeadRefName) } return nil } diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index 45e910b..b00c525 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -798,7 +798,7 @@ func TestSubmitCreatesAndTracksPR(t *testing.T) { t.Setenv("STACK_TEST_GH_LOG", ghStub.LogPath) t.Setenv("PATH", ghStub.Dir+string(os.PathListSeparator)+os.Getenv("PATH")) - executeCommand(t, runtime, "submit", "feature/a", "--yes", "--no-restack") + output := executeCommand(t, runtime, "submit", "feature/a", "--yes", "--no-restack") state, err = runtime.Store.ReadState(runtime.Context) if err != nil { @@ -815,10 +815,81 @@ func TestSubmitCreatesAndTracksPR(t *testing.T) { t.Fatalf("expected remote feature/a branch to exist") } + if !strings.Contains(output, `PR title: "add feature a" (commit subject)`) { + t.Fatalf("expected submit preview to include commit-subject title source, got %q", output) + } + if !strings.Contains(output, "PR body: generated default") { + t.Fatalf("expected submit preview to include generated body source, got %q", output) + } + log := readFile(t, ghStub.LogPath) if !strings.Contains(log, "pr create --base main --head feature/a") { t.Fatalf("expected gh create log entry, got %q", log) } + + ghState := readFile(t, ghStub.StatePath) + if !strings.Contains(ghState, `"title": "add feature a"`) { + t.Fatalf("expected gh state to capture commit-subject title, got %q", ghState) + } + if !strings.Contains(ghState, "Generated by `stack submit` because the tip commit body was empty.") { + t.Fatalf("expected gh state to capture generated default body, got %q", ghState) + } +} + +func TestSubmitFallsBackToBranchNameWhenCommitSubjectIsEmpty(t *testing.T) { + repo := testutil.SetupGitRepo(t) + remote := filepath.Join(t.TempDir(), "remote.git") + testutil.Run(t, repo, "git", "init", "--bare", remote) + testutil.Run(t, repo, "git", "remote", "add", "origin", remote) + testutil.Run(t, repo, "git", "push", "-u", "origin", "main") + + testutil.Run(t, repo, "git", "switch", "-c", "feature/a") + testutil.Run(t, repo, "git", "commit", "--allow-empty", "--allow-empty-message", "-m", "") + + runtime := newTestRuntime(repo) + mainHead, err := runtime.Git.ResolveRef(runtime.Context, "main") + if err != nil { + t.Fatalf("resolve main head: %v", err) + } + state := store.RepoState{ + Version: 1, + Repo: "hack-dance/stack", + DefaultRemote: "origin", + Trunk: "main", + Branches: map[string]store.BranchRecord{ + "feature/a": { + ParentBranch: "main", + RemoteName: "origin", + Restack: store.RestackMetadata{ + LastParentHeadOID: mainHead, + }, + }, + }, + } + if err := runtime.Store.WriteState(runtime.Context, state); err != nil { + t.Fatalf("write state: %v", err) + } + + ghStub := testutil.SetupGHStub(t, "hack-dance/stack", "main") + t.Setenv("STACK_TEST_GH_STATE", ghStub.StatePath) + t.Setenv("STACK_TEST_GH_LOG", ghStub.LogPath) + t.Setenv("PATH", ghStub.Dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + output := executeCommand(t, runtime, "submit", "feature/a", "--yes", "--no-restack") + if !strings.Contains(output, `PR title: "feature/a" (branch name fallback)`) { + t.Fatalf("expected branch-name title fallback in preview, got %q", output) + } + if !strings.Contains(output, "PR body: generated default") { + t.Fatalf("expected generated default body in preview, got %q", output) + } + + ghState := readFile(t, ghStub.StatePath) + if !strings.Contains(ghState, `"title": "feature/a"`) { + t.Fatalf("expected gh state to capture branch-name fallback title, got %q", ghState) + } + if !strings.Contains(ghState, "Stack branch `feature/a` targeting `main`.") || !strings.Contains(ghState, "Generated by `stack submit` because the tip commit body was empty.") { + t.Fatalf("expected gh state to capture generated body fallback, got %q", ghState) + } } func TestCreateRejectsDetachedHEAD(t *testing.T) { diff --git a/internal/github/client.go b/internal/github/client.go index 34752bc..aa30e7c 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -111,7 +111,7 @@ func (c *Client) FindPRByHead(ctx context.Context, branch string) (store.PullReq for _, pr := range open { numbers = append(numbers, fmt.Sprintf("#%d", pr.Number)) } - return store.PullRequest{}, fmt.Errorf("multiple open pull requests match head %q: %s", branch, strings.Join(numbers, ", ")) + return store.PullRequest{}, fmt.Errorf("multiple open pull requests match head %q: %s; close or retarget the duplicate until one open PR remains, then rerun `stack submit %s`", branch, strings.Join(numbers, ", "), branch) } } diff --git a/internal/github/client_test.go b/internal/github/client_test.go index 9e0a581..ca4b582 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -190,7 +190,7 @@ func TestFindPRByHeadRejectsAmbiguousOpenMatches(t *testing.T) { client := stackgh.NewClient(repo) _, err := client.FindPRByHead(context.Background(), "feature/a") - if err == nil || !strings.Contains(err.Error(), "multiple open pull requests match head") { + if err == nil || !strings.Contains(err.Error(), "multiple open pull requests match head") || !strings.Contains(err.Error(), "then rerun `stack submit feature/a`") { t.Fatalf("expected ambiguous open-pr error, got %v", err) } } diff --git a/internal/stack/summary.go b/internal/stack/summary.go index 7bf19f3..eece7bf 100644 --- a/internal/stack/summary.go +++ b/internal/stack/summary.go @@ -108,7 +108,7 @@ func BuildSummary(ctx context.Context, git *stackgit.Client, state store.RepoSta if !isAncestor { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityWarn, - Message: "branch is not currently based on parent", + Message: "branch is not currently based on parent; run `stack restack` or repair it manually", }) } } @@ -117,19 +117,24 @@ func BuildSummary(ctx context.Context, git *stackgit.Client, state store.RepoSta if record.Restack.LastParentHeadOID == "" { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityWarn, - Message: "missing restack anchor", + Message: "missing restack anchor; re-track or repair metadata before restacking", }) } else if summary.ParentHeadOID != "" && record.Restack.LastParentHeadOID != summary.ParentHeadOID { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityInfo, - Message: "parent moved since last recorded restack", + Message: "parent moved since last recorded restack; run `stack restack` before submitting", }) } if !summary.RemoteExists { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityInfo, - Message: "remote branch has not been pushed yet", + Message: "remote branch has not been pushed yet; run `stack submit` when ready", + }) + } else if record.PR.Number == 0 { + summary.Issues = append(summary.Issues, HealthIssue{ + Severity: SeverityWarn, + Message: "remote branch exists but no PR is linked; run `stack submit` to relink or create one", }) } @@ -137,31 +142,31 @@ func BuildSummary(ctx context.Context, git *stackgit.Client, state store.RepoSta if record.PR.State == "CLOSED" { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityWarn, - Message: "tracked PR is closed and needs repair or relink", + Message: "tracked PR is closed; repair or relink it before submitting again", }) } if record.PR.State == "MERGED" { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityInfo, - Message: "tracked PR is merged; descendants may need sync", + Message: "tracked PR is merged; run `stack sync` before moving descendants", }) } if record.PR.BaseRefName != "" && record.PR.BaseRefName != record.ParentBranch { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityWarn, - Message: fmt.Sprintf("cached PR base is %q, expected %q", record.PR.BaseRefName, record.ParentBranch), + Message: fmt.Sprintf("cached PR base is %q, expected %q; run `stack submit` to retarget it", record.PR.BaseRefName, record.ParentBranch), }) } if record.PR.HeadRefName != "" && record.PR.HeadRefName != branchName { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityWarn, - Message: fmt.Sprintf("cached PR head is %q, expected %q", record.PR.HeadRefName, branchName), + Message: fmt.Sprintf("cached PR head is %q, expected %q; relink the correct PR before continuing", record.PR.HeadRefName, branchName), }) } if summary.CurrentHeadOID != "" && record.PR.LastSeenHeadOID != "" && record.PR.LastSeenHeadOID != summary.CurrentHeadOID { summary.Issues = append(summary.Issues, HealthIssue{ Severity: SeverityInfo, - Message: "local branch head differs from last synced PR head", + Message: "local branch head differs from the last synced PR head; run `stack submit` to refresh it", }) } } diff --git a/internal/stack/summary_test.go b/internal/stack/summary_test.go index 76f1ba3..0e648ac 100644 --- a/internal/stack/summary_test.go +++ b/internal/stack/summary_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" stackgit "github.com/hack-dance/stack/internal/git" @@ -50,6 +51,61 @@ func TestBuildSummaryDetectsMissingAnchor(t *testing.T) { } } +func TestBuildSummaryFlagsUnlinkedRemoteBranchWithSubmitGuidance(t *testing.T) { + t.Parallel() + + repo := setupRepo(t) + remote := filepath.Join(t.TempDir(), "remote.git") + run(t, repo, "git", "init", "--bare", remote) + run(t, repo, "git", "remote", "add", "origin", remote) + run(t, repo, "git", "push", "-u", "origin", "main") + + run(t, repo, "git", "switch", "-c", "feature/a") + if err := os.WriteFile(filepath.Join(repo, "a.txt"), []byte("a\n"), 0o644); err != nil { + t.Fatalf("write file: %v", err) + } + run(t, repo, "git", "add", "a.txt") + run(t, repo, "git", "commit", "-m", "add a") + run(t, repo, "git", "push", "-u", "origin", "feature/a") + + client := stackgit.NewClient(repo) + mainHead := strings.TrimSpace(runOutput(t, repo, "git", "rev-parse", "main")) + state := store.RepoState{ + Version: 1, + Repo: "hack-dance/stack", + DefaultRemote: "origin", + Trunk: "main", + Branches: map[string]store.BranchRecord{ + "feature/a": { + ParentBranch: "main", + Restack: store.RestackMetadata{ + LastParentHeadOID: mainHead, + }, + }, + }, + } + + summary, err := stack.BuildSummary(context.Background(), client, state) + if err != nil { + t.Fatalf("build summary: %v", err) + } + + if len(summary.Branches) != 1 { + t.Fatalf("expected 1 branch summary, got %d", len(summary.Branches)) + } + + found := false + for _, issue := range summary.Branches[0].Issues { + if strings.Contains(issue.Message, "run `stack submit` to relink or create one") { + found = true + break + } + } + if !found { + t.Fatalf("expected actionable relink guidance in summary, got %+v", summary.Branches[0].Issues) + } +} + func setupRepo(t *testing.T) string { t.Helper() @@ -75,3 +131,15 @@ func run(t *testing.T, dir string, name string, args ...string) { t.Fatalf("%s %v: %v\n%s", name, args, err, string(output)) } } + +func runOutput(t *testing.T, dir string, name string, args ...string) string { + t.Helper() + + cmd := exec.Command(name, args...) + cmd.Dir = dir + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("%s %v: %v\n%s", name, args, err, string(output)) + } + return string(output) +}