diff --git a/CHANGELOG.md b/CHANGELOG.md index dd2d690..dc71faa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ 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. @@ -19,6 +20,8 @@ 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 +- PRs without any CI are reported as `no_ci` instead of `pending` forever. Previously `prw wait` and the MCP `wait_for_ci` tool blocked until their timeout on such PRs (repositories without CI, or stacked PRs whose base branch runs none). A PR is `no_ci` once it has no checks or statuses at all and hasn't been updated for 3 minutes, so CI that is still registering after a push isn't mistaken for missing CI. `prw wait` exits `4` for it; `prw run` doesn't notify about it. +- 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 4d5f6ff..f54da96 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,7 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you - **`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) @@ -143,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 @@ -228,6 +238,7 @@ prw wait "$PR_URL" --timeout 15m --json | `1` | CI failed (`failure` or `error`) | | `2` | Timed out (`--timeout`, default 30m) while CI was still pending | | `3` | Other error: invalid URL, API error, missing token, PR closed without merging | +| `4` | The PR has **no CI**: no checks or statuses were reported within 3 minutes of its last update (see below) | | `130` | Interrupted (Ctrl+C) | It fails fast — as soon as one check fails it exits `1` with the failed checks and their links, without waiting for the rest. If someone pushes to the PR meanwhile, it follows the new head commit. Transient errors (network, 5xx, rate limits) are retried up to 3 times in a row. Polls every 15s by default (`--interval`). @@ -250,7 +261,9 @@ With `--json`, stdout is a single object: } ``` -`state` is one of `success`, `failure`, `error`, `pending` (with `timed_out` or `interrupted` set), `merged`, or `closed`. +`state` is one of `success`, `failure`, `error`, `pending` (with `timed_out` or `interrupted` set), `merged`, `closed`, or `no_ci`. + +**PRs without CI.** Some PRs never get a single check — the repository has no CI, or none runs for the PR's base branch (for example a stacked PR whose base isn't `main`). Right after a push, though, every PR briefly has zero checks while CI registers them. So prw only concludes `no_ci` once the PR has had no checks or statuses at all **and** hasn't been updated for 3 minutes; until then it reports `pending`. `no_ci` exits with `4`, not `0`, so `prw wait && deploy` doesn't deploy a PR that no CI looked at. `prw run` shows `no_ci` in `prw list` but doesn't send a notification for it, and notifies normally if CI shows up later (e.g. after the PR is retargeted). ## Using prw from AI agents: `prw mcp` @@ -276,7 +289,7 @@ 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. | +| `get_ci_status` | Current state of a PR (`success`, `failure`, `error`, `pending`, `merged`, `closed`, `no_ci`), 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). | diff --git a/TASKS.md b/TASKS.md index d4ef03c..80ba30e 100644 --- a/TASKS.md +++ b/TASKS.md @@ -87,7 +87,10 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - ✅ #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. - ✅ #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. -- Next: #10 `prw watch --mine` / `--review-requested` +- ✅ #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. +- ✅ #18 `no_ci` state — done (0 checks + PR not updated for 3 min → `no_ci`; `prw wait` exits 4; no notification; verified live: 11 real PRs went from `pending` to `no_ci`, `prw wait` on one returns in 2s instead of the 30-min timeout) +- Next: #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/broadcast.go b/cmd/prw/broadcast.go index bf2138d..856ba5c 100644 --- a/cmd/prw/broadcast.go +++ b/cmd/prw/broadcast.go @@ -81,6 +81,7 @@ var broadcastCmd = &cobra.Command{ fmt.Printf("Error fetching status for %s/%s#%d: %v\n", pr.Owner, pr.Repo, pr.Number, err) continue } + github.ApplyNoCI(ci, ghPR.UpdatedAt, time.Now()) currentState := ci.State previousState := github.NormalizeState(pr.LastKnownState) 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 index 44d7240..89792ca 100644 --- a/cmd/prw/mcp.go +++ b/cmd/prw/mcp.go @@ -43,7 +43,7 @@ const ( 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.` +States: success, failure, error, pending, merged, closed, and no_ci (the PR has no checks or statuses at all; waiting longer will not help).` ) func init() { @@ -428,7 +428,7 @@ 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.", + "description": "Get the current CI status of a GitHub pull request: overall state (success, failure, error, pending, merged, closed, or no_ci when the PR has no checks at all), 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}, @@ -440,7 +440,7 @@ var mcpTools = []map[string]interface{}{ { "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'.", + "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'; a PR with no checks or statuses at all returns 'no_ci' instead of waiting until the timeout.", "inputSchema": map[string]interface{}{ "type": "object", "properties": map[string]interface{}{ @@ -545,6 +545,8 @@ func ciSummary(r *waitResult) string { 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) + case r.State == github.StateNoCI: + return fmt.Sprintf("%s has no CI: no checks or statuses were reported within %s of its last update, so nothing verified it.", pr, github.NoCIGracePeriod) } names := make([]string, len(r.FailedChecks)) for i, c := range r.FailedChecks { diff --git a/cmd/prw/noci_test.go b/cmd/prw/noci_test.go new file mode 100644 index 0000000..221fd01 --- /dev/null +++ b/cmd/prw/noci_test.go @@ -0,0 +1,91 @@ +package main + +import ( + "bytes" + "context" + "fmt" + "net/http" + "strings" + "testing" + "time" + + "github.com/devblac/prw/internal/github" +) + +func staleOpenPR(sha string, quietFor time.Duration) *github.PullRequest { + pr := openPR(sha) + pr.UpdatedAt = time.Now().Add(-quietFor) + return pr +} + +func TestWaitForCI_NoCIStopsInsteadOfWaitingForTheTimeout(t *testing.T) { + noSleep(t) + client := &scriptedClient{steps: []pollStep{{pr: staleOpenPR("sha", time.Hour)}}} + + result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + if err != nil { + t.Fatal(err) + } + if result.State != github.StateNoCI || client.polls != 1 { + t.Errorf("state=%q polls=%d, want no_ci after one poll", result.State, client.polls) + } + if code := exitCode(result.exitErr()); code != exitWaitNoCI { + t.Errorf("exit code = %d, want 4 (no CI must not look like success)", code) + } +} + +func TestWaitForCI_WaitsForCIToRegisterAfterAPush(t *testing.T) { + noSleep(t) + client := &scriptedClient{steps: []pollStep{ + {pr: staleOpenPR("new", 10*time.Second)}, // just pushed, nothing registered yet + {pr: staleOpenPR("new", 20*time.Second), runs: []github.CheckRun{run("Test", "queued", "")}}, + {pr: staleOpenPR("new", 30*time.Second), runs: []github.CheckRun{run("Test", "completed", "success")}}, + }} + + result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + if err != nil { + t.Fatal(err) + } + if result.State != "success" || client.polls != 3 { + t.Errorf("state=%q polls=%d, want success after 3 polls", result.State, client.polls) + } +} + +func TestPrintWaitResult_NoCI(t *testing.T) { + var buf bytes.Buffer + printWaitResult(&buf, &waitResult{Owner: "o", Repo: "r", Number: 1, State: github.StateNoCI, URL: "https://github.com/o/r/pull/1"}, false) + if !strings.Contains(buf.String(), "⚪ o/r#1") || !strings.Contains(buf.String(), "No CI: no checks or statuses") { + t.Errorf("output:\n%s", buf.String()) + } +} + +func TestCISummary_NoCI(t *testing.T) { + got := ciSummary(&waitResult{Owner: "o", Repo: "r", Number: 1, State: github.StateNoCI}) + if !strings.HasPrefix(got, "o/r#1 has no CI") { + t.Errorf("summary = %q", got) + } +} + +func TestWaitCmd_NoCIExitCode(t *testing.T) { + updated := time.Now().Add(-time.Hour).UTC().Format(time.RFC3339) + setupWaitCommand(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/pulls/1"): + fmt.Fprintf(w, `{"number":1,"title":"Stacked","state":"open","updated_at":%q,"head":{"sha":"abc1234"}}`, updated) + case strings.HasSuffix(r.URL.Path, "/status"): + fmt.Fprint(w, `{"state":"pending","total_count":0,"statuses":[]}`) + default: + fmt.Fprint(w, `{"total_count":0,"check_runs":[]}`) + } + }) + + out, err := captureStdout(func() error { + return waitCmd.RunE(waitCmd, []string{"https://github.com/owner/repo/pull/1"}) + }) + if code := exitCode(err); code != exitWaitNoCI { + t.Errorf("exit code = %d (err=%v), want 4", code, err) + } + if !strings.Contains(out, "No CI") { + t.Errorf("stdout:\n%s", out) + } +} diff --git a/cmd/prw/wait.go b/cmd/prw/wait.go index 7c8e4b1..8f44af2 100644 --- a/cmd/prw/wait.go +++ b/cmd/prw/wait.go @@ -24,6 +24,7 @@ const ( exitWaitFailed = 1 exitWaitTimeout = 2 exitWaitError = 3 + exitWaitNoCI = 4 exitWaitInterrupted = 130 // maxConsecutiveWaitErrors is how many transient API errors in a row `prw wait` @@ -82,6 +83,8 @@ Exit codes: 1 CI failed (failure or error) 2 timed out while CI was still pending 3 other error (invalid URL, API error, PR closed without merging) + 4 the PR has no CI: no checks or statuses reported within 3 minutes of its + last update (e.g. the repository has no CI, or none runs for its base branch) 130 interrupted (Ctrl+C) Progress goes to stderr; the result goes to stdout (as JSON with --json).`, @@ -177,6 +180,9 @@ func (r *waitResult) exitErr() error { return waitErrorf(exitWaitFailed, "CI of %s finished with %s (%d failed check(s))", pr, r.State, len(r.FailedChecks)) case "closed": return waitErrorf(exitWaitError, "%s was closed without merging", pr) + case github.StateNoCI: + // Not a success: `prw wait && deploy` must not deploy a PR that no CI looked at. + return waitErrorf(exitWaitNoCI, "%s has no CI: no checks or statuses were reported", pr) } return waitErrorf(exitWaitError, "unexpected state %q for %s", r.State, pr) } @@ -267,6 +273,7 @@ func pollOnce(client ciClient, result *waitResult) (string, error) { if err != nil { return "", err } + github.ApplyNoCI(ci, pr.UpdatedAt, time.Now()) result.State = ci.State result.FailedChecks = ci.FailedChecks result.TotalChecks = ci.Total @@ -313,6 +320,8 @@ func printWaitResult(w io.Writer, r *waitResult, asJSON bool) error { icon = "✅" case "failure", "error", "closed": icon = "❌" + case github.StateNoCI: + icon = "⚪" } header := fmt.Sprintf("%s %s/%s#%d", icon, r.Owner, r.Repo, r.Number) if r.Title != "" { @@ -328,6 +337,8 @@ func printWaitResult(w io.Writer, r *waitResult, asJSON bool) error { fmt.Fprintf(w, "Timed out after %s; CI is still %s\n", elapsed, r.State) case r.State == "merged" || r.State == "closed": fmt.Fprintf(w, "PR was %s (after %s)\n", r.State, elapsed) + case r.State == github.StateNoCI: + fmt.Fprintf(w, "No CI: no checks or statuses were reported for this PR (after %s)\n", elapsed) default: fmt.Fprintf(w, "CI: %s (%d check(s), %s)\n", r.State, r.TotalChecks, elapsed) } 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..cceccf2 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -108,6 +108,31 @@ type PullRequest struct { Head struct { SHA string `json:"sha"` } `json:"head"` + // UpdatedAt changes on every push (and on comments, labels, ...), so it is an + // upper bound for how recently the head commit was pushed. + UpdatedAt time.Time `json:"updated_at"` +} + +// StateNoCI is reported instead of "pending" for a PR that has no checks or +// statuses at all and wasn't updated for NoCIGracePeriod, i.e. one whose repository +// (or base branch) runs no CI for it. +const StateNoCI = "no_ci" + +// NoCIGracePeriod is how long after the last update of a PR prw keeps waiting for +// CI to report its first check before concluding the PR has none. GitHub Actions +// usually registers checks within seconds; external CI can take longer. +const NoCIGracePeriod = 3 * time.Minute + +// ApplyNoCI turns a "pending" status with no checks or statuses at all into StateNoCI +// once the PR has gone NoCIGracePeriod without updates. updatedAt is the PR's +// UpdatedAt; when it's unknown (zero), the status is left as pending. +func ApplyNoCI(ci *CIStatus, updatedAt, now time.Time) { + if ci.State != "pending" || ci.Total != 0 || updatedAt.IsZero() { + return + } + if now.Sub(updatedAt) >= NoCIGracePeriod { + ci.State = StateNoCI + } } // IsClosed reports whether the PR is closed, merged or not (GitHub reports both as @@ -232,6 +257,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/noci_test.go b/internal/github/noci_test.go new file mode 100644 index 0000000..8a89082 --- /dev/null +++ b/internal/github/noci_test.go @@ -0,0 +1,64 @@ +package github + +import ( + "encoding/json" + "testing" + "time" +) + +func TestApplyNoCI(t *testing.T) { + now := time.Date(2026, 10, 18, 12, 0, 0, 0, time.UTC) + tests := []struct { + name string + ci CIStatus + updatedAt time.Time + want string + }{ + {"no checks, quiet past the grace period", CIStatus{State: "pending"}, now.Add(-NoCIGracePeriod), StateNoCI}, + {"no checks, quiet for days", CIStatus{State: "pending"}, now.Add(-72 * time.Hour), StateNoCI}, + {"no checks yet, just pushed", CIStatus{State: "pending"}, now.Add(-30 * time.Second), "pending"}, + {"no checks, just short of the grace period", CIStatus{State: "pending"}, now.Add(-NoCIGracePeriod + time.Second), "pending"}, + {"no checks, unknown update time", CIStatus{State: "pending"}, time.Time{}, "pending"}, + {"checks still running", CIStatus{State: "pending", Total: 2, Pending: 1}, now.Add(-time.Hour), "pending"}, + {"success is left alone", CIStatus{State: "success", Total: 1}, now.Add(-time.Hour), "success"}, + {"failure is left alone", CIStatus{State: "failure", Total: 1}, now.Add(-time.Hour), "failure"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ci := tt.ci + ApplyNoCI(&ci, tt.updatedAt, now) + if ci.State != tt.want { + t.Errorf("state = %q, want %q", ci.State, tt.want) + } + }) + } +} + +func TestApplyNoCI_WithSummarizeCI(t *testing.T) { + now := time.Now() + old := now.Add(-time.Hour) + + // The shape the real API returns for a commit nothing reported on. + none := SummarizeCI(&CombinedStatus{State: "pending", TotalCount: 0}, nil) + ApplyNoCI(none, old, now) + if none.State != StateNoCI { + t.Errorf("empty CI = %q, want no_ci", none.State) + } + + // A legacy status that is still pending is a real signal, not "no CI". + legacy := SummarizeCI(&CombinedStatus{State: "pending", TotalCount: 1, Statuses: []CommitStatus{{Context: "ci", State: "pending"}}}, nil) + ApplyNoCI(legacy, old, now) + if legacy.State != "pending" { + t.Errorf("pending legacy status = %q, want pending", legacy.State) + } +} + +func TestPullRequest_DecodesUpdatedAt(t *testing.T) { + var pr PullRequest + if err := json.Unmarshal([]byte(`{"number":7,"updated_at":"2026-10-18T03:27:41Z"}`), &pr); err != nil { + t.Fatal(err) + } + if want := time.Date(2026, 10, 18, 3, 27, 41, 0, time.UTC); !pr.UpdatedAt.Equal(want) { + t.Errorf("UpdatedAt = %v, want %v", pr.UpdatedAt, want) + } +} 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/notify/webhook_format.go b/internal/notify/webhook_format.go index c91e6c3..4481065 100644 --- a/internal/notify/webhook_format.go +++ b/internal/notify/webhook_format.go @@ -29,6 +29,7 @@ const ( colorSuccess = 0x2EA043 colorFailure = 0xD1242F colorPending = 0xDBAB09 + colorNoCI = 0x8C959F ) // IsValidWebhookFormat reports whether value is a supported webhook format. @@ -110,6 +111,8 @@ func stateEmoji(state string) string { return "✅" case "failure", "error": return "❌" + case github.StateNoCI: + return "⚪" default: return "⏳" } @@ -121,6 +124,8 @@ func stateColor(state string) int { return colorSuccess case "failure", "error": return colorFailure + case github.StateNoCI: + return colorNoCI default: return colorPending } 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/noci_test.go b/internal/watcher/noci_test.go new file mode 100644 index 0000000..8dc8ed8 --- /dev/null +++ b/internal/watcher/noci_test.go @@ -0,0 +1,66 @@ +package watcher + +import ( + "testing" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/github" +) + +func quietPR(number int, sha string, quietFor time.Duration, now time.Time) *github.PullRequest { + pr := newPR(number, sha) + pr.State = "open" + pr.UpdatedAt = now.Add(-quietFor) + return pr +} + +func TestCheckPR_ReportsNoCIWithoutNotifying(t *testing.T) { + now := time.Date(2026, 10, 18, 12, 0, 0, 0, time.UTC) + client := &mockGitHubClient{prs: map[string]*github.PullRequest{"owner/repo/1": quietPR(1, "sha1", time.Hour, now)}} + notifier := &mockNotifier{} + cfg := &config.Config{WatchedPRs: []config.WatchedPR{watched(1, "pending")}} + w := New(client, cfg, notifier) + w.now = func() time.Time { return now } + + if _, err := w.checkPR(&cfg.WatchedPRs[0]); err != nil { + t.Fatal(err) + } + if got := cfg.WatchedPRs[0].LastKnownState; got != github.StateNoCI { + t.Errorf("state = %q, want no_ci", got) + } + if len(notifier.events) != 0 { + t.Errorf("pending → no_ci must not notify, got %+v", notifier.events[0]) + } +} + +func TestCheckPR_StaysPendingDuringGracePeriod(t *testing.T) { + now := time.Date(2026, 10, 18, 12, 0, 0, 0, time.UTC) + client := &mockGitHubClient{prs: map[string]*github.PullRequest{"owner/repo/1": quietPR(1, "sha1", 20*time.Second, now)}} + cfg := &config.Config{WatchedPRs: []config.WatchedPR{watched(1, "")}} + w := New(client, cfg, &mockNotifier{}) + w.now = func() time.Time { return now } + + w.checkPR(&cfg.WatchedPRs[0]) + if got := cfg.WatchedPRs[0].LastKnownState; got != "pending" { + t.Errorf("a PR pushed 20s ago must stay pending while CI registers, got %q", got) + } +} + +func TestCheckPR_NotifiesWhenCIShowsUpLater(t *testing.T) { + // e.g. a stacked PR is retargeted to main and CI starts running. + now := time.Date(2026, 10, 18, 12, 0, 0, 0, time.UTC) + client := &mockGitHubClient{ + prs: map[string]*github.PullRequest{"owner/repo/1": quietPR(1, "sha1", time.Minute, now)}, + checkRuns: map[string][]github.CheckRun{"sha1": {{Name: "Test", Status: "completed", Conclusion: "success"}}}, + } + notifier := &mockNotifier{} + cfg := &config.Config{WatchedPRs: []config.WatchedPR{watched(1, github.StateNoCI)}} + w := New(client, cfg, notifier) + w.now = func() time.Time { return now } + + w.checkPR(&cfg.WatchedPRs[0]) + if len(notifier.events) != 1 || notifier.events[0].PreviousState != github.StateNoCI || notifier.events[0].CurrentState != "success" { + t.Errorf("events = %+v, want one no_ci → success", notifier.events) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 81f29d2..d51eac0 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() @@ -198,12 +219,15 @@ func (w *Watcher) checkPR(pr *config.WatchedPR) (closed bool, err error) { if err != nil { return false, fmt.Errorf("failed to fetch status: %w", err) } + github.ApplyNoCI(ci, ghPR.UpdatedAt, w.now()) currentState := ci.State previousState := github.NormalizeState(pr.LastKnownState) - // Check if status changed - if previousState != "" && previousState != currentState && shouldNotify(w.config.NotificationFilter, currentState) { + // Check if status changed. Finding out that a PR has no CI is not a CI result, so + // it isn't announced (it would otherwise ping once for every such PR). + if previousState != "" && previousState != currentState && currentState != github.StateNoCI && + shouldNotify(w.config.NotificationFilter, currentState) { event := ¬ify.StatusChangeEvent{ Owner: pr.Owner, Repo: pr.Repo,