From 0695a7933eede73efc527767288190dc5f88c50f Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 12:58:58 -0500 Subject: [PATCH 1/8] Add log command for upstream commits in oldest-first order Expose the feature branch's Git history without requiring review state or launching tuicr. Delegate formatting and paging to Git so the log behaves like the underlying command, with optional patch output and whitespace filtering. --- README.md | 15 +++++++ internal/cli/cli.go | 2 +- internal/cli/log.go | 42 ++++++++++++++++++++ internal/cli/log_test.go | 86 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 internal/cli/log.go create mode 100644 internal/cli/log_test.go diff --git a/README.md b/README.md index 3824dd8..18f0dd9 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,21 @@ An absent upstream, detached HEAD, merge commits, or ambiguous fixup placement i If the branch tracks its own remote branch, the review range can be empty. Configure the intended upstream explicitly with Git. +### Show commits after the upstream + +```sh +umpire log +umpire log -p +umpire log -pw +``` + +This command runs `git log @{u}..HEAD --reverse`, so commits appear in oldest-first order. +Use `-p` or `--patch` to show patches. +Use `-w` or `--ignore-all-space` to ignore whitespace changes in patches. +Output uses Git's format and pager without reading or changing review state. +The command does not support `--json` or revision arguments. +Unlike review commands, it uses the upstream range directly and permits merge commits. + ### Count commits by review state ```sh diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 69dc791..791a008 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -102,7 +102,7 @@ func (a *application) command() *cobra.Command { _, err = fmt.Fprintf(a.output, "Review running: %t\n", running) return err }} - root.AddCommand(needsReview, feedback, approve, status, activity, a.reviewCommand()) + root.AddCommand(needsReview, feedback, approve, status, activity, a.reviewCommand(), a.logCommand()) return root } diff --git a/internal/cli/log.go b/internal/cli/log.go new file mode 100644 index 0000000..e03bd40 --- /dev/null +++ b/internal/cli/log.go @@ -0,0 +1,42 @@ +package cli + +import ( + "fmt" + "os/exec" + + "github.com/spf13/cobra" +) + +func (a *application) logCommand() *cobra.Command { + var patch, ignoreAllSpace bool + cmd := &cobra.Command{ + Use: "log", + Short: "Show commits after the upstream in oldest-first order", + Long: "Run git log @{u}..HEAD --reverse with optional patches and whitespace filtering.\nOutput uses Git's format and pager, not Umpire's review state.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + if a.json { + return fmt.Errorf("log does not support --json") + } + args := []string{"log", "@{u}..HEAD", "--reverse"} + if patch { + args = append(args, "-p") + } + if ignoreAllSpace { + args = append(args, "-w") + } + git := exec.CommandContext(cmd.Context(), "git", args...) + git.Dir = a.service.Repository.Dir + git.Stdin = a.input + git.Stdout = a.output + git.Stderr = a.terminal + if err := git.Run(); err != nil { + return fmt.Errorf("git log: %w", err) + } + return nil + }, + } + cmd.Flags().BoolVarP(&patch, "patch", "p", false, "Show the patch for each commit") + cmd.Flags().BoolVarP(&ignoreAllSpace, "ignore-all-space", "w", false, "Ignore whitespace changes in patches") + return cmd +} diff --git a/internal/cli/log_test.go b/internal/cli/log_test.go new file mode 100644 index 0000000..506f091 --- /dev/null +++ b/internal/cli/log_test.go @@ -0,0 +1,86 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestLogShowsUpstreamCommitsAndOptionalPatches(t *testing.T) { + dir := featureRepo(t) + path := filepath.Join(dir, "example.txt") + require.NoError(t, os.WriteFile(path, []byte("value = 1\n"), 0o600)) + git(t, dir, "add", "example.txt") + git(t, dir, "commit", "-m", "Add example") + require.NoError(t, os.WriteFile(path, []byte("value = 1\n"), 0o600)) + git(t, dir, "commit", "-am", "Change whitespace") + + for _, tc := range []struct { + name string + flags []string + patches int + }{ + {name: "default"}, + {name: "patch", flags: []string{"-p"}, patches: 2}, + {name: "whitespace without patch", flags: []string{"-w"}}, + {name: "patch ignoring whitespace", flags: []string{"-p", "-w"}, patches: 1}, + {name: "combined short flags", flags: []string{"-pw"}, patches: 1}, + {name: "long flags", flags: []string{"--patch", "--ignore-all-space"}, patches: 1}, + } { + t.Run(tc.name, func(t *testing.T) { + out, terminal, err := execute(t, dir, append([]string{"log"}, tc.flags...)...) + require.NoError(t, err) + require.Empty(t, terminal) + require.NotContains(t, out, " Base\n") + first := strings.Index(out, " Add feature\n") + second := strings.Index(out, " Add example\n") + third := strings.Index(out, " Change whitespace\n") + require.GreaterOrEqual(t, first, 0) + require.Greater(t, second, first) + require.Greater(t, third, second) + require.Equal(t, tc.patches, strings.Count(out, "diff --git")) + if tc.patches > 0 { + require.Contains(t, out, "+value = 1\n") + } + if tc.patches == 2 { + require.Contains(t, out, "+value = 1\n") + } else { + require.NotContains(t, out, "+value = 1\n") + } + }) + } + + git(t, dir, "reset", "--hard", "@{u}") + out, terminal, err := execute(t, dir, "log") + require.NoError(t, err) + require.Empty(t, out) + require.Empty(t, terminal) +} + +func TestLogReportsGitAndUsageErrors(t *testing.T) { + dir := featureRepo(t) + git(t, dir, "branch", "--unset-upstream") + out, terminal, err := execute(t, dir, "log") + require.Error(t, err) + require.Empty(t, out) + require.Contains(t, terminal, "no upstream configured") + + out, _, err = execute(t, t.TempDir(), "log") + require.Error(t, err) + require.Empty(t, out) + + for _, tc := range []struct { + args []string + message string + }{ + {args: []string{"log", "HEAD"}, message: "unknown command"}, + {args: []string{"log", "--unknown"}, message: "unknown flag"}, + {args: []string{"log", "--json"}, message: "log does not support --json"}, + } { + _, _, err := execute(t, dir, tc.args...) + require.ErrorContains(t, err, tc.message) + } +} From af9f95c8de1a5db5688400d044473a28aebc84cc Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 13:15:38 -0500 Subject: [PATCH 2/8] Support diff statistics in the log command Allow users to summarize changed files without displaying full patches. The --stat option also combines with patch output for more context. --- README.md | 2 ++ internal/cli/log.go | 8 ++++++-- internal/cli/log_test.go | 9 +++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 18f0dd9..5db7159 100644 --- a/README.md +++ b/README.md @@ -68,10 +68,12 @@ Configure the intended upstream explicitly with Git. umpire log umpire log -p umpire log -pw +umpire log --stat ``` This command runs `git log @{u}..HEAD --reverse`, so commits appear in oldest-first order. Use `-p` or `--patch` to show patches. +Use `--stat` to show diff statistics, with or without patches. Use `-w` or `--ignore-all-space` to ignore whitespace changes in patches. Output uses Git's format and pager without reading or changing review state. The command does not support `--json` or revision arguments. diff --git a/internal/cli/log.go b/internal/cli/log.go index e03bd40..999f013 100644 --- a/internal/cli/log.go +++ b/internal/cli/log.go @@ -8,11 +8,11 @@ import ( ) func (a *application) logCommand() *cobra.Command { - var patch, ignoreAllSpace bool + var patch, ignoreAllSpace, stat bool cmd := &cobra.Command{ Use: "log", Short: "Show commits after the upstream in oldest-first order", - Long: "Run git log @{u}..HEAD --reverse with optional patches and whitespace filtering.\nOutput uses Git's format and pager, not Umpire's review state.", + Long: "Run git log @{u}..HEAD --reverse with optional patches, diff statistics, and whitespace filtering.\nOutput uses Git's format and pager, not Umpire's review state.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { if a.json { @@ -25,6 +25,9 @@ func (a *application) logCommand() *cobra.Command { if ignoreAllSpace { args = append(args, "-w") } + if stat { + args = append(args, "--stat") + } git := exec.CommandContext(cmd.Context(), "git", args...) git.Dir = a.service.Repository.Dir git.Stdin = a.input @@ -38,5 +41,6 @@ func (a *application) logCommand() *cobra.Command { } cmd.Flags().BoolVarP(&patch, "patch", "p", false, "Show the patch for each commit") cmd.Flags().BoolVarP(&ignoreAllSpace, "ignore-all-space", "w", false, "Ignore whitespace changes in patches") + cmd.Flags().BoolVar(&stat, "stat", false, "Show diff statistics for each commit") return cmd } diff --git a/internal/cli/log_test.go b/internal/cli/log_test.go index 506f091..3ac2759 100644 --- a/internal/cli/log_test.go +++ b/internal/cli/log_test.go @@ -22,8 +22,11 @@ func TestLogShowsUpstreamCommitsAndOptionalPatches(t *testing.T) { name string flags []string patches int + stat bool }{ {name: "default"}, + {name: "stat", flags: []string{"--stat"}, stat: true}, + {name: "stat with patch", flags: []string{"--stat", "-p"}, patches: 2, stat: true}, {name: "patch", flags: []string{"-p"}, patches: 2}, {name: "whitespace without patch", flags: []string{"-w"}}, {name: "patch ignoring whitespace", flags: []string{"-p", "-w"}, patches: 1}, @@ -42,6 +45,12 @@ func TestLogShowsUpstreamCommitsAndOptionalPatches(t *testing.T) { require.Greater(t, second, first) require.Greater(t, third, second) require.Equal(t, tc.patches, strings.Count(out, "diff --git")) + if tc.stat { + require.Contains(t, out, "example.txt |") + require.Contains(t, out, "1 file changed, 1 insertion(+)") + } else { + require.NotContains(t, out, "example.txt |") + } if tc.patches > 0 { require.Contains(t, out, "+value = 1\n") } From fbc22486bbac8202b75084f298be83de2e13a900 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 13:25:10 -0500 Subject: [PATCH 3/8] Refactor: Share status snapshot and tally construction Extract functions for loading a consistent branch and review-state view and for counting its commits. The expanded status result will use the same view for both stack details and tallies. --- internal/review/service.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/internal/review/service.go b/internal/review/service.go index 3ab4d2a..3ef79b5 100644 --- a/internal/review/service.go +++ b/internal/review/service.go @@ -122,11 +122,7 @@ func (s Service) Feedback(ctx context.Context) (Result, error) { // List returns unresolved current stacks without changing review state. func (s Service) List(ctx context.Context) (Result, error) { - view, err := s.Repository.Snapshot(ctx) - if err != nil { - return Result{}, err - } - state, err := (Store{Path: view.Path}).Load() + view, state, err := s.snapshotState(ctx) if err != nil { return Result{}, err } @@ -145,14 +141,26 @@ func (s Service) ReviewRunning(ctx context.Context) (bool, error) { // Status returns ordered commit counts without changing review state. func (s Service) Status(ctx context.Context) ([]StateCount, error) { - view, err := s.Repository.Snapshot(ctx) + view, state, err := s.snapshotState(ctx) if err != nil { return nil, err } + return countStates(view, state), nil +} + +func (s Service) snapshotState(ctx context.Context) (Snapshot, State, error) { + view, err := s.Repository.Snapshot(ctx) + if err != nil { + return Snapshot{}, State{}, err + } state, err := (Store{Path: view.Path}).Load() if err != nil { - return nil, err + return Snapshot{}, State{}, err } + return view, state, nil +} + +func countStates(view Snapshot, state State) []StateCount { counts := []StateCount{ {State: "needs_review"}, {State: "reviewing"}, @@ -169,7 +177,7 @@ func (s Service) Status(ctx context.Context) ([]StateCount, error) { } } } - return counts, nil + return counts } func (s Service) selectRange(ctx context.Context, view Snapshot, first, last string) ([]Stack, error) { From d3ec59ec3452222ddef6842e8d398fe693d0c760 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 13:28:39 -0500 Subject: [PATCH 4/8] Show all review stacks in status with an optional filter Make status useful for inspecting the whole feature range while keeping needs-review available as an unresolved-only view. Include commit tallies alongside stack details in JSON, even when entries are filtered. Update the Pi consumer and agent instructions with the new envelope so review counts and feedback notices continue to use the same tallies. --- README.md | 21 ++++++-- internal/cli/cli.go | 11 ++-- internal/cli/render.go | 9 ---- internal/cli/status_test.go | 76 +++++++++++++++++++++++++--- internal/review/patch_status_test.go | 8 +-- internal/review/service.go | 18 +++++-- pi/extensions/umpire-support.test.ts | 36 +++++++++++++ pi/extensions/umpire.test.ts | 4 +- pi/extensions/umpire/support.ts | 14 +++-- skills/umpire/SKILL.md | 13 ++--- 10 files changed, 162 insertions(+), 48 deletions(-) create mode 100644 pi/extensions/umpire-support.test.ts diff --git a/README.md b/README.md index 5db7159..fd45638 100644 --- a/README.md +++ b/README.md @@ -79,22 +79,33 @@ Output uses Git's format and pager without reading or changing review state. The command does not support `--json` or revision arguments. Unlike review commands, it uses the upstream range directly and permits merge commits. -### Count commits by review state +### Show feature stacks and review state ```sh umpire status +umpire status --needs-review umpire status --json +umpire status --needs-review --json ``` -This read-only command counts commits in the current feature range, including attached fixups. +This read-only command displays all stacks in the current feature range, including approved stacks, in oldest-first order. +The table uses the same columns and details as `needs-review`, with fixups grouped in inclusive commit ranges. +Use `--needs-review` for text output identical to `umpire needs-review`. + +JSON output uses the same versioned result structure as `needs-review`, with `"command":"status"` and an additional `summary` array. +By default, `entries` includes all current stacks. +With `--needs-review`, `entries` omits approved stacks, but `summary` still counts the full feature range. +The result's `status` remains `pending` while unresolved stacks or active attempts exist, and `idle` otherwise. + +The summary counts commits in the current feature range, including attached fixups. Each commit inherits its stack's current state. Historical attempts and commits removed from the feature range do not contribute to the counts. Stacks with changed content need another review and count as `needs_review` until a new attempt records another state. Unchanged content can retain approval or pending feedback after commit hashes change. -The command always includes all five states, even when their counts are zero. -Text and JSON output use this order: `needs_review`, `reviewing`, `feedback`, `incomplete`, `approved`. -Unlike other commands, successful JSON output is a top-level array, not a versioned result object: +The `summary` array always includes all five states, even when their counts are zero. +It uses this order: `needs_review`, `reviewing`, `feedback`, `incomplete`, `approved`. +For example, the `summary` field can contain: ```json [ diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 791a008..1eb5907 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -78,16 +78,15 @@ func (a *application) command() *cobra.Command { } return a.write(result) }} - status := &cobra.Command{Use: "status", Short: "Show commit counts in each review state", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - counts, err := a.service.Status(cmd.Context()) + var onlyNeedsReview bool + status := &cobra.Command{Use: "status", Short: "Show all feature stacks and their review states", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + result, err := a.service.Status(cmd.Context(), onlyNeedsReview) if err != nil { return err } - if a.json { - return writeJSON(a.output, counts) - } - return renderStatus(a.output, counts) + return a.write(result) }} + status.Flags().BoolVar(&onlyNeedsReview, "needs-review", false, "Show unresolved stacks only, like needs-review") activity := &cobra.Command{Use: "review-running", Short: "Check whether a review process holds the shared worktree lock", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { running, err := a.service.ReviewRunning(cmd.Context()) if err != nil { diff --git a/internal/cli/render.go b/internal/cli/render.go index 578703c..b2500a3 100644 --- a/internal/cli/render.go +++ b/internal/cli/render.go @@ -16,15 +16,6 @@ import ( "github.com/nnutter/umpire/internal/review" ) -func renderStatus(output io.Writer, counts []review.StateCount) error { - var out strings.Builder - for _, count := range counts { - fmt.Fprintf(&out, "%s: %d\n", statusLabel(count.State), count.Count) - } - _, err := io.WriteString(output, out.String()) - return err -} - func render(output io.Writer, result review.Result) error { var text string var err error diff --git a/internal/cli/status_test.go b/internal/cli/status_test.go index c059d1e..cb176d0 100644 --- a/internal/cli/status_test.go +++ b/internal/cli/status_test.go @@ -32,18 +32,53 @@ func TestStatusCountsCurrentCommitsInStateOrder(t *testing.T) { })) before, err := os.ReadFile(view.Path) require.NoError(t, err) - want := `[{"state":"needs_review","count":2},{"state":"reviewing","count":1},{"state":"feedback","count":1},{"state":"incomplete","count":1},{"state":"approved","count":2}]` + want := []review.StateCount{{State: "needs_review", Count: 2}, {State: "reviewing", Count: 1}, {State: "feedback", Count: 1}, {State: "incomplete", Count: 1}, {State: "approved", Count: 2}} for _, args := range [][]string{{"status", "--json"}, {"--json", "status"}} { out, terminal, err := executeBinary(t, binary, dir, args...) require.NoError(t, err) require.Empty(t, terminal) - // Exact array order and zero fields are part of the command's contract. - require.Equal(t, want+"\n", out) + var result review.Result + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Equal(t, 1, result.Version) + require.Equal(t, "status", result.Command) + require.Equal(t, "pending", result.Status) + require.Equal(t, want, result.Summary) + require.Len(t, result.Entries, len(view.Stacks)) + for i, entry := range result.Entries { + require.Equal(t, view.Stacks[i], entry.Stack) + require.Equal(t, want[i].State, entry.Status) + } + require.Len(t, result.Active, 1) + require.Equal(t, "reviewing", result.Active[0].ID) } out, terminal, err := executeBinary(t, binary, dir, "status") require.NoError(t, err) require.Empty(t, terminal) - require.Equal(t, "Needs review: 2\nReviewing: 1\nFeedback: 1\nIncomplete: 1\nApproved: 2\n", out) + require.Contains(t, out, "COMMITS (inclusive)") + for i, stack := range view.Stacks { + require.Contains(t, out, shortStack(stack)) + require.Contains(t, out, stack.Subject) + require.Contains(t, out, statusLabel(want[i].State)) + } + filtered, terminal, err := executeBinary(t, binary, dir, "status", "--needs-review") + require.NoError(t, err) + require.Empty(t, terminal) + listed, _, err := executeBinary(t, binary, dir, "needs-review") + require.NoError(t, err) + require.Equal(t, listed, filtered) + require.NotContains(t, filtered, shortStack(view.Stacks[4])) + filtered, _, err = executeBinary(t, binary, dir, "status", "--needs-review", "--json") + require.NoError(t, err) + listed, _, err = executeBinary(t, binary, dir, "needs-review", "--json") + require.NoError(t, err) + var result, unresolved review.Result + require.NoError(t, json.Unmarshal([]byte(filtered), &result)) + require.NoError(t, json.Unmarshal([]byte(listed), &unresolved)) + require.Equal(t, "status", result.Command) + require.Equal(t, want, result.Summary) + unresolved.Command = "status" + unresolved.Summary = want + require.Equal(t, unresolved, result) after, err := os.ReadFile(view.Path) require.NoError(t, err) require.Equal(t, before, after) @@ -52,11 +87,15 @@ func TestStatusCountsCurrentCommitsInStateOrder(t *testing.T) { git(t, dir, "commit", "--allow-empty", "-m", "fixup! Approved") out, _, err = executeBinary(t, binary, dir, "status", "--json") require.NoError(t, err) - require.JSONEq(t, `[{"state":"needs_review","count":5},{"state":"reviewing","count":1},{"state":"feedback","count":1},{"state":"incomplete","count":1},{"state":"approved","count":0}]`+"\n", out) + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Equal(t, []review.StateCount{{State: "needs_review", Count: 5}, {State: "reviewing", Count: 1}, {State: "feedback", Count: 1}, {State: "incomplete", Count: 1}, {State: "approved", Count: 0}}, result.Summary) git(t, dir, "reset", "--hard", "main") out, _, err = executeBinary(t, binary, dir, "status", "--json") require.NoError(t, err) - require.JSONEq(t, `[{"state":"needs_review","count":0},{"state":"reviewing","count":0},{"state":"feedback","count":0},{"state":"incomplete","count":0},{"state":"approved","count":0}]`+"\n", out) + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Equal(t, []review.StateCount{{State: "needs_review"}, {State: "reviewing"}, {State: "feedback"}, {State: "incomplete"}, {State: "approved"}}, result.Summary) + require.Empty(t, result.Entries) + require.Equal(t, "pending", result.Status) // The historical active attempt still needs recovery. } func TestStatusWithoutReviewHistory(t *testing.T) { @@ -66,9 +105,32 @@ func TestStatusWithoutReviewHistory(t *testing.T) { out, terminal, err := execute(t, dir, "status", "--json") require.NoError(t, err) require.Empty(t, terminal) - require.JSONEq(t, `[{"state":"needs_review","count":1},{"state":"reviewing","count":0},{"state":"feedback","count":0},{"state":"incomplete","count":0},{"state":"approved","count":0}]`+"\n", out) + var result review.Result + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Equal(t, []review.StateCount{{State: "needs_review", Count: 1}, {State: "reviewing"}, {State: "feedback"}, {State: "incomplete"}, {State: "approved"}}, result.Summary) + require.Len(t, result.Entries, 1) + require.Equal(t, "needs_review", result.Entries[0].Status) _, err = os.Stat(view.Path) require.ErrorIs(t, err, os.ErrNotExist) + + _, _, err = execute(t, dir, "approve", "HEAD") + require.NoError(t, err) + out, _, err = execute(t, dir, "status", "--json") + require.NoError(t, err) + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Equal(t, "idle", result.Status) + require.Len(t, result.Entries, 1) + require.Equal(t, "approved", result.Entries[0].Status) + out, _, err = execute(t, dir, "status") + require.NoError(t, err) + require.Contains(t, out, "Approved") + require.Contains(t, out, "Add feature") + filtered, _, err := execute(t, dir, "status", "--needs-review") + require.NoError(t, err) + listed, _, err := execute(t, dir, "needs-review") + require.NoError(t, err) + require.Equal(t, listed, filtered) + require.Contains(t, filtered, "No reviews pending.") } func TestStatusOutsideGitRepository(t *testing.T) { diff --git a/internal/review/patch_status_test.go b/internal/review/patch_status_test.go index 0815403..7ae8723 100644 --- a/internal/review/patch_status_test.go +++ b/internal/review/patch_status_test.go @@ -47,9 +47,9 @@ func TestReviewStatusAfterStackRewrite(t *testing.T) { return nil })) service := Service{Repository: r} - counts, err := service.Status(t.Context()) + statusResult, err := service.Status(t.Context(), false) require.NoError(t, err) - for _, count := range counts { + for _, count := range statusResult.Summary { if count.State == tc.status { require.Equal(t, 2, count.Count) } @@ -84,9 +84,9 @@ func TestReviewStatusAfterStackRewrite(t *testing.T) { require.ErrorContains(t, err, "pending feedback") } } - counts, err = service.Status(t.Context()) + statusResult, err = service.Status(t.Context(), false) require.NoError(t, err) - for _, count := range counts { + for _, count := range statusResult.Summary { want := 0 if count.State == tc.want { want = len(result.View.Stacks[0].Commits) diff --git a/internal/review/service.go b/internal/review/service.go index 3ef79b5..e5fd845 100644 --- a/internal/review/service.go +++ b/internal/review/service.go @@ -24,6 +24,7 @@ type Result struct { Active []Attempt `json:"active"` Decision *Decision `json:"decision,omitzero"` Feedback []FeedbackRecord `json:"feedback,omitempty"` + Summary []StateCount `json:"summary,omitempty"` } // FeedbackRecord retains captured scope and marks obsolete or addressed notes. @@ -139,13 +140,22 @@ func (s Service) ReviewRunning(ctx context.Context) (bool, error) { return reviewRunning(ctx, path) } -// Status returns ordered commit counts without changing review state. -func (s Service) Status(ctx context.Context) ([]StateCount, error) { +// Status returns current stacks and ordered commit counts without changing review state. +// needsReview omits approved entries but does not filter the summary. +func (s Service) Status(ctx context.Context, needsReview bool) (Result, error) { view, state, err := s.snapshotState(ctx) if err != nil { - return nil, err + return Result{}, err } - return countStates(view, state), nil + result := makeResult("status", view, state) + result.Summary = countStates(view, state) + if !needsReview { + result.Entries = make([]Entry, 0, len(view.Stacks)) + for _, stack := range view.Stacks { + result.Entries = append(result.Entries, entryFor(state, stack)) + } + } + return result, nil } func (s Service) snapshotState(ctx context.Context) (Snapshot, State, error) { diff --git a/pi/extensions/umpire-support.test.ts b/pi/extensions/umpire-support.test.ts new file mode 100644 index 0000000..eb82f60 --- /dev/null +++ b/pi/extensions/umpire-support.test.ts @@ -0,0 +1,36 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { parseUmpireStatus } from "./umpire/support.ts"; + +const summary = [ + { state: "needs_review", count: 0 }, + { state: "reviewing", count: 0 }, + { state: "feedback", count: 0 }, + { state: "incomplete", count: 0 }, + { state: "approved", count: 3 }, +]; + +test("status parser reads idle summary and includes additional states in total", () => { + const result = { version: 1, command: "status", status: "idle", entries: [], summary: [...summary, { state: "future", count: 2 }] }; + assert.deepEqual(parseUmpireStatus(JSON.stringify(result)), { + needsReview: 0, total: 5, feedback: 0, line: "Needs Review: 0/5", + }); +}); + +test("status parser rejects unsupported envelopes and invalid summaries", () => { + const result = { version: 1, command: "status", status: "pending", summary }; + for (const invalid of [ + null, + summary, + { ...result, version: 2 }, + { ...result, command: "needs-review" }, + { ...result, status: "error" }, + { ...result, summary: undefined }, + { ...result, summary: summary.slice(1) }, + { ...result, summary: [...summary, summary[0]] }, + ...[-1, 0.5, "1"].map(count => ({ ...result, summary: [...summary.slice(0, -1), { state: "approved", count }] })), + ]) { + assert.equal(parseUmpireStatus(JSON.stringify(invalid)), undefined); + } + assert.equal(parseUmpireStatus("not json"), undefined); +}); diff --git a/pi/extensions/umpire.test.ts b/pi/extensions/umpire.test.ts index e7b5828..6aac403 100644 --- a/pi/extensions/umpire.test.ts +++ b/pi/extensions/umpire.test.ts @@ -27,13 +27,13 @@ function harness(mode: ExtensionContext["mode"] = "tui") { await activityGate; return activity; } - return { code: 0, killed: false, stdout: JSON.stringify([ + return { code: 0, killed: false, stdout: JSON.stringify({ version: 1, command: "status", status: "pending", summary: [ { state: "needs_review", count: 2 }, { state: "reviewing", count: 1 }, { state: "feedback", count: feedback }, { state: "incomplete", count: 0 }, { state: "approved", count: 0 }, - ]) }; + ] }) }; }, sendUserMessage: (message: string) => messages.push(message), } as unknown as ExtensionAPI; diff --git a/pi/extensions/umpire/support.ts b/pi/extensions/umpire/support.ts index b1c7ac6..6ea691e 100644 --- a/pi/extensions/umpire/support.ts +++ b/pi/extensions/umpire/support.ts @@ -2,7 +2,7 @@ * Pure helpers for the umpire extension. * * No Pi imports: node:test can load this file outside the Pi process. - * `umpire status --json` prints a top-level array of { state, count }. + * `umpire status --json` prints a versioned result with a summary of { state, count }. * Failures print a versioned object and exit non-zero. Callers check * the exit code before parsing. */ @@ -36,8 +36,8 @@ export function reviewStatusLine(needsReview: number, total: number): string { /** * Parse successful `umpire status --json` stdout. - * Returns undefined for error objects, malformed rows, or a missing required state. - * `total` is the sum of every state in the array, including states this parser does not name. + * Returns undefined for unsupported results, malformed rows, or a missing required state. + * `total` is the sum of every state in summary, including states this parser does not name. */ export function parseUmpireStatus(stdout: string): UmpireStatus | undefined { let parsed: unknown; @@ -46,10 +46,14 @@ export function parseUmpireStatus(stdout: string): UmpireStatus | undefined { } catch { return undefined; } - if (!Array.isArray(parsed)) return undefined; + if (parsed === null || typeof parsed !== "object" || Array.isArray(parsed)) return undefined; + const result = parsed as { version?: unknown; command?: unknown; status?: unknown; summary?: unknown }; + if (result.version !== 1 || result.command !== "status") return undefined; + if (result.status !== "pending" && result.status !== "idle") return undefined; + if (!Array.isArray(result.summary)) return undefined; const counts = new Map(); - for (const entry of parsed) { + for (const entry of result.summary) { if (entry === null || typeof entry !== "object") return undefined; const state = (entry as { state?: unknown }).state; const count = (entry as { count?: unknown }).count; diff --git a/skills/umpire/SKILL.md b/skills/umpire/SKILL.md index 846d845..978d4dc 100644 --- a/skills/umpire/SKILL.md +++ b/skills/umpire/SKILL.md @@ -12,15 +12,16 @@ The user controls review and approval. ## Check review progress -Use this read-only command when you need commit counts rather than individual stacks or saved feedback: +Use this read-only command for current stack details and commit counts: ```sh umpire status --json ``` -Successful output is a top-level array, not the versioned result object used by other commands. -Each element contains a `state` string and a `count` integer. -The array always includes these states in this order, including zero counts: +Successful output is a versioned result object with `"version":1` and `"command":"status"`. +The `entries` array includes all current stacks, including approved stacks. +The `summary` array contains elements with a `state` string and a `count` integer. +The summary always includes these states in this order, including zero counts: 1. `needs_review` 2. `reviewing` @@ -34,9 +35,9 @@ Historical attempts and removed commits do not contribute to the counts. Changed stacks lose prior approval and count as `needs_review` until a new attempt records another state. An empty feature range produces all five states with zero counts. -Check the exit code before interpreting the array. +Check the exit code and output version before interpreting the result. Command failures exit with status `1` and emit the standard versioned JSON error object instead. -Successful `status` output has no `version` field. +Use `umpire status --needs-review --json` to omit approved entries without filtering the summary. Use `umpire needs-review --json` for unresolved stack details and current commit IDs. Its `entries` omits approved stacks, but its `view.stacks` includes them. From ff74247971eac4fac85704b0211c3562208a85d2 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 13:35:47 -0500 Subject: [PATCH 5/8] Remove repository metadata before review tables Start table output with the stack list instead of branch, upstream, and state-file metadata. Keep those fields in the JSON view for consumers that need repository context. --- internal/cli/render.go | 4 ---- internal/cli/status_test.go | 6 ++++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/cli/render.go b/internal/cli/render.go index b2500a3..d9c61a4 100644 --- a/internal/cli/render.go +++ b/internal/cli/render.go @@ -33,10 +33,6 @@ func render(output io.Writer, result review.Result) error { func renderResult(result review.Result) (string, error) { var out strings.Builder - heading := lipgloss.NewStyle().Bold(true).Foreground(lipgloss.Color("12")) - fmt.Fprintln(&out, heading.Render("Feature review: "+safeText(result.View.Branch))) - fmt.Fprintln(&out, "Target: "+safeText(result.View.Target)) - fmt.Fprintln(&out, "State: "+safeText(result.View.Path)) if len(result.Entries) > 0 { rows := make([][]string, 0, len(result.Entries)) for _, entry := range result.Entries { diff --git a/internal/cli/status_test.go b/internal/cli/status_test.go index cb176d0..b421eb7 100644 --- a/internal/cli/status_test.go +++ b/internal/cli/status_test.go @@ -42,6 +42,9 @@ func TestStatusCountsCurrentCommitsInStateOrder(t *testing.T) { require.Equal(t, 1, result.Version) require.Equal(t, "status", result.Command) require.Equal(t, "pending", result.Status) + require.Equal(t, view.Branch, result.View.Branch) + require.Equal(t, view.Target, result.View.Target) + require.Equal(t, view.Path, result.View.Path) require.Equal(t, want, result.Summary) require.Len(t, result.Entries, len(view.Stacks)) for i, entry := range result.Entries { @@ -55,6 +58,9 @@ func TestStatusCountsCurrentCommitsInStateOrder(t *testing.T) { require.NoError(t, err) require.Empty(t, terminal) require.Contains(t, out, "COMMITS (inclusive)") + require.NotContains(t, out, "Feature review:") + require.NotContains(t, out, "Target:") + require.NotContains(t, out, "State:") for i, stack := range view.Stacks { require.Contains(t, out, shortStack(stack)) require.Contains(t, out, stack.Subject) From f18498abdf685bae6d685ea7c991276b9c6a8fa7 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 14:07:50 -0500 Subject: [PATCH 6/8] Replace needs-review command with filtered status Use status as the single entry point for inspecting feature stacks. Remove the redundant command and service method, and migrate the skill and Pi extension to status --needs-review --json. Filtered entries keep full-range summary counts for progress reporting. --- README.md | 14 +++++++------- internal/cli/cli.go | 11 ++--------- internal/cli/cli_test.go | 17 ++++++++++------- internal/cli/status_test.go | 26 ++++++++++++-------------- internal/cli/tuicr_functional_test.go | 6 +++--- internal/review/patch_status_test.go | 4 ++-- internal/review/service.go | 9 --------- internal/review/state_test.go | 6 +++--- pi/extensions/umpire.test.ts | 1 + pi/extensions/umpire.ts | 4 ++-- pi/extensions/umpire/support.ts | 4 ++-- skills/umpire/SKILL.md | 14 +++++++------- 12 files changed, 51 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index fd45638..c2af5e7 100644 --- a/README.md +++ b/README.md @@ -89,10 +89,10 @@ umpire status --needs-review --json ``` This read-only command displays all stacks in the current feature range, including approved stacks, in oldest-first order. -The table uses the same columns and details as `needs-review`, with fixups grouped in inclusive commit ranges. -Use `--needs-review` for text output identical to `umpire needs-review`. +The table shows each stack's status, inclusive commit range, and subject, with fixups grouped together. +Use `--needs-review` to omit approved stacks and show unresolved stacks and active attempts. -JSON output uses the same versioned result structure as `needs-review`, with `"command":"status"` and an additional `summary` array. +JSON output uses the shared versioned result structure, with `"command":"status"` and a `summary` array. By default, `entries` includes all current stacks. With `--needs-review`, `entries` omits approved stacks, but `summary` still counts the full feature range. The result's `status` remains `pending` while unresolved stacks or active attempts exist, and `idle` otherwise. @@ -123,15 +123,15 @@ Command failures use the standard JSON error object. ### List unresolved stacks ```sh -umpire needs-review -umpire needs-review --json +umpire status --needs-review +umpire status --needs-review --json ``` A stack contains one original commit and its immediately following `fixup!`, `amend!`, or `reword!` commits. Create feedback commits with `git commit --fixup=`, `--fixup=amend:`, or `--fixup=reword:`. Insert each feedback commit immediately after its target stack, before the next original commit. Targets can be originals or feedback commits within that stack, but their subjects must identify exactly one earlier commit. -The summary omits approved stacks and includes active attempts, even when their original stacks disappeared from history. +The filtered table omits approved stacks and includes active attempts, even when their original stacks disappeared from history. Current and historical feedback remain available in the result. ### Check live review activity @@ -317,7 +317,7 @@ An agent must inspect the result's status rather than equate exit status 0 with Command failures exit with status 1 and produce a JSON error object: ```json -{"version":1,"command":"needs-review","status":"error","error":"..."} +{"version":1,"command":"status","status":"error","error":"..."} ``` Help, version, and shell completion output retain their usual formats. diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 1eb5907..b919ea5 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -53,13 +53,6 @@ type application struct { func (a *application) command() *cobra.Command { root := &cobra.Command{Use: "umpire", Short: "Review feature-branch commit stacks with tuicr", Long: "Umpire tracks review of exact commit stacks against the configured Git upstream.\nApproval never transfers to rewritten commits or added fixups."} root.PersistentFlags().BoolVar(&a.json, "json", a.json, "Emit a versioned JSON result without interactive prompts") - needsReview := &cobra.Command{Use: "needs-review", Short: "Show unresolved stacks and active review attempts", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { - result, err := a.service.List(cmd.Context()) - if err != nil { - return err - } - return a.write(result) - }} feedback := &cobra.Command{Use: "feedback", Short: "Read current and historical saved feedback without launching tuicr", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { result, err := a.service.Feedback(cmd.Context()) if err != nil { @@ -86,7 +79,7 @@ func (a *application) command() *cobra.Command { } return a.write(result) }} - status.Flags().BoolVar(&onlyNeedsReview, "needs-review", false, "Show unresolved stacks only, like needs-review") + status.Flags().BoolVar(&onlyNeedsReview, "needs-review", false, "Show unresolved stacks and active review attempts") activity := &cobra.Command{Use: "review-running", Short: "Check whether a review process holds the shared worktree lock", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { running, err := a.service.ReviewRunning(cmd.Context()) if err != nil { @@ -101,7 +94,7 @@ func (a *application) command() *cobra.Command { _, err = fmt.Fprintf(a.output, "Review running: %t\n", running) return err }} - root.AddCommand(needsReview, feedback, approve, status, activity, a.reviewCommand(), a.logCommand()) + root.AddCommand(feedback, approve, status, activity, a.reviewCommand(), a.logCommand()) return root } diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 9bf2657..26fd56c 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -48,19 +48,19 @@ func execute(t *testing.T, dir string, args ...string) (string, string, error) { func TestCommandsShareStructuredResults(t *testing.T) { binary := buildUmpire(t) dir := featureRepo(t) - for _, args := range [][]string{{"needs-review", "--json"}, {"--json", "needs-review"}} { + for _, args := range [][]string{{"status", "--needs-review", "--json"}, {"--json", "status", "--needs-review"}} { out, terminal, err := executeBinary(t, binary, dir, args...) require.NoError(t, err) require.Empty(t, terminal) require.Equal(t, 1, strings.Count(out, "\n")) var result review.Result require.NoError(t, json.Unmarshal([]byte(out), &result)) - require.Equal(t, "needs-review", result.Command) + require.Equal(t, "status", result.Command) require.Equal(t, "needs_review", result.Entries[0].Status) require.Len(t, result.Entries[0].Stack.Commits[0], 40) require.NotContains(t, out, "\x1b") } - out, terminal, err := executeBinary(t, binary, dir, "needs-review") + out, terminal, err := executeBinary(t, binary, dir, "status", "--needs-review") require.NoError(t, err) require.Empty(t, terminal) require.Contains(t, out, "Needs review") @@ -75,7 +75,7 @@ func TestCommandsShareStructuredResults(t *testing.T) { require.Equal(t, "approved", approved.Status) require.Equal(t, "approve", approved.Command) require.Equal(t, "user-command", approved.Entries[0].Attempt.ApprovalSource) - out, _, err = executeBinary(t, binary, dir, "needs-review", "--json") + out, _, err = executeBinary(t, binary, dir, "status", "--needs-review", "--json") require.NoError(t, err) var listed review.Result require.NoError(t, json.Unmarshal([]byte(out), &listed)) @@ -88,11 +88,11 @@ func TestJSONErrorsIncludeArgumentAndGitFailures(t *testing.T) { dir := featureRepo(t) git(t, dir, "branch", "--unset-upstream") for _, args := range [][]string{ - {"needs-review", "--json"}, + {"status", "--needs-review", "--json"}, {"approve", "--json"}, {"review", "--recover", "--replace", "--json"}, {"not-a-command", "--json"}, - {"--json", "needs-review", "--unknown"}, + {"--json", "status", "--needs-review", "--unknown"}, } { out, terminal, err := executeBinary(t, binary, dir, args...) var exit *exec.ExitError @@ -159,7 +159,7 @@ func TestTerminalRenderingRetainsFeedbackAndRemovesControlSequences(t *testing.T attempt := review.Attempt{ID: "feedback", Stack: view.Stacks[0], Status: "feedback", StartedAt: "2026-01-01T00:00:00Z"} attempt.Review = jsontext.Value(`{"id":"saved","version":"1.3","reviewed":1,"files":2,"complete":false,"comments":[{"content":"Revise validation\u001b[31m","location":"line","path":"main.go","stored_line":"12"}],"sessionNotes":"Explain the change"}`) require.NoError(t, (review.Store{Path: view.Path}).Update(func(state *review.State) error { state.Attempts = append(state.Attempts, attempt); return nil })) - out, _, err := execute(t, dir, "needs-review") + out, _, err := execute(t, dir, "status", "--needs-review") require.NoError(t, err) require.Contains(t, out, "Feedback") require.Contains(t, out, "main.go:12: Revise validation") @@ -231,6 +231,9 @@ func TestHelpAndVersion(t *testing.T) { require.Contains(t, out, "--json") require.NotContains(t, out, "confirm") require.NotContains(t, out, "challenge") + out, _, err = execute(t, t.TempDir(), "needs-review") + require.ErrorContains(t, err, `unknown command "needs-review"`) + require.Empty(t, out) out, _, err = execute(t, t.TempDir(), "--version") require.NoError(t, err) require.Contains(t, out, "test-version") diff --git a/internal/cli/status_test.go b/internal/cli/status_test.go index b421eb7..ac3e71d 100644 --- a/internal/cli/status_test.go +++ b/internal/cli/status_test.go @@ -69,22 +69,23 @@ func TestStatusCountsCurrentCommitsInStateOrder(t *testing.T) { filtered, terminal, err := executeBinary(t, binary, dir, "status", "--needs-review") require.NoError(t, err) require.Empty(t, terminal) - listed, _, err := executeBinary(t, binary, dir, "needs-review") - require.NoError(t, err) - require.Equal(t, listed, filtered) + for _, stack := range view.Stacks[:4] { + require.Contains(t, filtered, shortStack(stack)) + } require.NotContains(t, filtered, shortStack(view.Stacks[4])) filtered, _, err = executeBinary(t, binary, dir, "status", "--needs-review", "--json") require.NoError(t, err) - listed, _, err = executeBinary(t, binary, dir, "needs-review", "--json") - require.NoError(t, err) - var result, unresolved review.Result + var result review.Result require.NoError(t, json.Unmarshal([]byte(filtered), &result)) - require.NoError(t, json.Unmarshal([]byte(listed), &unresolved)) require.Equal(t, "status", result.Command) require.Equal(t, want, result.Summary) - unresolved.Command = "status" - unresolved.Summary = want - require.Equal(t, unresolved, result) + require.Len(t, result.Entries, 4) + for i, entry := range result.Entries { + require.Equal(t, view.Stacks[i], entry.Stack) + require.Equal(t, want[i].State, entry.Status) + } + require.Len(t, result.View.Stacks, 5) + require.Len(t, result.Active, 1) after, err := os.ReadFile(view.Path) require.NoError(t, err) require.Equal(t, before, after) @@ -133,10 +134,7 @@ func TestStatusWithoutReviewHistory(t *testing.T) { require.Contains(t, out, "Add feature") filtered, _, err := execute(t, dir, "status", "--needs-review") require.NoError(t, err) - listed, _, err := execute(t, dir, "needs-review") - require.NoError(t, err) - require.Equal(t, listed, filtered) - require.Contains(t, filtered, "No reviews pending.") + require.Equal(t, "No reviews pending.\n", filtered) } func TestStatusOutsideGitRepository(t *testing.T) { diff --git a/internal/cli/tuicr_functional_test.go b/internal/cli/tuicr_functional_test.go index 925eaee..5da22a2 100644 --- a/internal/cli/tuicr_functional_test.go +++ b/internal/cli/tuicr_functional_test.go @@ -105,7 +105,7 @@ func TestRealTuicrFeedback(t *testing.T) { require.Equal(t, "Handle the empty case", saved.Comments[0].Content) require.Equal(t, "feature.txt", saved.Comments[0].Path) require.Equal(t, "line", saved.Comments[0].Location) - listed := runRealCommand(t, binary, dir, env, "needs-review") + listed := runRealCommand(t, binary, dir, env, "status", "--needs-review") require.Contains(t, listed, "Feedback") require.Contains(t, listed, "feature.txt:1: Handle the empty case") } @@ -229,7 +229,7 @@ func TestRealTuicrRecoveryPrompt(t *testing.T) { case "cancel": require.Contains(t, out, "Review cancelled. The attempt remains unchanged.") } - after := runRealCommand(t, binary, dir, env, "needs-review", "--json") + after := runRealCommand(t, binary, dir, env, "status", "--needs-review", "--json") var result struct { Status string `json:"status"` Entries []struct { @@ -281,7 +281,7 @@ func TestRealTuicrCleanReview(t *testing.T) { require.True(t, result.Entries[0].Attempt.Review.Complete) require.Equal(t, 2, result.Entries[0].Attempt.Review.Reviewed) require.Equal(t, 2, result.Entries[0].Attempt.Review.Files) - listed := runRealCommand(t, binary, dir, env, "needs-review", "--json") + listed := runRealCommand(t, binary, dir, env, "status", "--needs-review", "--json") var status struct { Status string `json:"status"` } diff --git a/internal/review/patch_status_test.go b/internal/review/patch_status_test.go index 7ae8723..f3a26b5 100644 --- a/internal/review/patch_status_test.go +++ b/internal/review/patch_status_test.go @@ -58,7 +58,7 @@ func TestReviewStatusAfterStackRewrite(t *testing.T) { require.NoError(t, err) rewriteTestStack(t, r, view, tc.rewrite) - result, err := service.List(t.Context()) + result, err := service.Status(t.Context(), true) require.NoError(t, err) require.NotEqual(t, view.Stacks[0].Key, result.View.Stacks[0].Key) if tc.want == "approved" { @@ -144,7 +144,7 @@ func TestStackRewriteDoesNotRestoreSupersededApproval(t *testing.T) { return nil })) gitTest(t, r.Dir, "commit", "--amend", "-m", "Reworded") - result, err := service.List(t.Context()) + result, err := service.Status(t.Context(), true) require.NoError(t, err) require.Len(t, result.Entries, 1) require.Equal(t, "needs_review", result.Entries[0].Status) diff --git a/internal/review/service.go b/internal/review/service.go index e5fd845..20e1a31 100644 --- a/internal/review/service.go +++ b/internal/review/service.go @@ -121,15 +121,6 @@ func (s Service) Feedback(ctx context.Context) (Result, error) { return result, nil } -// List returns unresolved current stacks without changing review state. -func (s Service) List(ctx context.Context) (Result, error) { - view, state, err := s.snapshotState(ctx) - if err != nil { - return Result{}, err - } - return makeResult("needs-review", view, state), nil -} - // ReviewRunning reports whether a process holds the shared review worktree lock. // It does not create the lock file or infer activity from persisted attempts. func (s Service) ReviewRunning(ctx context.Context) (bool, error) { diff --git a/internal/review/state_test.go b/internal/review/state_test.go index 7708045..fe45552 100644 --- a/internal/review/state_test.go +++ b/internal/review/state_test.go @@ -22,13 +22,13 @@ func TestApprovalAndSupersession(t *testing.T) { require.NoError(t, err) require.Equal(t, "approved", result.Status) require.Equal(t, "user-command", result.Entries[0].Attempt.ApprovalSource) - result, err = service.List(t.Context()) + result, err = service.Status(t.Context(), true) require.NoError(t, err) require.Equal(t, "idle", result.Status) require.Empty(t, result.Entries) gitTest(t, r.Dir, "commit", "--allow-empty", "-m", "amend! One") - result, err = service.List(t.Context()) + result, err = service.Status(t.Context(), true) require.NoError(t, err) require.Equal(t, "needs_review", result.Entries[0].Status) require.Equal(t, "approved", result.Entries[0].Previous.Status) @@ -38,7 +38,7 @@ func TestApprovalAndSupersession(t *testing.T) { require.NoError(t, store.Update(func(state *State) error { state.Attempts[0].Status = "feedback"; return nil })) gitTest(t, r.Dir, "reset", "--hard", first) gitTest(t, r.Dir, "commit", "--amend", "--allow-empty", "-m", "Rewritten\n\nChange-Id: "+id) - result, err = service.List(t.Context()) + result, err = service.Status(t.Context(), true) require.NoError(t, err) require.Equal(t, "feedback", result.Entries[0].Previous.Status) require.Equal(t, "needs_review", result.Entries[0].Status) diff --git a/pi/extensions/umpire.test.ts b/pi/extensions/umpire.test.ts index 6aac403..8a4f1a5 100644 --- a/pi/extensions/umpire.test.ts +++ b/pi/extensions/umpire.test.ts @@ -27,6 +27,7 @@ function harness(mode: ExtensionContext["mode"] = "tui") { await activityGate; return activity; } + assert.deepEqual(args, ["status", "--needs-review", "--json"]); return { code: 0, killed: false, stdout: JSON.stringify({ version: 1, command: "status", status: "pending", summary: [ { state: "needs_review", count: 2 }, { state: "reviewing", count: 1 }, diff --git a/pi/extensions/umpire.ts b/pi/extensions/umpire.ts index d504182..f68ad7f 100644 --- a/pi/extensions/umpire.ts +++ b/pi/extensions/umpire.ts @@ -1,7 +1,7 @@ /** * Umpire extension. * - * Reads `umpire status --json` for the current Git feature branch and + * Reads `umpire status --needs-review --json` for the current Git feature branch and * publishes the counts on `umpire:status`. Outside a Git repository it * publishes nothing. * @@ -55,7 +55,7 @@ export default function (pi: ExtensionAPI) { /** Publish status for a repository. Returns undefined when there is nothing to publish. */ async function readAndPublish(cwd: string): Promise { if (!(await isRepository(cwd))) return undefined; - const result = await pi.exec("umpire", ["status", "--json"], { + const result = await pi.exec("umpire", ["status", "--needs-review", "--json"], { cwd, timeout: STATUS_TIMEOUT_MS, }); diff --git a/pi/extensions/umpire/support.ts b/pi/extensions/umpire/support.ts index 6ea691e..6796b1c 100644 --- a/pi/extensions/umpire/support.ts +++ b/pi/extensions/umpire/support.ts @@ -2,7 +2,7 @@ * Pure helpers for the umpire extension. * * No Pi imports: node:test can load this file outside the Pi process. - * `umpire status --json` prints a versioned result with a summary of { state, count }. + * `umpire status --needs-review --json` prints a result with a summary of { state, count }. * Failures print a versioned object and exit non-zero. Callers check * the exit code before parsing. */ @@ -35,7 +35,7 @@ export function reviewStatusLine(needsReview: number, total: number): string { } /** - * Parse successful `umpire status --json` stdout. + * Parse successful `umpire status --needs-review --json` stdout. * Returns undefined for unsupported results, malformed rows, or a missing required state. * `total` is the sum of every state in summary, including states this parser does not name. */ diff --git a/skills/umpire/SKILL.md b/skills/umpire/SKILL.md index 978d4dc..48a0a5f 100644 --- a/skills/umpire/SKILL.md +++ b/skills/umpire/SKILL.md @@ -15,11 +15,12 @@ The user controls review and approval. Use this read-only command for current stack details and commit counts: ```sh -umpire status --json +umpire status --needs-review --json ``` Successful output is a versioned result object with `"version":1` and `"command":"status"`. -The `entries` array includes all current stacks, including approved stacks. +With `--needs-review`, the `entries` array omits approved stacks. +The `view.stacks` array includes all current stacks. The `summary` array contains elements with a `state` string and a `count` integer. The summary always includes these states in this order, including zero counts: @@ -37,10 +38,9 @@ An empty feature range produces all five states with zero counts. Check the exit code and output version before interpreting the result. Command failures exit with status `1` and emit the standard versioned JSON error object instead. -Use `umpire status --needs-review --json` to omit approved entries without filtering the summary. -Use `umpire needs-review --json` for unresolved stack details and current commit IDs. -Its `entries` omits approved stacks, but its `view.stacks` includes them. +Read `entries` for unresolved stack details and current commit IDs. +The summary counts the full feature range even when entries are filtered. Use `umpire feedback --json` for saved comments and session notes. Counts do not identify feedback targets or authorize approval. @@ -96,7 +96,7 @@ Recovery opens an interactive review and requires the user's choice. 5. Record the original branch tip and create a uniquely named local backup branch. 6. Identify each current target stack through Umpire's `view.stacks` data. -Use `umpire needs-review --json` when you need a fresh view of current stacks. +Use `umpire status --needs-review --json` when you need a fresh view of current stacks. Its `view.stacks` includes approved stacks too. Do not derive targets from comment file paths or commit subjects alone. Use full commit IDs. @@ -186,7 +186,7 @@ Git generates nested feedback subjects when the target is an existing feedback c 1. Check that the checkout uses the feature branch after integration. 2. Run the repository's required checks on the final history. -3. Run `umpire needs-review --json` to check stack formation. +3. Run `umpire status --needs-review --json` to check stack formation. 4. Check that each feedback commit immediately follows its target stack tip. 5. Report each attempt ID, addressed comment, new stack range, and validation result. From 8f7a70922d1d39630f15caa60b0dd3620a8b8d6f Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 14:12:10 -0500 Subject: [PATCH 7/8] Rename review commands to run and running Use shorter command names for launching reviews and checking live activity. Update recovery guidance, JSON command identifiers, agent instructions, and the Pi activity query so callers use the new names. --- README.md | 22 +++++++++++----------- internal/cli/activity_test.go | 4 ++-- internal/cli/cli.go | 8 ++++---- internal/cli/cli_test.go | 26 +++++++++++++++----------- internal/cli/render.go | 2 +- internal/cli/tuicr_functional_test.go | 14 +++++++------- internal/review/run.go | 10 +++++----- pi/extensions/umpire.test.ts | 2 +- pi/extensions/umpire.ts | 2 +- skills/umpire/SKILL.md | 4 ++-- 10 files changed, 49 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index c2af5e7..cfcde89 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,7 @@ Current and historical feedback remain available in the result. ### Check live review activity ```sh -umpire review-running --json +umpire running --json ``` Successful output is `{"version":1,"running":true}` or `{"version":1,"running":false}`. @@ -180,9 +180,9 @@ Active attempts remain visible in `active`, but collecting feedback does not res ### Review a stack ```sh -umpire review -umpire review HEAD -umpire review abc123..def456 +umpire run +umpire run HEAD +umpire run abc123..def456 ``` Without an argument, Umpire reviews each eligible, non-deferred stack in oldest-first order, once per invocation. @@ -218,9 +218,9 @@ An untouched review leaves no attempt in Umpire's history and does not require r ### Review a range of stacks ```sh -umpire review --start abc123 -umpire review --start abc123 --end def456 --json -umpire review --end def456 +umpire run --start abc123 +umpire run --start abc123 --end def456 --json +umpire run --end def456 ``` Each invocation reviews eligible stacks within the inclusive bounds in oldest-first order. @@ -249,9 +249,9 @@ In an interactive terminal, Umpire prompts to recover, replace, or cancel. With JSON output or redirected streams, it returns `decision_required` without a prompt. ```sh -umpire review --recover -umpire review --replace -umpire review --recover --session +umpire run --recover +umpire run --replace +umpire run --recover --session ``` Recovery reopens tuicr at the previous attempt's exact captured range and preserves its attempt ID. @@ -262,7 +262,7 @@ Neither operation proceeds while the review worktree is held by a live Umpire/tu `--session` accepts an exact saved session ID, index slug, or indexed `sessions/.json` path. It resolves ambiguous saved sessions and is not an arbitrary filesystem path. -If an interrupted original disappeared and no lineage identifies its replacement, use `umpire review --replace`. +If an interrupted original disappeared and no lineage identifies its replacement, use `umpire run --replace`. ### Record explicit user approval diff --git a/internal/cli/activity_test.go b/internal/cli/activity_test.go index 1150b47..3afe8b9 100644 --- a/internal/cli/activity_test.go +++ b/internal/cli/activity_test.go @@ -18,7 +18,7 @@ func TestReviewRunningReportsLiveLockNotPersistedAttempt(t *testing.T) { require.NoError(t, err) check := func(cwd string, running bool) { t.Helper() - out, terminal, err := executeBinary(t, binary, cwd, "review-running", "--json") + out, terminal, err := executeBinary(t, binary, cwd, "running", "--json") require.NoError(t, err) require.Empty(t, terminal) want := `{"version":1,"running":false}` @@ -51,7 +51,7 @@ func TestReviewRunningReportsLiveLockNotPersistedAttempt(t *testing.T) { } func TestReviewRunningOutsideRepository(t *testing.T) { - out, terminal, err := execute(t, t.TempDir(), "review-running", "--json") + out, terminal, err := execute(t, t.TempDir(), "running", "--json") require.Error(t, err) require.Empty(t, terminal) require.Contains(t, out, `"status":"error"`) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index b919ea5..36e410a 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -80,7 +80,7 @@ func (a *application) command() *cobra.Command { return a.write(result) }} status.Flags().BoolVar(&onlyNeedsReview, "needs-review", false, "Show unresolved stacks and active review attempts") - activity := &cobra.Command{Use: "review-running", Short: "Check whether a review process holds the shared worktree lock", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { + activity := &cobra.Command{Use: "running", Short: "Check whether a review process holds the shared worktree lock", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { running, err := a.service.ReviewRunning(cmd.Context()) if err != nil { return err @@ -94,14 +94,14 @@ func (a *application) command() *cobra.Command { _, err = fmt.Fprintf(a.output, "Review running: %t\n", running) return err }} - root.AddCommand(feedback, approve, status, activity, a.reviewCommand(), a.logCommand()) + root.AddCommand(feedback, approve, status, activity, a.runCommand(), a.logCommand()) return root } -func (a *application) reviewCommand() *cobra.Command { +func (a *application) runCommand() *cobra.Command { var recoverReview, replaceReview bool var session, start, end string - cmd := &cobra.Command{Use: "review []", Aliases: []string{"challenge"}, Short: "Review eligible stacks, or select one stack", Long: "Run tuicr in a stable detached worktree. Without a stack argument, review each\neligible stack in oldest-first order. A commit reference selects one stack. After\napproval or feedback, continue through the selected stacks. With --start or --end,\nlimit selection to the bounded range. Each eligible stack opens once per\ninvocation. Incomplete reviews, cancellation, required decisions, and errors stop\nthe command. Existing interrupted or feedback attempts require an explicit\nrecovery or replacement choice.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + cmd := &cobra.Command{Use: "run []", Aliases: []string{"challenge"}, Short: "Review eligible stacks, or select one stack", Long: "Run tuicr in a stable detached worktree. Without a stack argument, review each\neligible stack in oldest-first order. A commit reference selects one stack. After\napproval or feedback, continue through the selected stacks. With --start or --end,\nlimit selection to the bounded range. Each eligible stack opens once per\ninvocation. Incomplete reviews, cancellation, required decisions, and errors stop\nthe command. Existing interrupted or feedback attempts require an explicit\nrecovery or replacement choice.", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { options := review.ReviewOptions{Session: session, Start: start, End: end} if len(args) > 0 { options.Stack = args[0] diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 26fd56c..8b92e8f 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -90,7 +90,7 @@ func TestJSONErrorsIncludeArgumentAndGitFailures(t *testing.T) { for _, args := range [][]string{ {"status", "--needs-review", "--json"}, {"approve", "--json"}, - {"review", "--recover", "--replace", "--json"}, + {"run", "--recover", "--replace", "--json"}, {"not-a-command", "--json"}, {"--json", "status", "--needs-review", "--unknown"}, } { @@ -116,7 +116,7 @@ func TestReviewBoundsReachAutomaticSelection(t *testing.T) { state.Attempts = append(state.Attempts, review.Attempt{ID: "bounded", Stack: view.Stacks[1], Status: "incomplete", StartedAt: "2026-01-01T00:00:00Z"}) return nil })) - out, terminal, err := execute(t, dir, "review", "--start", "HEAD", "--end", "HEAD", "--json") + out, terminal, err := execute(t, dir, "run", "--start", "HEAD", "--end", "HEAD", "--json") require.NoError(t, err) require.Empty(t, terminal) var result review.Result @@ -142,14 +142,15 @@ func TestReviewAliasProducesDecisionWithoutPromptOrStateChanges(t *testing.T) { var result review.Result require.NoError(t, json.Unmarshal([]byte(out), &result)) require.Equal(t, "decision_required", result.Status) + require.Equal(t, "run", result.Command) require.NotNil(t, result.Decision) require.Equal(t, []string{"recover", "replace", "cancel"}, result.Decision.Choices) after, err := os.ReadFile(view.Path) require.NoError(t, err) require.Equal(t, before, after) - out, _, err = execute(t, dir, "review") + out, _, err = execute(t, dir, "run") require.NoError(t, err) - require.Contains(t, out, "umpire review --recover") + require.Contains(t, out, "umpire run --recover") } func TestTerminalRenderingRetainsFeedbackAndRemovesControlSequences(t *testing.T) { @@ -174,7 +175,7 @@ func TestReviewKeepsTuicrOutputOffJSONStdout(t *testing.T) { bin := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(bin, "tuicr"), []byte("#!/bin/sh\nprintf 'tuicr terminal output\\n' >&2\nprintf 'exported feedback\\n'\n"), 0o700)) t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) - out, terminal, err := execute(t, dir, "review", "--json") + out, terminal, err := execute(t, dir, "run", "--json") require.NoError(t, err) require.Equal(t, "tuicr terminal output\n", terminal) var result review.Result @@ -200,7 +201,7 @@ func TestTuicrKeepsWorktreeLockedAfterUmpireDies(t *testing.T) { script := "#!/bin/sh\nset -eu\nprintf started > \"$TEST_STARTED\"\nwhile [ ! -f \"$TEST_EXIT\" ]; do sleep 0.05; done\nprintf finished > \"$TEST_STARTED\"\n" require.NoError(t, os.WriteFile(filepath.Join(bin, "tuicr"), []byte(script), 0o700)) t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) - cmd := exec.CommandContext(t.Context(), binary, "review", "--json") + cmd := exec.CommandContext(t.Context(), binary, "run", "--json") cmd.Dir = dir require.NoError(t, cmd.Start()) defer func() { @@ -216,7 +217,7 @@ func TestTuicrKeepsWorktreeLockedAfterUmpireDies(t *testing.T) { require.Eventually(t, func() bool { _, err := os.Stat(started); return err == nil }, 5*time.Second, 10*time.Millisecond) require.NoError(t, cmd.Process.Kill()) require.Error(t, cmd.Wait()) - out, _, err := execute(t, dir, "review", "--replace", "--json") + out, _, err := execute(t, dir, "run", "--replace", "--json") require.ErrorContains(t, err, "worktree is in use") var result errorResult require.NoError(t, json.Unmarshal([]byte(out), &result)) @@ -227,13 +228,16 @@ func TestHelpAndVersion(t *testing.T) { out, _, err := execute(t, t.TempDir(), "--help") require.NoError(t, err) require.Contains(t, out, "approve") - require.Contains(t, out, "review") + require.Contains(t, out, "run") + require.Contains(t, out, "running") require.Contains(t, out, "--json") require.NotContains(t, out, "confirm") require.NotContains(t, out, "challenge") - out, _, err = execute(t, t.TempDir(), "needs-review") - require.ErrorContains(t, err, `unknown command "needs-review"`) - require.Empty(t, out) + for _, name := range []string{"needs-review", "review", "review-running"} { + out, _, err = execute(t, t.TempDir(), name) + require.ErrorContains(t, err, "unknown command") + require.Empty(t, out) + } out, _, err = execute(t, t.TempDir(), "--version") require.NoError(t, err) require.Contains(t, out, "test-version") diff --git a/internal/cli/render.go b/internal/cli/render.go index d9c61a4..d0ecc73 100644 --- a/internal/cli/render.go +++ b/internal/cli/render.go @@ -64,7 +64,7 @@ func renderResult(result review.Result) (string, error) { } if result.Decision != nil { fmt.Fprintf(&out, "Attempt: %s\n", safeText(result.Decision.AttemptID)) - fmt.Fprintln(&out, "Choose umpire review --recover, umpire review --replace, or leave the attempt unchanged.") + fmt.Fprintln(&out, "Choose umpire run --recover, umpire run --replace, or leave the attempt unchanged.") } return strings.TrimSpace(out.String()), nil } diff --git a/internal/cli/tuicr_functional_test.go b/internal/cli/tuicr_functional_test.go index 5da22a2..81dc87c 100644 --- a/internal/cli/tuicr_functional_test.go +++ b/internal/cli/tuicr_functional_test.go @@ -67,7 +67,7 @@ func TestRealTuicrFeedback(t *testing.T) { env := realTuicrEnvironment(t) binary := buildUmpire(t) dir := realFeatureRepo(t) - session := startTerminal(t, binary, dir, env, "review", "--json") + session := startTerminal(t, binary, dir, env, "run", "--json") session.waitFor(t, "feature.txt") list := runRealCommand(t, "tuicr", dir, env, "review", "list", "--all") var sessions []struct { @@ -114,7 +114,7 @@ func TestRealTuicrInterruptedRecovery(t *testing.T) { env := realTuicrEnvironment(t) binary := buildUmpire(t) dir := realFeatureRepo(t) - session := startTerminal(t, binary, dir, env, "review", "--json") + session := startTerminal(t, binary, dir, env, "run", "--json") session.waitFor(t, "feature.txt") session.send(t, ":set noreviewed\rr") session.waitFor(t, "1/2") @@ -143,7 +143,7 @@ func TestRealTuicrInterruptedRecovery(t *testing.T) { } return !saved[0].Active }, 5*time.Second, 20*time.Millisecond) - before := runRealCommand(t, binary, dir, env, "review", "--json") + before := runRealCommand(t, binary, dir, env, "run", "--json") var decision struct { Status string `json:"status"` Decision struct { @@ -153,7 +153,7 @@ func TestRealTuicrInterruptedRecovery(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(before), &decision)) require.Equal(t, "decision_required", decision.Status) require.NotEmpty(t, decision.Decision.AttemptID) - resumed := startTerminal(t, binary, dir, env, "review", "HEAD", "--recover", "--json") + resumed := startTerminal(t, binary, dir, env, "run", "HEAD", "--recover", "--json") resumed.waitFor(t, "feature.txt") resumed.waitFor(t, "1/2") resumed.send(t, ":set noreviewed\rr") @@ -186,7 +186,7 @@ func TestRealTuicrRecoveryPrompt(t *testing.T) { env := realTuicrEnvironment(t) binary := buildUmpire(t) dir := realFeatureRepo(t) - initial := startTerminal(t, binary, dir, env, "review", "--json") + initial := startTerminal(t, binary, dir, env, "run", "--json") initial.waitFor(t, "feature.txt") initial.send(t, ":set noreviewed\rr") initial.waitFor(t, "1/2") @@ -203,7 +203,7 @@ func TestRealTuicrRecoveryPrompt(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(before), &prior)) require.Equal(t, "incomplete", prior.Status) require.Len(t, prior.Entries, 1) - prompt := startTerminal(t, binary, dir, env, "review") + prompt := startTerminal(t, binary, dir, env, "run") prompt.waitFor(t, "A previous review attempt exists") switch choice { case "recover": @@ -256,7 +256,7 @@ func TestRealTuicrCleanReview(t *testing.T) { env := realTuicrEnvironment(t) binary := buildUmpire(t) dir := realFeatureRepo(t) - session := startTerminal(t, binary, dir, env, "review", "HEAD", "--json") + session := startTerminal(t, binary, dir, env, "run", "HEAD", "--json") session.waitFor(t, "feature.txt") session.send(t, ":set noreviewed\r") session.send(t, "rr") diff --git a/internal/review/run.go b/internal/review/run.go index 518ceda..0a9473c 100644 --- a/internal/review/run.go +++ b/internal/review/run.go @@ -198,7 +198,7 @@ func (r Reviewer) launch(ctx context.Context, path string, stack Stack, lock *os // Keep the worktree locked if Umpire dies while tuicr is still running. cmd.ExtraFiles = []*os.File{lock} if err := cmd.Run(); err != nil { - return fmt.Errorf("tuicr interrupted or failed; run umpire review to recover or replace: %w", err) + return fmt.Errorf("tuicr interrupted or failed; run umpire run to recover or replace: %w", err) } return nil } @@ -290,11 +290,11 @@ func (r Reviewer) recover(plan reviewPlan, attempt Attempt, selector string) (Re if err != nil { return Result{}, err } - result := makeResult("review", plan.view, state) + result := makeResult("run", plan.view, state) result.Status = "cancelled" return result, nil } - result := Result{Version: 1, Command: "review", Status: finished.Status, View: plan.view, Entries: []Entry{{Stack: finished.Stack, Status: finished.Status, Attempt: &finished}}, Active: []Attempt{}} + result := Result{Version: 1, Command: "run", Status: finished.Status, View: plan.view, Entries: []Entry{{Stack: finished.Stack, Status: finished.Status, Attempt: &finished}}, Active: []Attempt{}} if finished.Status == "incomplete" { result.Decision = &Decision{AttemptID: finished.ID, StackKey: finished.Stack.Key, Choices: []string{"recover", "replace", "cancel"}} } @@ -353,7 +353,7 @@ func (r Reviewer) reviewStack(ctx context.Context, options ReviewOptions, visite if err != nil { return Result{}, err } - result := makeResult("review", plan.view, plan.state) + result := makeResult("run", plan.view, plan.state) if plan.stack == nil { if plan.rangeComplete() { result.Status = "range_complete" @@ -526,7 +526,7 @@ func replacementStack(plan reviewPlan) (Stack, error) { return stack, nil } } - return Stack{}, fmt.Errorf("interrupted stack is no longer current; select its replacement with umpire review ") + return Stack{}, fmt.Errorf("interrupted stack is no longer current; select its replacement with umpire run ") } func (options ReviewOptions) hasBounds() bool { diff --git a/pi/extensions/umpire.test.ts b/pi/extensions/umpire.test.ts index 8a4f1a5..4033900 100644 --- a/pi/extensions/umpire.test.ts +++ b/pi/extensions/umpire.test.ts @@ -22,7 +22,7 @@ function harness(mode: ExtensionContext["mode"] = "tui") { on: (name: string, handler: Function) => handlers.set(name, handler), exec: async (command: string, args: string[]) => { if (command === "git") return { code: 0, killed: false, stdout: "true" }; - if (args[0] === "review-running") { + if (args[0] === "running") { activityCalls++; await activityGate; return activity; diff --git a/pi/extensions/umpire.ts b/pi/extensions/umpire.ts index f68ad7f..538e77c 100644 --- a/pi/extensions/umpire.ts +++ b/pi/extensions/umpire.ts @@ -86,7 +86,7 @@ export default function (pi: ExtensionAPI) { return; } if (aborted || status.feedback === lastNudgedFeedback) return; - const activity = await pi.exec("umpire", ["review-running", "--json"], { + const activity = await pi.exec("umpire", ["running", "--json"], { cwd: ctx.cwd, timeout: STATUS_TIMEOUT_MS, }); diff --git a/skills/umpire/SKILL.md b/skills/umpire/SKILL.md index 48a0a5f..1029937 100644 --- a/skills/umpire/SKILL.md +++ b/skills/umpire/SKILL.md @@ -84,7 +84,7 @@ Do not silently lose selected feedback or address the same attempt twice. If `active` contains an attempt, pause before rewriting history. Ask the user to finish or resolve the active review. This command does not collect uncaptured notes from interrupted tuicr sessions. -Do not use `umpire review --recover` to retrieve feedback. +Do not use `umpire run --recover` to retrieve feedback. Recovery opens an interactive review and requires the user's choice. ## Prepare the changes @@ -197,4 +197,4 @@ Keep backup branches until the user requests their removal. Remove temporary fix branches only after checking their commits are reachable from the feature branch. Leave recovery branches intact when integration fails. -Tell the user that the changes are ready for re-review with `umpire review`. +Tell the user that the changes are ready for re-review with `umpire run`. From e01b3705bd1238c427e54b9c8dc4df6ce6c20552 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 14:24:00 -0500 Subject: [PATCH 8/8] Rename Umpire to git-review across the project Keep the CLI, Go module, Pi integration, and agent skill under one name ahead of the repository rename. Align release and template metadata so future builds and generated updates retain the new identity. Use the new name for the disposable review checkout and lock as well. Existing branch review history stays in tuicr-reviews unchanged. --- .config/git.tpl.toml | 6 +- .github/workflows/update-tap.yaml | 2 +- CONTEXT.md | 2 +- README.md | 135 +++++++++--------- umpire.go => git_review.go | 0 umpire_test.go => git_review_test.go | 0 go.mod | 2 +- internal/cli/activity_test.go | 6 +- internal/cli/binary_test.go | 4 +- internal/cli/cli.go | 6 +- internal/cli/cli_test.go | 24 ++-- internal/cli/feedback_test.go | 4 +- internal/cli/log.go | 2 +- internal/cli/render.go | 4 +- internal/cli/status_test.go | 10 +- internal/cli/testmain_test.go | 2 +- internal/cli/tuicr_functional_test.go | 10 +- internal/review/git.go | 2 +- internal/review/run.go | 8 +- internal/review/testmain_test.go | 2 +- internal/review/worktree.go | 6 +- internal/review/worktree_test.go | 2 +- internal/testenv/git.go | 2 +- main.go | 4 +- mise.toml | 2 +- package.json | 8 +- ...ort.test.ts => git-review-support.test.ts} | 8 +- .../{umpire.test.ts => git-review.test.ts} | 9 +- pi/extensions/{umpire.ts => git-review.ts} | 40 +++--- .../{umpire => git-review}/support.ts | 26 ++-- skills/{umpire => git-review}/SKILL.md | 34 ++--- testmain_test.go | 2 +- 32 files changed, 192 insertions(+), 182 deletions(-) rename umpire.go => git_review.go (100%) rename umpire_test.go => git_review_test.go (100%) rename pi/extensions/{umpire-support.test.ts => git-review-support.test.ts} (81%) rename pi/extensions/{umpire.test.ts => git-review.test.ts} (97%) rename pi/extensions/{umpire.ts => git-review.ts} (82%) rename pi/extensions/{umpire => git-review}/support.ts (77%) rename skills/{umpire => git-review}/SKILL.md (86%) diff --git a/.config/git.tpl.toml b/.config/git.tpl.toml index 869e180..fb5e9e4 100644 --- a/.config/git.tpl.toml +++ b/.config/git.tpl.toml @@ -9,10 +9,10 @@ source = "git@github.com:nnutter/go-template.git" [answers] copyright_holder = "Nathan Nutter" copyright_year = "2026" -formula_name = "umpire" +formula_name = "git-review" go_version = "1.27.1" -module_path = "github.com/nnutter/umpire" -project_name = "umpire" +module_path = "github.com/nnutter/git-review" +project_name = "git-review" tap_owner = "nnutter" tap_repository = "homebrew-tap" with_homebrew = true diff --git a/.github/workflows/update-tap.yaml b/.github/workflows/update-tap.yaml index 192fcc5..04b7360 100644 --- a/.github/workflows/update-tap.yaml +++ b/.github/workflows/update-tap.yaml @@ -22,7 +22,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 15 env: - FORMULA_NAME: umpire + FORMULA_NAME: git-review TAP_OWNER: nnutter TAP_REPOSITORY: homebrew-tap steps: diff --git a/CONTEXT.md b/CONTEXT.md index f96a859..0fd4324 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -1,6 +1,6 @@ # Commit Review -Umpire tracks user review of changes on a feature branch. +git-review tracks user review of changes on a feature branch. ## Language diff --git a/README.md b/README.md index cfcde89..7ccd175 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ -# Umpire +# git-review -Umpire tracks user review of feature-branch commit stacks with [tuicr](https://github.com/agavra/tuicr). +git-review tracks user review of feature-branch commit stacks with [tuicr](https://github.com/agavra/tuicr). It runs independently of Pi or any other coding agent. Cobra and Fang provide the command interface, Lip Gloss renders summaries, and Huh provides the recovery prompt. @@ -12,34 +12,35 @@ Cobra and Fang provide the command interface, Lip Gloss renders summaries, and H - Go 1.27.1 or later to build from source. ```sh -go install github.com/nnutter/umpire@latest +go install github.com/nnutter/git-review@latest ``` -For a local build, run `go build -o umpire .`. +For a local build, run `go build -o git-review .`. +With `git-review` on `PATH`, you can also invoke it as `git review`. ## Agent skill -The portable [Umpire skill](skills/umpire/SKILL.md) instructs an agent to retrieve saved feedback and prepare amendments for re-review. -Load it in your agent or install `skills/umpire/` in the agent's supported skills location. -The initial workflow uses `umpire feedback --json` and native Git fixups targeting each stack tip. +The portable [git-review skill](skills/git-review/SKILL.md) instructs an agent to retrieve saved feedback and prepare amendments for re-review. +Load it in your agent or install `skills/git-review/` in the agent's supported skills location. +The initial workflow uses `git-review feedback --json` and native Git fixups targeting each stack tip. It replays later commits without autosquashing so feedback commits remain adjacent to their targets. The user launches reviews and controls approval. -The skill does not register `/umpire` commands or implement background workers. +The skill does not register `/git-review` commands or implement background workers. ## Pi integration -Install the Pi extension and Umpire skill from this repository: +Install the Pi extension and git-review skill from this repository: ```sh -pi install git:github.com/nnutter/umpire +pi install git:github.com/nnutter/git-review ``` -Install the Umpire CLI separately and ensure `umpire` is on `PATH`. +Install the git-review CLI separately and ensure `git-review` is on `PATH`. The package does not install the CLI or tuicr. -Remove any manually installed copies of the Umpire extension or skill to avoid loading them twice. +Remove any manually installed copies of the git-review extension or skill to avoid loading them twice. -The extension source lives in [`pi/extensions/umpire.ts`](pi/extensions/umpire.ts). -It publishes review counts for other extensions and asks the model to use the Umpire skill when feedback exists after an agent run. +The extension source lives in [`pi/extensions/git-review.ts`](pi/extensions/git-review.ts). +It publishes review counts for other extensions and asks the model to use the git-review skill when feedback exists after an agent run. It defers model notices while a review process holds the shared worktree lock. Deferred feedback remains eligible for notification after the review ends. Activity-query failures suppress notices rather than assume that review ended. @@ -56,7 +57,7 @@ For local development, run `pi install .` from this checkout. ## Commands Run commands from the feature checkout or one of its subdirectories. -Umpire reviews commits after the merge-base with `@{upstream}`. +git-review reviews commits after the merge-base with `@{upstream}`. It does not fetch branches or use Pi's target override. An absent upstream, detached HEAD, merge commits, or ambiguous fixup placement is an error. If the branch tracks its own remote branch, the review range can be empty. @@ -65,10 +66,10 @@ Configure the intended upstream explicitly with Git. ### Show commits after the upstream ```sh -umpire log -umpire log -p -umpire log -pw -umpire log --stat +git-review log +git-review log -p +git-review log -pw +git-review log --stat ``` This command runs `git log @{u}..HEAD --reverse`, so commits appear in oldest-first order. @@ -82,10 +83,10 @@ Unlike review commands, it uses the upstream range directly and permits merge co ### Show feature stacks and review state ```sh -umpire status -umpire status --needs-review -umpire status --json -umpire status --needs-review --json +git-review status +git-review status --needs-review +git-review status --json +git-review status --needs-review --json ``` This read-only command displays all stacks in the current feature range, including approved stacks, in oldest-first order. @@ -123,8 +124,8 @@ Command failures use the standard JSON error object. ### List unresolved stacks ```sh -umpire status --needs-review -umpire status --needs-review --json +git-review status --needs-review +git-review status --needs-review --json ``` A stack contains one original commit and its immediately following `fixup!`, `amend!`, or `reword!` commits. @@ -137,7 +138,7 @@ Current and historical feedback remain available in the result. ### Check live review activity ```sh -umpire running --json +git-review running --json ``` Successful output is `{"version":1,"running":true}` or `{"version":1,"running":false}`. @@ -150,13 +151,13 @@ Command failures exit with status 1 and emit the standard versioned JSON error o ### Collect saved feedback ```sh -umpire feedback -umpire feedback --json +git-review feedback +git-review feedback --json ``` -This read-only command returns notes persisted by Umpire, including reviews completed outside an agent session. +This read-only command returns notes persisted by git-review, including reviews completed outside an agent session. It does not launch tuicr, recover an attempt, or change either review store. -Interrupted sessions whose notes were not captured by Umpire still require explicit interactive recovery. +Interrupted sessions whose notes were not captured by git-review still require explicit interactive recovery. Feedback results use the versioned result envelope, with `"command":"feedback"`. The optional `feedback` array contains records with these fields: @@ -180,28 +181,28 @@ Active attempts remain visible in `active`, but collecting feedback does not res ### Review a stack ```sh -umpire run -umpire run HEAD -umpire run abc123..def456 +git-review run +git-review run HEAD +git-review run abc123..def456 ``` -Without an argument, Umpire reviews each eligible, non-deferred stack in oldest-first order, once per invocation. +Without an argument, git-review reviews each eligible, non-deferred stack in oldest-first order, once per invocation. Approval or feedback does not stop this sequence. A commit reference selects only its containing stack. The displayed `original..tip` form has inclusive endpoints and must identify exactly one complete stack. An explicit selection can review an approved stack again or select a later stack without approving earlier stacks. -Umpire launches tuicr directly in a stable, detached worktree checked out at the stack's tip. +git-review launches tuicr directly in a stable, detached worktree checked out at the stack's tip. The tuicr revision expression uses the original's parent through the stack tip, so the original commit is included. Tuicr's editor therefore opens the reviewed file version rather than the feature checkout's current version. Make fixes in the feature checkout, not the review worktree. The review checkout is disposable and is only for exploring the reviewed code, not making fixes. -Before reuse, Umpire discards tracked edits, untracked files, and ignored artifacts in that checkout. +Before reuse, git-review discards tracked edits, untracked files, and ignored artifacts in that checkout. It does not forcibly remove nested Git repositories. -Umpire refuses to overwrite unrelated directories or attached branch checkouts at the review path. -It also refuses to use a worktree held by another Umpire review. +git-review refuses to overwrite unrelated directories or attached branch checkouts at the review path. +It also refuses to use a worktree held by another git-review process. -After tuicr exits, Umpire validates its saved review: +After tuicr exits, git-review validates its saved review: | Saved result | Status | | --- | --- | @@ -213,18 +214,18 @@ After tuicr exits, Umpire validates its saved review: Commit-message review marks count toward completion. A successful process exit is not proof of approval. -An untouched review leaves no attempt in Umpire's history and does not require recovery on the next invocation. +An untouched review leaves no attempt in git-review's history and does not require recovery on the next invocation. ### Review a range of stacks ```sh -umpire run --start abc123 -umpire run --start abc123 --end def456 --json -umpire run --end def456 +git-review run --start abc123 +git-review run --start abc123 --end def456 --json +git-review run --end def456 ``` Each invocation reviews eligible stacks within the inclusive bounds in oldest-first order. -After approval or feedback, Umpire continues to the next eligible stack. +After approval or feedback, git-review continues to the next eligible stack. Each eligible stack opens at most once per invocation. Feedback remains unresolved and requires an explicit recovery or replacement choice on a later invocation. An incomplete review, cancellation, a required decision, or an error stops the command. @@ -245,30 +246,30 @@ The result's view and entries still describe the feature branch, not only the bo ### Recover or replace an attempt An interrupted, incomplete, or feedback attempt on the selected stack requires an explicit choice. -In an interactive terminal, Umpire prompts to recover, replace, or cancel. +In an interactive terminal, git-review prompts to recover, replace, or cancel. With JSON output or redirected streams, it returns `decision_required` without a prompt. ```sh -umpire run --recover -umpire run --replace -umpire run --recover --session +git-review run --recover +git-review run --replace +git-review run --recover --session ``` Recovery reopens tuicr at the previous attempt's exact captured range and preserves its attempt ID. Tuicr restores saved comments and review marks for that checkout and range. -Umpire records the result only after tuicr exits. +git-review records the result only after tuicr exits. Replacement starts another attempt and retains the previous attempt in history. -Neither operation proceeds while the review worktree is held by a live Umpire/tuicr process. +Neither operation proceeds while the review worktree is held by a live git-review/tuicr process. `--session` accepts an exact saved session ID, index slug, or indexed `sessions/.json` path. It resolves ambiguous saved sessions and is not an arbitrary filesystem path. -If an interrupted original disappeared and no lineage identifies its replacement, use `umpire run --replace`. +If an interrupted original disappeared and no lineage identifies its replacement, use `git-review run --replace`. ### Record explicit user approval ```sh -umpire approve -umpire approve +git-review approve +git-review approve ``` Endpoints are inclusive and accept Git commit references such as abbreviated SHAs or `HEAD`. @@ -289,9 +290,9 @@ Other successful commands produce a versioned JSON object. JSON and terminal output consume the same typed result. JSON retains full commit IDs, attempts, saved comments, session notes, and optional previous-version lineage. Tuicr renders its interactive view through the controlling terminal, with diagnostics on stderr, not the JSON stream. -Umpire launches tuicr with `--stdout` to bypass clipboard confirmation and discards its Markdown export. +git-review launches tuicr with `--stdout` to bypass clipboard confirmation and discards its Markdown export. Feedback comes from the persisted review session. -`--json` disables Umpire's recovery prompt, but a new tuicr review still requires an interactive terminal. +`--json` disables git-review's recovery prompt, but a new tuicr review still requires an interactive terminal. Successful results other than `status` contain: @@ -338,29 +339,29 @@ After a crashed writer, remove a stale state lock only after confirming that no Each repository uses one persistent, detached review checkout: ```text -/umpire/worktree +/git-review/worktree ``` All feature checkouts and branches sharing that Git common directory use the same review checkout and lock. -Umpire checks out the selected stack tip so tuicr's editor opens the correct file version. +git-review checks out the selected stack tip so tuicr's editor opens the correct file version. Tuicr associates saved sessions with the checkout path and commit range, not just individual file paths. -Identical stacks can reuse the same saved session across branches, while Umpire's attempt history remains branch-specific. +Identical stacks can reuse the same saved session across branches, while git-review's attempt history remains branch-specific. No migration from the previous cache-based checkout location is performed. -The adjacent `worktree.umpire.lock` file uses an operating-system lock that releases when its last holder exits. +The adjacent `worktree.git-review.lock` file uses an operating-system lock that releases when its last holder exits. Do not delete this lock file to bypass an active review. The checkout remains in place between reviews, so routine reviews do not create disposable worktrees that need garbage collection. -Other tools must not use or edit this checkout while Umpire holds its lock. +Other tools must not use or edit this checkout while git-review holds its lock. Prepare fixes in the feature checkout or a separate writable workspace, never in the review checkout. -Umpire reads tuicr's platform review store, then checks the other platform's location for existing sessions. +git-review reads tuicr's platform review store, then checks the other platform's location for existing sessions. Supported formats are review index `2.0` and saved session `1.3`. It never writes tuicr annotations or rewrites feature-branch history. Compatibility with Pi's branch state requires Pi to adopt the `tuicr-reviews` path. ## Changed stacks and Change-Ids -Umpire records an optional `patchHash` for each stack's combined diff, from the original commit's parent to the final fixup. +git-review records an optional `patchHash` for each stack's combined diff, from the original commit's parent to the final fixup. The hash uses SHA-256 with a `v1:` prefix and includes paths, file modes, whitespace, binary content, and diff context. It excludes commit messages, authors, timestamps, and commit identities. @@ -380,9 +381,9 @@ Feedback from obsolete content remains historical context, not a global blocker. Changed stack content supersedes a prior result but does not prove that feedback was addressed. The next review supplies that check. -When an original commit has a Gerrit `Change-Id` trailer, Umpire uses it to connect stack versions within the same repository and feature branch. +When an original commit has a Gerrit `Change-Id` trailer, git-review uses it to connect stack versions within the same repository and feature branch. It never transfers approval through a Change-Id. -Umpire rejects malformed or multiple trailers and duplicate Change-Ids on current original commits. +git-review rejects malformed or multiple trailers and duplicate Change-Ids on current original commits. It does not generate Change-Ids or guess lineage for split or combined changes. ## Development @@ -403,11 +404,11 @@ mise run functional-tests ``` This task requires tuicr on `PATH` and permission to create pseudo-terminals. -It runs the compiled Umpire binary against real tuicr, using terminal key input and tuicr's public annotation CLI. -It checks complete reviews, incomplete feedback, and interactive recovery of a saved review after Umpire is killed. +It runs the compiled git-review binary against real tuicr, using terminal key input and tuicr's public annotation CLI. +It checks complete reviews, incomplete feedback, and interactive recovery of a saved review after git-review is killed. It also selects recover, replace, and cancel through the actual terminal prompt and checks the resulting status through another CLI invocation. A terminal emulator reconstructs screen updates instead of matching raw ANSI output. -The real-tuicr tests skip in the standard suite unless `UMPIRE_REAL_TUICR=1` is set. +The real-tuicr tests skip in the standard suite unless `GIT_REVIEW_REAL_TUICR=1` is set. When enabled, a missing tuicr executable or unavailable pseudo-terminal fails the tests. The suite is verified with tuicr v0.27.0. diff --git a/umpire.go b/git_review.go similarity index 100% rename from umpire.go rename to git_review.go diff --git a/umpire_test.go b/git_review_test.go similarity index 100% rename from umpire_test.go rename to git_review_test.go diff --git a/go.mod b/go.mod index df80b5a..821cbe6 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/nnutter/umpire +module github.com/nnutter/git-review go 1.27.1 diff --git a/internal/cli/activity_test.go b/internal/cli/activity_test.go index 3afe8b9..da913ed 100644 --- a/internal/cli/activity_test.go +++ b/internal/cli/activity_test.go @@ -8,11 +8,11 @@ import ( "github.com/stretchr/testify/require" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/internal/review" ) func TestReviewRunningReportsLiveLockNotPersistedAttempt(t *testing.T) { - binary := buildUmpire(t) + binary := buildGitReview(t) dir := featureRepo(t) path, err := review.WorktreePath(dir) require.NoError(t, err) @@ -38,7 +38,7 @@ func TestReviewRunningReportsLiveLockNotPersistedAttempt(t *testing.T) { })) check(dir, false) require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) - lock, err := os.OpenFile(path+".umpire.lock", os.O_CREATE|os.O_RDWR, 0o600) + lock, err := os.OpenFile(path+".git-review.lock", os.O_CREATE|os.O_RDWR, 0o600) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, lock.Close()) }) require.NoError(t, syscall.Flock(int(lock.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)) diff --git a/internal/cli/binary_test.go b/internal/cli/binary_test.go index a414304..698280f 100644 --- a/internal/cli/binary_test.go +++ b/internal/cli/binary_test.go @@ -9,9 +9,9 @@ import ( "github.com/stretchr/testify/require" ) -func buildUmpire(t *testing.T) string { +func buildGitReview(t *testing.T) string { t.Helper() - binary := filepath.Join(t.TempDir(), "umpire") + binary := filepath.Join(t.TempDir(), "git-review") cmd := exec.CommandContext(t.Context(), "go", "build", "-o", binary, "../..") out, err := cmd.CombinedOutput() require.NoError(t, err, "%s", out) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 36e410a..ca835cb 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -1,4 +1,4 @@ -// Package cli exposes Umpire's review workflow through Cobra and Fang. +// Package cli exposes git-review's review workflow through Cobra and Fang. package cli import ( @@ -16,7 +16,7 @@ import ( "github.com/charmbracelet/x/term" "github.com/spf13/cobra" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/internal/review" ) // Execute runs a command with explicit process streams and checkout location. @@ -51,7 +51,7 @@ type application struct { } func (a *application) command() *cobra.Command { - root := &cobra.Command{Use: "umpire", Short: "Review feature-branch commit stacks with tuicr", Long: "Umpire tracks review of exact commit stacks against the configured Git upstream.\nApproval never transfers to rewritten commits or added fixups."} + root := &cobra.Command{Use: "git-review", Short: "Review feature-branch commit stacks with tuicr", Long: "git-review tracks review of exact commit stacks against the configured Git upstream.\nApproval never transfers to rewritten commits or added fixups."} root.PersistentFlags().BoolVar(&a.json, "json", a.json, "Emit a versioned JSON result without interactive prompts") feedback := &cobra.Command{Use: "feedback", Short: "Read current and historical saved feedback without launching tuicr", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { result, err := a.service.Feedback(cmd.Context()) diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 8b92e8f..7a0e563 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/internal/review" ) func git(t *testing.T, dir string, args ...string) string { @@ -46,10 +46,18 @@ func execute(t *testing.T, dir string, args ...string) (string, string, error) { } func TestCommandsShareStructuredResults(t *testing.T) { - binary := buildUmpire(t) + binary := buildGitReview(t) dir := featureRepo(t) - for _, args := range [][]string{{"status", "--needs-review", "--json"}, {"--json", "status", "--needs-review"}} { - out, terminal, err := executeBinary(t, binary, dir, args...) + t.Setenv("PATH", filepath.Dir(binary)+string(os.PathListSeparator)+os.Getenv("PATH")) + for _, tc := range []struct { + binary string + args []string + }{ + {binary: binary, args: []string{"status", "--needs-review", "--json"}}, + {binary: binary, args: []string{"--json", "status", "--needs-review"}}, + {binary: "git", args: []string{"review", "status", "--needs-review", "--json"}}, + } { + out, terminal, err := executeBinary(t, tc.binary, dir, tc.args...) require.NoError(t, err) require.Empty(t, terminal) require.Equal(t, 1, strings.Count(out, "\n")) @@ -84,7 +92,7 @@ func TestCommandsShareStructuredResults(t *testing.T) { } func TestJSONErrorsIncludeArgumentAndGitFailures(t *testing.T) { - binary := buildUmpire(t) + binary := buildGitReview(t) dir := featureRepo(t) git(t, dir, "branch", "--unset-upstream") for _, args := range [][]string{ @@ -150,7 +158,7 @@ func TestReviewAliasProducesDecisionWithoutPromptOrStateChanges(t *testing.T) { require.Equal(t, before, after) out, _, err = execute(t, dir, "run") require.NoError(t, err) - require.Contains(t, out, "umpire run --recover") + require.Contains(t, out, "git-review run --recover") } func TestTerminalRenderingRetainsFeedbackAndRemovesControlSequences(t *testing.T) { @@ -185,12 +193,12 @@ func TestReviewKeepsTuicrOutputOffJSONStdout(t *testing.T) { require.NotContains(t, out, "exported feedback") } -func TestTuicrKeepsWorktreeLockedAfterUmpireDies(t *testing.T) { +func TestTuicrKeepsWorktreeLockedAfterGitReviewDies(t *testing.T) { dir := featureRepo(t) t.Setenv("XDG_CACHE_HOME", t.TempDir()) t.Setenv("XDG_DATA_HOME", t.TempDir()) bin := t.TempDir() - binary := filepath.Join(bin, "umpire") + binary := filepath.Join(bin, "git-review") build := exec.CommandContext(t.Context(), "go", "build", "-o", binary, "../..") buildOut, err := build.CombinedOutput() require.NoError(t, err, "%s", buildOut) diff --git a/internal/cli/feedback_test.go b/internal/cli/feedback_test.go index c9cabbd..3f6d2b7 100644 --- a/internal/cli/feedback_test.go +++ b/internal/cli/feedback_test.go @@ -8,11 +8,11 @@ import ( "github.com/stretchr/testify/require" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/internal/review" ) func TestFeedbackReadsPersistedCurrentAndHistoricalNotes(t *testing.T) { - binary := buildUmpire(t) + binary := buildGitReview(t) dir := featureRepo(t) repository := review.Repository{Dir: dir} original, err := repository.Snapshot(t.Context()) diff --git a/internal/cli/log.go b/internal/cli/log.go index 999f013..5960d4d 100644 --- a/internal/cli/log.go +++ b/internal/cli/log.go @@ -12,7 +12,7 @@ func (a *application) logCommand() *cobra.Command { cmd := &cobra.Command{ Use: "log", Short: "Show commits after the upstream in oldest-first order", - Long: "Run git log @{u}..HEAD --reverse with optional patches, diff statistics, and whitespace filtering.\nOutput uses Git's format and pager, not Umpire's review state.", + Long: "Run git log @{u}..HEAD --reverse with optional patches, diff statistics, and whitespace filtering.\nOutput uses Git's format and pager, not git-review's review state.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { if a.json { diff --git a/internal/cli/render.go b/internal/cli/render.go index d0ecc73..0ff3261 100644 --- a/internal/cli/render.go +++ b/internal/cli/render.go @@ -13,7 +13,7 @@ import ( "github.com/charmbracelet/colorprofile" "github.com/charmbracelet/x/ansi" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/internal/review" ) func render(output io.Writer, result review.Result) error { @@ -64,7 +64,7 @@ func renderResult(result review.Result) (string, error) { } if result.Decision != nil { fmt.Fprintf(&out, "Attempt: %s\n", safeText(result.Decision.AttemptID)) - fmt.Fprintln(&out, "Choose umpire run --recover, umpire run --replace, or leave the attempt unchanged.") + fmt.Fprintln(&out, "Choose git-review run --recover, git-review run --replace, or leave the attempt unchanged.") } return strings.TrimSpace(out.String()), nil } diff --git a/internal/cli/status_test.go b/internal/cli/status_test.go index ac3e71d..fdc8444 100644 --- a/internal/cli/status_test.go +++ b/internal/cli/status_test.go @@ -8,11 +8,11 @@ import ( "github.com/stretchr/testify/require" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/internal/review" ) func TestStatusCountsCurrentCommitsInStateOrder(t *testing.T) { - binary := buildUmpire(t) + binary := buildGitReview(t) dir := featureRepo(t) git(t, dir, "commit", "--allow-empty", "-m", "fixup! Add feature") for _, subject := range []string{"Active", "Feedback", "Partial", "Approved"} { @@ -138,9 +138,9 @@ func TestStatusWithoutReviewHistory(t *testing.T) { } func TestStatusOutsideGitRepository(t *testing.T) { - binary := buildUmpire(t) + binary := buildGitReview(t) dir := t.TempDir() - const message = "umpire must be used from inside a Git repository" + const message = "git-review must be used from inside a Git repository" for _, args := range [][]string{{"status"}, {"status", "--json"}} { t.Run(args[len(args)-1], func(t *testing.T) { out, terminal, err := executeBinary(t, binary, dir, args...) @@ -156,7 +156,7 @@ func TestStatusOutsideGitRepository(t *testing.T) { require.Equal(t, message, failed.Error) } else { require.Empty(t, out) - require.Contains(t, terminal, "Umpire must be used from inside a Git repository.") + require.Contains(t, terminal, "Git-Review must be used from inside a Git repository.") require.NotContains(t, terminal, "git symbolic-ref") require.NotContains(t, terminal, "detached HEAD") require.NotContains(t, terminal, "exit status") diff --git a/internal/cli/testmain_test.go b/internal/cli/testmain_test.go index 3175a77..2e6360f 100644 --- a/internal/cli/testmain_test.go +++ b/internal/cli/testmain_test.go @@ -3,7 +3,7 @@ package cli import ( "testing" - "github.com/nnutter/umpire/internal/testenv" + "github.com/nnutter/git-review/internal/testenv" ) func TestMain(m *testing.M) { testenv.Run(m) } diff --git a/internal/cli/tuicr_functional_test.go b/internal/cli/tuicr_functional_test.go index 81dc87c..40de62b 100644 --- a/internal/cli/tuicr_functional_test.go +++ b/internal/cli/tuicr_functional_test.go @@ -16,7 +16,7 @@ import ( func realTuicrEnvironment(t *testing.T) []string { t.Helper() - if os.Getenv("UMPIRE_REAL_TUICR") != "1" { + if os.Getenv("GIT_REVIEW_REAL_TUICR") != "1" { t.Skip("run mise run functional-tests to exercise real tuicr") } _, err := exec.LookPath("tuicr") @@ -65,7 +65,7 @@ func realFeatureRepo(t *testing.T) string { func TestRealTuicrFeedback(t *testing.T) { env := realTuicrEnvironment(t) - binary := buildUmpire(t) + binary := buildGitReview(t) dir := realFeatureRepo(t) session := startTerminal(t, binary, dir, env, "run", "--json") session.waitFor(t, "feature.txt") @@ -112,7 +112,7 @@ func TestRealTuicrFeedback(t *testing.T) { func TestRealTuicrInterruptedRecovery(t *testing.T) { env := realTuicrEnvironment(t) - binary := buildUmpire(t) + binary := buildGitReview(t) dir := realFeatureRepo(t) session := startTerminal(t, binary, dir, env, "run", "--json") session.waitFor(t, "feature.txt") @@ -184,7 +184,7 @@ func TestRealTuicrRecoveryPrompt(t *testing.T) { for _, choice := range []string{"recover", "replace", "cancel"} { t.Run(choice, func(t *testing.T) { env := realTuicrEnvironment(t) - binary := buildUmpire(t) + binary := buildGitReview(t) dir := realFeatureRepo(t) initial := startTerminal(t, binary, dir, env, "run", "--json") initial.waitFor(t, "feature.txt") @@ -254,7 +254,7 @@ func TestRealTuicrRecoveryPrompt(t *testing.T) { func TestRealTuicrCleanReview(t *testing.T) { env := realTuicrEnvironment(t) - binary := buildUmpire(t) + binary := buildGitReview(t) dir := realFeatureRepo(t) session := startTerminal(t, binary, dir, env, "run", "HEAD", "--json") session.waitFor(t, "feature.txt") diff --git a/internal/review/git.go b/internal/review/git.go index b9f2251..fc4f0a8 100644 --- a/internal/review/git.go +++ b/internal/review/git.go @@ -197,7 +197,7 @@ func (r Repository) gitOutput(ctx context.Context, args ...string) (string, erro out, err := cmd.Output() if err != nil { if strings.HasPrefix(stderr.String(), "fatal: not a git repository") { - return "", fmt.Errorf("umpire must be used from inside a Git repository") + return "", fmt.Errorf("git-review must be used from inside a Git repository") } return "", fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) } diff --git a/internal/review/run.go b/internal/review/run.go index 0a9473c..c901cc6 100644 --- a/internal/review/run.go +++ b/internal/review/run.go @@ -192,13 +192,13 @@ func (r Reviewer) launch(ctx context.Context, path string, stack Stack, lock *os cmd.Dir = path cmd.Stdin = r.Input // --stdout bypasses tuicr's clipboard confirmation. The TUI uses /dev/tty; - // Umpire reads persisted feedback instead of the exported markdown. + // git-review reads persisted feedback instead of the exported markdown. cmd.Stdout = io.Discard cmd.Stderr = r.Terminal - // Keep the worktree locked if Umpire dies while tuicr is still running. + // Keep the worktree locked if git-review dies while tuicr is still running. cmd.ExtraFiles = []*os.File{lock} if err := cmd.Run(); err != nil { - return fmt.Errorf("tuicr interrupted or failed; run umpire run to recover or replace: %w", err) + return fmt.Errorf("tuicr interrupted or failed; use git-review run to recover or replace: %w", err) } return nil } @@ -526,7 +526,7 @@ func replacementStack(plan reviewPlan) (Stack, error) { return stack, nil } } - return Stack{}, fmt.Errorf("interrupted stack is no longer current; select its replacement with umpire run ") + return Stack{}, fmt.Errorf("interrupted stack is no longer current; select its replacement with git-review run ") } func (options ReviewOptions) hasBounds() bool { diff --git a/internal/review/testmain_test.go b/internal/review/testmain_test.go index 1ccb988..c5e54a0 100644 --- a/internal/review/testmain_test.go +++ b/internal/review/testmain_test.go @@ -3,7 +3,7 @@ package review import ( "testing" - "github.com/nnutter/umpire/internal/testenv" + "github.com/nnutter/git-review/internal/testenv" ) func TestMain(m *testing.M) { testenv.Run(m) } diff --git a/internal/review/worktree.go b/internal/review/worktree.go index 8268253..68c9c33 100644 --- a/internal/review/worktree.go +++ b/internal/review/worktree.go @@ -21,12 +21,12 @@ func WorktreePath(repo string) (string, error) { if err != nil { return "", err } - return filepath.Join(common, "umpire", "worktree"), nil + return filepath.Join(common, "git-review", "worktree"), nil } // reviewRunning probes an existing lock without creating it. func reviewRunning(ctx context.Context, path string) (running bool, err error) { - file, err := os.OpenFile(path+".umpire.lock", os.O_RDONLY, 0) + file, err := os.OpenFile(path+".git-review.lock", os.O_RDONLY, 0) if errors.Is(err, os.ErrNotExist) { return false, nil } @@ -50,7 +50,7 @@ func lockWorktree(path string) (*os.File, error) { if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return nil, err } - file, err := os.OpenFile(path+".umpire.lock", os.O_CREATE|os.O_RDWR, 0o600) + file, err := os.OpenFile(path+".git-review.lock", os.O_CREATE|os.O_RDWR, 0o600) if err != nil { return nil, err } diff --git a/internal/review/worktree_test.go b/internal/review/worktree_test.go index 1038255..120869c 100644 --- a/internal/review/worktree_test.go +++ b/internal/review/worktree_test.go @@ -16,7 +16,7 @@ func TestReviewCheckoutIsSharedAcrossFeatureWorktrees(t *testing.T) { path, err := WorktreePath(r.Dir) require.NoError(t, err) common := gitTest(t, r.Dir, "rev-parse", "--path-format=absolute", "--git-common-dir") - require.Equal(t, filepath.Join(common, "umpire", "worktree"), path) + require.Equal(t, filepath.Join(common, "git-review", "worktree"), path) otherPath, err := WorktreePath(other) require.NoError(t, err) require.Equal(t, path, otherPath) diff --git a/internal/testenv/git.go b/internal/testenv/git.go index 387f94c..db51895 100644 --- a/internal/testenv/git.go +++ b/internal/testenv/git.go @@ -1,4 +1,4 @@ -// Package testenv provides process isolation for Umpire's test suites. +// Package testenv provides process isolation for git-review's test suites. // It is imported only by test files, never by the command or review library. package testenv diff --git a/main.go b/main.go index b2c7385..ccb1690 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,11 @@ -// Command umpire is the umpire command line tool. +// Command git-review is the git-review command line tool. package main import ( "context" "os" - "github.com/nnutter/umpire/internal/cli" + "github.com/nnutter/git-review/internal/cli" ) func main() { diff --git a/mise.toml b/mise.toml index bd4b545..ac1091e 100644 --- a/mise.toml +++ b/mise.toml @@ -20,7 +20,7 @@ run = ["go test -race -shuffle=on ./...", "node --test pi/extensions/*.test.ts"] [tasks.functional-tests] description = "Test the compiled CLI and real tuicr through pseudo-terminals" -env.UMPIRE_REAL_TUICR = "1" +env.GIT_REVIEW_REAL_TUICR = "1" run = "go test -race -shuffle=on -count=1 ./internal/cli -run 'TestCommandsShareStructuredResults|TestJSONErrorsIncludeArgumentAndGitFailures|TestRealTuicr'" [tasks.coverage] diff --git a/package.json b/package.json index bdc97a9..261aae0 100644 --- a/package.json +++ b/package.json @@ -1,13 +1,13 @@ { - "name": "umpire-pi", + "name": "git-review-pi", "version": "0.0.0", "private": true, "type": "module", - "description": "Pi integration and agent skill for Umpire commit reviews", + "description": "Pi integration and agent skill for git-review commit reviews", "keywords": ["pi-package"], "pi": { - "extensions": ["./pi/extensions/umpire.ts"], - "skills": ["./skills/umpire"], + "extensions": ["./pi/extensions/git-review.ts"], + "skills": ["./skills/git-review"], "prompts": [], "themes": [] }, diff --git a/pi/extensions/umpire-support.test.ts b/pi/extensions/git-review-support.test.ts similarity index 81% rename from pi/extensions/umpire-support.test.ts rename to pi/extensions/git-review-support.test.ts index eb82f60..01f75cc 100644 --- a/pi/extensions/umpire-support.test.ts +++ b/pi/extensions/git-review-support.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { parseUmpireStatus } from "./umpire/support.ts"; +import { parseGitReviewStatus } from "./git-review/support.ts"; const summary = [ { state: "needs_review", count: 0 }, @@ -12,7 +12,7 @@ const summary = [ test("status parser reads idle summary and includes additional states in total", () => { const result = { version: 1, command: "status", status: "idle", entries: [], summary: [...summary, { state: "future", count: 2 }] }; - assert.deepEqual(parseUmpireStatus(JSON.stringify(result)), { + assert.deepEqual(parseGitReviewStatus(JSON.stringify(result)), { needsReview: 0, total: 5, feedback: 0, line: "Needs Review: 0/5", }); }); @@ -30,7 +30,7 @@ test("status parser rejects unsupported envelopes and invalid summaries", () => { ...result, summary: [...summary, summary[0]] }, ...[-1, 0.5, "1"].map(count => ({ ...result, summary: [...summary.slice(0, -1), { state: "approved", count }] })), ]) { - assert.equal(parseUmpireStatus(JSON.stringify(invalid)), undefined); + assert.equal(parseGitReviewStatus(JSON.stringify(invalid)), undefined); } - assert.equal(parseUmpireStatus("not json"), undefined); + assert.equal(parseGitReviewStatus("not json"), undefined); }); diff --git a/pi/extensions/umpire.test.ts b/pi/extensions/git-review.test.ts similarity index 97% rename from pi/extensions/umpire.test.ts rename to pi/extensions/git-review.test.ts index 4033900..6898722 100644 --- a/pi/extensions/umpire.test.ts +++ b/pi/extensions/git-review.test.ts @@ -3,8 +3,8 @@ import { EventEmitter } from "node:events"; import test from "node:test"; import { setImmediate } from "node:timers/promises"; import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent"; -import umpire from "./umpire.ts"; -import { MISE_AFTER_ON_STOP_EVENT, MISE_COORDINATES_EVENT, UMPIRE_STATUS_EVENT } from "./umpire/support.ts"; +import gitReview from "./git-review.ts"; +import { MISE_AFTER_ON_STOP_EVENT, MISE_COORDINATES_EVENT } from "./git-review/support.ts"; function harness(mode: ExtensionContext["mode"] = "tui") { const events = new EventEmitter(); @@ -22,6 +22,7 @@ function harness(mode: ExtensionContext["mode"] = "tui") { on: (name: string, handler: Function) => handlers.set(name, handler), exec: async (command: string, args: string[]) => { if (command === "git") return { code: 0, killed: false, stdout: "true" }; + assert.equal(command, "git-review"); if (args[0] === "running") { activityCalls++; await activityGate; @@ -38,8 +39,8 @@ function harness(mode: ExtensionContext["mode"] = "tui") { }, sendUserMessage: (message: string) => messages.push(message), } as unknown as ExtensionAPI; - events.on(UMPIRE_STATUS_EVENT, data => published.push(data)); - umpire(pi); + events.on("git-review:status", data => published.push(data)); + gitReview(pi); return { events, messages, published, ctx, setActivity: (result: typeof activity) => { activity = result; }, diff --git a/pi/extensions/umpire.ts b/pi/extensions/git-review.ts similarity index 82% rename from pi/extensions/umpire.ts rename to pi/extensions/git-review.ts index 538e77c..f762283 100644 --- a/pi/extensions/umpire.ts +++ b/pi/extensions/git-review.ts @@ -1,15 +1,15 @@ /** - * Umpire extension. + * git-review extension. * - * Reads `umpire status --needs-review --json` for the current Git feature branch and - * publishes the counts on `umpire:status`. Outside a Git repository it + * Reads `git-review status --needs-review --json` for the current Git feature branch and + * publishes the counts on `git-review:status`. Outside a Git repository it * publishes nothing. * * Recap (and anything else) can ask for a fresh post with - * `umpire:request-status`. The request object may set `refresh(cwd)`. + * `git-review:request-status`. The request object may set `refresh(cwd)`. * * On settle, when feedback exists and no review process is running, the extension tells - * the model to use the umpire skill. The same count is not sent again + * the model to use the git-review skill. The same count is not sent again * until it changes, so an unanswered notice does not loop. That notice * runs from `mise-hooks:after-on-stop` when mise-hooks is loaded, so it * follows the mise on-stop hook. Otherwise it runs on `agent_settled`. @@ -22,13 +22,13 @@ import { feedbackNotice, MISE_AFTER_ON_STOP_EVENT, MISE_COORDINATES_EVENT, - parseUmpireStatus, - UMPIRE_READY_EVENT, - UMPIRE_REQUEST_STATUS_EVENT, - UMPIRE_STATUS_EVENT, - type UmpireStatus, - type UmpireStatusRequest, -} from "./umpire/support.ts"; + parseGitReviewStatus, + GIT_REVIEW_READY_EVENT, + GIT_REVIEW_REQUEST_STATUS_EVENT, + GIT_REVIEW_STATUS_EVENT, + type GitReviewStatus, + type GitReviewStatusRequest, +} from "./git-review/support.ts"; const GIT_TIMEOUT_MS = 5_000; const STATUS_TIMEOUT_MS = 15_000; @@ -53,16 +53,16 @@ export default function (pi: ExtensionAPI) { } /** Publish status for a repository. Returns undefined when there is nothing to publish. */ - async function readAndPublish(cwd: string): Promise { + async function readAndPublish(cwd: string): Promise { if (!(await isRepository(cwd))) return undefined; - const result = await pi.exec("umpire", ["status", "--needs-review", "--json"], { + const result = await pi.exec("git-review", ["status", "--needs-review", "--json"], { cwd, timeout: STATUS_TIMEOUT_MS, }); if (result.killed || result.code !== 0) return undefined; - const status = parseUmpireStatus(result.stdout); + const status = parseGitReviewStatus(result.stdout); if (!status) return undefined; - pi.events.emit(UMPIRE_STATUS_EVENT, { + pi.events.emit(GIT_REVIEW_STATUS_EVENT, { needsReview: status.needsReview, total: status.total, feedback: status.feedback, @@ -86,7 +86,7 @@ export default function (pi: ExtensionAPI) { return; } if (aborted || status.feedback === lastNudgedFeedback) return; - const activity = await pi.exec("umpire", ["running", "--json"], { + const activity = await pi.exec("git-review", ["running", "--json"], { cwd: ctx.cwd, timeout: STATUS_TIMEOUT_MS, }); @@ -131,9 +131,9 @@ export default function (pi: ExtensionAPI) { hooks.push((ctx, aborted) => onStop(ctx, aborted)); }); - pi.events.on(UMPIRE_REQUEST_STATUS_EVENT, (data) => { + pi.events.on(GIT_REVIEW_REQUEST_STATUS_EVENT, (data) => { if (data === null || typeof data !== "object" || Array.isArray(data)) return; - (data as UmpireStatusRequest).refresh = async (cwd: string) => { + (data as GitReviewStatusRequest).refresh = async (cwd: string) => { await readAndPublish(cwd); }; }); @@ -141,7 +141,7 @@ export default function (pi: ExtensionAPI) { // Announce after the listeners above are registered. Mise may already // be loaded, in which case it answers this event. If mise loads later, // it emits MISE_COORDINATES_EVENT itself. - pi.events.emit(UMPIRE_READY_EVENT, undefined); + pi.events.emit(GIT_REVIEW_READY_EVENT, undefined); pi.on("session_start", async (_event, ctx) => { stopMonitoring(); diff --git a/pi/extensions/umpire/support.ts b/pi/extensions/git-review/support.ts similarity index 77% rename from pi/extensions/umpire/support.ts rename to pi/extensions/git-review/support.ts index 6796b1c..d3facc7 100644 --- a/pi/extensions/umpire/support.ts +++ b/pi/extensions/git-review/support.ts @@ -1,32 +1,32 @@ /** - * Pure helpers for the umpire extension. + * Pure helpers for the git-review extension. * * No Pi imports: node:test can load this file outside the Pi process. - * `umpire status --needs-review --json` prints a result with a summary of { state, count }. + * `git-review status --needs-review --json` prints a result with a summary of { state, count }. * Failures print a versioned object and exit non-zero. Callers check * the exit code before parsing. */ -export const UMPIRE_STATUS_EVENT = "umpire:status"; -export const UMPIRE_REQUEST_STATUS_EVENT = "umpire:request-status"; +export const GIT_REVIEW_STATUS_EVENT = "git-review:status"; +export const GIT_REVIEW_REQUEST_STATUS_EVENT = "git-review:request-status"; /** Mise emits this with a mutable hook list at the end of on-stop. */ export const MISE_AFTER_ON_STOP_EVENT = "mise-hooks:after-on-stop"; -/** Umpire emits this at load. Mise answers with MISE_COORDINATES_EVENT. */ -export const UMPIRE_READY_EVENT = "umpire:ready"; +/** git-review emits this at load. Mise answers with MISE_COORDINATES_EVENT. */ +export const GIT_REVIEW_READY_EVENT = "git-review:ready"; export const MISE_COORDINATES_EVENT = "mise-hooks:coordinates-on-stop"; const REQUIRED_STATES = ["needs_review", "reviewing", "feedback", "incomplete", "approved"] as const; -export type UmpireStatus = { +export type GitReviewStatus = { needsReview: number; total: number; feedback: number; line: string; }; -export type UmpireStatusRequest = { +export type GitReviewStatusRequest = { refresh?: (cwd: string) => Promise; }; @@ -35,11 +35,11 @@ export function reviewStatusLine(needsReview: number, total: number): string { } /** - * Parse successful `umpire status --needs-review --json` stdout. + * Parse successful `git-review status --needs-review --json` stdout. * Returns undefined for unsupported results, malformed rows, or a missing required state. * `total` is the sum of every state in summary, including states this parser does not name. */ -export function parseUmpireStatus(stdout: string): UmpireStatus | undefined { +export function parseGitReviewStatus(stdout: string): GitReviewStatus | undefined { let parsed: unknown; try { parsed = JSON.parse(stdout); @@ -79,7 +79,7 @@ export function parseUmpireStatus(stdout: string): UmpireStatus | undefined { }; } -/** Accept only the status object umpire publishes. */ +/** Accept only the status object git-review publishes. */ export function reviewLineFromEvent(data: unknown): string | undefined { if (data === null || typeof data !== "object") return undefined; const record = data as { needsReview?: unknown; total?: unknown; line?: unknown; feedback?: unknown }; @@ -95,7 +95,7 @@ export function reviewLineFromEvent(data: unknown): string | undefined { export function feedbackNotice(count: number): string { if (count === 1) { - return "Umpire has 1 commit in the feedback state. Use the umpire skill to address it."; + return "git-review has 1 commit in the feedback state. Use the git-review skill to address it."; } - return `Umpire has ${count} commits in the feedback state. Use the umpire skill to address them.`; + return `git-review has ${count} commits in the feedback state. Use the git-review skill to address them.`; } diff --git a/skills/umpire/SKILL.md b/skills/git-review/SKILL.md similarity index 86% rename from skills/umpire/SKILL.md rename to skills/git-review/SKILL.md index 1029937..2271ac0 100644 --- a/skills/umpire/SKILL.md +++ b/skills/git-review/SKILL.md @@ -1,12 +1,12 @@ --- -name: umpire -description: Retrieve saved Umpire review feedback, address comments, and rewrite Git history into adjacent commit stacks. Use when the user asks to address Umpire feedback, insert fixup commits, or prepare amendments for re-review. -compatibility: Requires Git and umpire on PATH, a feature branch with a configured upstream, and permission to rewrite local history. +name: git-review +description: Retrieve saved git-review feedback, address comments, and rewrite Git history into adjacent commit stacks. Use when the user asks to address git-review feedback, insert fixup commits, or prepare amendments for re-review. +compatibility: Requires Git and git-review on PATH, a feature branch with a configured upstream, and permission to rewrite local history. --- -# Address Umpire feedback +# Address git-review feedback -Use this skill to address reviews that the user completed through Umpire. +Use this skill to address reviews that the user completed through git-review. This skill does not launch reviews or register harness commands. The user controls review and approval. @@ -15,7 +15,7 @@ The user controls review and approval. Use this read-only command for current stack details and commit counts: ```sh -umpire status --needs-review --json +git-review status --needs-review --json ``` Successful output is a versioned result object with `"version":1` and `"command":"status"`. @@ -41,7 +41,7 @@ Command failures exit with status `1` and emit the standard versioned JSON error Read `entries` for unresolved stack details and current commit IDs. The summary counts the full feature range even when entries are filtered. -Use `umpire feedback --json` for saved comments and session notes. +Use `git-review feedback --json` for saved comments and session notes. Counts do not identify feedback targets or authorize approval. ## Retrieve feedback @@ -49,7 +49,7 @@ Counts do not identify feedback targets or authorize approval. Run this command from the feature checkout: ```sh -umpire feedback --json +git-review feedback --json ``` Inspect the JSON result, not only the exit code. @@ -84,7 +84,7 @@ Do not silently lose selected feedback or address the same attempt twice. If `active` contains an attempt, pause before rewriting history. Ask the user to finish or resolve the active review. This command does not collect uncaptured notes from interrupted tuicr sessions. -Do not use `umpire run --recover` to retrieve feedback. +Do not use `git-review run --recover` to retrieve feedback. Recovery opens an interactive review and requires the user's choice. ## Prepare the changes @@ -94,9 +94,9 @@ Recovery opens an interactive review and requires the user's choice. 3. Preserve unrelated user changes before changing branches or rewriting history. 4. Summarize the selected feedback and proposed fixes. 5. Record the original branch tip and create a uniquely named local backup branch. -6. Identify each current target stack through Umpire's `view.stacks` data. +6. Identify each current target stack through git-review's `view.stacks` data. -Use `umpire status --needs-review --json` when you need a fresh view of current stacks. +Use `git-review status --needs-review --json` when you need a fresh view of current stacks. Its `view.stacks` includes approved stacks too. Do not derive targets from comment file paths or commit subjects alone. Use full commit IDs. @@ -107,9 +107,9 @@ This keeps older target commit IDs unchanged while you replay later history. Refresh the current view when a target no longer matches the captured stack. Do not guess lineage for split or combined changes. -Make fixes in the feature checkout, not `/umpire/worktree`. +Make fixes in the feature checkout, not `/git-review/worktree`. The review checkout is disposable and is only for code exploration. -Do not edit Umpire's state file or tuicr's saved sessions. +Do not edit git-review's state file or tuicr's saved sessions. ## Create native Git feedback commits @@ -186,15 +186,15 @@ Git generates nested feedback subjects when the target is an existing feedback c 1. Check that the checkout uses the feature branch after integration. 2. Run the repository's required checks on the final history. -3. Run `umpire status --needs-review --json` to check stack formation. +3. Run `git-review status --needs-review --json` to check stack formation. 4. Check that each feedback commit immediately follows its target stack tip. 5. Report each attempt ID, addressed comment, new stack range, and validation result. -Leave review eligibility and approval decisions to Umpire. -Do not call `umpire approve`, launch a review, or claim user approval. +Leave review eligibility and approval decisions to git-review. +Do not call `git-review approve`, launch a review, or claim user approval. Do not change stored state to mark feedback addressed. Keep backup branches until the user requests their removal. Remove temporary fix branches only after checking their commits are reachable from the feature branch. Leave recovery branches intact when integration fails. -Tell the user that the changes are ready for re-review with `umpire run`. +Tell the user that the changes are ready for re-review with `git-review run`. diff --git a/testmain_test.go b/testmain_test.go index ab446b5..bd4f6de 100644 --- a/testmain_test.go +++ b/testmain_test.go @@ -3,7 +3,7 @@ package main import ( "testing" - "github.com/nnutter/umpire/internal/testenv" + "github.com/nnutter/git-review/internal/testenv" ) func TestMain(m *testing.M) { testenv.Run(m) }