From 51d862298e105de6d820ddc23877456ffa304ac0 Mon Sep 17 00:00:00 2001 From: Dominic Black Date: Wed, 4 Nov 2026 17:08:10 +0000 Subject: [PATCH] Add git stack doctor to check and fix git-stack's setup When something git-stack leans on is missing or in a state it can't work with, it only finds out part way through a command, and the error rarely says what to do: an old gh-stack, gh logged out, or stack files gh-stack 0.2 kept per worktree that make gh stack 0.3 stop with "cannot migrate stack catalogs". git stack doctor runs a list of checks, each with the checks that only make sense once it passes (gh-stack's version under gh-stack, under gh): git and its version, gh and its login, gh-stack and its version, git-stack's own aliases and whether a newer release is out, gh stack's stack file and whether any worktree still keeps its own, and in a repository an unfinished restack, tracked branches that are gone and an empty branch under others. Outside a repository the ones that need none still run. Each problem says what to do about it, and with --fix the ones git-stack can put right itself are fixed (install or upgrade gh-stack, set missing aliases without replacing any the user set, update git-stack, move the per worktree files into the shared one, forget branches that are gone) before everything is checked again. It exits non-zero while anything still fails. The checks live with what they check through a small doctor.Provider port: the backend checks gh and gh-stack, gitconfig the aliases, update the release, and the use case adds git and the stack checks. stack_doctor gives MCP clients the same, with the checks flattened (each naming its parent), outside a repository too, but reports a newer release rather than replacing the binary the server runs as. The server's instructions point not_installed and state_migration errors at it. --- README.md | 29 +++ cmd/doctor.go | 99 +++++++++ cmd/mcp.go | 14 ++ cmd/root.go | 23 ++ cmd/update.go | 44 ++-- docs/mcp.md | 1 + docs/style.md | 1 + pkg/app/app.go | 6 + pkg/app/doctor.go | 340 +++++++++++++++++++++++++++++ pkg/app/doctor_test.go | 158 ++++++++++++++ pkg/backend/ghstack/doctor.go | 212 ++++++++++++++++++ pkg/backend/ghstack/doctor_test.go | 86 ++++++++ pkg/doctor/doctor.go | 100 +++++++++ pkg/git/git.go | 27 +++ pkg/git/version_internal_test.go | 16 ++ pkg/gitconfig/doctor.go | 62 ++++++ pkg/gitconfig/doctor_test.go | 39 ++++ pkg/mcp/instructions.go | 2 +- pkg/mcp/server.go | 9 +- pkg/mcp/server_test.go | 45 +++- pkg/mcp/tools.go | 36 +++ pkg/ui/doctor.go | 87 ++++++++ pkg/ui/doctor_test.go | 36 +++ pkg/ui/report.go | 1 + pkg/update/doctor.go | 77 +++++++ pkg/update/doctor_test.go | 47 ++++ 26 files changed, 1575 insertions(+), 22 deletions(-) create mode 100644 cmd/doctor.go create mode 100644 pkg/app/doctor.go create mode 100644 pkg/app/doctor_test.go create mode 100644 pkg/backend/ghstack/doctor.go create mode 100644 pkg/backend/ghstack/doctor_test.go create mode 100644 pkg/doctor/doctor.go create mode 100644 pkg/git/version_internal_test.go create mode 100644 pkg/gitconfig/doctor.go create mode 100644 pkg/gitconfig/doctor_test.go create mode 100644 pkg/ui/doctor.go create mode 100644 pkg/ui/doctor_test.go create mode 100644 pkg/update/doctor.go create mode 100644 pkg/update/doctor_test.go diff --git a/README.md b/README.md index e1b6a83..3bd2312 100644 --- a/README.md +++ b/README.md @@ -819,6 +819,35 @@ Flags: - `--rebase` rebase the commits onto trunk as they are - `--squash` squash each PR into one commit + +### `git stack doctor` + +Checks that git-stack has everything it needs and, inside a repository, that your stacks are in +good shape. Each check has the checks that only make sense once it passes hanging off it, so a +missing gh doesn't also report a missing login: + +- git, and that it's 2.40 or later +- gh, its login, the gh-stack extension and that it's v0.3.0 or later +- git-stack's git aliases (one you set to something else yourself is listed, never replaced) and + whether a newer git-stack release is out +- gh stack's stack file, and that no worktree still keeps its own from gh-stack 0.2 (the cause of + gh stack's `cannot migrate stack catalogs` error) +- no restack waiting to be finished, every tracked branch still exists, and no branch with no + commits of its own sits under others (submit would stop there) + +Every problem comes with what to do about it, and it exits non-zero while anything fails. Outside a +repository it runs the checks that don't need one. + +```sh +git stack doctor +git stack doctor --fix # install or upgrade gh-stack, set missing aliases, update + # git-stack, move per worktree stack files into the shared one, + # forget branches that are gone; then check again +``` + +Flags: +- `--fix` fix what can be fixed automatically, then check again + ## Configuration Everything is plain `git config`, so set it globally or per repo as you like. diff --git a/cmd/doctor.go b/cmd/doctor.go new file mode 100644 index 0000000..2625a9b --- /dev/null +++ b/cmd/doctor.go @@ -0,0 +1,99 @@ +package cmd + +import ( + "context" + "fmt" + "runtime" + + "github.com/spf13/cobra" + + "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/gitconfig" + "github.com/DomBlack/git-stack/pkg/stack" + "github.com/DomBlack/git-stack/pkg/ui" + "github.com/DomBlack/git-stack/pkg/update" +) + +func newDoctorCmd(c *cli) *cobra.Command { + var fix bool + cmd := &cobra.Command{ + Use: "doctor", + Short: "Check that git-stack has what it needs, and fix what it can", + Long: `Runs a list of checks, each with the checks that only make sense once it +passes: git and its version, gh and its login, the gh-stack extension and its +version, git-stack's own aliases and whether a newer release is out, and +inside a repository gh stack's stack file and the stacks +themselves (an interrupted restack, tracked branches that are gone, an empty +branch under others that submit would stop at). + +Every problem says what to do about it. With --fix, the ones git-stack can +put right itself are fixed (installing or upgrading gh-stack, setting the +missing aliases, updating git-stack, moving stack files gh-stack 0.2 kept per +worktree into the shared one, forgetting branches that are gone) and everything is checked again. It exits non-zero while +anything still fails.`, + Example: ` # see what's wrong + git stack doctor + + # and fix what can be fixed + git stack doctor --fix`, + Args: cobra.NoArgs, + ValidArgsFunction: completeNothing, + RunE: func(cmd *cobra.Command, _ []string) error { + ctx := cmd.Context() + a, repo, err := c.doctorApp(ctx) + if err != nil { + return err + } + target, err := c.updatePath() + if err != nil { + return err + } + info := c.currentVersion() + extra := []doctor.Provider{ + gitconfig.New(c.runtime().Git), + &update.Doctor{ + Client: c.updaterClient(info), + Options: update.Options{Current: info, Target: target, GOOS: runtime.GOOS, GOARCH: runtime.GOARCH}, + AfterUpdate: func(ctx context.Context, exe string) error { + return refreshInstall(ctx, c, exe) + }, + }, + } + res, err := a.Doctor(ctx, repo, app.DoctorOptions{Fix: fix, Extra: extra}) + if err != nil { + return err + } + rep := c.report() + rep.Print(ui.RenderDoctor(res.Checks, rep.Styles(), rep.Links())) + return doctorOutcome(rep, res) + }, + } + cmd.Flags().BoolVar(&fix, "fix", false, "fix what can be fixed automatically, then check again") + return cmd +} + +// doctorOutcome prints the result line, and is an error while a check +// still fails. +func doctorOutcome(rep *ui.Reporter, res app.DoctorResult) error { + fixed := "" + if res.Fixed > 0 { + fixed = fmt.Sprintf(", %d fixed", res.Fixed) + } + if res.Failed > 0 { + e := stack.Newf(stack.KindPartial, "%d %s failed%s", res.Failed, pluralWord(res.Failed, "check", "checks"), fixed) + if res.Fixable > 0 { + e = e.WithSteps("git stack doctor --fix fixes what it can") + } + return e + } + if res.Warned > 0 { + rep.Partial("Checked git-stack's setup: %d %s%s", res.Warned, pluralWord(res.Warned, "warning", "warnings"), fixed) + if res.Fixable > 0 { + rep.NextStep("git stack doctor --fix fixes what it can") + } + return nil + } + rep.Success("Checked git-stack's setup: everything is in order%s", fixed) + return nil +} diff --git a/cmd/mcp.go b/cmd/mcp.go index c5bee87..843444c 100644 --- a/cmd/mcp.go +++ b/cmd/mcp.go @@ -3,6 +3,7 @@ package cmd import ( "context" "os" + "runtime" "github.com/spf13/cobra" @@ -11,10 +12,13 @@ import ( "github.com/DomBlack/git-stack/pkg/backend/ghstack" "github.com/DomBlack/git-stack/pkg/cache" "github.com/DomBlack/git-stack/pkg/config" + "github.com/DomBlack/git-stack/pkg/doctor" "github.com/DomBlack/git-stack/pkg/exec" "github.com/DomBlack/git-stack/pkg/forge/github" "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/gitconfig" mcpserver "github.com/DomBlack/git-stack/pkg/mcp" + "github.com/DomBlack/git-stack/pkg/update" "github.com/DomBlack/git-stack/pkg/version" ) @@ -50,6 +54,15 @@ agents such as Claude Code and Codex can drive git-stack. Register it with Cwd: cwd, Git: g, Log: log, + NewBareApp: func(context.Context) (*app.App, error) { + return app.New(app.Deps{Git: g, Log: log, Doctors: []doctor.Provider{ghstack.New(runner, g)}}), nil + }, + Doctors: []doctor.Provider{ + gitconfig.New(g), + // No AfterUpdate: the server reports a newer release but + // never replaces the binary it runs as. + &update.Doctor{Client: update.New(version.Current()), Options: update.Options{Current: version.Current(), GOOS: runtime.GOOS, GOARCH: runtime.GOARCH}}, + }, NewApp: func(ctx context.Context, repo git.Repo) (*app.App, error) { cfg, err := config.Load(ctx, g, repo) if err != nil { @@ -63,6 +76,7 @@ agents such as Claude Code and Codex can drive git-stack. Register it with Command: cfg.AICommand, Model: cfg.AIModel, ExtraPrompt: cfg.AIExtraPrompt, Timeout: cfg.AITimeout, }, log), Cache: cache.New(repo), Config: cfg, Log: log, + Doctors: []doctor.Provider{backend}, }), nil }, }) diff --git a/cmd/root.go b/cmd/root.go index 1fc7746..5675a46 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,6 +22,7 @@ import ( "github.com/DomBlack/git-stack/pkg/backend/ghstack" "github.com/DomBlack/git-stack/pkg/cache" "github.com/DomBlack/git-stack/pkg/config" + "github.com/DomBlack/git-stack/pkg/doctor" "github.com/DomBlack/git-stack/pkg/exec" "github.com/DomBlack/git-stack/pkg/forge/github" "github.com/DomBlack/git-stack/pkg/git" @@ -196,6 +197,7 @@ func (c *cli) app(ctx context.Context) (*app.App, git.Repo, error) { Config: cfg, Log: rt.Log, Progress: rt.Report.Step, + Doctors: []doctor.Provider{backend}, } if rt.Interactive { deps.Prompter = ui.Prompter{In: rt.Streams.In, Out: rt.Streams.Err, Ctx: ctx, Wait: rt.Report.Waiting} @@ -206,6 +208,26 @@ func (c *cli) app(ctx context.Context) (*app.App, git.Repo, error) { return a, repo, nil } +// doctorApp wires the use cases for git stack doctor, which also runs +// outside a repository: then repo is nil and only the checks that don't +// need one run. +func (c *cli) doctorApp(ctx context.Context) (*app.App, *git.Repo, error) { + rt := c.runtime() + repo, err := rt.Repo(ctx) + if err != nil { + if se, ok := errors.AsType[*stack.Error](err); !ok || se.Kind != stack.KindNotRepo { + return nil, nil, err + } + backend := ghstack.New(rt.Runner, rt.Git) + return app.New(app.Deps{Git: rt.Git, Log: rt.Log, Progress: rt.Report.Step, Doctors: []doctor.Provider{backend}}), nil, nil + } + a, repo, err := c.app(ctx) + if err != nil { + return nil, nil, err + } + return a, &repo, nil +} + // completionApp wires a read-only App for shell completion: metadata comes // from the backend's local file and only git may run. func (c *cli) completionApp(ctx context.Context) (*app.App, git.Repo, error) { @@ -348,6 +370,7 @@ func newRootCmd(c *cli) *cobra.Command { newCompletionCmd(c), newVersionCmd(c), newUpdateCmd(c), + newDoctorCmd(c), ) c.ownTerminal(root) return root diff --git a/cmd/update.go b/cmd/update.go index 8b3ad26..810682c 100644 --- a/cmd/update.go +++ b/cmd/update.go @@ -16,6 +16,7 @@ import ( "github.com/DomBlack/git-stack/pkg/stack" "github.com/DomBlack/git-stack/pkg/ui" "github.com/DomBlack/git-stack/pkg/update" + "github.com/DomBlack/git-stack/pkg/version" ) func newUpdateCmd(c *cli) *cobra.Command { @@ -43,21 +44,11 @@ the new release without prompting. Only the changes are printed.`, ValidArgsFunction: completeNothing, RunE: func(cmd *cobra.Command, _ []string) error { info := c.currentVersion() - target := c.updateTarget - if target == "" { - exe, err := os.Executable() - if err != nil { - return err - } - if resolved, err := filepath.EvalSymlinks(exe); err == nil { - exe = resolved - } - target = exe - } - client := c.updateClient - if client == nil { - client = update.New(info) + target, err := c.updatePath() + if err != nil { + return err } + client := c.updaterClient(info) rep := c.report() opts := update.Options{ Current: info, Target: target, GOOS: runtime.GOOS, GOARCH: runtime.GOARCH, Check: check, Force: force, @@ -67,7 +58,7 @@ the new release without prompting. Only the changes are printed.`, notes []update.ReleaseNote notesErr error ) - err := rep.Step(cmd.Context(), app.PhaseUpdate, "Checking for updates", func(ctx context.Context) error { + err = rep.Step(cmd.Context(), app.PhaseUpdate, "Checking for updates", func(ctx context.Context) error { var err error plan, err = client.Plan(ctx, opts) if err != nil { @@ -127,6 +118,29 @@ the new release without prompting. Only the changes are printed.`, return cmd } +// updatePath is the binary update replaces: this one, symlinks resolved. +func (c *cli) updatePath() (string, error) { + if c.updateTarget != "" { + return c.updateTarget, nil + } + exe, err := os.Executable() + if err != nil { + return "", err + } + if resolved, err := filepath.EvalSymlinks(exe); err == nil { + exe = resolved + } + return exe, nil +} + +// updaterClient is the release source, unless a test injected one. +func (c *cli) updaterClient(info version.Info) *update.Client { + if c.updateClient != nil { + return c.updateClient + } + return update.New(info) +} + // maxNotes is how many releases update lists before "and N more". const maxNotes = 10 diff --git a/docs/mcp.md b/docs/mcp.md index e01bf0a..28446b5 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -28,6 +28,7 @@ path is resolved with `git rev-parse`; anything outside a repository returns `no | `stack_fold` | Merge the current branch into the branch below it, one branch and one PR; the parent survives and is checked out, or with `keep` the current branch takes the parent's place; nothing is rebased, and the next `stack_submit` closes the PR of the branch that went | `keep` | destructive | | `stack_untrack` | Stop tracking a branch and every branch above it, changing no commit, ref or checkout; with `only` just that branch, and the branches above are restacked onto its parent; pull requests stay open and the next `stack_submit` regroups a stack GitHub knows; `next_steps` says how to track them again | `branch` (default current), `only` | destructive | | `stack_rename` | Give the current branch a new name, keeping its place, commits and base; a pushed branch is renamed on GitHub first (keeping its PR and retargeting PRs based on it, `remote` in the result) and nothing changes if GitHub refuses; `local` renames only this repository and leaves the PR on the old name | `name` (required), `local` | destructive, open-world | +| `stack_doctor` | Check git-stack's setup and the repository's stacks: git, gh and its login, the gh-stack extension and its version (v0.3.0 or later), gh stack's stack file (files still kept per worktree by gh-stack 0.2), git-stack's aliases and whether a newer release is out (reported only; updating is `git stack update`), an interrupted restack, tracked branches that are gone, empty branches under others; outside a repository (no `repo_path`) only the checks that need none. `checks` is flat, each with `parent`, `status` (`ok`, `warn`, `fail`, `skipped`, `fixed`), `summary`, `details`, `next_steps` and `fix` when it can put it right itself; `fix_error` says why a fix didn't work | `fix` (run the fixes, then check again) | open-world | | `stack_init` | Track branches that already exist as a new stack on a trunk; only the metadata changes, and `needsRestack` lists the branches that aren't on their parent | `branches` (bottom to top, default current), `base` | destructive (it writes the metadata, though no branch moves) | | `stack_pop` | Delete the current branch and take it out of its stack, leaving its changes staged on its parent; the branches above are restacked onto the parent first, and its open PR is closed by the next `stack_submit`; refuses with nothing changed when tracked files have changes, the branch isn't on its parent or the parent is checked out in another worktree | | destructive | | `stack_continue` | Finish an interrupted restack, modify, reorder, move or pop once the conflicts are resolved and git added | `stage_all` | destructive | diff --git a/docs/style.md b/docs/style.md index bf4d8ea..b86f27c 100644 --- a/docs/style.md +++ b/docs/style.md @@ -66,6 +66,7 @@ read as section markers rather than confetti because they never show up anywhere | merge | 🔀 | Merging 3 pull requests into main… | | rename | 🏷️ | Renaming feat-a on origin… | | refresh | 📥 | Fetching pull requests… (only when the PR cache is stale) | +| doctor | 🩺 | Checking git-stack's setup…, then Fixing: gh extension upgrade stack… | | install | 🔧 | (install prints a report, no spinner) | ## Spinners diff --git a/pkg/app/app.go b/pkg/app/app.go index 40944e8..b19d1d8 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -9,6 +9,7 @@ import ( "github.com/DomBlack/git-stack/pkg/ai" "github.com/DomBlack/git-stack/pkg/cache" "github.com/DomBlack/git-stack/pkg/config" + "github.com/DomBlack/git-stack/pkg/doctor" "github.com/DomBlack/git-stack/pkg/forge" "github.com/DomBlack/git-stack/pkg/git" "github.com/DomBlack/git-stack/pkg/stack" @@ -27,6 +28,9 @@ type Deps struct { Config *config.Config Log *slog.Logger Prompter Prompter + // Doctors contribute checks to Doctor beyond the ones the use cases + // make themselves (the backend checks gh and gh-stack). + Doctors []doctor.Provider // Progress, when set, wraps slow operations (backend calls, AI calls) so // the CLI can show a headline and spinner. It must run fn and return its // error. The phase says what kind of work it is; the message is the @@ -56,6 +60,8 @@ const ( // PhaseRefresh: fetching pull request state from the forge for a command // that only reads it (log, submit's look at which PRs exist). PhaseRefresh Phase = "refresh" + // PhaseDoctor: checking (and fixing) git-stack's setup. + PhaseDoctor Phase = "doctor" ) // Prompter asks the user a question. It is nil under --no-interactive and in diff --git a/pkg/app/doctor.go b/pkg/app/doctor.go new file mode 100644 index 0000000..aea10be --- /dev/null +++ b/pkg/app/doctor.go @@ -0,0 +1,340 @@ +package app + +import ( + "context" + "errors" + "fmt" + "slices" + "strings" + + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/shell" + "github.com/DomBlack/git-stack/pkg/stack" + "github.com/DomBlack/git-stack/pkg/version" +) + +// DoctorOptions controls Doctor. +type DoctorOptions struct { + // Fix runs the fix of every check that failed or warned and has one, + // then checks again. + Fix bool + // Extra are checks this run adds to the ones from Deps.Doctors (the + // CLI adds git-stack's own aliases and release). + Extra []doctor.Provider +} + +// DoctorResult is every check, in the order they ran, each with the checks +// that hang off it. +type DoctorResult struct { + // Checks is the tree, for rendering; List is the same checks flat, + // each naming its parent, for JSON. + Checks []doctor.Result `json:"-"` + List []doctor.Result `json:"checks"` + // Failed and Warned count the checks left failing or warning; Fixed + // those --fix put right. + Failed int `json:"failed"` + Warned int `json:"warned"` + Fixed int `json:"fixed"` + // Fixable counts the checks still failing or warning that --fix could + // try (zero after a --fix run unless a fix failed). + Fixable int `json:"fixable"` +} + +// Doctor checks that git-stack has what it needs (git, gh, gh-stack and its +// version) and, inside a repository, that its stacks are in good shape. With +// Fix it tries to put right what it can and checks again. repo is nil +// outside a repository. Problems are in the result, not the error; the +// error is only for being interrupted. +func (a *App) Doctor(ctx context.Context, repo *git.Repo, o DoctorOptions) (DoctorResult, error) { + checks := []doctor.Check{a.gitCheck()} + for _, p := range slices.Concat(a.d.Doctors, o.Extra) { + checks = append(checks, p.Checks(repo)...) + } + if repo != nil { + checks = append(checks, a.stackChecks(*repo)) + } + + var results []doctor.Result + tried := map[string]error{} + // A fix can let a check further down run for the first time (gh-stack + // installed, so its version can be looked at), so this goes round until + // there is nothing new to fix. + for { + err := a.progress(ctx, PhaseDoctor, "Checking git-stack's setup", func(ctx context.Context) error { + results = runChecks(ctx, checks) + return ctx.Err() + }) + if err != nil { + return DoctorResult{}, err + } + if !o.Fix { + break + } + var todo []doctor.Check + walkFixable(checks, results, func(c doctor.Check) { + if _, done := tried[c.ID]; !done { + todo = append(todo, c) + } + }) + if len(todo) == 0 { + break + } + for _, c := range todo { + err := a.progress(ctx, PhaseDoctor, "Fixing: "+c.FixLabel, func(ctx context.Context) error { + return c.Fix(ctx) + }) + if ctx.Err() != nil { + return DoctorResult{}, ctx.Err() + } + tried[c.ID] = err + } + } + + res := DoctorResult{Checks: results} + markFixes(res.Checks, tried, &res) + res.List = doctor.Flatten(res.Checks) + return res, nil +} + +// runChecks runs each check and, unless it failed, the ones under it. +func runChecks(ctx context.Context, checks []doctor.Check) []doctor.Result { + out := make([]doctor.Result, 0, len(checks)) + for _, c := range checks { + r := doctor.Result{ID: c.ID, Title: c.Title} + if ctx.Err() != nil { + r.Status = doctor.Skipped + out = append(out, r) + continue + } + f := c.Run(ctx) + r.Status, r.Summary, r.Details, r.Steps = f.Status, f.Summary, f.Details, f.Steps + if c.Fix != nil && (f.Status == doctor.Fail || f.Status == doctor.Warn) { + r.Fix = c.FixLabel + } + if f.Status == doctor.Fail { + r.Checks = skipped(c.Sub, c.Title) + } else { + r.Checks = runChecks(ctx, c.Sub) + } + out = append(out, r) + } + return out +} + +// skipped marks checks that couldn't run because the one above failed. +func skipped(checks []doctor.Check, parent string) []doctor.Result { + out := make([]doctor.Result, 0, len(checks)) + for _, c := range checks { + out = append(out, doctor.Result{ID: c.ID, Title: c.Title, Status: doctor.Skipped, + Summary: "skipped; needs " + parent, Checks: skipped(c.Sub, parent)}) + } + return out +} + +// walkFixable calls fn with every check whose result failed or warned and +// that has a fix. +func walkFixable(checks []doctor.Check, results []doctor.Result, fn func(doctor.Check)) { + for i, c := range checks { + if i >= len(results) { + return + } + r := results[i] + if c.Fix != nil && (r.Status == doctor.Fail || r.Status == doctor.Warn) { + fn(c) + } + walkFixable(c.Sub, r.Checks, fn) + } +} + +// markFixes turns the checks a fix put right into Fixed, notes why a fix +// that didn't work failed, and counts what is left. +func markFixes(results []doctor.Result, tried map[string]error, res *DoctorResult) { + for i := range results { + r := &results[i] + if err, ok := tried[r.ID]; ok { + switch { + case err != nil: + r.FixError = errText(err) + case r.Status == doctor.OK: + r.Status = doctor.Fixed + default: + r.FixError = "the fix ran but the check still doesn't pass" + } + } + switch r.Status { + case doctor.Fail: + res.Failed++ + case doctor.Warn: + res.Warned++ + case doctor.Fixed: + res.Fixed++ + } + if r.Fix != "" && r.FixError == "" && (r.Status == doctor.Fail || r.Status == doctor.Warn) { + res.Fixable++ + } + markFixes(r.Checks, tried, res) + } +} + +func errText(err error) string { + if se, ok := errors.AsType[*stack.Error](err); ok { + return se.Msg + } + return err.Error() +} + +// gitCheck looks at git itself. +func (a *App) gitCheck() doctor.Check { + return doctor.Check{ + ID: "git", + Title: "git", + Run: func(ctx context.Context) doctor.Finding { + v, err := a.d.Git.Version(ctx) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "can't run git", Details: []string{err.Error()}, + Steps: []string{"install git " + git.MinVersion + " or later"}} + } + if v == "" { + return doctor.Finding{Status: doctor.Warn, Summary: "can't tell which version of git this is; git-stack needs " + git.MinVersion + " or later"} + } + if version.Compare("v"+v, "v"+git.MinVersion) < 0 { + return doctor.Finding{Status: doctor.Fail, Summary: fmt.Sprintf("git %s is older than %s, which git-stack needs", v, git.MinVersion), + Steps: []string{"upgrade git"}} + } + return doctor.Finding{Status: doctor.OK, Summary: "git " + v} + }, + } +} + +// stackChecks looks at the stacks themselves. +func (a *App) stackChecks(repo git.Repo) doctor.Check { + var graph *stack.Graph + return doctor.Check{ + ID: "stacks", + Title: "stacks", + Run: func(ctx context.Context) doctor.Finding { + g, err := a.d.Meta.Load(ctx, repo) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "can't read the stacks", Details: []string{errText(err)}} + } + graph = g + n := 0 + for _, s := range g.Stacks { + n += len(s.Branches) + } + return doctor.Finding{Status: doctor.OK, Summary: fmt.Sprintf("%d %s in %d %s", + n, pluralise(n, "branch", "branches"), len(g.Stacks), pluralise(len(g.Stacks), "stack", "stacks"))} + }, + Sub: []doctor.Check{ + { + ID: "stacks.restack", + Title: "no restack is waiting to be finished", + Run: func(ctx context.Context) doctor.Finding { + err := a.noRebaseActive(ctx, repo) + if err == nil { + return doctor.Finding{Status: doctor.OK, Summary: "nothing in progress"} + } + if se, ok := errors.AsType[*stack.Error](err); ok { + return doctor.Finding{Status: doctor.Warn, Summary: se.Msg, Steps: se.NextSteps} + } + return doctor.Finding{Status: doctor.Fail, Summary: err.Error()} + }, + }, + a.missingCheck(repo, &graph), + a.emptyCheck(repo, &graph), + }, + } +} + +// missingCheck finds tracked branches that no longer exist locally (deleted +// by hand); sync would drop them too, this does it without the network. +func (a *App) missingCheck(repo git.Repo, graph **stack.Graph) doctor.Check { + missing := func(ctx context.Context) ([]string, error) { + branches, err := a.d.Git.Branches(ctx, repo) + if err != nil { + return nil, err + } + have := map[string]bool{} + for _, b := range branches { + have[b.Name] = true + } + var out []string + for _, s := range (*graph).Stacks { + for _, b := range s.Branches { + if !have[b.Name] { + out = append(out, b.Name) + } + } + } + return out, nil + } + return doctor.Check{ + ID: "stacks.missing", + Title: "every tracked branch exists", + Run: func(ctx context.Context) doctor.Finding { + names, err := missing(ctx) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "can't list branches", Details: []string{err.Error()}} + } + if len(names) == 0 { + return doctor.Finding{Status: doctor.OK, Summary: "they all do"} + } + return doctor.Finding{Status: doctor.Warn, + Summary: fmt.Sprintf("%d tracked %s no longer %s", len(names), pluralise(len(names), "branch", "branches"), pluralise(len(names), "exists", "exist")), + Details: names, Steps: []string{"git stack sync drops them too"}} + }, + Fix: func(ctx context.Context) error { + names, err := missing(ctx) + if err != nil { + return err + } + return a.d.Meta.Update(ctx, repo, func(g *stack.Graph) error { + for i := range g.Stacks { + g.Stacks[i].Branches = slices.DeleteFunc(g.Stacks[i].Branches, func(b stack.Branch) bool { return slices.Contains(names, b.Name) }) + } + return nil + }) + }, + FixLabel: "stop tracking them", + } +} + +// emptyCheck finds branches with no commits of their own under other +// branches, which can't get a pull request for the ones above to stack on. +func (a *App) emptyCheck(repo git.Repo, graph **stack.Graph) doctor.Check { + return doctor.Check{ + ID: "stacks.empty", + Title: "no empty branch in the middle of a stack", + Run: func(ctx context.Context) doctor.Finding { + f := doctor.Finding{Status: doctor.OK, Summary: "none"} + for _, s := range (*graph).Stacks { + var empty []string + parent := s.Trunk + for i, b := range s.Branches { + n, err := a.d.Git.CountCommits(ctx, repo, parent, b.Name) + parent = b.Name + if err == nil && n == 0 && !b.Merged() && i < len(s.Branches)-1 { + empty = append(empty, b.Name) + } + } + if len(empty) == 0 { + continue + } + f.Status = doctor.Warn + order := slices.DeleteFunc(s.Names(), func(n string) bool { return slices.Contains(empty, n) }) + order = append(order, empty...) + for i, n := range order { + order[i], _ = shell.Arg(n) + } + f.Details = append(f.Details, fmt.Sprintf("%s in %s's stack", joinNames(empty), s.Bottom())) + bottom, _ := shell.Arg(s.Bottom()) + f.Steps = append(f.Steps, fmt.Sprintf("git stack checkout %s, then git stack reorder %s", bottom, strings.Join(order, " "))) + } + if f.Status == doctor.Warn { + f.Summary = "a branch with no commits of its own sits under others; submit stops there" + } + return f + }, + } +} diff --git a/pkg/app/doctor_test.go b/pkg/app/doctor_test.go new file mode 100644 index 0000000..c4e7fdf --- /dev/null +++ b/pkg/app/doctor_test.go @@ -0,0 +1,158 @@ +package app_test + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/exec" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/git/gittest" + "github.com/DomBlack/git-stack/pkg/stack" +) + +// fakeDoctor is a tool that is missing until its fix installs it, with a +// version check under it that only runs once it is there. +type fakeDoctor struct { + installed bool + fixErr error + fixes int +} + +func (f *fakeDoctor) Checks(*git.Repo) []doctor.Check { + return []doctor.Check{{ + ID: "tool", Title: "tool", + Run: func(context.Context) doctor.Finding { + if !f.installed { + return doctor.Finding{Status: doctor.Fail, Summary: "missing"} + } + return doctor.Finding{Status: doctor.OK, Summary: "there"} + }, + Fix: func(context.Context) error { + f.fixes++ + if f.fixErr != nil { + return f.fixErr + } + f.installed = true + return nil + }, + FixLabel: "install tool", + Sub: []doctor.Check{{ + ID: "tool.version", Title: "tool version", + Run: func(context.Context) doctor.Finding { return doctor.Finding{Status: doctor.Warn, Summary: "old"} }, + }}, + }} +} + +func find(results []doctor.Result, id string) doctor.Result { + for _, r := range results { + if r.ID == id { + return r + } + if got := find(r.Checks, id); got.ID != "" { + return got + } + } + return doctor.Result{} +} + +func TestDoctorReportsFixesAndSkipsUnder(t *testing.T) { + gittest.Isolate(t) + g := git.New(exec.New()) + tool := &fakeDoctor{} + a := app.New(app.Deps{Git: g, Doctors: []doctor.Provider{tool}}) + + res, err := a.Doctor(context.Background(), nil, app.DoctorOptions{}) + if err != nil { + t.Fatal(err) + } + if r := find(res.Checks, "git"); r.Status != doctor.OK || !strings.HasPrefix(r.Summary, "git ") { + t.Errorf("git = %+v", r) + } + if r := find(res.Checks, "tool"); r.Status != doctor.Fail || r.Fix != "install tool" { + t.Errorf("tool = %+v", r) + } + if r := find(res.Checks, "tool.version"); r.Status != doctor.Skipped { + t.Errorf("version should be skipped under a failing tool: %+v", r) + } + if res.Failed != 1 || res.Fixable != 1 || tool.fixes != 0 { + t.Errorf("res = %+v, fixes %d", res, tool.fixes) + } + if find(res.Checks, "stacks").ID != "" { + t.Error("stack checks need a repository") + } + + res, err = a.Doctor(context.Background(), nil, app.DoctorOptions{Fix: true}) + if err != nil { + t.Fatal(err) + } + if r := find(res.Checks, "tool"); r.Status != doctor.Fixed { + t.Errorf("tool = %+v", r) + } + if r := find(res.Checks, "tool.version"); r.Status != doctor.Warn { + t.Errorf("version should run once the tool is there: %+v", r) + } + if res.Failed != 0 || res.Fixed != 1 || res.Warned != 1 || tool.fixes != 1 { + t.Errorf("res = %+v, fixes %d", res, tool.fixes) + } +} + +func TestDoctorFixThatFails(t *testing.T) { + gittest.Isolate(t) + tool := &fakeDoctor{fixErr: errors.New("no network")} + a := app.New(app.Deps{Git: git.New(exec.New()), Doctors: []doctor.Provider{tool}}) + res, err := a.Doctor(context.Background(), nil, app.DoctorOptions{Fix: true}) + if err != nil { + t.Fatal(err) + } + if r := find(res.Checks, "tool"); r.Status != doctor.Fail || r.FixError != "no network" { + t.Errorf("tool = %+v", r) + } + if tool.fixes != 1 || res.Failed != 1 || res.Fixable != 0 { + t.Errorf("a failed fix is tried once and not offered again: %+v, fixes %d", res, tool.fixes) + } +} + +func TestDoctorStackChecks(t *testing.T) { + gittest.Isolate(t) + dir := gittest.InitRepo(t) + gittest.Run(t, dir, "switch", "-q", "-c", "a") + gittest.Commit(t, dir, "a.txt", "a", "feat: a") + gittest.Run(t, dir, "branch", "empty") + gittest.Run(t, dir, "switch", "-q", "-c", "b") + gittest.Commit(t, dir, "b.txt", "b", "feat: b") + g := git.New(exec.New()) + repo, err := g.Discover(context.Background(), dir) + if err != nil { + t.Fatal(err) + } + meta := memMeta{stack.NewGraph([]stack.Stack{{Trunk: "main", Branches: []stack.Branch{{Name: "a"}, {Name: "empty"}, {Name: "gone"}, {Name: "b"}}}})} + a := app.New(app.Deps{Git: g, Meta: meta}) + + res, err := a.Doctor(context.Background(), &repo, app.DoctorOptions{}) + if err != nil { + t.Fatal(err) + } + if r := find(res.Checks, "stacks.empty"); r.Status != doctor.Warn || + !slices.Contains(r.Steps, "git stack checkout a, then git stack reorder a gone b empty") { + t.Errorf("empty = %+v", r) + } + if r := find(res.Checks, "stacks.missing"); r.Status != doctor.Warn || !slices.Equal(r.Details, []string{"gone"}) || r.Fix == "" { + t.Errorf("missing = %+v", r) + } + + res, err = a.Doctor(context.Background(), &repo, app.DoctorOptions{Fix: true}) + if err != nil { + t.Fatal(err) + } + if r := find(res.Checks, "stacks.missing"); r.Status != doctor.Fixed { + t.Errorf("missing = %+v", r) + } + if got := strings.Join(meta.g.Stacks[0].Names(), ","); got != "a,empty,b" { + t.Errorf("stack = %s", got) + } +} diff --git a/pkg/backend/ghstack/doctor.go b/pkg/backend/ghstack/doctor.go new file mode 100644 index 0000000..8dce32d --- /dev/null +++ b/pkg/backend/ghstack/doctor.go @@ -0,0 +1,212 @@ +package ghstack + +import ( + "context" + "errors" + "fmt" + "path/filepath" + "strings" + + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/exec" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/stack" +) + +var _ doctor.Provider = (*Backend)(nil) + +// Checks implements doctor.Provider: gh, its login, the gh-stack extension +// and its version, and in a repository gh stack's stack file. repo is nil +// outside a repository. +func (b *Backend) Checks(repo *git.Repo) []doctor.Check { + checks := []doctor.Check{b.ghCheck()} + if repo != nil { + checks = append(checks, b.fileCheck(*repo)) + } + return checks +} + +func (b *Backend) ghCheck() doctor.Check { + return doctor.Check{ + ID: "gh", + Title: "GitHub CLI (gh)", + Run: func(ctx context.Context) doctor.Finding { + res, err := b.run.Run(ctx, exec.Cmd{Name: "gh", Args: []string{"--version"}, Env: ghEnv, Unset: ghUnset}) + if err != nil { + if _, ok := errors.AsType[*exec.NotFoundError](err); ok { + return doctor.Finding{Status: doctor.Fail, Summary: "gh is not installed", + Steps: []string{"install it from https://cli.github.com, then run gh auth login"}} + } + return doctor.Finding{Status: doctor.Fail, Summary: "gh --version failed", Details: errLines(err)} + } + first, _, _ := strings.Cut(res.Out(), "\n") + return doctor.Finding{Status: doctor.OK, Summary: first} + }, + Sub: []doctor.Check{b.authCheck(), b.extensionCheck()}, + } +} + +func (b *Backend) authCheck() doctor.Check { + return doctor.Check{ + ID: "gh.auth", + Title: "gh is logged in", + Run: func(ctx context.Context) doctor.Finding { + res, err := b.run.Run(ctx, exec.Cmd{Name: "gh", Args: []string{"auth", "status"}, Env: ghEnv, Unset: ghUnset}) + if err != nil { + if errors.Is(err, context.Canceled) { + return doctor.Finding{Status: doctor.Skipped, Summary: "interrupted"} + } + return doctor.Finding{Status: doctor.Fail, Summary: "gh is not logged in", + Details: errLines(err), + Steps: []string{"run gh auth login (gh-stack needs an OAuth login, a personal access token is not enough)"}} + } + out := res.Out() + "\n" + res.Err() + for line := range strings.SplitSeq(out, "\n") { + line = strings.TrimSpace(strings.TrimLeft(line, "✓- ")) + if strings.HasPrefix(line, "Logged in to ") { + return doctor.Finding{Status: doctor.OK, Summary: line} + } + } + return doctor.Finding{Status: doctor.OK, Summary: "logged in"} + }, + } +} + +func (b *Backend) extensionCheck() doctor.Check { + return doctor.Check{ + ID: "gh-stack", + Title: "gh-stack extension", + Run: func(ctx context.Context) doctor.Finding { + v, err := b.Extension(ctx) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "gh-stack is not installed", Details: errLines(err)} + } + if v == "" { + v = "installed" + } + return doctor.Finding{Status: doctor.OK, Summary: "gh-stack " + v} + }, + Fix: func(ctx context.Context) error { + preflightOnce.Delete(b) + return b.ghExtension(ctx, "install", extensionName) + }, + FixLabel: "gh extension install " + extensionName, + Sub: []doctor.Check{b.versionCheck()}, + } +} + +func (b *Backend) versionCheck() doctor.Check { + return doctor.Check{ + ID: "gh-stack.version", + Title: "gh-stack is new enough", + Run: func(ctx context.Context) doctor.Finding { + v, err := b.Extension(ctx) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "gh-stack is not installed", Details: errLines(err)} + } + switch { + case TooOld(v): + return doctor.Finding{Status: doctor.Fail, + Summary: fmt.Sprintf("gh-stack %s is older than %s, which git-stack needs", v, MinVersion)} + case !strings.HasPrefix(v, "v"): + return doctor.Finding{Status: doctor.Warn, + Summary: fmt.Sprintf("gh-stack %q is a local build, so its version can't be checked; git-stack needs %s or later", v, MinVersion)} + } + return doctor.Finding{Status: doctor.OK, Summary: fmt.Sprintf("%s (needs %s or later)", v, MinVersion)} + }, + Fix: func(ctx context.Context) error { + preflightOnce.Delete(b) + return b.ghExtension(ctx, "upgrade", "stack") + }, + FixLabel: "gh extension upgrade stack", + } +} + +// ghExtension runs `gh extension ` captured, for doctor's fixes. +func (b *Backend) ghExtension(ctx context.Context, args ...string) error { + _, err := b.run.Run(ctx, exec.Cmd{Name: "gh", Args: append([]string{"extension"}, args...), Env: ghEnv, Unset: ghUnset}) + if err != nil { + return mapError([]string{"extension"}, err) + } + return nil +} + +func (b *Backend) fileCheck(repo git.Repo) doctor.Check { + return doctor.Check{ + ID: "stack-file", + Title: "gh stack's stack file", + Run: func(context.Context) doctor.Finding { + c, err := readCatalog(repo) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "can't read the stack file", Details: errLines(err), + Steps: []string{"fix or move aside " + catalogPath(repo) + ", then run git stack doctor again"}} + } + n := len(c.graph.Stacks) + if n == 1 { + return doctor.Finding{Status: doctor.OK, Summary: "1 stack in " + catalogPath(repo)} + } + return doctor.Finding{Status: doctor.OK, Summary: fmt.Sprintf("%d stacks in %s", n, catalogPath(repo))} + }, + Sub: []doctor.Check{b.layoutCheck(repo)}, + } +} + +// layoutCheck looks for per-worktree files left from gh-stack 0.2, the +// thing that makes gh stack 0.3 stop with "cannot migrate stack catalogs". +func (b *Backend) layoutCheck(repo git.Repo) doctor.Check { + return doctor.Check{ + ID: "stack-file.shared", + Title: "stacks are in the shared file", + Run: func(context.Context) doctor.Finding { + if MigrationPending(repo) { + return doctor.Finding{Status: doctor.Fail, + Summary: "gh stack is part way through moving stacks into the shared file", + Details: []string{filepath.Join(catalogDir(repo), migrationJournal)}, + Steps: []string{"run gh stack view to let it finish"}} + } + legacy := LegacyFiles(repo) + if len(legacy) == 0 { + return doctor.Finding{Status: doctor.OK, Summary: "every worktree uses " + catalogPath(repo)} + } + f := doctor.Finding{Status: doctor.Fail, + Summary: fmt.Sprintf("%d %s from gh-stack 0.2 still kept per worktree", len(legacy), plural(len(legacy), "stack file", "stack files")), + Details: legacy} + if err := checkMovable(repo, legacy); err != nil { + if se, ok := errors.AsType[*stack.Error](err); ok { + f.Details = append(f.Details, se.Msg) + f.Steps = se.NextSteps + } + } + return f + }, + Fix: func(ctx context.Context) error { + _, err := b.Migrate(ctx, repo) + return err + }, + FixLabel: "move them into the shared file, keeping each as a .pre-worktree-migration backup", + } +} + +// errLines is an error as detail lines: its message, then any detail. +func errLines(err error) []string { + if se, ok := errors.AsType[*stack.Error](err); ok { + lines := []string{se.Msg} + if d := strings.TrimSpace(se.Detail); d != "" { + lines = append(lines, strings.Split(d, "\n")...) + } + return lines + } + if ee, ok := errors.AsType[*exec.ExitError](err); ok { + if msg := strings.TrimSpace(ee.Result.Err()); msg != "" { + return strings.Split(msg, "\n") + } + } + return []string{err.Error()} +} + +func plural(n int, one, many string) string { + if n == 1 { + return one + } + return many +} diff --git a/pkg/backend/ghstack/doctor_test.go b/pkg/backend/ghstack/doctor_test.go new file mode 100644 index 0000000..8eab6da --- /dev/null +++ b/pkg/backend/ghstack/doctor_test.go @@ -0,0 +1,86 @@ +//go:build unix + +package ghstack + +import ( + "context" + "os" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/exec" + "github.com/DomBlack/git-stack/pkg/exec/exectest" +) + +func runAll(ctx context.Context, checks []doctor.Check, out map[string]doctor.Finding) { + for _, c := range checks { + f := c.Run(ctx) + out[c.ID] = f + if f.Status != doctor.Fail { + runAll(ctx, c.Sub, out) + } + } +} + +func TestDoctorChecksGhAndGhStack(t *testing.T) { + ctx := context.Background() + f := exectest.New() + f.On("gh", "--version").Reply("gh version 2.62.0 (2026-10-01)\nhttps://github.com/cli/cli/releases/tag/v2.62.0\n") + f.On("gh", "auth", "status").Reply("github.com\n ✓ Logged in to github.com account dom (keyring)\n") + f.On("gh", "extension", "list").Reply("gh stack\tgithub/gh-stack\tv0.2.1\n") + f.On("gh", "extension", "upgrade", "stack").Do(func(exec.Cmd) (exec.Result, error) { + f.On("gh", "extension", "list").Reply("gh stack\tgithub/gh-stack\tv0.3.0\n") + return exec.Result{}, nil + }) + b := New(f, nil) + got := map[string]doctor.Finding{} + checks := b.Checks(nil) + runAll(ctx, checks, got) + if got["gh"].Summary != "gh version 2.62.0 (2026-10-01)" || got["gh.auth"].Summary != "Logged in to github.com account dom (keyring)" { + t.Errorf("gh = %+v, auth = %+v", got["gh"], got["gh.auth"]) + } + if v := got["gh-stack.version"]; v.Status != doctor.Fail || !strings.Contains(v.Summary, "older than v0.3.0") { + t.Errorf("version = %+v", v) + } + if _, ok := got["stack-file"]; ok { + t.Error("the stack file needs a repository") + } + if err := checks[0].Sub[1].Sub[0].Fix(ctx); err != nil { + t.Fatal(err) + } + if v := checks[0].Sub[1].Sub[0].Run(ctx); v.Status != doctor.OK { + t.Errorf("after the upgrade = %+v", v) + } + + f = exectest.New() + f.On("gh").NotFound() + got = map[string]doctor.Finding{} + runAll(ctx, New(f, nil).Checks(nil), got) + if g := got["gh"]; g.Status != doctor.Fail || len(g.Steps) == 0 { + t.Errorf("missing gh = %+v", g) + } + if _, ok := got["gh.auth"]; ok { + t.Error("nothing under a missing gh should run") + } +} + +func TestDoctorMovesWorktreeFiles(t *testing.T) { + b, repo, mainPath, wtPath := updateFixture(t) + ctx := context.Background() + got := map[string]doctor.Finding{} + checks := b.Checks(&repo) + runAll(ctx, checks[1:], got) + if l := got["stack-file.shared"]; l.Status != doctor.Fail || len(l.Details) != 1 || l.Details[0] != wtPath { + t.Fatalf("layout = %+v", l) + } + if err := checks[1].Sub[0].Fix(ctx); err != nil { + t.Fatal(err) + } + if l := checks[1].Sub[0].Run(ctx); l.Status != doctor.OK { + t.Errorf("after the fix = %+v", l) + } + if data, _ := os.ReadFile(mainPath); !strings.Contains(string(data), `"branch": "w"`) { + t.Errorf("catalog lacks w:\n%s", data) + } +} diff --git a/pkg/doctor/doctor.go b/pkg/doctor/doctor.go new file mode 100644 index 0000000..5a3ea49 --- /dev/null +++ b/pkg/doctor/doctor.go @@ -0,0 +1,100 @@ +// Package doctor holds the shape of git-stack's health checks: what a check +// looks at, what it found and how it can put it right. The checks themselves +// live with whatever they check (the backend checks gh and gh-stack, the use +// cases check the stacks) and pkg/app runs them. +package doctor + +import ( + "context" + + "github.com/DomBlack/git-stack/pkg/git" +) + +// Status is the outcome of one check. +type Status string + +const ( + // OK: nothing wrong. + OK Status = "ok" + // Warn: it works, but something is worth knowing or tidying. + Warn Status = "warn" + // Fail: something git-stack needs is missing or broken. + Fail Status = "fail" + // Skipped: not run, because the check it hangs off failed or doesn't + // apply here (outside a repository, say). + Skipped Status = "skipped" + // Fixed: it failed or warned, and --fix put it right. + Fixed Status = "fixed" +) + +// Finding is what a check saw. +type Finding struct { + Status Status + // Summary is one line: "gh-stack v0.3.0", "2 branches have no commits". + Summary string + // Details are extra lines, one idea each (a branch, a file). + Details []string + // Steps are things the user can run to deal with it by hand. + Steps []string +} + +// Check is one thing to look at, with the checks that only make sense once +// it passes. +type Check struct { + // ID is stable and machine readable ("gh-stack.version"). + ID string + // Title names what is checked ("gh-stack is up to date"). + Title string + // Run looks and reports; it must not change anything. + Run func(ctx context.Context) Finding + // Fix tries to put right what Run found wrong; nil when that has to be + // done by hand. FixLabel says what it will do ("gh extension upgrade + // stack"). + Fix func(ctx context.Context) error + FixLabel string + // Sub run after this check unless it failed. + Sub []Check +} + +// Result is a check that has run. +type Result struct { + ID string `json:"id"` + Title string `json:"title"` + Status Status `json:"status"` + Summary string `json:"summary,omitempty"` + Details []string `json:"details,omitempty"` + Steps []string `json:"next_steps,omitempty"` + // Fix is what --fix does (or did) about it, when it can do anything. + Fix string `json:"fix,omitempty"` + // FixError is why the fix didn't work, when one was tried and failed. + FixError string `json:"fix_error,omitempty"` + // Parent is the ID of the check this one hangs off, in a Flatten list. + Parent string `json:"parent,omitempty"` + // Checks are the checks that hang off this one. They are left out of + // JSON (a schema can't describe the recursion); Flatten lists them + // with Parent set instead. + Checks []Result `json:"-"` +} + +// Flatten lists every result depth first, each with Parent set and its +// Checks left out. +func Flatten(results []Result) []Result { + var out []Result + var walk func(rs []Result, parent string) + walk = func(rs []Result, parent string) { + for _, r := range rs { + sub := r.Checks + r.Checks, r.Parent = nil, parent + out = append(out, r) + walk(sub, r.ID) + } + } + walk(results, "") + return out +} + +// Provider contributes checks. repo is nil outside a repository; checks +// that need one are left out then. +type Provider interface { + Checks(repo *git.Repo) []Check +} diff --git a/pkg/git/git.go b/pkg/git/git.go index 221b7bb..d081505 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -333,6 +333,33 @@ func (c *Client) CheckRefFormat(ctx context.Context, name string) error { return nil } +// MinVersion is the oldest git git-stack supports. +const MinVersion = "2.40.0" + +// Version returns git's version as x.y.z ("2.47.1"; a vendor suffix such +// as "(Apple Git-154)" or ".windows.1" is dropped). +func (c *Client) Version(ctx context.Context) (string, error) { + res, err := c.git(ctx, "", "version") + if err != nil { + return "", err + } + return parseVersion(res.Out()), nil +} + +// parseVersion takes the x.y.z out of `git version` output, "" when there +// is none. +func parseVersion(out string) string { + fields := strings.Fields(out) + if len(fields) < 3 { + return "" + } + parts := strings.Split(fields[2], ".") + if len(parts) < 3 { + return "" + } + return strings.Join(parts[:3], ".") +} + // ListCmds returns `git --list-cmds=` (e.g. "builtins" or "main,others"). func (c *Client) ListCmds(ctx context.Context, spec string) ([]string, error) { res, err := c.git(ctx, "", "--list-cmds="+spec) diff --git a/pkg/git/version_internal_test.go b/pkg/git/version_internal_test.go new file mode 100644 index 0000000..643344a --- /dev/null +++ b/pkg/git/version_internal_test.go @@ -0,0 +1,16 @@ +package git + +import "testing" + +func TestParseVersion(t *testing.T) { + for in, want := range map[string]string{ + "git version 2.47.1": "2.47.1", + "git version 2.39.5 (Apple Git-154)": "2.39.5", + "git version 2.45.2.windows.1": "2.45.2", + "something else": "", + } { + if got := parseVersion(in); got != want { + t.Errorf("parseVersion(%q) = %q, want %q", in, got, want) + } + } +} diff --git a/pkg/gitconfig/doctor.go b/pkg/gitconfig/doctor.go new file mode 100644 index 0000000..4cd7c86 --- /dev/null +++ b/pkg/gitconfig/doctor.go @@ -0,0 +1,62 @@ +package gitconfig + +import ( + "context" + "fmt" + "strings" + + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/git" +) + +var _ doctor.Provider = (*Manager)(nil) + +// Checks implements doctor.Provider: whether git-stack's git aliases are +// set. One you set to something else yourself is listed but never counts +// against you, and the fix never replaces it. +func (m *Manager) Checks(*git.Repo) []doctor.Check { + return []doctor.Check{{ + ID: "aliases", + Title: "git aliases", + Run: func(ctx context.Context) doctor.Finding { + plan, err := m.Plan(ctx, DefaultAliases) + if err != nil { + return doctor.Finding{Status: doctor.Fail, Summary: "can't read git's aliases", Details: []string{err.Error()}} + } + var missing, theirs []string + for _, e := range plan.Entries { + switch e.Status { + case StatusNew: + missing = append(missing, "git "+e.Alias.Name) + case StatusConflict: + theirs = append(theirs, fmt.Sprintf("git %s is %q, not %q", e.Alias.Name, e.Existing, e.Alias.Command)) + } + } + f := doctor.Finding{Status: doctor.OK, Details: theirs} + switch { + case len(missing) == len(plan.Entries): + f.Status = doctor.Warn + f.Summary = "not installed, so git ss, git c and the rest aren't there" + f.Steps = []string{"git stack install sets them up with completion and agent registration"} + case len(missing) > 0: + f.Status = doctor.Warn + f.Summary = fmt.Sprintf("%d missing: %s", len(missing), strings.Join(missing, ", ")) + default: + f.Summary = fmt.Sprintf("%d set", len(plan.Entries)-len(theirs)) + } + if len(theirs) > 0 { + f.Steps = append(f.Steps, "git stack install --force replaces the ones you set yourself") + } + return f + }, + Fix: func(ctx context.Context) error { + plan, err := m.Plan(ctx, DefaultAliases) + if err != nil { + return err + } + _, _, err = m.Apply(ctx, plan, false) + return err + }, + FixLabel: "set the missing aliases (never replacing one you set yourself)", + }} +} diff --git a/pkg/gitconfig/doctor_test.go b/pkg/gitconfig/doctor_test.go new file mode 100644 index 0000000..121e650 --- /dev/null +++ b/pkg/gitconfig/doctor_test.go @@ -0,0 +1,39 @@ +package gitconfig_test + +import ( + "context" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/doctor" + "github.com/DomBlack/git-stack/pkg/exec" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/git/gittest" + "github.com/DomBlack/git-stack/pkg/gitconfig" +) + +func TestDoctorAliases(t *testing.T) { + gittest.Isolate(t) + home := t.TempDir() + ctx := context.Background() + m := gitconfig.New(git.New(exec.New())) + c := m.Checks(nil)[0] + if f := c.Run(ctx); f.Status != doctor.Warn || !strings.Contains(f.Summary, "not installed") { + t.Errorf("none set = %+v", f) + } + gittest.Run(t, home, "config", "--global", "alias.co", "checkout") // the user's own + if err := c.Fix(ctx); err != nil { + t.Fatal(err) + } + f := c.Run(ctx) + if f.Status != doctor.OK || len(f.Details) != 1 || !strings.Contains(f.Details[0], `git co is "checkout"`) { + t.Errorf("after the fix = %+v", f) + } + if got := gittest.Run(t, home, "config", "--global", "alias.co"); got != "checkout" { + t.Errorf("the user's alias was replaced: %q", got) + } + gittest.Run(t, home, "config", "--global", "--unset", "alias.ss") + if f := c.Run(ctx); f.Status != doctor.Warn || f.Summary != "1 missing: git ss" { + t.Errorf("one missing = %+v", f) + } +} diff --git a/pkg/mcp/instructions.go b/pkg/mcp/instructions.go index 405717e..a526d08 100644 --- a/pkg/mcp/instructions.go +++ b/pkg/mcp/instructions.go @@ -6,6 +6,6 @@ const Instructions = `git-stack manages stacked branches and stacked pull reques Never run git rebase or git commit --amend by hand inside a stack: descendants would not be restacked. Use stack_modify / stack_restack instead. -Errors are JSON objects {code, message, next_steps, files?, branch?, checks?}. On code "conflict": resolve the listed files, run git add on them, then call stack_continue (or stack_abort to give up). Code "not_at_top": gh stack can only add branches at the top of a stack; call stack_navigate {direction: "top"} first, or pass insert: true to stack_create to put the branch right above the current one. Code "interaction_required": the operation would need a terminal; pass the required text (message, pull_requests) instead. Code "signing_failed": git could not sign the commit (usually an SSH signing key not loaded into ssh-agent; the server cannot answer a passphrase prompt); nothing was committed, so tell the user to run the ssh-add in next_steps (or turn signing off for the repository) and call the tool again; from a restack the rebase is left in progress, so call stack_continue once signing works, or stack_abort to put the branches back. Code "partial" from stack_sync: everything else was done, but the branches in the result's notUpdated could not be updated (reason locked: another git process held the lock file in the entry's lock field, of kind lockKind: "index" for that checkout's index or "ref" for the branch's own ref, which can apply to a branch with no checkout; use those fields rather than assuming an index lock; refused: git would not move the checkout for another reason, such as a merge in progress there; entries with reason dirty or untracked list the files in the way and don't fail the sync); follow next_steps and call stack_sync again. +Errors are JSON objects {code, message, next_steps, files?, branch?, checks?}. On code "conflict": resolve the listed files, run git add on them, then call stack_continue (or stack_abort to give up). Code "not_at_top": gh stack can only add branches at the top of a stack; call stack_navigate {direction: "top"} first, or pass insert: true to stack_create to put the branch right above the current one. Code "interaction_required": the operation would need a terminal; pass the required text (message, pull_requests) instead. Code "signing_failed": git could not sign the commit (usually an SSH signing key not loaded into ssh-agent; the server cannot answer a passphrase prompt); nothing was committed, so tell the user to run the ssh-add in next_steps (or turn signing off for the repository) and call the tool again; from a restack the rebase is left in progress, so call stack_continue once signing works, or stack_abort to put the branches back. Codes "not_installed" and "state_migration": call stack_doctor to see what is missing or in the way, and stack_doctor with fix: true to put right what it can (install or upgrade gh-stack, move stacks gh-stack 0.2 kept per worktree into the shared file). Code "partial" from stack_sync: everything else was done, but the branches in the result's notUpdated could not be updated (reason locked: another git process held the lock file in the entry's lock field, of kind lockKind: "index" for that checkout's index or "ref" for the branch's own ref, which can apply to a branch with no checkout; use those fields rather than assuming an index lock; refused: git would not move the checkout for another reason, such as a merge in progress there; entries with reason dirty or untracked list the files in the way and don't fail the sync); follow next_steps and call stack_sync again. Every tool accepts repo_path (an absolute path inside the repository); otherwise the client's first root, then the server's working directory, is used. stack_view is read-only; stack_submit pushes to the remote, stack_merge merges pull requests on the forge, and stack_sync deletes local branches.` diff --git a/pkg/mcp/server.go b/pkg/mcp/server.go index bf983a0..5b35a6a 100644 --- a/pkg/mcp/server.go +++ b/pkg/mcp/server.go @@ -16,6 +16,7 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/doctor" "github.com/DomBlack/git-stack/pkg/git" "github.com/DomBlack/git-stack/pkg/stack" ) @@ -29,7 +30,13 @@ type Options struct { // NewApp builds the use cases for a repository. It must never produce // an App that prompts or hands over a terminal. NewApp func(ctx context.Context, repo git.Repo) (*app.App, error) - Log *slog.Logger + // NewBareApp builds the use cases outside any repository, for + // stack_doctor; nil means stack_doctor needs a repository too. + NewBareApp func(ctx context.Context) (*app.App, error) + // Doctors are checks stack_doctor adds to the App's own (git-stack's + // aliases and release). + Doctors []doctor.Provider + Log *slog.Logger } // Server is the MCP server. diff --git a/pkg/mcp/server_test.go b/pkg/mcp/server_test.go index 3cadd65..047d287 100644 --- a/pkg/mcp/server_test.go +++ b/pkg/mcp/server_test.go @@ -123,10 +123,11 @@ func newHarness(t *testing.T, roots ...string) *harness { h.log = &strings.Builder{} srv := New(Options{ - Version: "test", - Cwd: t.TempDir(), // deliberately not the repo: tests must pass repo_path or roots - Git: g, - Log: slog.New(slog.NewTextHandler(h.log, &slog.HandlerOptions{Level: slog.LevelDebug})), + Version: "test", + Cwd: t.TempDir(), // deliberately not the repo: tests must pass repo_path or roots + Git: g, + Log: slog.New(slog.NewTextHandler(h.log, &slog.HandlerOptions{Level: slog.LevelDebug})), + NewBareApp: func(context.Context) (*app.App, error) { return app.New(app.Deps{Git: g}), nil }, NewApp: func(context.Context, git.Repo) (*app.App, error) { cfg := config.Defaults() cfg.CacheTTL = 0 @@ -228,7 +229,7 @@ func TestToolsAreListedWithAnnotationsAndSchemas(t *testing.T) { for _, tool := range res.Tools { got[tool.Name] = tool } - for _, name := range []string{"stack_view", "stack_create", "stack_modify", "stack_absorb", "stack_reorder", "stack_move", "stack_fold", "stack_rename", "stack_untrack", "stack_init", "stack_pop", "stack_restack", "stack_undo", "stack_navigate", "stack_submit", "stack_sync"} { + for _, name := range []string{"stack_view", "stack_create", "stack_modify", "stack_absorb", "stack_reorder", "stack_move", "stack_fold", "stack_rename", "stack_untrack", "stack_doctor", "stack_init", "stack_pop", "stack_restack", "stack_undo", "stack_navigate", "stack_submit", "stack_sync"} { if got[name] == nil { t.Errorf("missing tool %s", name) } @@ -935,3 +936,37 @@ func TestRename(t *testing.T) { t.Errorf("taken = %+v", te) } } + +func TestDoctor(t *testing.T) { + h := newHarness(t) + defer h.assertNoStdout() + var res app.DoctorResult + h.call("stack_doctor", nil, &res) + byID := map[string]string{} + for _, c := range res.List { + byID[c.ID] = string(c.Status) + "<" + c.Parent + } + if byID["git"] != "ok<" || byID["stacks"] != "ok<" || byID["stacks.missing"] != "ok