diff --git a/CHANGELOG.md b/CHANGELOG.md index 7002679..b989c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `prw run` now notifies once (`merged` or `closed`) and automatically stops watching a PR once it is merged or closed, instead of polling it forever. This notification always fires, regardless of `--on`/`notification_filter`. ### Added +- Auto-watch: `prw watch --mine` follows all your open PRs and `prw watch --review-requested` the PRs where your review is requested, across every repository. Matches are added immediately; `prw run` refreshes the searches every 5 minutes. Review-request PRs are dropped when the request goes away; PRs watched by hand are never touched by it. `prw unwatch --mine` / `--review-requested` turns it off and removes the PRs it added. `prw list` gains an `ADDED BY` column (and `source` in `--json`); `prw config show` shows `auto_watch`. +- `prw mcp`: a Model Context Protocol server over stdio for AI coding agents, with tools `get_ci_status`, `wait_for_ci` (progress notifications, cancellation, fail-fast), `list_watched`, `watch_pr` and `unwatch_pr`. Implemented without new dependencies (the official Go SDK needs Go 1.25). - Conditional requests (ETags): repeated polls of unchanged resources are answered with `304 Not Modified`, which doesn't count against the authenticated rate limit. Measured against the real API: after the first poll, 5 further polls of a PR (15 requests) consumed 0 quota. Previously each watched PR cost 3 requests per poll (~540/hour at the default 20s interval), so about 9 PRs exhausted the 5,000/hour limit. - Rate-limit handling: 403/429 rate-limit responses are told apart from missing-permission 403s. `prw run` stops the current round and pauses until the reset time; `prw wait` waits for the reset (still bounded by `--timeout`) instead of giving up. - `prw wait `: blocks until the PR's CI finishes and exits `0` (success or merged), `1` (failed), `2` (timeout), `3` (other error) or `130` (interrupted). Fails fast on the first failed check, follows new pushes, retries transient API errors, and prints a JSON result with `--json`. Progress goes to stderr. @@ -18,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Integration tests against the real GitHub API (`make integration`, build tag `integration`), run nightly by a new `Integration` workflow. They fail against the old status-only logic. ### Fixed +- PR owner and repository names are compared case-insensitively (as on GitHub), so `DevBlac/PRW#5` and `devblac/prw#5` are no longer two separate watch-list entries. - Requests without a token no longer send an empty `Authorization: Bearer ` header, which GitHub rejects with 401 even for public repositories. - `MultiNotifier` (used whenever more than one notification channel is enabled) stopped at the first failing notifier. A down webhook silently suppressed the console and native notifications that were supposed to run after it; now every notifier always runs and their errors are combined. - The README's "Prebuilt binaries" install snippet was pinned to `v0.2.0`, two releases behind the current `v0.3.0`. It now resolves the actual latest release tag first. diff --git a/README.md b/README.md index 1527673..669de8f 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,8 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you ## Features - **`prw wait`**: block until a PR's CI finishes and exit 0/1/2 — for scripts, pipelines and AI coding agents +- **MCP server** (`prw mcp`): lets Claude Code, Claude Desktop, Cursor and other agents check and wait for PR CI as tools +- **Auto-watch**: `prw watch --mine` / `--review-requested` follow your PRs and review requests across all repositories, no URLs needed - **Auto-cleanup**: merged/closed PRs are notified once and dropped from the watch list by `prw run` - **Watch multiple PRs** from different repositories simultaneously - **Instant notifications** when CI status changes (pending, success, failure, error) @@ -142,6 +144,15 @@ prw watch https://github.com/owner/repo/pull/123 The tool will fetch the PR details and add it to your watch list. +**Or let prw find your PRs** — no URLs to copy: + +```bash +prw watch --mine # all your open PRs, in every repository +prw watch --review-requested # open PRs where your review is requested (incl. your teams) +``` + +These follow a GitHub search instead of a fixed list: matching PRs are added right away, and `prw run` searches again every 5 minutes, so new PRs show up by themselves. Merged or closed PRs are dropped after their final notification, and review-request PRs are also dropped once your review is no longer requested. `prw list` shows which PRs were added automatically (`ADDED BY`). Undo with `prw unwatch --mine` / `--review-requested`, which removes only the PRs that search added — PRs you watched by hand are kept. + ### 3. Start the watcher ```bash @@ -251,6 +262,37 @@ With `--json`, stdout is a single object: `state` is one of `success`, `failure`, `error`, `pending` (with `timed_out` or `interrupted` set), `merged`, or `closed`. +## Using prw from AI agents: `prw mcp` + +`prw mcp` runs a [Model Context Protocol](https://modelcontextprotocol.io) server over stdio, so coding agents can check a PR's CI and wait for it after pushing — and read which checks failed — without screen-scraping `gh` output. + +**Claude Code:** + +```bash +claude mcp add prw -- prw mcp +``` + +**Claude Desktop, Cursor and other clients** (add to the client's `mcpServers` JSON config): + +```json +{ + "mcpServers": { + "prw": { "command": "prw", "args": ["mcp"] } + } +} +``` + +Tools: + +| Tool | What it does | +|------|--------------| +| `get_ci_status` | Current state of a PR (`success`, `failure`, `error`, `pending`, `merged`, `closed`), check counts, failed checks with links. Returns immediately. | +| `wait_for_ci` | Blocks until CI finishes (`timeout_seconds` default 600, max 3600). Fails fast on the first failed check, follows new pushes, sends progress notifications. On timeout it returns `pending` with `timed_out: true` so the agent can call it again. | +| `list_watched` | PRs on the local watch list with their last known state. | +| `watch_pr` / `unwatch_pr` | Add or remove a PR from the watch list (a running `prw run` picks it up). | + +Results include a one-line summary for the model plus the full data as JSON (and as `structuredContent`). Authentication works like every other command — with `gh auth login` there's nothing to configure. All tools share one GitHub client, so the ETag cache keeps repeated polling free. + ## Configuration Configuration is stored in `~/.prw/config.json`. You can manage settings via the `config` subcommand. diff --git a/TASKS.md b/TASKS.md index 36974eb..fdbec24 100644 --- a/TASKS.md +++ b/TASKS.md @@ -86,7 +86,10 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - 🟡 #8 `gh` extension distribution — code part done (token from `GH_TOKEN` / `gh auth token`, tested with a fake gh binary and end-to-end). Remaining: publishing as `gh-prw` needs a repo named `gh-prw` (new repo or rename) + precompiled release assets named `gh-prw--` — owner decision. - ✅ #13 `prw wait ` — done (exit codes 0/1/2/3/130, fail-fast, follows new pushes, retries transient errors, `--json`; verified live against devblac/prw#5 and real process exit codes) - ✅ #9 API efficiency — done (ETag/If-None-Match cache; measured live: 15 steady-state requests = 0 quota; rate-limit detection + pause in `run`, wait-for-reset in `wait`). GraphQL batching not needed for now. -- Next: #14 MCP server (builds on `waitForCI`) +- ✅ #14 MCP server — done (`prw mcp`: 5 tools, progress + cancellation, protocol 2025-06-18/2025-03-26/2024-11-05; verified end-to-end over stdio against the real API with the token from `gh auth token`). Not yet tried inside a real MCP client. +- ✅ #10 `prw watch --mine` / `--review-requested` — done (search-backed subscriptions refreshed every 5 min by `prw run`; verified live: 18 open PRs across 7 repos found, CI checked, cleanly removed by `unwatch --mine`) +- 🆕 #18 **Report "no CI" instead of `pending` forever** (found while testing #10: 14 of 18 real PRs have zero checks — repos without CI, or stacked PRs whose base isn't `main`). Today `prw wait` / `wait_for_ci` would wait until the timeout on them. Needs a grace period after the last push (CI takes a moment to register checks) before concluding "no CI". Impact 4, effort S–M. +- Next: #18, then #11 review & mergeability events ## Planned diff --git a/cmd/prw/autowatch_test.go b/cmd/prw/autowatch_test.go new file mode 100644 index 0000000..860334c --- /dev/null +++ b/cmd/prw/autowatch_test.go @@ -0,0 +1,268 @@ +package main + +import ( + "fmt" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// setupAutoWatchCmd points commands at a temp config (with a token) and a fake GitHub +// whose search returns mine/reviewRequested PR numbers in owner/repo. +func setupAutoWatchCmd(t *testing.T, mine, reviewRequested []int) *int { + t.Helper() + path := filepath.Join(t.TempDir(), ".prw", "config.json") + oldPath := config.ConfigPath + config.ConfigPath = func() (string, error) { return path, nil } + t.Cleanup(func() { config.ConfigPath = oldPath }) + cfg := config.DefaultConfig() + cfg.GitHubToken = "test-token" + if err := cfg.Save(); err != nil { + t.Fatal(err) + } + + searches := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/search/issues": + searches++ + numbers := mine + if strings.Contains(r.URL.Query().Get("q"), "review-requested:@me") { + numbers = reviewRequested + } + items := make([]string, len(numbers)) + for i, n := range numbers { + items[i] = fmt.Sprintf(`{"number":%d,"title":"PR %d","html_url":"https://github.com/owner/repo/pull/%d","pull_request":{}}`, n, n, n) + } + fmt.Fprintf(w, `{"total_count":%d,"items":[%s]}`, len(items), strings.Join(items, ",")) + case strings.HasPrefix(r.URL.Path, "/repos/owner/repo/pulls/"): + n := strings.TrimPrefix(r.URL.Path, "/repos/owner/repo/pulls/") + fmt.Fprintf(w, `{"number":%s,"title":"PR %s","state":"open","head":{"sha":"sha%s"}}`, n, n, n) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + oldClient := newGitHubClient + newGitHubClient = func(token string) *github.Client { + c := github.NewClient(token) + c.BaseURL = server.URL + c.HTTPClient = server.Client() + return c + } + t.Cleanup(func() { newGitHubClient = oldClient }) + + resetFlags := func() { + watchMine, watchReviewRequested, unwatchMine, unwatchReviewRequested = false, false, false, false + } + resetFlags() + t.Cleanup(resetFlags) + return &searches +} + +func watchListSummary(t *testing.T) string { + t.Helper() + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + var parts []string + for _, pr := range cfg.WatchedPRs { + src := pr.Source + if src == "" { + src = "manual" + } + parts = append(parts, fmt.Sprintf("%d:%s", pr.Number, src)) + } + return strings.Join(parts, " ") +} + +func TestWatchCmd_MineAddsSubscriptionAndCurrentPRs(t *testing.T) { + searches := setupAutoWatchCmd(t, []int{1, 2}, nil) + watchMine = true + + out, err := captureStdout(func() error { return watchCmd.RunE(watchCmd, nil) }) + if err != nil { + t.Fatalf("watch --mine: %v", err) + } + for _, want := range []string{"Now auto-watching your open PRs.", "+ owner/repo#1 - PR 1", "+ owner/repo#2 - PR 2", "searches again every 5 minutes"} { + if !strings.Contains(out, want) { + t.Errorf("output missing %q:\n%s", want, out) + } + } + cfg, _ := config.Load() + if !cfg.HasAutoWatch(config.AutoWatchMine) || cfg.HasAutoWatch(config.AutoWatchReviewRequested) { + t.Errorf("AutoWatch = %v", cfg.AutoWatch) + } + if got := watchListSummary(t); got != "1:mine 2:mine" { + t.Errorf("watch list = %s", got) + } + + // Running it again is idempotent. + out, err = captureStdout(func() error { return watchCmd.RunE(watchCmd, nil) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Already auto-watching your open PRs.") || !strings.Contains(out, "No new PRs to add right now.") { + t.Errorf("second run output:\n%s", out) + } + if *searches != 2 { + t.Errorf("searches = %d", *searches) + } +} + +func TestWatchCmd_BothSourcesAndAURL(t *testing.T) { + setupAutoWatchCmd(t, []int{1}, []int{2}) + watchMine, watchReviewRequested = true, true + + if _, err := captureStdout(func() error { + return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/9"}) + }); err != nil { + t.Fatal(err) + } + if got := watchListSummary(t); got != "9:manual 1:mine 2:review-requested" { + t.Errorf("watch list = %s", got) + } +} + +func TestWatchCmd_ByHandClaimsAnAutoWatchedPR(t *testing.T) { + setupAutoWatchCmd(t, []int{1}, nil) + watchMine = true + captureStdout(func() error { return watchCmd.RunE(watchCmd, nil) }) + watchMine = false + + out, err := captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/1"}) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "already being watched") { + t.Errorf("output: %s", out) + } + if got := watchListSummary(t); got != "1:manual" { + t.Errorf("watching by hand must claim the PR, got %s", got) + } + + // ... so turning auto-watch off keeps it. + unwatchMine = true + if _, err := captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, nil) }); err != nil { + t.Fatal(err) + } + if got := watchListSummary(t); got != "1:manual" { + t.Errorf("watch list after unwatch --mine = %s", got) + } +} + +func TestUnwatchCmd_MineRemovesSubscriptionAndItsPRs(t *testing.T) { + setupAutoWatchCmd(t, []int{1, 2}, []int{3}) + watchMine, watchReviewRequested = true, true + captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/9"}) }) + + unwatchMine = true + out, err := captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, nil) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Stopped auto-watching your open PRs; removed 2 PR(s) it had added.") { + t.Errorf("output: %s", out) + } + cfg, _ := config.Load() + if cfg.HasAutoWatch(config.AutoWatchMine) || !cfg.HasAutoWatch(config.AutoWatchReviewRequested) { + t.Errorf("AutoWatch = %v", cfg.AutoWatch) + } + if got := watchListSummary(t); got != "9:manual 3:review-requested" { + t.Errorf("watch list = %s", got) + } + + out, _ = captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, nil) }) + if !strings.Contains(out, "Was not auto-watching your open PRs.") { + t.Errorf("second unwatch --mine output: %s", out) + } +} + +func TestWatchCmd_Args(t *testing.T) { + setupAutoWatchCmd(t, nil, nil) + + if err := watchCmd.Args(watchCmd, nil); err == nil || !strings.Contains(err.Error(), "--mine") { + t.Errorf("no URL and no flag must fail with a hint, got %v", err) + } + if err := watchCmd.Args(watchCmd, []string{"a", "b"}); err == nil { + t.Error("two URLs must fail") + } + if err := watchCmd.Args(watchCmd, []string{"https://github.com/o/r/pull/1"}); err != nil { + t.Errorf("one URL: %v", err) + } + watchMine = true + if err := watchCmd.Args(watchCmd, nil); err != nil { + t.Errorf("--mine without URL: %v", err) + } + if err := watchCmd.Args(watchCmd, []string{"a", "b"}); err == nil { + t.Error("--mine with two URLs must fail") + } + + if err := unwatchCmd.Args(unwatchCmd, nil); err == nil { + t.Error("unwatch with nothing must fail") + } + unwatchReviewRequested = true + if err := unwatchCmd.Args(unwatchCmd, nil); err != nil { + t.Errorf("unwatch --review-requested: %v", err) + } +} + +func TestWatchCmd_MineNeedsToken(t *testing.T) { + setupAutoWatchCmd(t, nil, nil) + config.Update(func(c *config.Config) error { c.GitHubToken = ""; return nil }) + t.Setenv("GITHUB_TOKEN", "") + watchMine = true + + if err := watchCmd.RunE(watchCmd, nil); err != errMissingToken { + t.Errorf("err = %v, want errMissingToken", err) + } + if cfg, _ := config.Load(); len(cfg.AutoWatch) != 0 { + t.Error("auto-watch must not be enabled without a token") + } +} + +func TestListAndConfigShow_ShowAutoWatch(t *testing.T) { + setupAutoWatchCmd(t, []int{1}, nil) + watchMine = true + captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{"https://github.com/owner/repo/pull/9"}) }) + + out, err := captureStdout(func() error { return listCmd.RunE(listCmd, nil) }) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, "Auto-watching your open PRs (--mine).") || !strings.Contains(out, "ADDED BY") { + t.Errorf("list output:\n%s", out) + } + lines := strings.Split(out, "\n") + var row1, row9 string + for _, l := range lines { + if strings.Contains(l, "#1 ") { + row1 = l + } + if strings.Contains(l, "#9 ") { + row9 = l + } + } + if !strings.Contains(row1, "--mine") || !strings.Contains(row9, "you") { + t.Errorf("ADDED BY column wrong:\n%s", out) + } + + oldJSON := listJSON + listJSON = true + defer func() { listJSON = oldJSON }() + out, _ = captureStdout(func() error { return listCmd.RunE(listCmd, nil) }) + if !strings.Contains(out, `"source": "mine"`) { + t.Errorf("JSON list should include the source:\n%s", out) + } + + out, _ = captureStdout(func() error { return configShowCmd.RunE(configShowCmd, nil) }) + if !strings.Contains(out, "auto_watch: mine") { + t.Errorf("config show output:\n%s", out) + } +} diff --git a/cmd/prw/main.go b/cmd/prw/main.go index 911a5be..a7a7665 100644 --- a/cmd/prw/main.go +++ b/cmd/prw/main.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strconv" + "strings" "syscall" "text/tabwriter" "time" @@ -52,6 +53,10 @@ func init() { rootCmd.AddCommand(completionCmd) listCmd.Flags().BoolVar(&listJSON, "json", false, "output watched PRs as JSON") + watchCmd.Flags().BoolVar(&watchMine, "mine", false, "automatically watch all your open PRs") + watchCmd.Flags().BoolVar(&watchReviewRequested, "review-requested", false, "automatically watch open PRs where your review is requested") + unwatchCmd.Flags().BoolVar(&unwatchMine, "mine", false, "stop auto-watching your open PRs and remove the PRs it added") + unwatchCmd.Flags().BoolVar(&unwatchReviewRequested, "review-requested", false, "stop auto-watching review requests and remove the PRs it added") runCmd.Flags().StringVar(¬ifyFilter, "on", "", "notify on: change, fail, or success") runCmd.Flags().BoolVar(&runOnce, "once", false, "check watched PRs once and exit") runCmd.Flags().BoolVar(¬ifyNative, "notify-native", false, "enable native OS notifications (macOS/Linux/Windows)") @@ -70,63 +75,187 @@ var errMissingToken = errors.New("missing GITHUB_TOKEN; run 'gh auth login', set // newGitHubClient allows tests to inject a custom GitHub client. var newGitHubClient = github.NewClient -var watchCmd = &cobra.Command{ - Use: "watch ", - Short: "Add a PR to the watch list", - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - prURL := args[0] +var ( + watchMine bool + watchReviewRequested bool + unwatchMine bool + unwatchReviewRequested bool +) - owner, repo, number, err := github.ParsePRURL(prURL) - if err != nil { - return fmt.Errorf("invalid PR URL: %w", err) - } +// autoWatchFlags returns the auto-watch sources selected by a command's flags. +func autoWatchFlags(mine, reviewRequested bool) []string { + var sources []string + if mine { + sources = append(sources, config.AutoWatchMine) + } + if reviewRequested { + sources = append(sources, config.AutoWatchReviewRequested) + } + return sources +} - cfg, err := config.Load() - if err != nil { - return fmt.Errorf("failed to load config: %w", err) +// prURLOrAutoWatchArgs requires a PR URL unless an auto-watch flag is set. +func prURLOrAutoWatchArgs(flagsSet func() bool) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if flagsSet() { + return cobra.MaximumNArgs(1)(cmd, args) } - - token := cfg.GetToken() - if token == "" { - return errMissingToken + if len(args) != 1 { + return fmt.Errorf("expected a PR URL (or --mine / --review-requested), got %d argument(s)", len(args)) } + return nil + } +} - // Try to fetch the PR to validate it exists and get title - client := newGitHubClient(token) - pr, err := client.GetPullRequest(owner, repo, number) - if err != nil { - return fmt.Errorf("failed to fetch PR: %w", err) +var watchCmd = &cobra.Command{ + Use: "watch [PR_URL]", + Short: "Add a PR to the watch list, or follow your PRs automatically", + Long: `Add a pull request to the watch list. + +With --mine or --review-requested, prw instead follows a GitHub search and keeps +the watch list up to date by itself: matching PRs are added right away, and +'prw run' refreshes the search every 5 minutes. + + --mine your open PRs (author:@me), in every repository + --review-requested open PRs where your review is requested (incl. your teams) + +PRs added by --review-requested are dropped again once your review is no longer +requested. Merged or closed PRs are always dropped by 'prw run' after it notifies. +Undo with 'prw unwatch --mine' / 'prw unwatch --review-requested'.`, + Example: ` prw watch https://github.com/owner/repo/pull/123 + prw watch --mine + prw watch --mine --review-requested`, + Args: prURLOrAutoWatchArgs(func() bool { return watchMine || watchReviewRequested }), + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 1 { + if err := watchSinglePR(args[0]); err != nil { + return err + } } - - watchedPR := config.WatchedPR{ - Owner: owner, - Repo: repo, - Number: number, - Title: pr.Title, + if sources := autoWatchFlags(watchMine, watchReviewRequested); len(sources) > 0 { + return enableAutoWatch(sources) } + return nil + }, +} - // Re-read the config under a lock instead of saving the copy loaded before the - // network call: a running `prw run` may have updated the file in the meantime. - added := false - if err := config.Update(func(c *config.Config) error { - added = c.AddPR(watchedPR) - if !added { - return config.ErrNoChanges - } - return nil - }); err != nil { - return fmt.Errorf("failed to save config: %w", err) - } +func watchSinglePR(prURL string) error { + owner, repo, number, err := github.ParsePRURL(prURL) + if err != nil { + return fmt.Errorf("invalid PR URL: %w", err) + } + + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("failed to load config: %w", err) + } + + token := cfg.GetToken() + if token == "" { + return errMissingToken + } + + // Try to fetch the PR to validate it exists and get title + client := newGitHubClient(token) + pr, err := client.GetPullRequest(owner, repo, number) + if err != nil { + return fmt.Errorf("failed to fetch PR: %w", err) + } + + watchedPR := config.WatchedPR{ + Owner: owner, + Repo: repo, + Number: number, + Title: pr.Title, + } + // Re-read the config under a lock instead of saving the copy loaded before the + // network call: a running `prw run` may have updated the file in the meantime. + added, claimed := false, false + if err := config.Update(func(c *config.Config) error { + added = c.AddPR(watchedPR) if !added { - fmt.Printf("PR %s/%s#%d is already being watched.\n", owner, repo, number) - return nil + // Watching by hand a PR that auto-watch added keeps it even if that + // auto-watch is turned off later. + claimed = c.MarkManual(owner, repo, number) + } + if !added && !claimed { + return config.ErrNoChanges } + return nil + }); err != nil { + return fmt.Errorf("failed to save config: %w", err) + } - fmt.Printf("Now watching: %s/%s#%d - %s\n", owner, repo, number, pr.Title) + if !added { + fmt.Printf("PR %s/%s#%d is already being watched.\n", owner, repo, number) return nil - }, + } + + fmt.Printf("Now watching: %s/%s#%d - %s\n", owner, repo, number, pr.Title) + return nil +} + +func enableAutoWatch(sources []string) error { + cfg, err := config.Load() + if err != nil { + return fmt.Errorf("failed to load config: %w", err) + } + token := cfg.GetToken() + if token == "" { + return errMissingToken + } + + var enabled []string + if err := config.Update(func(c *config.Config) error { + for _, source := range sources { + if c.EnableAutoWatch(source) { + enabled = append(enabled, source) + } + } + if len(enabled) == 0 { + return config.ErrNoChanges + } + return nil + }); err != nil { + return fmt.Errorf("failed to save config: %w", err) + } + for _, source := range sources { + state := "Now auto-watching" + if !contains(enabled, source) { + state = "Already auto-watching" + } + fmt.Printf("%s %s.\n", state, watcher.AutoWatchDescription[source]) + } + + // Run the searches now so the user sees the result immediately. + res, err := watcher.SyncAutoWatch(newGitHubClient(token), sources) + for _, warning := range res.Warnings { + fmt.Printf("Warning: %s\n", warning) + } + if err != nil { + return fmt.Errorf("auto-watch is enabled, but searching GitHub failed (prw run will retry): %w", err) + } + if len(res.Added) == 0 { + fmt.Println("No new PRs to add right now.") + } + for _, pr := range res.Added { + fmt.Printf(" + %s/%s#%d - %s\n", pr.Owner, pr.Repo, pr.Number, pr.Title) + } + for _, pr := range res.Removed { + fmt.Printf(" - %s/%s#%d (review no longer requested)\n", pr.Owner, pr.Repo, pr.Number) + } + fmt.Println("'prw run' keeps this list up to date (searches again every 5 minutes).") + return nil +} + +func contains(list []string, s string) bool { + for _, v := range list { + if v == s { + return true + } + } + return false } var listCmd = &cobra.Command{ @@ -142,14 +271,20 @@ var listCmd = &cobra.Command{ return outputJSONList(cfg.WatchedPRs) } + for _, source := range cfg.AutoWatch { + fmt.Printf("Auto-watching %s (--%s).\n", watcher.AutoWatchDescription[source], source) + } if len(cfg.WatchedPRs) == 0 { fmt.Println("No PRs being watched.") return nil } + if len(cfg.AutoWatch) > 0 { + fmt.Println() + } w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) - fmt.Fprintln(w, "REPO\tPR\tSTATUS\tLAST CHECKED\tTITLE") - fmt.Fprintln(w, "----\t--\t------\t------------\t-----") + fmt.Fprintln(w, "REPO\tPR\tSTATUS\tLAST CHECKED\tADDED BY\tTITLE") + fmt.Fprintln(w, "----\t--\t------\t------------\t--------\t-----") for _, pr := range cfg.WatchedPRs { repo := fmt.Sprintf("%s/%s", pr.Owner, pr.Repo) @@ -166,7 +301,11 @@ var listCmd = &cobra.Command{ if len(title) > 50 { title = title[:47] + "..." } - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", repo, prNum, status, lastChecked, title) + addedBy := "you" + if pr.Source != "" { + addedBy = "--" + pr.Source + } + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n", repo, prNum, status, lastChecked, addedBy, title) } w.Flush() @@ -175,36 +314,89 @@ var listCmd = &cobra.Command{ } var unwatchCmd = &cobra.Command{ - Use: "unwatch ", - Short: "Remove a PR from the watch list", - Args: cobra.ExactArgs(1), + Use: "unwatch [PR_URL]", + Short: "Remove a PR from the watch list, or stop following your PRs", + Long: `Remove a pull request from the watch list. + +With --mine or --review-requested, stop that auto-watch and remove the PRs it +added. PRs you added by hand with 'prw watch ' are kept. + +Unwatching a single PR that auto-watch added only lasts until the next search +finds it again; use --mine / --review-requested to stop following those PRs.`, + Example: ` prw unwatch https://github.com/owner/repo/pull/123 + prw unwatch --review-requested`, + Args: prURLOrAutoWatchArgs(func() bool { return unwatchMine || unwatchReviewRequested }), RunE: func(cmd *cobra.Command, args []string) error { - prURL := args[0] - - owner, repo, number, err := github.ParsePRURL(prURL) - if err != nil { - return fmt.Errorf("invalid PR URL: %w", err) - } - - removed := false - if err := config.Update(func(c *config.Config) error { - removed = c.RemovePR(owner, repo, number) - if !removed { - return config.ErrNoChanges + if len(args) == 1 { + if err := unwatchSinglePR(args[0]); err != nil { + return err } - return nil - }); err != nil { - return fmt.Errorf("failed to update config: %w", err) } + if sources := autoWatchFlags(unwatchMine, unwatchReviewRequested); len(sources) > 0 { + return disableAutoWatch(sources) + } + return nil + }, +} +func unwatchSinglePR(prURL string) error { + owner, repo, number, err := github.ParsePRURL(prURL) + if err != nil { + return fmt.Errorf("invalid PR URL: %w", err) + } + + removed := false + if err := config.Update(func(c *config.Config) error { + removed = c.RemovePR(owner, repo, number) if !removed { - fmt.Printf("PR %s/%s#%d is not being watched.\n", owner, repo, number) - return nil + return config.ErrNoChanges } + return nil + }); err != nil { + return fmt.Errorf("failed to update config: %w", err) + } - fmt.Printf("Stopped watching: %s/%s#%d\n", owner, repo, number) + if !removed { + fmt.Printf("PR %s/%s#%d is not being watched.\n", owner, repo, number) return nil - }, + } + + fmt.Printf("Stopped watching: %s/%s#%d\n", owner, repo, number) + return nil +} + +func disableAutoWatch(sources []string) error { + type outcome struct { + wasOn bool + removed []config.WatchedPR + } + results := map[string]outcome{} + if err := config.Update(func(c *config.Config) error { + changed := false + for _, source := range sources { + wasOn, removed := c.DisableAutoWatch(source) + results[source] = outcome{wasOn, removed} + if wasOn || len(removed) > 0 { + changed = true + } + } + if !changed { + return config.ErrNoChanges + } + return nil + }); err != nil { + return fmt.Errorf("failed to update config: %w", err) + } + + for _, source := range sources { + r := results[source] + if !r.wasOn && len(r.removed) == 0 { + fmt.Printf("Was not auto-watching %s.\n", watcher.AutoWatchDescription[source]) + continue + } + fmt.Printf("Stopped auto-watching %s; removed %d PR(s) it had added.\n", watcher.AutoWatchDescription[source], len(r.removed)) + } + return nil } var runCmd = &cobra.Command{ @@ -296,6 +488,12 @@ var configShowCmd = &cobra.Command{ _, tokenSource := cfg.ResolveToken() fmt.Printf("github_token: %s\n", tokenSource) + autoWatch := "off" + if len(cfg.AutoWatch) > 0 { + autoWatch = strings.Join(cfg.AutoWatch, ", ") + } + fmt.Printf("auto_watch: %s\n", autoWatch) + fmt.Printf("\nWatched PRs: %d\n", len(cfg.WatchedPRs)) return nil @@ -424,6 +622,7 @@ type listPROutput struct { Status string `json:"status"` LastChecked *time.Time `json:"last_checked,omitempty"` Title string `json:"title,omitempty"` + Source string `json:"source,omitempty"` // auto-watch source that added it; empty if added by hand } func outputJSONList(prs []config.WatchedPR) error { @@ -447,6 +646,7 @@ func outputJSONList(prs []config.WatchedPR) error { Status: status, LastChecked: lastChecked, Title: pr.Title, + Source: pr.Source, }) } diff --git a/cmd/prw/mcp.go b/cmd/prw/mcp.go new file mode 100644 index 0000000..44d7240 --- /dev/null +++ b/cmd/prw/mcp.go @@ -0,0 +1,711 @@ +package main + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/signal" + "strings" + "sync" + "syscall" + "time" + + "github.com/spf13/cobra" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" + "github.com/devblac/prw/internal/version" +) + +// prw speaks the Model Context Protocol over stdio: newline-delimited JSON-RPC 2.0 +// messages on stdin/stdout, logs on stderr. Only the parts an MCP tool server needs +// are implemented (initialize, ping, tools/list, tools/call, progress, cancellation); +// the official Go SDK would require raising the module's Go version. + +// mcpProtocolVersions are the protocol revisions this server can speak, newest first. +var mcpProtocolVersions = []string{"2025-06-18", "2025-03-26", "2024-11-05"} + +const ( + rpcParseError = -32700 + rpcInvalidRequest = -32600 + rpcMethodNotFound = -32601 + rpcInvalidParams = -32602 + + mcpDefaultWaitTimeout = 10 * time.Minute + mcpMaxWaitTimeout = time.Hour + mcpDefaultWaitInterval = 15 * time.Second + mcpMinWaitInterval = 5 * time.Second + + mcpInstructions = `prw reports the CI status of GitHub pull requests, aggregating GitHub Actions check runs and legacy commit statuses. +Use get_ci_status for a one-off check, and wait_for_ci after pushing to a PR to block until CI finishes (it fails fast and lists failed checks with links). +States: success, failure, error, pending, merged, closed.` +) + +func init() { + rootCmd.AddCommand(mcpCmd) +} + +var mcpCmd = &cobra.Command{ + Use: "mcp", + Short: "Run an MCP server on stdio so AI agents can check and wait for PR CI", + Long: `Run a Model Context Protocol (MCP) server over stdin/stdout. + +AI coding agents (Claude Code, Claude Desktop, Cursor, ...) can then call these tools: + get_ci_status current CI state of a PR, with failed checks and links + wait_for_ci block until a PR's CI finishes (fails fast on the first failed check) + list_watched PRs on prw's watch list and their last known state + watch_pr add a PR to the watch list (picked up by a running 'prw run') + unwatch_pr remove a PR from the watch list + +Authentication is the same as for every other command (gh auth login, GITHUB_TOKEN, ...).`, + Example: ` # Claude Code + claude mcp add prw -- prw mcp + + # Claude Desktop / Cursor (mcpServers in the client's JSON config) + "prw": { "command": "prw", "args": ["mcp"] }`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + return newMCPServer(os.Stdin, os.Stdout, os.Stderr, defaultMCPClient).Serve(ctx) + }, +} + +// mcpClient is what the MCP tools need from GitHub. +type mcpClient interface { + ciClient +} + +func defaultMCPClient() (mcpClient, error) { + cfg, err := config.Load() + if err != nil { + return nil, fmt.Errorf("failed to load config: %w", err) + } + token := cfg.GetToken() + if token == "" { + return nil, errMissingToken + } + return newGitHubClient(token), nil +} + +type rpcRequest struct { + JSONRPC string `json:"jsonrpc"` + ID json.RawMessage `json:"id,omitempty"` + Method string `json:"method,omitempty"` + Params json.RawMessage `json:"params,omitempty"` +} + +type rpcError struct { + Code int `json:"code"` + Message string `json:"message"` +} + +type rpcResponse struct { + JSONRPC string `json:"jsonrpc"` + ID json.RawMessage `json:"id"` + Result interface{} `json:"result,omitempty"` + Error *rpcError `json:"error,omitempty"` +} + +type rpcNotification struct { + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + Params interface{} `json:"params,omitempty"` +} + +type mcpServer struct { + in io.Reader + out io.Writer + logOut io.Writer + + writeMu sync.Mutex + + clientFactory func() (mcpClient, error) + clientMu sync.Mutex + client mcpClient // created on first use and shared, so the ETag cache is too + + inflightMu sync.Mutex + inflight map[string]context.CancelFunc + calls sync.WaitGroup +} + +func newMCPServer(in io.Reader, out, logOut io.Writer, clientFactory func() (mcpClient, error)) *mcpServer { + return &mcpServer{ + in: in, + out: out, + logOut: logOut, + clientFactory: clientFactory, + inflight: make(map[string]context.CancelFunc), + } +} + +// Serve handles messages until stdin is closed or ctx is done. Tool calls run +// concurrently so that pings and cancellations are handled during a long wait. +func (s *mcpServer) Serve(ctx context.Context) error { + ctx, cancel := context.WithCancel(ctx) + defer func() { + cancel() + s.calls.Wait() + }() + + lines := make(chan []byte) + readErr := make(chan error, 1) + go func() { + scanner := bufio.NewScanner(s.in) + scanner.Buffer(make([]byte, 0, 64*1024), 10*1024*1024) + for scanner.Scan() { + line := append([]byte(nil), scanner.Bytes()...) + select { + case lines <- line: + case <-ctx.Done(): + return + } + } + readErr <- scanner.Err() + }() + + for { + select { + case <-ctx.Done(): + return nil + case err := <-readErr: + // stdin closed (err is nil on EOF): the client is shutting down. Let calls in + // flight finish and answer; a client that can't wait sends SIGTERM, which + // cancels ctx. + finished := make(chan struct{}) + go func() { + s.calls.Wait() + close(finished) + }() + select { + case <-finished: + case <-ctx.Done(): + } + return err + case line := <-lines: + s.handleLine(ctx, line) + } + } +} + +func (s *mcpServer) handleLine(ctx context.Context, line []byte) { + line = bytes.TrimSpace(line) + if len(line) == 0 { + return + } + if line[0] == '[' { + s.respondError(json.RawMessage("null"), rpcInvalidRequest, "batch requests are not supported") + return + } + + var req rpcRequest + if err := json.Unmarshal(line, &req); err != nil { + s.respondError(json.RawMessage("null"), rpcParseError, "parse error: "+err.Error()) + return + } + if req.Method == "" { + return // a response to a request we never send; ignore + } + if len(req.ID) == 0 || string(req.ID) == "null" { + s.handleNotification(req) + return + } + + switch req.Method { + case "initialize": + s.respond(req.ID, s.initializeResult(req.Params)) + case "ping": + s.respond(req.ID, struct{}{}) + case "tools/list": + s.respond(req.ID, map[string]interface{}{"tools": mcpTools}) + case "tools/call": + s.startToolCall(ctx, req) + default: + s.respondError(req.ID, rpcMethodNotFound, "method not found: "+req.Method) + } +} + +func (s *mcpServer) handleNotification(req rpcRequest) { + if req.Method != "notifications/cancelled" { + return // notifications/initialized and others need no action + } + var params struct { + RequestID json.RawMessage `json:"requestId"` + } + if json.Unmarshal(req.Params, ¶ms) != nil { + return + } + s.inflightMu.Lock() + cancel := s.inflight[string(params.RequestID)] + s.inflightMu.Unlock() + if cancel != nil { + cancel() + } +} + +func (s *mcpServer) initializeResult(params json.RawMessage) interface{} { + var p struct { + ProtocolVersion string `json:"protocolVersion"` + } + _ = json.Unmarshal(params, &p) + negotiated := mcpProtocolVersions[0] + for _, v := range mcpProtocolVersions { + if v == p.ProtocolVersion { + negotiated = v + } + } + return map[string]interface{}{ + "protocolVersion": negotiated, + "capabilities": map[string]interface{}{"tools": map[string]interface{}{}}, + "serverInfo": map[string]string{"name": "prw", "version": version.Version}, + "instructions": mcpInstructions, + } +} + +func (s *mcpServer) startToolCall(ctx context.Context, req rpcRequest) { + var params struct { + Name string `json:"name"` + Arguments json.RawMessage `json:"arguments"` + Meta struct { + ProgressToken json.RawMessage `json:"progressToken"` + } `json:"_meta"` + } + if err := json.Unmarshal(req.Params, ¶ms); err != nil || params.Name == "" { + s.respondError(req.ID, rpcInvalidParams, "tools/call needs a tool name") + return + } + handler, ok := mcpToolHandlers[params.Name] + if !ok { + s.respondError(req.ID, rpcInvalidParams, "unknown tool: "+params.Name) + return + } + + callCtx, cancel := context.WithCancel(ctx) + key := string(req.ID) + s.inflightMu.Lock() + s.inflight[key] = cancel + s.inflightMu.Unlock() + + s.calls.Add(1) + go func() { + defer s.calls.Done() + defer func() { + s.inflightMu.Lock() + delete(s.inflight, key) + s.inflightMu.Unlock() + cancel() + }() + + var progress io.Writer = io.Discard + if len(params.Meta.ProgressToken) > 0 && string(params.Meta.ProgressToken) != "null" { + progress = &mcpProgressWriter{server: s, token: params.Meta.ProgressToken} + } + + result := handler(callCtx, s, params.Arguments, progress) + if callCtx.Err() != nil { + // Cancelled by the client (or the server is shutting down): per the spec the + // receiver should not answer a cancelled request. + return + } + s.respond(req.ID, result) + }() +} + +// githubClient returns the shared GitHub client, creating it on first use so a missing +// token is reported as a tool error instead of preventing the server from starting. +func (s *mcpServer) githubClient() (mcpClient, error) { + s.clientMu.Lock() + defer s.clientMu.Unlock() + if s.client != nil { + return s.client, nil + } + c, err := s.clientFactory() + if err != nil { + return nil, err + } + s.client = c + return c, nil +} + +func (s *mcpServer) respond(id json.RawMessage, result interface{}) { + s.write(rpcResponse{JSONRPC: "2.0", ID: id, Result: result}) +} + +func (s *mcpServer) respondError(id json.RawMessage, code int, message string) { + s.write(rpcResponse{JSONRPC: "2.0", ID: id, Error: &rpcError{Code: code, Message: message}}) +} + +func (s *mcpServer) write(msg interface{}) { + data, err := json.Marshal(msg) + if err != nil { + fmt.Fprintf(s.logOut, "prw mcp: failed to encode message: %v\n", err) + return + } + s.writeMu.Lock() + defer s.writeMu.Unlock() + if _, err := s.out.Write(append(data, '\n')); err != nil { + fmt.Fprintf(s.logOut, "prw mcp: failed to write message: %v\n", err) + } +} + +// mcpProgressWriter turns each line waitForCI prints into a notifications/progress +// message for the tool call that asked for progress. +type mcpProgressWriter struct { + server *mcpServer + token json.RawMessage + buf []byte + count int +} + +func (w *mcpProgressWriter) Write(p []byte) (int, error) { + w.buf = append(w.buf, p...) + for { + i := bytes.IndexByte(w.buf, '\n') + if i < 0 { + return len(p), nil + } + line := strings.TrimSpace(string(w.buf[:i])) + w.buf = w.buf[i+1:] + if line == "" { + continue + } + w.count++ + w.server.write(rpcNotification{ + JSONRPC: "2.0", + Method: "notifications/progress", + Params: map[string]interface{}{ + "progressToken": w.token, + "progress": w.count, + "message": line, + }, + }) + } +} + +// --- tools --- + +type mcpToolHandler func(ctx context.Context, s *mcpServer, args json.RawMessage, progress io.Writer) mcpToolResult + +type mcpToolResult struct { + Content []mcpContent `json:"content"` + StructuredContent interface{} `json:"structuredContent,omitempty"` + IsError bool `json:"isError,omitempty"` +} + +type mcpContent struct { + Type string `json:"type"` + Text string `json:"text"` +} + +func toolError(format string, args ...interface{}) mcpToolResult { + return mcpToolResult{Content: []mcpContent{{Type: "text", Text: fmt.Sprintf(format, args...)}}, IsError: true} +} + +// toolOK returns a short human summary followed by the JSON data, plus the data as +// structuredContent for clients that support it. +func toolOK(summary string, data interface{}) mcpToolResult { + encoded, err := json.MarshalIndent(data, "", " ") + if err != nil { + return toolError("failed to encode result: %v", err) + } + return mcpToolResult{ + Content: []mcpContent{{Type: "text", Text: summary + "\n\n" + string(encoded)}}, + StructuredContent: data, + } +} + +var prURLSchema = map[string]interface{}{ + "type": "string", + "description": "GitHub pull request URL, e.g. https://github.com/owner/repo/pull/123", +} + +var mcpTools = []map[string]interface{}{ + { + "name": "get_ci_status", + "title": "Get PR CI status", + "description": "Get the current CI status of a GitHub pull request: overall state (success, failure, error, pending, merged, closed), check counts, and failed checks with links. Aggregates GitHub Actions check runs and legacy commit statuses. Returns immediately.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{"pr_url": prURLSchema}, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, + }, + { + "name": "wait_for_ci", + "title": "Wait for PR CI", + "description": "Block until a GitHub pull request's CI finishes, then return the result. Fails fast: returns as soon as any check fails, with the failed checks and links. Follows new pushes to the PR. If the timeout is reached first, returns state 'pending' with timed_out=true (call again to keep waiting). A merge while waiting returns state 'merged'.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{ + "pr_url": prURLSchema, + "timeout_seconds": map[string]interface{}{ + "type": "integer", + "description": "Give up after this many seconds (default 600, max 3600).", + "minimum": 1, + "maximum": int(mcpMaxWaitTimeout / time.Second), + }, + "interval_seconds": map[string]interface{}{ + "type": "integer", + "description": "Seconds between polls (default 15, min 5).", + "minimum": int(mcpMinWaitInterval / time.Second), + }, + }, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": true}, + }, + { + "name": "list_watched", + "title": "List watched PRs", + "description": "List the pull requests on prw's local watch list with their last known CI state. Reads local state only; use get_ci_status for fresh data.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": true, "openWorldHint": false}, + }, + { + "name": "watch_pr", + "title": "Watch a PR", + "description": "Add a GitHub pull request to prw's watch list so a running 'prw run' sends desktop/Slack/Discord notifications when its CI status changes. Idempotent.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{"pr_url": prURLSchema}, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": false, "destructiveHint": false, "idempotentHint": true, "openWorldHint": true}, + }, + { + "name": "unwatch_pr", + "title": "Stop watching a PR", + "description": "Remove a GitHub pull request from prw's watch list. Idempotent.", + "inputSchema": map[string]interface{}{ + "type": "object", + "properties": map[string]interface{}{"pr_url": prURLSchema}, + "required": []string{"pr_url"}, + "additionalProperties": false, + }, + "annotations": map[string]interface{}{"readOnlyHint": false, "destructiveHint": true, "idempotentHint": true, "openWorldHint": false}, + }, +} + +var mcpToolHandlers map[string]mcpToolHandler + +func init() { + // Assigned in init to avoid an initialization cycle (handlers reference mcpServer). + mcpToolHandlers = map[string]mcpToolHandler{ + "get_ci_status": toolGetCIStatus, + "wait_for_ci": toolWaitForCI, + "list_watched": toolListWatched, + "watch_pr": toolWatchPR, + "unwatch_pr": toolUnwatchPR, + } +} + +type prURLArgs struct { + PRURL string `json:"pr_url"` +} + +func parsePRArg(args json.RawMessage) (owner, repo string, number int, err error) { + var a prURLArgs + if len(args) > 0 { + if err := json.Unmarshal(args, &a); err != nil { + return "", "", 0, fmt.Errorf("invalid arguments: %v", err) + } + } + if strings.TrimSpace(a.PRURL) == "" { + return "", "", 0, errors.New("pr_url is required") + } + owner, repo, number, err = github.ParsePRURL(a.PRURL) + if err != nil { + return "", "", 0, fmt.Errorf("invalid pr_url %q: %v", a.PRURL, err) + } + return owner, repo, number, nil +} + +func ciSummary(r *waitResult) string { + pr := fmt.Sprintf("%s/%s#%d", r.Owner, r.Repo, r.Number) + switch { + case r.TimedOut: + return fmt.Sprintf("Timed out after %s: CI of %s is still pending (%d of %d check(s) running). Call wait_for_ci again to keep waiting.", + formatElapsed(r.ElapsedSeconds), pr, r.PendingChecks, r.TotalChecks) + case r.State == "merged" || r.State == "closed": + return fmt.Sprintf("%s was %s.", pr, r.State) + case r.State == "pending": + return fmt.Sprintf("CI of %s is pending: %s.", pr, pendingSummary(r)) + case r.State == "success": + return fmt.Sprintf("CI of %s passed (%d check(s)).", pr, r.TotalChecks) + } + names := make([]string, len(r.FailedChecks)) + for i, c := range r.FailedChecks { + names[i] = c.Name + } + summary := fmt.Sprintf("CI of %s finished with %s.", pr, r.State) + if len(names) > 0 { + summary += " Failed: " + strings.Join(names, ", ") + "." + } + return summary +} + +func toolGetCIStatus(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + result := &waitResult{Owner: owner, Repo: repo, Number: number, URL: github.FormatPRURL(owner, repo, number)} + if _, err := pollOnce(client, result); err != nil { + return toolError("failed to get CI status of %s/%s#%d: %v", owner, repo, number, err) + } + return toolOK(ciSummary(result), result) +} + +func toolWaitForCI(ctx context.Context, s *mcpServer, args json.RawMessage, progress io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + var opts struct { + TimeoutSeconds int `json:"timeout_seconds"` + IntervalSeconds int `json:"interval_seconds"` + } + if err := json.Unmarshal(args, &opts); err != nil { + return toolError("invalid arguments: %v", err) + } + timeout := mcpDefaultWaitTimeout + if opts.TimeoutSeconds != 0 { + if opts.TimeoutSeconds < 0 { + return toolError("timeout_seconds must be positive") + } + timeout = time.Duration(opts.TimeoutSeconds) * time.Second + } + if timeout > mcpMaxWaitTimeout { + timeout = mcpMaxWaitTimeout + } + interval := mcpDefaultWaitInterval + if opts.IntervalSeconds != 0 { + interval = time.Duration(opts.IntervalSeconds) * time.Second + } + if interval < mcpMinWaitInterval { + interval = mcpMinWaitInterval + } + + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + + waitCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + result, err := waitForCI(waitCtx, client, owner, repo, number, interval, progress) + if err != nil { + return toolError("failed waiting for CI of %s/%s#%d: %v", owner, repo, number, err) + } + return toolOK(ciSummary(result), result) +} + +type watchedPROutput struct { + URL string `json:"url"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + Title string `json:"title,omitempty"` + LastState string `json:"last_known_state"` + LastChecked *time.Time `json:"last_checked,omitempty"` +} + +func toolListWatched(ctx context.Context, s *mcpServer, _ json.RawMessage, _ io.Writer) mcpToolResult { + cfg, err := config.Load() + if err != nil { + return toolError("failed to load config: %v", err) + } + prs := make([]watchedPROutput, 0, len(cfg.WatchedPRs)) + for _, pr := range cfg.WatchedPRs { + state := github.NormalizeState(pr.LastKnownState) + if state == "" { + state = "unknown" + } + out := watchedPROutput{ + URL: github.FormatPRURL(pr.Owner, pr.Repo, pr.Number), Owner: pr.Owner, Repo: pr.Repo, + Number: pr.Number, Title: pr.Title, LastState: state, + } + if !pr.LastChecked.IsZero() { + t := pr.LastChecked + out.LastChecked = &t + } + prs = append(prs, out) + } + summary := fmt.Sprintf("%d PR(s) on the watch list.", len(prs)) + if len(prs) == 0 { + summary = "No PRs on the watch list. Add one with watch_pr." + } + return toolOK(summary, map[string]interface{}{"watched_prs": prs}) +} + +func toolWatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + client, err := s.githubClient() + if err != nil { + return toolError("%v", err) + } + pr, err := client.GetPullRequest(owner, repo, number) + if err != nil { + return toolError("failed to fetch %s/%s#%d: %v", owner, repo, number, err) + } + + added := false + if err := config.Update(func(c *config.Config) error { + added = c.AddPR(config.WatchedPR{Owner: owner, Repo: repo, Number: number, Title: pr.Title}) + if !added { + return config.ErrNoChanges + } + return nil + }); err != nil { + return toolError("failed to save config: %v", err) + } + + summary := fmt.Sprintf("Now watching %s/%s#%d - %s. A running 'prw run' picks it up on its next poll.", owner, repo, number, pr.Title) + if !added { + summary = fmt.Sprintf("%s/%s#%d was already being watched.", owner, repo, number) + } + return toolOK(summary, map[string]interface{}{"url": github.FormatPRURL(owner, repo, number), "title": pr.Title, "added": added}) +} + +func toolUnwatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { + owner, repo, number, err := parsePRArg(args) + if err != nil { + return toolError("%v", err) + } + removed := false + if err := config.Update(func(c *config.Config) error { + removed = c.RemovePR(owner, repo, number) + if !removed { + return config.ErrNoChanges + } + return nil + }); err != nil { + return toolError("failed to update config: %v", err) + } + + summary := fmt.Sprintf("Stopped watching %s/%s#%d.", owner, repo, number) + if !removed { + summary = fmt.Sprintf("%s/%s#%d was not being watched.", owner, repo, number) + } + return toolOK(summary, map[string]interface{}{"url": github.FormatPRURL(owner, repo, number), "removed": removed}) +} diff --git a/cmd/prw/mcp_test.go b/cmd/prw/mcp_test.go new file mode 100644 index 0000000..fffea80 --- /dev/null +++ b/cmd/prw/mcp_test.go @@ -0,0 +1,518 @@ +package main + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "io" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// mcpHarness runs an mcpServer over in-memory pipes and lets a test exchange messages. +type mcpHarness struct { + t *testing.T + in *io.PipeWriter + out *bufio.Scanner + done chan error + cancel context.CancelFunc + pending []map[string]interface{} // messages read while waiting for something else + mu sync.Mutex +} + +func startMCP(t *testing.T, factory func() (mcpClient, error)) *mcpHarness { + t.Helper() + path := filepath.Join(t.TempDir(), ".prw", "config.json") + oldPath := config.ConfigPath + config.ConfigPath = func() (string, error) { return path, nil } + t.Cleanup(func() { config.ConfigPath = oldPath }) + + inR, inW := io.Pipe() + outR, outW := io.Pipe() + ctx, cancel := context.WithCancel(context.Background()) + h := &mcpHarness{t: t, in: inW, out: bufio.NewScanner(outR), done: make(chan error, 1), cancel: cancel} + h.out.Buffer(make([]byte, 0, 64*1024), 10*1024*1024) + + srv := newMCPServer(inR, outW, io.Discard, factory) + go func() { + h.done <- srv.Serve(ctx) + outW.Close() + }() + t.Cleanup(func() { + cancel() + inW.Close() + select { + case <-h.done: + case <-time.After(5 * time.Second): + t.Error("server did not stop") + } + }) + return h +} + +func (h *mcpHarness) send(msg string) { + h.t.Helper() + if _, err := io.WriteString(h.in, msg+"\n"); err != nil { + h.t.Fatalf("write: %v", err) + } +} + +func (h *mcpHarness) readMessage() map[string]interface{} { + h.t.Helper() + type res struct { + m map[string]interface{} + err error + } + ch := make(chan res, 1) + go func() { + if !h.out.Scan() { + ch <- res{err: fmt.Errorf("stream closed: %v", h.out.Err())} + return + } + var m map[string]interface{} + err := json.Unmarshal(h.out.Bytes(), &m) + ch <- res{m, err} + }() + select { + case r := <-ch: + if r.err != nil { + h.t.Fatalf("read: %v", r.err) + } + return r.m + case <-time.After(5 * time.Second): + h.t.Fatal("timed out waiting for a message") + } + return nil +} + +// response reads messages until the response with the given id, keeping others. +func (h *mcpHarness) response(id int) map[string]interface{} { + h.t.Helper() + for i, m := range h.pending { + if v, ok := m["id"].(float64); ok && int(v) == id { + h.pending = append(h.pending[:i], h.pending[i+1:]...) + return m + } + } + for { + m := h.readMessage() + if v, ok := m["id"].(float64); ok && int(v) == id { + return m + } + h.pending = append(h.pending, m) + } +} + +func (h *mcpHarness) call(id int, method string, params interface{}) map[string]interface{} { + h.t.Helper() + p, _ := json.Marshal(params) + h.send(fmt.Sprintf(`{"jsonrpc":"2.0","id":%d,"method":%q,"params":%s}`, id, method, p)) + return h.response(id) +} + +func (h *mcpHarness) initialize() { + h.t.Helper() + resp := h.call(1, "initialize", map[string]interface{}{ + "protocolVersion": "2025-06-18", + "capabilities": map[string]interface{}{}, + "clientInfo": map[string]string{"name": "test", "version": "1"}, + }) + if resp["error"] != nil { + h.t.Fatalf("initialize failed: %v", resp["error"]) + } + h.send(`{"jsonrpc":"2.0","method":"notifications/initialized"}`) +} + +// toolResult extracts the tool result and its text from a tools/call response. +func toolResultOf(t *testing.T, resp map[string]interface{}) (result map[string]interface{}, text string) { + t.Helper() + if resp["error"] != nil { + t.Fatalf("unexpected JSON-RPC error: %v", resp["error"]) + } + result = resp["result"].(map[string]interface{}) + content := result["content"].([]interface{}) + if len(content) != 1 { + t.Fatalf("expected 1 content item, got %v", content) + } + item := content[0].(map[string]interface{}) + if item["type"] != "text" { + t.Fatalf("content type = %v", item["type"]) + } + return result, item["text"].(string) +} + +// setSleep replaces sleepFunc until the end of the test. Call it before startMCP: +// cleanups run in reverse order, so the original is restored only after the server +// (and any tool call still reading sleepFunc) has stopped. +func setSleep(t *testing.T, fn func(context.Context, time.Duration)) { + t.Helper() + old := sleepFunc + sleepFunc = fn + t.Cleanup(func() { sleepFunc = old }) +} + +func staticClient(c mcpClient) func() (mcpClient, error) { + return func() (mcpClient, error) { return c, nil } +} + +func successClient() *scriptedClient { + return &scriptedClient{steps: []pollStep{{ + pr: openPR("abc1234"), + runs: []github.CheckRun{run("Test", "completed", "success"), run("Build", "completed", "success")}, + }}} +} + +func TestMCP_Initialize(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + + resp := h.call(1, "initialize", map[string]interface{}{"protocolVersion": "2025-03-26", "capabilities": map[string]interface{}{}}) + result := resp["result"].(map[string]interface{}) + if result["protocolVersion"] != "2025-03-26" { + t.Errorf("must echo a supported requested version, got %v", result["protocolVersion"]) + } + if _, ok := result["capabilities"].(map[string]interface{})["tools"]; !ok { + t.Errorf("tools capability missing: %v", result["capabilities"]) + } + if info := result["serverInfo"].(map[string]interface{}); info["name"] != "prw" { + t.Errorf("serverInfo = %v", info) + } + if result["instructions"] == "" { + t.Error("instructions should be set") + } + + unknown := h.call(2, "initialize", map[string]interface{}{"protocolVersion": "1999-01-01"}) + if v := unknown["result"].(map[string]interface{})["protocolVersion"]; v != mcpProtocolVersions[0] { + t.Errorf("unsupported version should get the latest supported, got %v", v) + } +} + +func TestMCP_ToolsList(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + h.initialize() + + resp := h.call(2, "tools/list", map[string]interface{}{}) + tools := resp["result"].(map[string]interface{})["tools"].([]interface{}) + names := map[string]bool{} + for _, raw := range tools { + tool := raw.(map[string]interface{}) + name := tool["name"].(string) + names[name] = true + if tool["description"] == "" { + t.Errorf("%s has no description", name) + } + schema := tool["inputSchema"].(map[string]interface{}) + if schema["type"] != "object" { + t.Errorf("%s inputSchema type = %v", name, schema["type"]) + } + if _, ok := mcpToolHandlers[name]; !ok { + t.Errorf("%s is listed but has no handler", name) + } + } + for _, want := range []string{"get_ci_status", "wait_for_ci", "list_watched", "watch_pr", "unwatch_pr"} { + if !names[want] { + t.Errorf("tool %s missing from tools/list", want) + } + } + if len(names) != len(mcpToolHandlers) { + t.Errorf("%d tools listed, %d handlers", len(names), len(mcpToolHandlers)) + } +} + +func TestMCP_PingAndUnknownMethod(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + + if resp := h.call(1, "ping", nil); resp["error"] != nil || resp["result"] == nil { + t.Errorf("ping response = %v", resp) + } + resp := h.call(2, "resources/list", map[string]interface{}{}) + if code := resp["error"].(map[string]interface{})["code"]; code != float64(rpcMethodNotFound) { + t.Errorf("error code = %v, want %d", code, rpcMethodNotFound) + } +} + +func TestMCP_MalformedInput(t *testing.T) { + h := startMCP(t, staticClient(successClient())) + + h.send(`{not json`) + if code := h.readMessage()["error"].(map[string]interface{})["code"]; code != float64(rpcParseError) { + t.Errorf("parse error code = %v", code) + } + h.send(`[{"jsonrpc":"2.0","id":1,"method":"ping"}]`) + if code := h.readMessage()["error"].(map[string]interface{})["code"]; code != float64(rpcInvalidRequest) { + t.Errorf("batch error code = %v", code) + } + h.send(``) // blank lines are ignored + if resp := h.call(3, "tools/call", map[string]interface{}{"name": "no_such_tool"}); resp["error"].(map[string]interface{})["code"] != float64(rpcInvalidParams) { + t.Errorf("unknown tool: %v", resp) + } + if resp := h.call(4, "tools/call", map[string]interface{}{}); resp["error"].(map[string]interface{})["code"] != float64(rpcInvalidParams) { + t.Errorf("missing name: %v", resp) + } +} + +func TestMCP_GetCIStatus(t *testing.T) { + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{{ + pr: openPR("abc1234"), + runs: []github.CheckRun{run("Lint", "completed", "failure"), run("Test", "in_progress", "")}, + }}})) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "get_ci_status", "arguments": map[string]string{"pr_url": "https://github.com/owner/repo/pull/1"}, + })) + if result["isError"] == true { + t.Fatalf("unexpected tool error: %s", text) + } + if !strings.HasPrefix(text, "CI of owner/repo#1 finished with failure. Failed: Lint.") { + t.Errorf("summary = %q", strings.SplitN(text, "\n", 2)[0]) + } + structured := result["structuredContent"].(map[string]interface{}) + if structured["state"] != "failure" || structured["total_checks"] != float64(2) || structured["pending_checks"] != float64(1) { + t.Errorf("structuredContent = %v", structured) + } + failed := structured["failed_checks"].([]interface{}) + if len(failed) != 1 || failed[0].(map[string]interface{})["url"] != "https://github.com/o/r/runs/Lint" { + t.Errorf("failed_checks = %v", failed) + } +} + +func TestMCP_ToolErrorsAreReportedAsIsError(t *testing.T) { + tests := []struct { + name string + factory func() (mcpClient, error) + tool string + args interface{} + want string + }{ + {"missing pr_url", staticClient(successClient()), "get_ci_status", map[string]string{}, "pr_url is required"}, + {"invalid pr_url", staticClient(successClient()), "wait_for_ci", map[string]string{"pr_url": "nope"}, "invalid pr_url"}, + {"wrong argument type", staticClient(successClient()), "wait_for_ci", map[string]interface{}{"pr_url": "https://github.com/o/r/pull/1", "timeout_seconds": "soon"}, "invalid arguments"}, + {"missing token", func() (mcpClient, error) { return nil, errMissingToken }, "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "gh auth login"}, + {"API error", staticClient(&scriptedClient{steps: []pollStep{{prErr: &github.APIError{StatusCode: 404, Body: "Not Found"}}}}), "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "404"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := startMCP(t, tt.factory) + h.initialize() + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{"name": tt.tool, "arguments": tt.args})) + if result["isError"] != true { + t.Errorf("expected isError, got %v", result) + } + if !strings.Contains(text, tt.want) { + t.Errorf("text %q should contain %q", text, tt.want) + } + }) + } +} + +func TestMCP_WaitForCI_WithProgress(t *testing.T) { + noSleep(t) + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{ + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "in_progress", "")}}, + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }})) + h.initialize() + + resp := h.call(2, "tools/call", map[string]interface{}{ + "name": "wait_for_ci", + "arguments": map[string]interface{}{"pr_url": "https://github.com/owner/repo/pull/1", "timeout_seconds": 60}, + "_meta": map[string]interface{}{"progressToken": "tok-1"}, + }) + result, text := toolResultOf(t, resp) + if result["isError"] == true || result["structuredContent"].(map[string]interface{})["state"] != "success" { + t.Fatalf("unexpected result: %s", text) + } + + var progress []map[string]interface{} + for _, m := range h.pending { + if m["method"] == "notifications/progress" { + progress = append(progress, m["params"].(map[string]interface{})) + } + } + if len(progress) != 1 { + t.Fatalf("expected 1 progress notification, got %d: %v", len(progress), h.pending) + } + if progress[0]["progressToken"] != "tok-1" || progress[0]["progress"] != float64(1) || + !strings.Contains(progress[0]["message"].(string), "1 of 1 check(s) still running") { + t.Errorf("progress = %v", progress[0]) + } +} + +func TestMCP_WaitForCI_NoProgressWithoutToken(t *testing.T) { + noSleep(t) + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{ + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "in_progress", "")}}, + {pr: openPR("abc1234"), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }})) + h.initialize() + + h.call(2, "tools/call", map[string]interface{}{"name": "wait_for_ci", "arguments": map[string]string{"pr_url": "https://github.com/o/r/pull/1"}}) + if len(h.pending) != 0 { + t.Errorf("no notifications expected without a progressToken, got %v", h.pending) + } +} + +func TestMCP_WaitForCI_Timeout(t *testing.T) { + setSleep(t, func(ctx context.Context, d time.Duration) { <-ctx.Done() }) + + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}}})) + h.initialize() + + result, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{ + "name": "wait_for_ci", "arguments": map[string]interface{}{"pr_url": "https://github.com/o/r/pull/1", "timeout_seconds": 1}, + })) + if result["isError"] == true { + t.Fatalf("a timeout is a normal result, not a tool error: %s", text) + } + if s := result["structuredContent"].(map[string]interface{}); s["timed_out"] != true || s["state"] != "pending" { + t.Errorf("structuredContent = %v", s) + } + if !strings.Contains(text, "Call wait_for_ci again") { + t.Errorf("summary should tell the agent how to continue: %q", text) + } +} + +func TestMCP_PingWhileWaitingAndCancellation(t *testing.T) { + setSleep(t, func(ctx context.Context, d time.Duration) { <-ctx.Done() }) // wait until cancelled + + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}}})) + h.initialize() + + h.send(`{"jsonrpc":"2.0","id":7,"method":"tools/call","params":{"name":"wait_for_ci","arguments":{"pr_url":"https://github.com/o/r/pull/1"}}}`) + + // The long call must not block other requests. + if resp := h.call(8, "ping", nil); resp["result"] == nil { + t.Fatalf("ping during wait_for_ci failed: %v", resp) + } + + h.send(`{"jsonrpc":"2.0","method":"notifications/cancelled","params":{"requestId":7,"reason":"user aborted"}}`) + + // No response must be sent for the cancelled request; the next ping proves the + // server is alive and that nothing for id 7 arrived before it. + if resp := h.call(9, "ping", nil); resp["result"] == nil { + t.Fatalf("ping after cancel failed: %v", resp) + } + time.Sleep(100 * time.Millisecond) + h.send(`{"jsonrpc":"2.0","id":10,"method":"ping"}`) + h.response(10) + for _, m := range h.pending { + if m["id"] == float64(7) { + t.Errorf("a cancelled request must not get a response: %v", m) + } + } +} + +func TestMCP_FinishesInFlightCallsWhenStdinCloses(t *testing.T) { + release := make(chan struct{}) + setSleep(t, func(ctx context.Context, d time.Duration) { <-release }) + + h := startMCP(t, staticClient(&scriptedClient{steps: []pollStep{ + {pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}, + {pr: openPR("sha"), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }})) + h.initialize() + + h.send(`{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"wait_for_ci","arguments":{"pr_url":"https://github.com/o/r/pull/1"}}}`) + time.Sleep(50 * time.Millisecond) + h.in.Close() + close(release) + + result, _ := toolResultOf(t, h.response(5)) + if result["structuredContent"].(map[string]interface{})["state"] != "success" { + t.Errorf("in-flight call should complete after stdin closes: %v", result) + } + select { + case err := <-h.done: + if err != nil { + t.Errorf("Serve returned %v", err) + } + h.done <- nil // let the cleanup see it + case <-time.After(5 * time.Second): + t.Fatal("server did not exit after stdin closed") + } +} + +func TestMCP_WatchListTools(t *testing.T) { + pr := openPR("sha") + pr.Number, pr.Title = 42, "Ship it" + client := &scriptedClient{steps: []pollStep{{pr: pr}}} + h := startMCP(t, staticClient(client)) + h.initialize() + url := map[string]string{"pr_url": "https://github.com/owner/repo/pull/42"} + + _, text := toolResultOf(t, h.call(2, "tools/call", map[string]interface{}{"name": "list_watched", "arguments": map[string]string{}})) + if !strings.HasPrefix(text, "No PRs on the watch list") { + t.Errorf("empty list summary = %q", text) + } + + result, text := toolResultOf(t, h.call(3, "tools/call", map[string]interface{}{"name": "watch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["added"] != true || !strings.Contains(text, "Ship it") { + t.Errorf("watch_pr: %s", text) + } + result, _ = toolResultOf(t, h.call(4, "tools/call", map[string]interface{}{"name": "watch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["added"] != false { + t.Error("watch_pr must be idempotent") + } + + result, _ = toolResultOf(t, h.call(5, "tools/call", map[string]interface{}{"name": "list_watched"})) + prs := result["structuredContent"].(map[string]interface{})["watched_prs"].([]interface{}) + if len(prs) != 1 { + t.Fatalf("watched_prs = %v", prs) + } + if p := prs[0].(map[string]interface{}); p["url"] != "https://github.com/owner/repo/pull/42" || p["title"] != "Ship it" || p["last_known_state"] != "unknown" { + t.Errorf("watched PR = %v", p) + } + + loaded, _ := config.Load() + if len(loaded.WatchedPRs) != 1 || loaded.WatchedPRs[0].Number != 42 { + t.Errorf("watch_pr must persist to the config file: %+v", loaded.WatchedPRs) + } + + result, _ = toolResultOf(t, h.call(6, "tools/call", map[string]interface{}{"name": "unwatch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["removed"] != true { + t.Error("unwatch_pr should remove the PR") + } + result, _ = toolResultOf(t, h.call(7, "tools/call", map[string]interface{}{"name": "unwatch_pr", "arguments": url})) + if result["structuredContent"].(map[string]interface{})["removed"] != false { + t.Error("unwatch_pr must be idempotent") + } +} + +func TestMCP_SharesOneGitHubClient(t *testing.T) { + created := 0 + h := startMCP(t, func() (mcpClient, error) { + created++ + return successClient(), nil + }) + h.initialize() + for id := 2; id <= 4; id++ { + h.call(id, "tools/call", map[string]interface{}{"name": "get_ci_status", "arguments": map[string]string{"pr_url": "https://github.com/o/r/pull/1"}}) + } + if created != 1 { + t.Errorf("client created %d times; it must be shared so the ETag cache is reused", created) + } +} + +func TestCISummary(t *testing.T) { + tests := []struct { + r waitResult + want string + }{ + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "success", TotalChecks: 3}, "CI of o/r#1 passed (3 check(s))."}, + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "error"}, "CI of o/r#1 finished with error."}, + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "merged"}, "o/r#1 was merged."}, + {waitResult{Owner: "o", Repo: "r", Number: 1, State: "pending", SHA: "abcdef123", TotalChecks: 2, PendingChecks: 1}, "CI of o/r#1 is pending: 1 of 2 check(s) still running on abcdef1."}, + } + for _, tt := range tests { + if got := ciSummary(&tt.r); got != tt.want { + t.Errorf("ciSummary(%s) = %q, want %q", tt.r.State, got, tt.want) + } + } +} diff --git a/internal/config/autowatch_test.go b/internal/config/autowatch_test.go new file mode 100644 index 0000000..5c75e96 --- /dev/null +++ b/internal/config/autowatch_test.go @@ -0,0 +1,85 @@ +package config + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestEnableAndDisableAutoWatch(t *testing.T) { + c := &Config{} + if !c.EnableAutoWatch(AutoWatchMine) || c.EnableAutoWatch(AutoWatchMine) { + t.Fatal("EnableAutoWatch must report the first enable only") + } + c.EnableAutoWatch(AutoWatchReviewRequested) + if !c.HasAutoWatch(AutoWatchMine) || !c.HasAutoWatch(AutoWatchReviewRequested) { + t.Fatalf("AutoWatch = %v", c.AutoWatch) + } + + c.WatchedPRs = []WatchedPR{ + {Owner: "o", Repo: "r", Number: 1, Source: AutoWatchMine}, + {Owner: "o", Repo: "r", Number: 2}, + {Owner: "o", Repo: "r", Number: 3, Source: AutoWatchReviewRequested}, + {Owner: "o", Repo: "r", Number: 4, Source: AutoWatchMine}, + } + wasOn, removed := c.DisableAutoWatch(AutoWatchMine) + if !wasOn || len(removed) != 2 || removed[0].Number != 1 || removed[1].Number != 4 { + t.Errorf("DisableAutoWatch(mine) = %v, %+v", wasOn, removed) + } + if len(c.WatchedPRs) != 2 || c.WatchedPRs[0].Number != 2 || c.WatchedPRs[1].Number != 3 { + t.Errorf("manual and other-source PRs must be kept: %+v", c.WatchedPRs) + } + if c.HasAutoWatch(AutoWatchMine) || !c.HasAutoWatch(AutoWatchReviewRequested) { + t.Errorf("AutoWatch = %v", c.AutoWatch) + } + + if wasOn, removed := c.DisableAutoWatch(AutoWatchMine); wasOn || len(removed) != 0 { + t.Error("disabling twice must be a no-op") + } + c.DisableAutoWatch(AutoWatchReviewRequested) + if c.AutoWatch != nil { + t.Errorf("AutoWatch should be nil when empty so it's omitted from JSON, got %#v", c.AutoWatch) + } +} + +func TestMarkManual(t *testing.T) { + c := &Config{WatchedPRs: []WatchedPR{{Owner: "o", Repo: "r", Number: 1, Source: AutoWatchMine}, {Owner: "o", Repo: "r", Number: 2}}} + if !c.MarkManual("O", "R", 1) || c.WatchedPRs[0].Source != "" { + t.Errorf("MarkManual should clear the source: %+v", c.WatchedPRs[0]) + } + if c.MarkManual("o", "r", 1) || c.MarkManual("o", "r", 2) || c.MarkManual("o", "r", 99) { + t.Error("MarkManual must report false when nothing changes") + } +} + +func TestPRMatchingIsCaseInsensitive(t *testing.T) { + c := &Config{} + c.AddPR(WatchedPR{Owner: "DevBlac", Repo: "PRW", Number: 5}) + if c.AddPR(WatchedPR{Owner: "devblac", Repo: "prw", Number: 5}) { + t.Error("owner/repo differing only in case must not create a duplicate") + } + if !c.ApplyPRState(WatchedPR{Owner: "devblac", Repo: "prw", Number: 5, LastKnownState: "success"}) || c.WatchedPRs[0].LastKnownState != "success" { + t.Error("ApplyPRState must match case-insensitively") + } + if !c.RemovePR("DEVBLAC", "prw", 5) || len(c.WatchedPRs) != 0 { + t.Error("RemovePR must match case-insensitively") + } +} + +func TestAutoWatchFieldsAreOmittedWhenUnused(t *testing.T) { + data, err := json.Marshal(&Config{WatchedPRs: []WatchedPR{{Owner: "o", Repo: "r", Number: 1}}}) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "auto_watch") || strings.Contains(string(data), "source") { + t.Errorf("existing configs must not gain new keys: %s", data) + } +} + +func TestApplyPRStateKeepsSource(t *testing.T) { + c := &Config{WatchedPRs: []WatchedPR{{Owner: "o", Repo: "r", Number: 1, Source: AutoWatchMine}}} + c.ApplyPRState(WatchedPR{Owner: "o", Repo: "r", Number: 1, LastKnownState: "failure"}) + if c.WatchedPRs[0].Source != AutoWatchMine { + t.Error("saving polling state must not drop the auto-watch source") + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 4a76593..fb78d24 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,10 +28,70 @@ type Config struct { NotificationFilter string `json:"notification_filter,omitempty"` NotificationNative bool `json:"notification_native,omitempty"` + // AutoWatch lists the searches (AutoWatchMine, AutoWatchReviewRequested) whose + // matching PRs are added to the watch list automatically. + AutoWatch []string `json:"auto_watch,omitempty"` + // Watched PRs WatchedPRs []WatchedPR `json:"watched_prs"` } +// Auto-watch sources. +const ( + AutoWatchMine = "mine" + AutoWatchReviewRequested = "review-requested" +) + +// AutoWatchSources are the valid auto-watch sources, in display order. +var AutoWatchSources = []string{AutoWatchMine, AutoWatchReviewRequested} + +// HasAutoWatch reports whether source is enabled. +func (c *Config) HasAutoWatch(source string) bool { + for _, s := range c.AutoWatch { + if s == source { + return true + } + } + return false +} + +// EnableAutoWatch turns source on and reports whether it was off. +func (c *Config) EnableAutoWatch(source string) bool { + if c.HasAutoWatch(source) { + return false + } + c.AutoWatch = append(c.AutoWatch, source) + return true +} + +// DisableAutoWatch turns source off and removes the PRs it added (PRs watched by hand +// are kept). It returns whether the source was on and the PRs removed. +func (c *Config) DisableAutoWatch(source string) (wasOn bool, removed []WatchedPR) { + kept := c.AutoWatch[:0] + for _, s := range c.AutoWatch { + if s == source { + wasOn = true + continue + } + kept = append(kept, s) + } + c.AutoWatch = kept + if len(c.AutoWatch) == 0 { + c.AutoWatch = nil + } + + prs := c.WatchedPRs[:0] + for _, pr := range c.WatchedPRs { + if pr.Source == source { + removed = append(removed, pr) + continue + } + prs = append(prs, pr) + } + c.WatchedPRs = prs + return wasOn, removed +} + // WatchedPR represents a pull request being watched. type WatchedPR struct { Owner string `json:"owner"` @@ -41,6 +101,9 @@ type WatchedPR struct { LastKnownState string `json:"last_known_state,omitempty"` LastChecked time.Time `json:"last_checked,omitempty"` Title string `json:"title,omitempty"` + // Source is the auto-watch source that added this PR, or empty if it was added by + // hand with `prw watch `. + Source string `json:"source,omitempty"` } // DefaultConfig returns a config with sensible defaults. @@ -213,7 +276,7 @@ func acquireLock(configPath string) (release func(), err error) { func (c *Config) ApplyPRState(pr WatchedPR) bool { for i := range c.WatchedPRs { w := &c.WatchedPRs[i] - if w.Owner == pr.Owner && w.Repo == pr.Repo && w.Number == pr.Number { + if samePR(*w, pr.Owner, pr.Repo, pr.Number) { w.LastKnownSHA = pr.LastKnownSHA w.LastKnownState = pr.LastKnownState w.LastChecked = pr.LastChecked @@ -262,10 +325,28 @@ func RemovePRs(prs []WatchedPR) error { }) } +// samePR reports whether pr is owner/repo#number. GitHub owner and repository names +// are case-insensitive, so "DevBlac/PRW" and "devblac/prw" are the same repository. +func samePR(pr WatchedPR, owner, repo string, number int) bool { + return pr.Number == number && strings.EqualFold(pr.Owner, owner) && strings.EqualFold(pr.Repo, repo) +} + +// MarkManual clears the auto-watch Source of a watched PR, so that disabling that +// auto-watch source no longer removes it. It reports whether anything changed. +func (c *Config) MarkManual(owner, repo string, number int) bool { + for i := range c.WatchedPRs { + if samePR(c.WatchedPRs[i], owner, repo, number) && c.WatchedPRs[i].Source != "" { + c.WatchedPRs[i].Source = "" + return true + } + } + return false +} + // AddPR adds a PR to the watched list if not already present. func (c *Config) AddPR(pr WatchedPR) bool { for _, existing := range c.WatchedPRs { - if existing.Owner == pr.Owner && existing.Repo == pr.Repo && existing.Number == pr.Number { + if samePR(existing, pr.Owner, pr.Repo, pr.Number) { return false } } @@ -276,7 +357,7 @@ func (c *Config) AddPR(pr WatchedPR) bool { // RemovePR removes a PR from the watched list. func (c *Config) RemovePR(owner, repo string, number int) bool { for i, pr := range c.WatchedPRs { - if pr.Owner == owner && pr.Repo == repo && pr.Number == number { + if samePR(pr, owner, repo, number) { c.WatchedPRs = append(c.WatchedPRs[:i], c.WatchedPRs[i+1:]...) return true } @@ -287,7 +368,7 @@ func (c *Config) RemovePR(owner, repo string, number int) bool { // UpdatePR updates the state of a watched PR. func (c *Config) UpdatePR(owner, repo string, number int, sha, state string) { for i := range c.WatchedPRs { - if c.WatchedPRs[i].Owner == owner && c.WatchedPRs[i].Repo == repo && c.WatchedPRs[i].Number == number { + if samePR(c.WatchedPRs[i], owner, repo, number) { c.WatchedPRs[i].LastKnownSHA = sha c.WatchedPRs[i].LastKnownState = state c.WatchedPRs[i].LastChecked = time.Now() diff --git a/internal/github/client.go b/internal/github/client.go index 11012cd..2a771c3 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -232,6 +232,75 @@ func (c *Client) GetCheckRuns(owner, repo, ref string) ([]CheckRun, error) { return runs, nil } +// SearchResultPR is a pull request returned by SearchPullRequests. +type SearchResultPR struct { + Owner string + Repo string + Number int + Title string +} + +// SearchResult holds the pull requests matching a search query. +type SearchResult struct { + PRs []SearchResultPR + // Total is how many items GitHub says match; it can exceed len(PRs) when the + // results were capped. + Total int + // Incomplete is set when GitHub timed out before finding every match, or when the + // results were capped, so absence from PRs doesn't prove a PR stopped matching. + Incomplete bool +} + +const ( + searchPerPage = 100 + maxSearchPages = 3 +) + +type searchIssuesResponse struct { + TotalCount int `json:"total_count"` + IncompleteResults bool `json:"incomplete_results"` + Items []struct { + Number int `json:"number"` + Title string `json:"title"` + HTMLURL string `json:"html_url"` + PullRequest *struct{} `json:"pull_request"` + } `json:"items"` +} + +// SearchPullRequests runs a GitHub issue search (e.g. "is:pr is:open author:@me") and +// returns the pull requests it finds, up to 300. Qualifiers like @me need a token. +func (c *Client) SearchPullRequests(query string) (*SearchResult, error) { + result := &SearchResult{} + for page := 1; page <= maxSearchPages; page++ { + path := fmt.Sprintf("/search/issues?q=%s&per_page=%d&page=%d", url.QueryEscape(query), searchPerPage, page) + var resp searchIssuesResponse + if err := c.getJSON(path, &resp); err != nil { + return nil, err + } + result.Total = resp.TotalCount + if resp.IncompleteResults { + result.Incomplete = true + } + for _, item := range resp.Items { + if item.PullRequest == nil { + continue // an issue, not a PR + } + owner, repo, number, err := ParsePRURL(item.HTMLURL) + if err != nil { + continue + } + result.PRs = append(result.PRs, SearchResultPR{Owner: owner, Repo: repo, Number: number, Title: item.Title}) + } + if len(resp.Items) < searchPerPage || page*searchPerPage >= resp.TotalCount { + return result, nil + } + } + if result.Total > len(result.PRs) { + result.Incomplete = true + } + return result, nil +} + // FetchCIStatus fetches commit statuses and check runs for a commit and aggregates them. func FetchCIStatus(c CIFetcher, owner, repo, ref string) (*CIStatus, error) { status, err := c.GetCombinedStatus(owner, repo, ref) diff --git a/internal/github/search_test.go b/internal/github/search_test.go new file mode 100644 index 0000000..3fcc3aa --- /dev/null +++ b/internal/github/search_test.go @@ -0,0 +1,117 @@ +package github + +import ( + "fmt" + "net/http" + "strconv" + "strings" + "testing" +) + +func searchItem(owner, repo string, number int, isPR bool) string { + pr := "" + if isPR { + pr = `,"pull_request":{"url":"x"}` + } + return fmt.Sprintf(`{"number":%d,"title":"%s %d","html_url":"https://github.com/%s/%s/pull/%d"%s}`, number, repo, number, owner, repo, number, pr) +} + +func TestSearchPullRequests_ParsesResultsAndSkipsIssues(t *testing.T) { + var gotQuery string + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/search/issues" { + t.Errorf("path = %s", r.URL.Path) + } + gotQuery = r.URL.Query().Get("q") + fmt.Fprintf(w, `{"total_count":3,"incomplete_results":false,"items":[%s,%s,%s]}`, + searchItem("Alice", "api", 1, true), searchItem("bob", "web", 2, false), searchItem("bob", "web", 3, true)) + }) + + res, err := c.SearchPullRequests("is:pr is:open author:@me") + if err != nil { + t.Fatal(err) + } + if gotQuery != "is:pr is:open author:@me" { + t.Errorf("query sent = %q (must round-trip through URL encoding)", gotQuery) + } + if len(res.PRs) != 2 || res.Incomplete { + t.Fatalf("result = %+v", res) + } + if p := res.PRs[0]; p.Owner != "Alice" || p.Repo != "api" || p.Number != 1 || p.Title != "api 1" { + t.Errorf("first PR = %+v", p) + } + if p := res.PRs[1]; p.Number != 3 { + t.Errorf("an issue must be skipped, got %+v", p) + } +} + +func TestSearchPullRequests_Paginates(t *testing.T) { + var pages []string + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + page := r.URL.Query().Get("page") + pages = append(pages, page) + n, _ := strconv.Atoi(page) + count := searchPerPage + if n == 2 { + count = 5 + } + items := make([]string, count) + for i := range items { + items[i] = searchItem("o", "r", n*1000+i, true) + } + fmt.Fprintf(w, `{"total_count":%d,"items":[%s]}`, searchPerPage+5, strings.Join(items, ",")) + }) + + res, err := c.SearchPullRequests("q") + if err != nil { + t.Fatal(err) + } + if len(res.PRs) != searchPerPage+5 || len(pages) != 2 || res.Incomplete { + t.Errorf("got %d PRs over pages %v, incomplete=%v", len(res.PRs), pages, res.Incomplete) + } +} + +func TestSearchPullRequests_CapsAndMarksIncomplete(t *testing.T) { + requests := 0 + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + requests++ + items := make([]string, searchPerPage) + for i := range items { + items[i] = searchItem("o", "r", requests*1000+i, true) + } + fmt.Fprintf(w, `{"total_count":5000,"items":[%s]}`, strings.Join(items, ",")) + }) + + res, err := c.SearchPullRequests("q") + if err != nil { + t.Fatal(err) + } + if requests != maxSearchPages || len(res.PRs) != maxSearchPages*searchPerPage { + t.Errorf("requests=%d PRs=%d", requests, len(res.PRs)) + } + if !res.Incomplete || res.Total != 5000 { + t.Errorf("capped results must be marked incomplete: %+v", res.Total) + } +} + +func TestSearchPullRequests_IncompleteFromGitHub(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"total_count":1,"incomplete_results":true,"items":[%s]}`, searchItem("o", "r", 1, true)) + }) + res, err := c.SearchPullRequests("q") + if err != nil { + t.Fatal(err) + } + if !res.Incomplete { + t.Error("incomplete_results from GitHub must be propagated") + } +} + +func TestSearchPullRequests_Error(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"message":"Validation Failed"}`, http.StatusUnprocessableEntity) + }) + if _, err := c.SearchPullRequests("q"); err == nil || !strings.Contains(err.Error(), "422") { + t.Errorf("expected a 422 error, got %v", err) + } +} diff --git a/internal/watcher/autowatch.go b/internal/watcher/autowatch.go new file mode 100644 index 0000000..297fdd5 --- /dev/null +++ b/internal/watcher/autowatch.go @@ -0,0 +1,157 @@ +package watcher + +import ( + "fmt" + "strings" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// AutoWatchInterval is how often `prw run` refreshes the auto-watch searches. Search +// results change slowly and the search API has its own, lower rate limit (30/min), so +// this is much longer than the CI poll interval. +const AutoWatchInterval = 5 * time.Minute + +// autoWatchQueries maps each auto-watch source to its GitHub search query. +var autoWatchQueries = map[string]string{ + config.AutoWatchMine: "is:pr is:open archived:false author:@me", + config.AutoWatchReviewRequested: "is:pr is:open archived:false review-requested:@me", +} + +// AutoWatchDescription is a human-readable name for each source. +var AutoWatchDescription = map[string]string{ + config.AutoWatchMine: "your open PRs", + config.AutoWatchReviewRequested: "PRs where your review is requested", +} + +// Searcher runs GitHub pull request searches. +type Searcher interface { + SearchPullRequests(query string) (*github.SearchResult, error) +} + +// SyncResult reports what SyncAutoWatch changed. +type SyncResult struct { + Added []config.WatchedPR + Removed []config.WatchedPR + // Warnings are non-fatal problems, e.g. a search that failed or was truncated. + Warnings []string +} + +// SyncAutoWatch runs the search of every source in sources and updates the watch list: +// matching PRs that aren't watched yet are added (tagged with their source), and PRs +// added for "review-requested" that no longer match are removed, since the request +// was withdrawn or answered. PRs added for "mine" are not removed when they stop +// matching: that means they were merged or closed, and `prw run` should still notify +// about it before dropping them. PRs watched by hand are never touched. +// +// It returns an error only if every search failed; other problems are Warnings. +func SyncAutoWatch(client Searcher, sources []string) (*SyncResult, error) { + result := &SyncResult{} + found := map[string]*github.SearchResult{} + var lastErr error + + for _, source := range sources { + query, ok := autoWatchQueries[source] + if !ok { + result.Warnings = append(result.Warnings, fmt.Sprintf("unknown auto-watch source %q ignored", source)) + continue + } + res, err := client.SearchPullRequests(query) + if err != nil { + lastErr = err + result.Warnings = append(result.Warnings, fmt.Sprintf("search for %s failed: %v", AutoWatchDescription[source], err)) + continue + } + if res.Incomplete { + result.Warnings = append(result.Warnings, fmt.Sprintf("search for %s returned %d of %d results; PRs that no longer match are kept", + AutoWatchDescription[source], len(res.PRs), res.Total)) + } + found[source] = res + } + if len(found) == 0 && lastErr != nil { + return result, lastErr + } + + err := config.Update(func(c *config.Config) error { + changed := false + for _, source := range sources { + res, ok := found[source] + if !ok || !c.HasAutoWatch(source) { + continue // search failed, or the source was disabled meanwhile + } + + matches := make(map[string]bool, len(res.PRs)) + for _, pr := range res.PRs { + matches[prKey(pr.Owner, pr.Repo, pr.Number)] = true + added := config.WatchedPR{Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, Title: pr.Title, Source: source} + if c.AddPR(added) { + result.Added = append(result.Added, added) + changed = true + } + } + + if source != config.AutoWatchReviewRequested || res.Incomplete { + continue + } + kept := c.WatchedPRs[:0] + for _, pr := range c.WatchedPRs { + if pr.Source == source && !matches[prKey(pr.Owner, pr.Repo, pr.Number)] { + result.Removed = append(result.Removed, pr) + changed = true + continue + } + kept = append(kept, pr) + } + c.WatchedPRs = kept + } + if !changed { + return config.ErrNoChanges + } + return nil + }) + if err != nil { + return result, fmt.Errorf("failed to update watch list: %w", err) + } + return result, nil +} + +func prKey(owner, repo string, number int) string { + return strings.ToLower(owner) + "/" + strings.ToLower(repo) + "#" + fmt.Sprint(number) +} + +// syncAutoWatch refreshes the auto-watch searches during `prw run` and reports changes. +func (w *Watcher) syncAutoWatch() { + sources := w.config.AutoWatch + if fresh, err := config.Load(); err == nil { + sources = fresh.AutoWatch // `prw watch --mine` may have run since startup + } + if len(sources) == 0 { + return + } + w.lastAutoWatch = w.now() + + searcher, ok := w.client.(Searcher) + if !ok { + return + } + res, err := SyncAutoWatch(searcher, sources) + for _, warning := range res.Warnings { + fmt.Printf("Warning: auto-watch: %s\n", warning) + } + if err != nil { + // Per-search failures (including rate limits) are already in Warnings; this is + // either "every search failed" or a config write error. + if limited, _ := github.IsRateLimited(err); !limited { + fmt.Printf("Warning: auto-watch failed: %v\n", err) + } + return + } + for _, pr := range res.Added { + fmt.Printf("➕ Auto-watching %s/%s#%d - %s (%s)\n", pr.Owner, pr.Repo, pr.Number, pr.Title, AutoWatchDescription[pr.Source]) + } + for _, pr := range res.Removed { + fmt.Printf("➖ No longer watching %s/%s#%d - review no longer requested\n", pr.Owner, pr.Repo, pr.Number) + } +} diff --git a/internal/watcher/autowatch_test.go b/internal/watcher/autowatch_test.go new file mode 100644 index 0000000..08fbe63 --- /dev/null +++ b/internal/watcher/autowatch_test.go @@ -0,0 +1,303 @@ +package watcher + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + "sync" + "testing" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +// fakeSearcher answers SearchPullRequests by source (looked up from the query). +type fakeSearcher struct { + mockGitHubClient + results map[string]*github.SearchResult + errs map[string]error + searches int +} + +func (f *fakeSearcher) SearchPullRequests(query string) (*github.SearchResult, error) { + f.searches++ + for source, q := range autoWatchQueries { + if q == query { + if err := f.errs[source]; err != nil { + return nil, err + } + if res := f.results[source]; res != nil { + return res, nil + } + return &github.SearchResult{}, nil + } + } + return nil, fmt.Errorf("unexpected query %q", query) +} + +func found(numbers ...int) *github.SearchResult { + res := &github.SearchResult{Total: len(numbers)} + for _, n := range numbers { + res.PRs = append(res.PRs, github.SearchResultPR{Owner: "owner", Repo: "repo", Number: n, Title: fmt.Sprintf("PR %d", n)}) + } + return res +} + +func saveConfig(t *testing.T, cfg *config.Config) { + t.Helper() + if err := cfg.Save(); err != nil { + t.Fatal(err) + } +} + +func watchList(t *testing.T) string { + t.Helper() + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + var items []string + for _, pr := range cfg.WatchedPRs { + src := pr.Source + if src == "" { + src = "manual" + } + items = append(items, fmt.Sprintf("%d:%s", pr.Number, src)) + } + sort.Strings(items) + return strings.Join(items, " ") +} + +func TestSyncAutoWatch_AddsMatchesTaggedWithSource(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{{Owner: "owner", Repo: "repo", Number: 1}}, // watched by hand + }) + s := &fakeSearcher{results: map[string]*github.SearchResult{ + config.AutoWatchMine: found(1, 2), + config.AutoWatchReviewRequested: found(3), + }} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if len(res.Added) != 2 || len(res.Removed) != 0 { + t.Errorf("added=%+v removed=%+v", res.Added, res.Removed) + } + if got := watchList(t); got != "1:manual 2:mine 3:review-requested" { + t.Errorf("watch list = %s (a PR watched by hand must stay manual)", got) + } + cfg, _ := config.Load() + for _, pr := range cfg.WatchedPRs { + if pr.Number == 2 && pr.Title != "PR 2" { + t.Errorf("title should come from the search result, got %q", pr.Title) + } + } +} + +func TestSyncAutoWatch_DropsWithdrawnReviewRequestsButKeepsMine(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{ + {Owner: "owner", Repo: "repo", Number: 10, Source: config.AutoWatchMine}, // merged: no longer matches + {Owner: "owner", Repo: "repo", Number: 20, Source: config.AutoWatchReviewRequested}, // review withdrawn + {Owner: "owner", Repo: "repo", Number: 21, Source: config.AutoWatchReviewRequested}, // still requested + {Owner: "owner", Repo: "repo", Number: 30}, // manual + }, + }) + s := &fakeSearcher{results: map[string]*github.SearchResult{ + config.AutoWatchMine: found(), + config.AutoWatchReviewRequested: found(21), + }} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if len(res.Removed) != 1 || res.Removed[0].Number != 20 { + t.Errorf("removed = %+v", res.Removed) + } + if got := watchList(t); got != "10:mine 21:review-requested 30:manual" { + t.Errorf("watch list = %s (mine PRs are left for prw run to notify and drop)", got) + } +} + +func TestSyncAutoWatch_IncompleteResultsNeverRemove(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{{Owner: "owner", Repo: "repo", Number: 20, Source: config.AutoWatchReviewRequested}}, + }) + partial := found(21) + partial.Incomplete, partial.Total = true, 2 + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchReviewRequested: partial}} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if got := watchList(t); got != "20:review-requested 21:review-requested" { + t.Errorf("watch list = %s", got) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "returned 1 of 2") { + t.Errorf("warnings = %v", res.Warnings) + } +} + +func TestSyncAutoWatch_OneFailedSearchDoesNotBlockTheOther(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{{Owner: "owner", Repo: "repo", Number: 20, Source: config.AutoWatchReviewRequested}}, + }) + s := &fakeSearcher{ + results: map[string]*github.SearchResult{config.AutoWatchMine: found(1)}, + errs: map[string]error{config.AutoWatchReviewRequested: errors.New("boom")}, + } + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatalf("a partial failure is a warning, got %v", err) + } + if got := watchList(t); got != "1:mine 20:review-requested" { + t.Errorf("watch list = %s (failed search must not remove its PRs)", got) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "boom") { + t.Errorf("warnings = %v", res.Warnings) + } +} + +func TestSyncAutoWatch_AllSearchesFailing(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{AutoWatch: []string{config.AutoWatchMine}}) + boom := errors.New("network down") + s := &fakeSearcher{errs: map[string]error{config.AutoWatchMine: boom}} + + if _, err := SyncAutoWatch(s, []string{config.AutoWatchMine}); !errors.Is(err, boom) { + t.Errorf("err = %v, want the search error", err) + } +} + +func TestSyncAutoWatch_SkipsSourceDisabledMeanwhile(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{}) // `prw unwatch --mine` ran after the caller read the config + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchMine: found(1)}} + + res, err := SyncAutoWatch(s, []string{config.AutoWatchMine}) + if err != nil { + t.Fatal(err) + } + if len(res.Added) != 0 || watchList(t) != "" { + t.Errorf("a disabled source must not add PRs: %+v / %s", res.Added, watchList(t)) + } +} + +func TestSyncAutoWatch_UnknownSource(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{AutoWatch: []string{"bogus"}}) + res, err := SyncAutoWatch(&fakeSearcher{}, []string{"bogus"}) + if err != nil { + t.Fatal(err) + } + if len(res.Warnings) != 1 || !strings.Contains(res.Warnings[0], "unknown auto-watch source") { + t.Errorf("warnings = %v", res.Warnings) + } +} + +func TestRun_SyncsOnStartupEvenWithEmptyWatchList(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{PollIntervalSeconds: 60, AutoWatch: []string{config.AutoWatchMine}} + saveConfig(t, cfg) + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchMine: found(7)}} + s.prs = map[string]*github.PullRequest{"owner/repo/7": newPR(7, "sha7")} + s.statuses = map[string]*github.CombinedStatus{"sha7": {State: "success", TotalCount: 1}} + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + New(s, cfg, &mockNotifier{}).Run(ctx) + + if s.searches != 1 { + t.Errorf("searches = %d, want 1 on startup", s.searches) + } + loaded, _ := config.Load() + if len(loaded.WatchedPRs) != 1 || loaded.WatchedPRs[0].LastKnownState != "success" { + t.Errorf("the auto-watched PR should be added and checked on startup: %+v", loaded.WatchedPRs) + } +} + +func TestRun_RefreshesSearchesOnlyEveryAutoWatchInterval(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{PollIntervalSeconds: 1, AutoWatch: []string{config.AutoWatchMine}} + saveConfig(t, cfg) + s := &fakeSearcher{} + w := New(s, cfg, &mockNotifier{}) + + start := time.Now() + var mu sync.Mutex + clock := start + w.now = func() time.Time { + mu.Lock() + defer mu.Unlock() + return clock + } + + ctx, cancel := context.WithTimeout(context.Background(), 2500*time.Millisecond) + defer cancel() + go func() { + // After ~1.5s (startup + one tick), jump the clock past the interval. + time.Sleep(1500 * time.Millisecond) + mu.Lock() + clock = start.Add(AutoWatchInterval + time.Second) + mu.Unlock() + }() + w.Run(ctx) + + if s.searches != 2 { + t.Errorf("searches = %d, want 2 (startup + once after the interval elapsed)", s.searches) + } +} + +func TestRunOnce_SyncsAutoWatch(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{AutoWatch: []string{config.AutoWatchReviewRequested}} + saveConfig(t, cfg) + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchReviewRequested: found(3)}} + s.prs = map[string]*github.PullRequest{"owner/repo/3": newPR(3, "sha3")} + + if err := New(s, cfg, &mockNotifier{}).RunOnce(context.Background()); err != nil { + t.Fatal(err) + } + if got := watchList(t); got != "3:review-requested" { + t.Errorf("watch list = %s", got) + } +} + +func TestRun_PicksUpAutoWatchEnabledWhileRunning(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{PollIntervalSeconds: 1, WatchedPRs: []config.WatchedPR{watched(1, "success")}} + saveConfig(t, cfg) + s := &fakeSearcher{results: map[string]*github.SearchResult{config.AutoWatchMine: found(2)}} + s.prs = map[string]*github.PullRequest{"owner/repo/1": newPR(1, "sha1"), "owner/repo/2": newPR(2, "sha2")} + + ctx, cancel := context.WithTimeout(context.Background(), 1500*time.Millisecond) + defer cancel() + go func() { + time.Sleep(300 * time.Millisecond) + // `prw watch --mine` in another terminal. + config.Update(func(c *config.Config) error { c.EnableAutoWatch(config.AutoWatchMine); return nil }) + }() + New(s, cfg, &mockNotifier{}).Run(ctx) + + if s.searches != 1 { + t.Errorf("searches = %d, want 1 after auto-watch was enabled", s.searches) + } + if got := watchList(t); got != "1:manual 2:mine" { + t.Errorf("watch list = %s", got) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 81f29d2..1dc9ada 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -34,8 +34,12 @@ type Watcher struct { pausedUntil time.Time // now is the clock; tests replace it. now func() time.Time + // lastAutoWatch is when the auto-watch searches last ran. + lastAutoWatch time.Time } +const noPRsMessage = "No PRs being watched. Add some with 'prw watch ', or 'prw watch --mine' to follow your open PRs." + // defaultRateLimitPause is used when GitHub doesn't say when the limit resets. const defaultRateLimitPause = time.Minute @@ -56,12 +60,19 @@ func (w *Watcher) Run(ctx context.Context) error { defer ticker.Stop() fmt.Printf("Starting watcher with %d second poll interval...\n", w.config.PollIntervalSeconds) - if len(w.config.WatchedPRs) == 0 { - fmt.Println("No PRs being watched. Add some with 'prw watch '.") + if len(w.config.WatchedPRs) == 0 && len(w.config.AutoWatch) == 0 { + fmt.Println(noPRsMessage) return nil } + for _, source := range w.config.AutoWatch { + fmt.Printf("Auto-watching %s (refreshed every %s).\n", AutoWatchDescription[source], AutoWatchInterval) + } // Check immediately on startup + if len(w.config.AutoWatch) > 0 { + w.syncAutoWatch() + w.reloadWatchedPRs() + } w.checkAllPRs() for { @@ -74,6 +85,10 @@ func (w *Watcher) Run(ctx context.Context) error { continue // rate limited; the pause message was printed when it started } w.reloadWatchedPRs() + if len(w.config.AutoWatch) > 0 && w.now().Sub(w.lastAutoWatch) >= AutoWatchInterval { + w.syncAutoWatch() + w.reloadWatchedPRs() + } w.checkAllPRs() } } @@ -91,13 +106,19 @@ func (w *Watcher) reloadWatchedPRs() { fmt.Printf("Watch list changed: now watching %d PR(s).\n", len(fresh.WatchedPRs)) } w.config.WatchedPRs = fresh.WatchedPRs + // `prw watch --mine` / `prw unwatch --mine` may have changed the searches too. + w.config.AutoWatch = fresh.AutoWatch } // RunOnce checks all watched PRs a single time and returns. func (w *Watcher) RunOnce(ctx context.Context) error { fmt.Printf("Running one-time check with %d second poll interval...\n", w.config.PollIntervalSeconds) + if len(w.config.AutoWatch) > 0 { + w.syncAutoWatch() + w.reloadWatchedPRs() + } if len(w.config.WatchedPRs) == 0 { - fmt.Println("No PRs being watched. Add some with 'prw watch '.") + fmt.Println(noPRsMessage) return nil } w.checkAllPRs()