diff --git a/CHANGELOG.md b/CHANGELOG.md index 81731be..240e5a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Auto-watch on Bitbucket Cloud: `prw watch --mine` / `--review-requested` and `prw run` also find your Bitbucket PRs (when a Bitbucket token or `bitbucket_user` is set): your open PRs in every workspace (or `bitbucket_workspaces`), and open PRs listing you as a reviewer that you haven't approved or requested changes on, in repositories active in the last 30 days (rescanned every 15 minutes). Without a GitHub token, auto-watch works on Bitbucket alone. + +### Changed +- `prw run` checks Bitbucket PRs less often as the watch list grows (every 18 seconds × the number of Bitbucket PRs, at least a minute), keeping checks around 600 of Bitbucket's 1,000 requests per hour. + ## [0.5.0] - 2026-10-31 prw now works with Bitbucket Cloud as well as GitHub, and `prw dashboard` shows all your PRs across repositories in one live table, grouped by task, with one column per target branch — handy when a change travels dev → rc → master as cherry-picks. diff --git a/README.md b/README.md index 8106131..81fd6ed 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,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 - **Bitbucket Cloud**: watch, wait for and list the builds of Bitbucket PRs (Pipelines, Jenkins, ...) with the same commands -- **Auto-watch**: `prw watch --mine` / `--review-requested` follow your PRs and review requests across all repositories, no URLs needed +- **Auto-watch**: `prw watch --mine` / `--review-requested` follow your PRs and review requests across all repositories, on GitHub and Bitbucket, 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) @@ -156,7 +156,7 @@ 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. +These follow a search instead of a fixed list, on GitHub and — when a Bitbucket token or `bitbucket_user` is set — on Bitbucket Cloud too (see [Bitbucket Cloud](#bitbucket-cloud)): 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 @@ -404,8 +404,9 @@ prw checks https://bitbucket.org/acme/api/pull-requests/42 - **CI** comes from the commit's build statuses, so it covers Bitbucket Pipelines, Jenkins and anything else that reports to Bitbucket, each with its link and duration. A stopped build counts as failed. - **Reviews**: participants who approved or requested changes. **Merge state** comes from the PR's merge checks: `conflict`, `blocked` (e.g. missing approvals or successful builds), or `ready`. - `prw run` notifies about Bitbucket PRs just like GitHub ones, `prw list` shows them as `bitbucket:workspace/repo`, and the MCP tools accept their URLs. -- Bitbucket Cloud allows 1,000 API requests per hour, so `prw run` checks each Bitbucket PR at most once a minute whatever the poll interval (about 8 PRs fit in the limit). A rate limit on one service doesn't pause the other. -- Not yet for Bitbucket: `--mine` / `--review-requested`, `--logs` and `prw flaky`. +- **Auto-watch**: `prw watch --mine` follows your open PRs in all your workspaces (or those in `bitbucket_workspaces`). `--review-requested` follows open PRs that list you as a reviewer and that you haven't approved or requested changes on yet; Bitbucket can only search those per repository, so prw scans the repositories active in the last 30 days (up to 60 per workspace) every 15 minutes. +- Bitbucket Cloud allows 1,000 API requests per hour, so `prw run` spaces out the checks of Bitbucket PRs whatever the poll interval: each is checked at most once a minute, and less often as the list grows (every 18 seconds × the number of Bitbucket PRs, e.g. every 9 minutes with 30), so checks use about 600 requests an hour and leave room for auto-watch and other commands. A rate limit on one service doesn't pause the other. +- Not yet for Bitbucket: `--logs` and `prw flaky`. **Authentication.** Public repositories work without credentials. For private ones, create an [Atlassian API token](https://id.atlassian.com/manage-profile/security/api-tokens) with Bitbucket read scopes (repositories and pull requests) and give prw your Atlassian email with it: diff --git a/TASKS.md b/TASKS.md index eae5e2f..821023b 100644 --- a/TASKS.md +++ b/TASKS.md @@ -100,6 +100,7 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - 🆕 Bitbucket Cloud support (requested by the owner, who uses Bitbucket at work). Phase 1 done: PR identity carries the host; a Bitbucket client maps PRs, build statuses (Pipelines, Jenkins), participants and merge checks onto the existing types, so run / wait / checks / list / broadcast / MCP work unchanged; per-host rate-limit pauses; ≥60s between checks of a Bitbucket PR (1,000 req/h limit). Verified live on public atlassian/atlassian-connect-express PRs. Next: auto-watch (`--mine` / `--review-requested`) on Bitbucket, Pipelines step logs for `--logs`, then GitHub Enterprise (#2) on the same host abstraction. - ✅ `prw dashboard` (owner's use case: ~25 repos × dev/rc/master cherry-picks at work) — live table grouped by task across GitHub + Bitbucket, smart CI re-reads. Bitbucket discovery endpoints verified live anonymously (workspace user PRs, merged since, recent repos, reviewer filter); `/user` and `/user/workspaces` still need a check with the owner's token. Verified live on the owner's GitHub PRs. - 🚀 v0.5.0 released with Bitbucket Cloud and `prw dashboard` (PRs #24–#25). +- ✅ Bitbucket auto-watch (`--mine` / `--review-requested`): per-host finders, review scan every 15 min skipping PRs already reviewed, check interval scaled to the number of Bitbucket PRs (600 req/h budget). Verified live with a public Bitbucket user + the owner's GitHub account in one watch list. - Next: #17 packaging (Homebrew/Scoop) + keychain token storage, #8 `gh` extension publishing — both need owner decisions ## Planned diff --git a/cmd/prw/autowatch_test.go b/cmd/prw/autowatch_test.go index 860334c..e5554b1 100644 --- a/cmd/prw/autowatch_test.go +++ b/cmd/prw/autowatch_test.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "net/http" "net/http/httptest" @@ -217,9 +218,10 @@ 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", "") + t.Setenv("BITBUCKET_TOKEN", "") watchMine = true - if err := watchCmd.RunE(watchCmd, nil); err != errMissingToken { + if err := watchCmd.RunE(watchCmd, nil); !errors.Is(err, errMissingToken) { t.Errorf("err = %v, want errMissingToken", err) } if cfg, _ := config.Load(); len(cfg.AutoWatch) != 0 { diff --git a/cmd/prw/clients.go b/cmd/prw/clients.go index efb8021..6c5a405 100644 --- a/cmd/prw/clients.go +++ b/cmd/prw/clients.go @@ -9,6 +9,7 @@ import ( "github.com/devblac/prw/internal/config" "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" + "github.com/devblac/prw/internal/watcher" ) // prClient is everything the commands use from a code host's client. Both the @@ -41,8 +42,8 @@ func clientFor(cfg *config.Config, host string) (prClient, error) { // needsGitHub reports whether the watch list or auto-watch involves GitHub. func needsGitHub(cfg *config.Config) bool { - if len(cfg.AutoWatch) > 0 { - return true // auto-watch searches GitHub + if len(cfg.AutoWatch) > 0 && !bitbucketConfigured(cfg) { + return true // auto-watch has nothing else to search } for _, pr := range cfg.WatchedPRs { if !forge.IsBitbucket(pr.Host) { @@ -52,6 +53,35 @@ func needsGitHub(cfg *config.Config) bool { return len(cfg.WatchedPRs) == 0 // nothing yet: GitHub is the default } +// bitbucketConfigured reports whether prw knows who you are on Bitbucket, which +// finding your PRs there needs. +func bitbucketConfigured(cfg *config.Config) bool { + auth, _ := cfg.ResolveBitbucketAuth() + return auth.Token != "" || cfg.BitbucketUser != "" +} + +// autoWatchFinders returns the auto-watch finders of the configured accounts; bb is +// the Bitbucket client to use. +func autoWatchFinders(cfg *config.Config, bb *bitbucket.Client) []watcher.Finder { + var finders []watcher.Finder + if token := cfg.GetToken(); token != "" { + finders = append(finders, watcher.GitHubFinder{Searcher: newGitHubClient(token)}) + } + if f := bitbucketFinder(cfg, bb); f != nil { + finders = append(finders, f) + } + return finders +} + +// bitbucketFinder returns the Bitbucket auto-watch finder, or nil without a Bitbucket +// account. +func bitbucketFinder(cfg *config.Config, bb *bitbucket.Client) *watcher.BitbucketFinder { + if !bitbucketConfigured(cfg) { + return nil + } + return &watcher.BitbucketFinder{API: bb, User: cfg.BitbucketUser, Workspaces: cfg.BitbucketWorkspaces} +} + // bitbucketAuthHint explains how to authenticate when Bitbucket refuses a request: // it answers 404 rather than 401/403 for private repositories without access. func bitbucketAuthHint(host string, err error) string { diff --git a/cmd/prw/dashboard.go b/cmd/prw/dashboard.go index 9bf3aac..3126fa6 100644 --- a/cmd/prw/dashboard.go +++ b/cmd/prw/dashboard.go @@ -192,7 +192,7 @@ type dashboardView struct { } func (v dashboardView) render(w io.Writer, items []dashboard.Item, warnings, requests []string, now time.Time, live bool) { - var mine, review, targets []string + var targets []string var mineItems, reviewItems []dashboard.Item for _, it := range items { targets = append(targets, it.Target) @@ -202,8 +202,6 @@ func (v dashboardView) render(w io.Writer, items []dashboard.Item, warnings, req mineItems = append(mineItems, it) } } - _ = mine - _ = review cols := dashboard.Columns(targets, v.order) open := 0 diff --git a/cmd/prw/main.go b/cmd/prw/main.go index 954e215..25f6246 100644 --- a/cmd/prw/main.go +++ b/cmd/prw/main.go @@ -114,15 +114,20 @@ var watchCmd = &cobra.Command{ 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) +With --mine or --review-requested, prw instead finds your PRs on GitHub and on +Bitbucket Cloud (when a Bitbucket token or bitbucket_user is set) and keeps the +watch list up to date by itself: matching PRs are added right away, and +'prw run' searches again every 5 minutes. + + --mine your open PRs, in every repository (on Bitbucket, in every + workspace, or those in bitbucket_workspaces) + --review-requested open PRs where your review is requested (on GitHub incl. + your teams; on Bitbucket, PRs listing you as a reviewer in + repositories active in the last 30 days, rescanned every + 15 minutes) 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. +requested (on Bitbucket: once you approve or request changes). 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 @@ -204,9 +209,10 @@ func enableAutoWatch(sources []string) error { if err != nil { return fmt.Errorf("failed to load config: %w", err) } - token := cfg.GetToken() - if token == "" { - return errMissingToken + bbAuth, _ := cfg.ResolveBitbucketAuth() + finders := autoWatchFinders(cfg, newBitbucketClient(bbAuth.Email, bbAuth.Token)) + if len(finders) == 0 { + return fmt.Errorf("%w; for Bitbucket, set 'prw config set bitbucket_email' and 'prw config set bitbucket_token'", errMissingToken) } var enabled []string @@ -232,21 +238,21 @@ func enableAutoWatch(sources []string) error { } // Run the searches now so the user sees the result immediately. - res, err := watcher.SyncAutoWatch(context.Background(), newGitHubClient(token), sources) + res, err := watcher.Sync(context.Background(), finders, 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) + return fmt.Errorf("auto-watch is enabled, but searching 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) + fmt.Printf(" + %s - %s\n", watcher.DisplayPR(pr), pr.Title) } for _, pr := range res.Removed { - fmt.Printf(" - %s/%s#%d (review no longer requested)\n", pr.Owner, pr.Repo, pr.Number) + fmt.Printf(" - %s (review no longer requested)\n", watcher.DisplayPR(pr)) } fmt.Println("'prw run' keeps this list up to date (searches again every 5 minutes).") return nil @@ -448,7 +454,11 @@ Press Ctrl+C to stop.`, w := watcher.New(client, cfg, notifier) bbAuth, _ := cfg.ResolveBitbucketAuth() - w.SetClient(forge.Bitbucket, newBitbucketClient(bbAuth.Email, bbAuth.Token)) + bb := newBitbucketClient(bbAuth.Email, bbAuth.Token) + w.SetClient(forge.Bitbucket, bb) + if f := bitbucketFinder(cfg, bb); f != nil { + w.AddFinder(f) + } // Setup signal handling ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/bitbucket/client.go b/internal/bitbucket/client.go index 022985c..a9e7e0b 100644 --- a/internal/bitbucket/client.go +++ b/internal/bitbucket/client.go @@ -114,6 +114,7 @@ type pullRequest struct { Participants []struct { User struct { UUID string `json:"uuid"` + AccountID string `json:"account_id"` DisplayName string `json:"display_name"` Nickname string `json:"nickname"` } `json:"user"` diff --git a/internal/bitbucket/discover.go b/internal/bitbucket/discover.go index 56716a0..583120d 100644 --- a/internal/bitbucket/discover.go +++ b/internal/bitbucket/discover.go @@ -14,7 +14,7 @@ import ( const prListFields = "next,values.id,values.title,values.state,values.updated_on," + "values.source.branch.name,values.source.commit.hash," + "values.destination.branch.name,values.destination.repository.full_name," + - "values.participants.approved,values.participants.state,values.participants.user.uuid" + "values.participants.approved,values.participants.state,values.participants.user.uuid,values.participants.user.account_id" const ( listPageLen = 50 @@ -36,6 +36,9 @@ type ListedPR struct { // participant requested changes. Approvals int ChangesRequested bool + // ReviewedBy holds the UUIDs and account ids of the participants who approved or + // requested changes. + ReviewedBy []string } func toListed(pr pullRequest) ListedPR { @@ -52,7 +55,10 @@ func toListed(pr pullRequest) ListedPR { l.ChangesRequested = true case p.Approved: l.Approvals++ + default: + continue } + l.ReviewedBy = append(l.ReviewedBy, p.User.UUID, p.User.AccountID) } return l } diff --git a/internal/watcher/autowatch.go b/internal/watcher/autowatch.go index bf53781..e87b54a 100644 --- a/internal/watcher/autowatch.go +++ b/internal/watcher/autowatch.go @@ -9,6 +9,7 @@ import ( "time" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -36,6 +37,44 @@ type Searcher interface { GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) } +// Found is the result of an auto-watch source on one host. +type Found struct { + PRs []github.SearchResultPR + // Partial says why some PRs may be missing (a truncated search, a workspace that + // failed); PRs that no longer match are then kept. + Partial string +} + +// Finder finds the open PRs of an auto-watch source on one code host. +type Finder interface { + Host() string // forge.GitHub, forge.Bitbucket + Find(ctx context.Context, source string) (*Found, error) + GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) +} + +// GitHubFinder finds PRs with GitHub's search. +type GitHubFinder struct { + Searcher +} + +func (GitHubFinder) Host() string { return forge.GitHub } + +func (f GitHubFinder) Find(ctx context.Context, source string) (*Found, error) { + query, ok := autoWatchQueries[source] + if !ok { + return nil, fmt.Errorf("unknown auto-watch source %q", source) + } + res, err := f.SearchPullRequests(ctx, query) + if err != nil { + return nil, err + } + found := &Found{PRs: res.PRs} + if res.Incomplete { + found.Partial = fmt.Sprintf("returned %d of %d results", len(res.PRs), res.Total) + } + return found, nil +} + // SyncResult reports what SyncAutoWatch changed. type SyncResult struct { Added []config.WatchedPR @@ -44,71 +83,88 @@ type SyncResult struct { 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 that stopped matching because they were merged or -// closed are kept instead (for "mine" that's always the reason), so that `prw run` -// still notifies about it before dropping them. PRs watched by hand are never touched. +// SyncAutoWatch runs GitHub's search for every source in sources and updates the watch +// list; see Sync. +func SyncAutoWatch(ctx context.Context, client Searcher, sources []string) (*SyncResult, error) { + return Sync(ctx, []Finder{GitHubFinder{client}}, sources) +} + +// Sync runs every source in sources on every finder's host and updates the watch list: +// matching PRs that aren't watched yet are added (tagged with their source and host), +// and PRs added for "review-requested" that no longer match are removed, since the +// request was withdrawn or answered. PRs that stopped matching because they were +// merged or closed are kept instead (for "mine" that's always the reason), so that +// `prw run` still notifies about it before dropping them. PRs watched by hand are never +// touched. // // It returns an error if every search failed or ctx was cancelled (then nothing is // changed); other problems are Warnings. -func SyncAutoWatch(ctx context.Context, client Searcher, sources []string) (*SyncResult, error) { +func Sync(ctx context.Context, finders []Finder, sources []string) (*SyncResult, error) { result := &SyncResult{} - found := map[string]*github.SearchResult{} + type search struct { + finder Finder + source string + found *Found + } + var searches []search var lastErr error for _, source := range sources { - query, ok := autoWatchQueries[source] - if !ok { + if _, ok := AutoWatchDescription[source]; !ok { result.Warnings = append(result.Warnings, fmt.Sprintf("unknown auto-watch source %q ignored", source)) continue } - res, err := client.SearchPullRequests(ctx, 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)) + for _, f := range finders { + found, err := f.Find(ctx, source) + if err != nil { + lastErr = err + result.Warnings = append(result.Warnings, fmt.Sprintf("%s search for %s failed: %v", serviceName(f.Host()), AutoWatchDescription[source], err)) + continue + } + if found.Partial != "" { + result.Warnings = append(result.Warnings, fmt.Sprintf("%s search for %s %s; PRs that no longer match are kept", + serviceName(f.Host()), AutoWatchDescription[source], found.Partial)) + } + searches = append(searches, search{f, source, found}) } - found[source] = res } - if len(found) == 0 && lastErr != nil { + if len(searches) == 0 && lastErr != nil { return result, lastErr } - withdrawn := withdrawnReviewRequests(ctx, client, found) + withdrawn := map[string]bool{} + for _, s := range searches { + if s.source == config.AutoWatchReviewRequested && s.found.Partial == "" { + withdrawnReviewRequests(ctx, s.finder, s.found, withdrawn) + } + } if err := ctx.Err(); err != nil { return result, err // interrupted: lookups that didn't finish prove nothing } 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 + for _, s := range searches { + if !c.HasAutoWatch(s.source) { + continue // 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} + host := s.finder.Host() + matches := make(map[string]bool, len(s.found.PRs)) + for _, pr := range s.found.PRs { + matches[prKey(host, pr.Owner, pr.Repo, pr.Number)] = true + added := config.WatchedPR{Host: forge.StorageHost(host), Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, Title: pr.Title, Source: s.source} if c.AddPR(added) { result.Added = append(result.Added, added) changed = true } } - if source != config.AutoWatchReviewRequested || res.Incomplete { + if s.source != config.AutoWatchReviewRequested || s.found.Partial != "" { continue } kept := c.WatchedPRs[:0] for _, pr := range c.WatchedPRs { - key := prKey(pr.Owner, pr.Repo, pr.Number) - if pr.Source == source && !matches[key] && withdrawn[key] { + key := prKey(pr.Host, pr.Owner, pr.Repo, pr.Number) + if pr.Source == s.source && forge.NormalizeHost(pr.Host) == host && !matches[key] && withdrawn[key] { result.Removed = append(result.Removed, pr) changed = true continue @@ -128,33 +184,29 @@ func SyncAutoWatch(ctx context.Context, client Searcher, sources []string) (*Syn return result, nil } -// withdrawnReviewRequests finds the review-request PRs that can be dropped: they no -// longer match their search and are still open (so the request was withdrawn or -// answered), or are gone (404). The search only returns open PRs, so a merged or -// closed PR looks the same until it's looked up; those are kept for the regular check -// to notify about. A failed lookup (network, rate limit, cancellation) proves nothing, -// so that PR is kept until the next sync. It runs before the config is locked, as it -// makes API calls. -func withdrawnReviewRequests(ctx context.Context, client Searcher, found map[string]*github.SearchResult) map[string]bool { - res, ok := found[config.AutoWatchReviewRequested] - if !ok || res.Incomplete { - return nil // nothing will be removed - } +// withdrawnReviewRequests marks in withdrawn the review-request PRs on f's host that +// can be dropped: they no longer match their search and are still open (so the request +// was withdrawn or answered), or are gone (404). The search only returns open PRs, so a +// merged or closed PR looks the same until it's looked up; those are kept for the +// regular check to notify about. A failed lookup (network, rate limit, cancellation) +// proves nothing, so that PR is kept until the next sync. It runs before the config is +// locked, as it makes API calls. +func withdrawnReviewRequests(ctx context.Context, f Finder, found *Found, withdrawn map[string]bool) { cfg, err := config.Load() if err != nil { - return nil + return } - matches := make(map[string]bool, len(res.PRs)) - for _, pr := range res.PRs { - matches[prKey(pr.Owner, pr.Repo, pr.Number)] = true + host := f.Host() + matches := make(map[string]bool, len(found.PRs)) + for _, pr := range found.PRs { + matches[prKey(host, pr.Owner, pr.Repo, pr.Number)] = true } - withdrawn := map[string]bool{} for _, pr := range cfg.WatchedPRs { - key := prKey(pr.Owner, pr.Repo, pr.Number) - if pr.Source != config.AutoWatchReviewRequested || matches[key] { + key := prKey(pr.Host, pr.Owner, pr.Repo, pr.Number) + if pr.Source != config.AutoWatchReviewRequested || forge.NormalizeHost(pr.Host) != host || matches[key] { continue } - ghPR, err := client.GetPullRequest(ctx, pr.Owner, pr.Repo, pr.Number) + ghPR, err := f.GetPullRequest(ctx, pr.Owner, pr.Repo, pr.Number) var apiErr *github.APIError switch { case err == nil: @@ -163,11 +215,35 @@ func withdrawnReviewRequests(ctx context.Context, client Searcher, found map[str withdrawn[key] = true // deleted, or no longer visible to us } } - return withdrawn } -func prKey(owner, repo string, number int) string { - return strings.ToLower(owner) + "/" + strings.ToLower(repo) + "#" + fmt.Sprint(number) +func prKey(host, owner, repo string, number int) string { + return forge.NormalizeHost(host) + ":" + strings.ToLower(owner) + "/" + strings.ToLower(repo) + "#" + fmt.Sprint(number) +} + +// AddFinder adds an auto-watch finder for a host other than GitHub (GitHub's is the +// GitHub client). +func (w *Watcher) AddFinder(f Finder) { + w.finders = append(w.finders, f) +} + +// autoWatchFinders returns the finders auto-watch runs on. +func (w *Watcher) autoWatchFinders() []Finder { + var finders []Finder + if s, ok := w.client.(Searcher); ok && !isNilClient(w.client) { + finders = append(finders, GitHubFinder{s}) + } + return append(finders, w.finders...) +} + +// DisplayPR names a watched PR: owner/repo#N, prefixed with "bitbucket:" for +// Bitbucket. +func DisplayPR(pr config.WatchedPR) string { + name := fmt.Sprintf("%s/%s#%d", pr.Owner, pr.Repo, pr.Number) + if forge.IsBitbucket(pr.Host) { + name = "bitbucket:" + name + } + return name } // syncAutoWatch refreshes the auto-watch searches during `prw run` and reports changes. @@ -181,11 +257,11 @@ func (w *Watcher) syncAutoWatch(ctx context.Context) { } w.lastAutoWatch = w.now() - searcher, ok := w.client.(Searcher) - if !ok { + finders := w.autoWatchFinders() + if len(finders) == 0 { return } - res, err := SyncAutoWatch(ctx, searcher, sources) + res, err := Sync(ctx, finders, sources) if ctx.Err() != nil { return // stopping; the failed searches and lookups aren't worth reporting } @@ -201,9 +277,9 @@ func (w *Watcher) syncAutoWatch(ctx context.Context) { 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]) + fmt.Printf("➕ Auto-watching %s - %s (%s)\n", DisplayPR(pr), 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) + fmt.Printf("➖ No longer watching %s - review no longer requested\n", DisplayPR(pr)) } } diff --git a/internal/watcher/bitbucket_finder.go b/internal/watcher/bitbucket_finder.go new file mode 100644 index 0000000..536f296 --- /dev/null +++ b/internal/watcher/bitbucket_finder.go @@ -0,0 +1,180 @@ +package watcher + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "github.com/devblac/prw/internal/bitbucket" + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" + "github.com/devblac/prw/internal/github" +) + +// BitbucketLister is what BitbucketFinder needs from the Bitbucket client. +type BitbucketLister interface { + CurrentUser(ctx context.Context) (string, error) + Workspaces(ctx context.Context) ([]string, error) + UserPullRequests(ctx context.Context, workspace, user, state string, updatedSince time.Time) ([]bitbucket.ListedPR, error) + RecentRepos(ctx context.Context, workspace string, since time.Time, max int) ([]string, error) + ReviewPullRequests(ctx context.Context, workspace, repo, user string) ([]bitbucket.ListedPR, error) + GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) +} + +// Bitbucket can only list a user's PRs per workspace, and find PRs to review per +// repository, so the review scan is the expensive part: it covers the repositories +// active in the last 30 days (at most 60 per workspace) and runs every 15 minutes. +const ( + BitbucketReviewScanEvery = 15 * time.Minute + bitbucketReviewWindow = 30 * 24 * time.Hour + bitbucketMaxReviewRepos = 60 +) + +// BitbucketFinder finds your open PRs and the open PRs you are a reviewer of (and +// haven't approved or requested changes on yet) across your Bitbucket workspaces. +type BitbucketFinder struct { + API BitbucketLister + // User is a UUID or account id; empty means the token's owner. + User string + // Workspaces to look in; empty means all of the token owner's workspaces. + Workspaces []string + Now func() time.Time + + lastReviewScan time.Time + reviewFound *Found +} + +func (f *BitbucketFinder) Host() string { return forge.Bitbucket } + +func (f *BitbucketFinder) GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) { + return f.API.GetPullRequest(ctx, owner, repo, number) +} + +func (f *BitbucketFinder) now() time.Time { + if f.Now != nil { + return f.Now() + } + return time.Now() +} + +func (f *BitbucketFinder) resolve(ctx context.Context) error { + if f.User == "" { + user, err := f.API.CurrentUser(ctx) + if err != nil { + return fmt.Errorf("finding out who you are needs a Bitbucket token (prw config set bitbucket_token), or set bitbucket_user: %w", err) + } + f.User = user + } + if len(f.Workspaces) == 0 { + ws, err := f.API.Workspaces(ctx) + if err == nil && len(ws) == 0 { + err = errors.New("none found") + } + if err != nil { + return fmt.Errorf("listing your workspaces failed; set them with 'prw config set bitbucket_workspaces ws1,ws2': %w", err) + } + f.Workspaces = ws + } + return nil +} + +func (f *BitbucketFinder) Find(ctx context.Context, source string) (*Found, error) { + if err := f.resolve(ctx); err != nil { + return nil, err + } + switch source { + case config.AutoWatchMine: + return f.mine(ctx) + case config.AutoWatchReviewRequested: + if f.reviewFound == nil || f.now().Sub(f.lastReviewScan) >= BitbucketReviewScanEvery { + found, err := f.scanReviews(ctx) + if err != nil { + return nil, err + } + f.reviewFound, f.lastReviewScan = found, f.now() + } + return f.reviewFound, nil + } + return nil, fmt.Errorf("unknown auto-watch source %q", source) +} + +func (f *BitbucketFinder) mine(ctx context.Context) (*Found, error) { + found := &Found{} + var failed []string + var lastErr error + for _, ws := range f.Workspaces { + prs, err := f.API.UserPullRequests(ctx, ws, f.User, "OPEN", time.Time{}) + if err != nil { + failed, lastErr = append(failed, ws), err + continue + } + found.PRs = append(found.PRs, toSearchPRs(prs, nil)...) + } + return partial(found, failed, len(f.Workspaces), lastErr) +} + +func (f *BitbucketFinder) scanReviews(ctx context.Context) (*Found, error) { + found := &Found{} + var failed []string + var lastErr error + scanned := 0 + for _, ws := range f.Workspaces { + repos, err := f.API.RecentRepos(ctx, ws, f.now().Add(-bitbucketReviewWindow), bitbucketMaxReviewRepos) + scanned++ + if err != nil { + failed, lastErr = append(failed, ws), err + continue + } + for _, repo := range repos { + prs, err := f.API.ReviewPullRequests(ctx, ws, repo, f.User) + scanned++ + if err != nil { + failed, lastErr = append(failed, ws+"/"+repo), err + continue + } + found.PRs = append(found.PRs, toSearchPRs(prs, func(p bitbucket.ListedPR) bool { return !f.reviewed(p) })...) + } + if len(repos) == bitbucketMaxReviewRepos && found.Partial == "" { + found.Partial = fmt.Sprintf("covered only the %d most recently active repositories of %s", bitbucketMaxReviewRepos, ws) + } + } + return partial(found, failed, scanned, lastErr) +} + +// reviewed reports whether the user already approved or requested changes on p: like +// on GitHub, the review is then no longer pending. +func (f *BitbucketFinder) reviewed(p bitbucket.ListedPR) bool { + for _, id := range p.ReviewedBy { + if id != "" && strings.EqualFold(id, f.User) { + return true + } + } + return false +} + +// partial fails when every request failed, and otherwise notes what was missed. +func partial(found *Found, failed []string, total int, lastErr error) (*Found, error) { + if len(failed) == 0 { + return found, nil + } + if len(failed) == total { + return nil, lastErr + } + if found.Partial == "" { + found.Partial = fmt.Sprintf("failed for %s (%v)", strings.Join(failed, ", "), lastErr) + } + return found, nil +} + +func toSearchPRs(prs []bitbucket.ListedPR, keep func(bitbucket.ListedPR) bool) []github.SearchResultPR { + var out []github.SearchResultPR + for _, p := range prs { + if keep != nil && !keep(p) { + continue + } + out = append(out, github.SearchResultPR{Owner: p.Workspace, Repo: p.Repo, Number: p.Number, Title: p.Title}) + } + return out +} diff --git a/internal/watcher/bitbucket_finder_test.go b/internal/watcher/bitbucket_finder_test.go new file mode 100644 index 0000000..e587263 --- /dev/null +++ b/internal/watcher/bitbucket_finder_test.go @@ -0,0 +1,195 @@ +package watcher + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/devblac/prw/internal/bitbucket" + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" + "github.com/devblac/prw/internal/github" +) + +const me = "{me}" + +// fakeLister serves Bitbucket's list endpoints from maps keyed by workspace and +// "workspace/repo". +type fakeLister struct { + mockGitHubClient + workspaces []string + mine map[string][]bitbucket.ListedPR + repos map[string][]string + reviews map[string][]bitbucket.ListedPR + errs map[string]error // by workspace or "workspace/repo" + userCalls int + scans int +} + +func (f *fakeLister) CurrentUser(ctx context.Context) (string, error) { + f.userCalls++ + return me, nil +} + +func (f *fakeLister) Workspaces(ctx context.Context) ([]string, error) { return f.workspaces, nil } + +func (f *fakeLister) UserPullRequests(ctx context.Context, ws, user, state string, since time.Time) ([]bitbucket.ListedPR, error) { + if user != me || state != "OPEN" { + return nil, fmt.Errorf("unexpected user %s / state %s", user, state) + } + return f.mine[ws], f.errs[ws] +} + +func (f *fakeLister) RecentRepos(ctx context.Context, ws string, since time.Time, max int) ([]string, error) { + f.scans++ + return f.repos[ws], f.errs[ws] +} + +func (f *fakeLister) ReviewPullRequests(ctx context.Context, ws, repo, user string) ([]bitbucket.ListedPR, error) { + return f.reviews[ws+"/"+repo], f.errs[ws+"/"+repo] +} + +func listed(ws, repo string, number int, reviewedBy ...string) bitbucket.ListedPR { + return bitbucket.ListedPR{Workspace: ws, Repo: repo, Number: number, Title: fmt.Sprintf("PR %d", number), State: "OPEN", ReviewedBy: reviewedBy} +} + +func numbers(f *Found) []int { + var n []int + for _, pr := range f.PRs { + n = append(n, pr.Number) + } + return n +} + +func TestBitbucketFinder_MineAcrossWorkspaces(t *testing.T) { + api := &fakeLister{ + workspaces: []string{"acme", "labs"}, + mine: map[string][]bitbucket.ListedPR{"acme": {listed("acme", "api", 1)}, "labs": {listed("labs", "web", 2)}}, + } + f := &BitbucketFinder{API: api} + got, err := f.Find(context.Background(), config.AutoWatchMine) + if err != nil { + t.Fatal(err) + } + if fmt.Sprint(numbers(got)) != "[1 2]" || got.Partial != "" { + t.Errorf("found %v, partial %q", numbers(got), got.Partial) + } + if got.PRs[1].Owner != "labs" || got.PRs[1].Repo != "web" { + t.Errorf("PR = %+v", got.PRs[1]) + } + f.Find(context.Background(), config.AutoWatchMine) + if api.userCalls != 1 { + t.Errorf("the user must be looked up once, was %d times", api.userCalls) + } +} + +func TestBitbucketFinder_FailedWorkspaceIsPartial(t *testing.T) { + api := &fakeLister{ + mine: map[string][]bitbucket.ListedPR{"acme": {listed("acme", "api", 1)}}, + errs: map[string]error{"labs": errors.New("boom")}, + } + f := &BitbucketFinder{API: api, User: me, Workspaces: []string{"acme", "labs"}} + got, err := f.Find(context.Background(), config.AutoWatchMine) + if err != nil || got.Partial == "" || len(got.PRs) != 1 { + t.Errorf("got %+v, %v; want acme's PR and a partial note", got, err) + } + + f.Workspaces = []string{"labs"} + if _, err := f.Find(context.Background(), config.AutoWatchMine); err == nil { + t.Error("every workspace failing must be an error") + } +} + +func TestBitbucketFinder_ReviewsSkipReviewedAndRescanEvery15Minutes(t *testing.T) { + now := time.Date(2026, 10, 31, 12, 0, 0, 0, time.UTC) + api := &fakeLister{ + repos: map[string][]string{"acme": {"api", "web"}}, + reviews: map[string][]bitbucket.ListedPR{ + "acme/api": {listed("acme", "api", 1), listed("acme", "api", 2, "{other}", "")}, + "acme/web": {listed("acme", "web", 3, me)}, // already approved by me + }, + } + f := &BitbucketFinder{API: api, User: me, Workspaces: []string{"acme"}, Now: func() time.Time { return now }} + got, err := f.Find(context.Background(), config.AutoWatchReviewRequested) + if err != nil { + t.Fatal(err) + } + if fmt.Sprint(numbers(got)) != "[1 2]" { + t.Errorf("found %v, want PRs awaiting my review only", numbers(got)) + } + + now = now.Add(BitbucketReviewScanEvery - time.Second) + f.Find(context.Background(), config.AutoWatchReviewRequested) + if api.scans != 1 { + t.Errorf("rescanned after %s: %d scans", BitbucketReviewScanEvery-time.Second, api.scans) + } + now = now.Add(time.Second) + f.Find(context.Background(), config.AutoWatchReviewRequested) + if api.scans != 2 { + t.Errorf("no rescan after %s: %d scans", BitbucketReviewScanEvery, api.scans) + } +} + +func TestBitbucketFinder_FailedRepoMakesReviewScanPartial(t *testing.T) { + api := &fakeLister{ + repos: map[string][]string{"acme": {"api", "web"}}, + reviews: map[string][]bitbucket.ListedPR{"acme/api": {listed("acme", "api", 1)}}, + errs: map[string]error{"acme/web": errors.New("boom")}, + } + f := &BitbucketFinder{API: api, User: me, Workspaces: []string{"acme"}} + got, err := f.Find(context.Background(), config.AutoWatchReviewRequested) + if err != nil || got.Partial == "" { + t.Errorf("got %+v, %v; a failed repository must make the scan partial", got, err) + } +} + +func TestSync_TwoHostsKeepTheirPRsApart(t *testing.T) { + useTempConfig(t) + saveConfig(t, &config.Config{ + AutoWatch: []string{config.AutoWatchMine, config.AutoWatchReviewRequested}, + WatchedPRs: []config.WatchedPR{ + // Same owner/repo#number on both hosts: only Bitbucket's review was withdrawn. + {Owner: "acme", Repo: "api", Number: 7, Source: config.AutoWatchReviewRequested}, + {Host: forge.Bitbucket, Owner: "acme", Repo: "api", Number: 7, Source: config.AutoWatchReviewRequested}, + }, + }) + gh := &fakeSearcher{results: map[string]*github.SearchResult{ + config.AutoWatchMine: found(1), + config.AutoWatchReviewRequested: {PRs: []github.SearchResultPR{{Owner: "acme", Repo: "api", Number: 7}}, Total: 1}, + }} + bbAPI := &fakeLister{ + mine: map[string][]bitbucket.ListedPR{"acme": {listed("acme", "api", 1)}}, + repos: map[string][]string{"acme": {"api"}}, + } + bbAPI.prs = map[string]*github.PullRequest{"acme/api/7": openReviewPR(7)} + bb := &BitbucketFinder{API: bbAPI, User: me, Workspaces: []string{"acme"}} + + res, err := Sync(context.Background(), []Finder{GitHubFinder{gh}, bb}, []string{config.AutoWatchMine, config.AutoWatchReviewRequested}) + if err != nil { + t.Fatal(err) + } + if len(res.Added) != 2 || res.Added[0].Host != "" || res.Added[1].Host != forge.Bitbucket { + t.Errorf("added = %+v, want #1 on each host", res.Added) + } + if len(res.Removed) != 1 || res.Removed[0].Host != forge.Bitbucket { + t.Errorf("removed = %+v, want only Bitbucket's #7", res.Removed) + } + cfg, _ := config.Load() + var names []string + for _, pr := range cfg.WatchedPRs { + names = append(names, DisplayPR(pr)) + } + if fmt.Sprint(names) != "[acme/api#7 owner/repo#1 bitbucket:acme/api#1]" { + t.Errorf("watch list = %v", names) + } +} + +func TestBitbucketInterval(t *testing.T) { + for n, want := range map[int]time.Duration{0: time.Minute, 3: time.Minute, 5: 90 * time.Second, 20: 6 * time.Minute, 30: 9 * time.Minute, 100: 30 * time.Minute} { + if got := BitbucketInterval(n); got != want { + t.Errorf("BitbucketInterval(%d) = %s, want %s", n, got, want) + } + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index fbb41bd..b130193 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -44,6 +44,8 @@ type Watcher struct { now func() time.Time // lastAutoWatch is when the auto-watch searches last ran. lastAutoWatch time.Time + // finders are the auto-watch finders of hosts other than GitHub (AddFinder). + finders []Finder } const noPRsMessage = "No PRs being watched. Add some with 'prw watch ', or 'prw watch --mine' to follow your open PRs." @@ -52,10 +54,26 @@ const noPRsMessage = "No PRs being watched. Add some with 'prw watch ', const defaultRateLimitPause = time.Minute // BitbucketMinInterval is the least time between two checks of a Bitbucket PR, -// whatever the poll interval: Bitbucket Cloud allows 1,000 requests per hour and a -// check costs about two, so this keeps ~8 PRs within the limit. +// whatever the poll interval. Bitbucket Cloud allows 1,000 requests per hour and a +// check costs up to three, so with more PRs each is checked less often: the checks +// get bitbucketCheckBudget requests per hour, leaving the rest to auto-watch and other +// commands. const BitbucketMinInterval = time.Minute +const ( + bitbucketCheckBudget = 600 // requests per hour + bitbucketCheckCost = 3 // PR + merge checks + build statuses +) + +// BitbucketInterval is the least time between two checks of each of n Bitbucket PRs. +func BitbucketInterval(n int) time.Duration { + d := time.Duration(n*bitbucketCheckCost) * time.Hour / bitbucketCheckBudget + if d < BitbucketMinInterval { + return BitbucketMinInterval + } + return d +} + // New creates a new Watcher. func New(client GitHubClient, cfg *config.Config, notifier notify.Notifier) *Watcher { return &Watcher{ @@ -216,18 +234,28 @@ func (w *Watcher) checkAllPRs(ctx context.Context) { } // due reports whether pr should be checked this round: not while its host is rate -// limited, and Bitbucket PRs at most every BitbucketMinInterval. +// limited, and Bitbucket PRs at most every BitbucketInterval. func (w *Watcher) due(pr config.WatchedPR) bool { host := forge.NormalizeHost(pr.Host) if w.now().Before(w.pausedUntil[host]) { return false } - if host == forge.Bitbucket && !pr.LastChecked.IsZero() && w.now().Sub(pr.LastChecked) < BitbucketMinInterval { + if host == forge.Bitbucket && !pr.LastChecked.IsZero() && w.now().Sub(pr.LastChecked) < BitbucketInterval(w.bitbucketPRs()) { return false } return true } +func (w *Watcher) bitbucketPRs() int { + n := 0 + for _, pr := range w.config.WatchedPRs { + if forge.IsBitbucket(pr.Host) { + n++ + } + } + return n +} + func (w *Watcher) pauseForRateLimit(host string, resetAt time.Time) { host = forge.NormalizeHost(host) now := w.now()