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 3824dd8..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,28 +57,56 @@ 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. Configure the intended upstream explicitly with Git. -### Count commits by review state +### Show commits after the upstream ```sh -umpire status -umpire status --json +git-review log +git-review log -p +git-review log -pw +git-review log --stat ``` -This read-only command counts commits in the current feature range, including attached fixups. +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. +Unlike review commands, it uses the upstream range directly and permits merge commits. + +### Show feature stacks and review state + +```sh +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. +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 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. + +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 [ @@ -95,21 +124,21 @@ Command failures use the standard JSON error object. ### List unresolved stacks ```sh -umpire needs-review -umpire 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. 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 ```sh -umpire review-running --json +git-review running --json ``` Successful output is `{"version":1,"running":true}` or `{"version":1,"running":false}`. @@ -122,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: @@ -152,28 +181,28 @@ 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 +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 | | --- | --- | @@ -185,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 review --start abc123 -umpire review --start abc123 --end def456 --json -umpire review --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. @@ -217,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 review --recover -umpire review --replace -umpire review --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 review --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`. @@ -261,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: @@ -289,7 +318,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. @@ -310,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. @@ -352,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 @@ -375,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 1150b47..da913ed 100644 --- a/internal/cli/activity_test.go +++ b/internal/cli/activity_test.go @@ -8,17 +8,17 @@ 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) 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}` @@ -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)) @@ -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/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 69dc791..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,15 +51,8 @@ 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") - 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 { @@ -78,17 +71,16 @@ 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) }} - 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 { + status.Flags().BoolVar(&onlyNeedsReview, "needs-review", false, "Show unresolved stacks and active review attempts") + 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 @@ -102,14 +94,14 @@ 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(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 9bf2657..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,21 +46,29 @@ 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{{"needs-review", "--json"}, {"--json", "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")) 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 +83,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)) @@ -84,15 +92,15 @@ 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{ - {"needs-review", "--json"}, + {"status", "--needs-review", "--json"}, {"approve", "--json"}, - {"review", "--recover", "--replace", "--json"}, + {"run", "--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 @@ -116,7 +124,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 +150,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, "git-review run --recover") } func TestTerminalRenderingRetainsFeedbackAndRemovesControlSequences(t *testing.T) { @@ -159,7 +168,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") @@ -174,7 +183,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 @@ -184,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) @@ -200,7 +209,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 +225,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,10 +236,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") + 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/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 new file mode 100644 index 0000000..5960d4d --- /dev/null +++ b/internal/cli/log.go @@ -0,0 +1,46 @@ +package cli + +import ( + "fmt" + "os/exec" + + "github.com/spf13/cobra" +) + +func (a *application) logCommand() *cobra.Command { + 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, 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 { + 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") + } + if stat { + args = append(args, "--stat") + } + 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") + 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 new file mode 100644 index 0000000..3ac2759 --- /dev/null +++ b/internal/cli/log_test.go @@ -0,0 +1,95 @@ +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 + 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}, + {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.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") + } + 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) + } +} diff --git a/internal/cli/render.go b/internal/cli/render.go index 578703c..0ff3261 100644 --- a/internal/cli/render.go +++ b/internal/cli/render.go @@ -13,18 +13,9 @@ import ( "github.com/charmbracelet/colorprofile" "github.com/charmbracelet/x/ansi" - "github.com/nnutter/umpire/internal/review" + "github.com/nnutter/git-review/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 @@ -42,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 { @@ -77,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 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 c059d1e..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"} { @@ -32,18 +32,60 @@ 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, 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 { + 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)") + 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) + 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) + 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) + var result review.Result + require.NoError(t, json.Unmarshal([]byte(filtered), &result)) + require.Equal(t, "status", result.Command) + require.Equal(t, want, result.Summary) + 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) @@ -52,11 +94,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,15 +112,35 @@ 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) + require.Equal(t, "No reviews pending.\n", filtered) } 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...) @@ -90,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 925eaee..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,9 +65,9 @@ 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, "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 { @@ -105,16 +105,16 @@ 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") } func TestRealTuicrInterruptedRecovery(t *testing.T) { env := realTuicrEnvironment(t) - binary := buildUmpire(t) + binary := buildGitReview(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") @@ -184,9 +184,9 @@ 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, "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": @@ -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 { @@ -254,9 +254,9 @@ 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, "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") @@ -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/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/patch_status_test.go b/internal/review/patch_status_test.go index 0815403..f3a26b5 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) } @@ -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" { @@ -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) @@ -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/run.go b/internal/review/run.go index 518ceda..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 review 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 } @@ -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 git-review run ") } func (options ReviewOptions) hasBounds() bool { diff --git a/internal/review/service.go b/internal/review/service.go index 3ab4d2a..20e1a31 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. @@ -120,19 +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, err := s.Repository.Snapshot(ctx) - if err != nil { - return Result{}, err - } - state, err := (Store{Path: view.Path}).Load() - 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) { @@ -143,16 +131,37 @@ 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 Result{}, err + } + 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) { view, err := s.Repository.Snapshot(ctx) if err != nil { - return nil, err + 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 +178,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) { 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/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/git-review-support.test.ts b/pi/extensions/git-review-support.test.ts new file mode 100644 index 0000000..01f75cc --- /dev/null +++ b/pi/extensions/git-review-support.test.ts @@ -0,0 +1,36 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { parseGitReviewStatus } from "./git-review/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(parseGitReviewStatus(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(parseGitReviewStatus(JSON.stringify(invalid)), undefined); + } + assert.equal(parseGitReviewStatus("not json"), undefined); +}); diff --git a/pi/extensions/umpire.test.ts b/pi/extensions/git-review.test.ts similarity index 94% rename from pi/extensions/umpire.test.ts rename to pi/extensions/git-review.test.ts index e7b5828..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,23 +22,25 @@ 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") { + assert.equal(command, "git-review"); + if (args[0] === "running") { activityCalls++; await activityGate; return activity; } - return { code: 0, killed: false, stdout: JSON.stringify([ + 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 }, { state: "feedback", count: feedback }, { state: "incomplete", count: 0 }, { state: "approved", count: 0 }, - ]) }; + ] }) }; }, 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 d504182..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 --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", "--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", ["review-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 62% rename from pi/extensions/umpire/support.ts rename to pi/extensions/git-review/support.ts index b1c7ac6..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 --json` prints a top-level array 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,21 +35,25 @@ 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. + * 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); } 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; @@ -75,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 }; @@ -91,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 78% rename from skills/umpire/SKILL.md rename to skills/git-review/SKILL.md index 846d845..2271ac0 100644 --- a/skills/umpire/SKILL.md +++ b/skills/git-review/SKILL.md @@ -1,26 +1,28 @@ --- -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. ## 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 +git-review status --needs-review --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"`. +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: 1. `needs_review` 2. `reviewing` @@ -34,13 +36,12 @@ 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 needs-review --json` for unresolved stack details and current commit IDs. -Its `entries` omits approved stacks, but its `view.stacks` includes them. -Use `umpire feedback --json` for saved comments and session notes. +Read `entries` for unresolved stack details and current commit IDs. +The summary counts the full feature range even when entries are filtered. +Use `git-review feedback --json` for saved comments and session notes. Counts do not identify feedback targets or authorize approval. ## Retrieve feedback @@ -48,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. @@ -83,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 `git-review run --recover` to retrieve feedback. Recovery opens an interactive review and requires the user's choice. ## Prepare the changes @@ -93,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 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. @@ -106,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 @@ -185,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 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 review`. +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) }