diff --git a/CHANGELOG.md b/CHANGELOG.md index 21a074e..2f6d344 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Bitbucket Cloud pull requests: `prw watch`, `run`, `wait`, `checks`, `list`, `broadcast` and the MCP tools accept `bitbucket.org///pull-requests/` URLs. CI comes from the commit's build statuses (Bitbucket Pipelines, Jenkins, ...), reviews from participants' approvals and change requests, the merge state from the PR's merge checks. Authenticate with an Atlassian API token (`bitbucket_email` + `bitbucket_token`, or `BITBUCKET_EMAIL` / `BITBUCKET_TOKEN`) or an access token alone; public repositories need none. To stay within Bitbucket's 1,000 requests/hour, `prw run` checks each Bitbucket PR at most once a minute, and rate limits pause only the service that hit them. Auto-watch, `--logs` and `prw flaky` are GitHub-only for now. + +### Changed +- `prw config set` no longer echoes token values. +- Watch-list entries, notifications (`host` in the generic webhook payload), `prw list --json`, `prw wait --json` / `prw checks --json` and MCP results carry the PR's host; GitHub entries keep an empty one, so existing config files are unchanged. + ## [0.4.0] - 2026-10-24 prw now covers the whole PR lifecycle and is built for scripts and AI agents: it detects GitHub Actions results (previously reported as `pending` forever), works with Slack and Discord webhooks, and adds `prw wait`, `prw checks`, `prw flaky`, an MCP server, auto-watching your PRs, review and merge-conflict events, and failed-job log excerpts. It reuses your `gh` login and polls with conditional requests that cost no API quota. diff --git a/README.md b/README.md index 86eb893..53a1e5f 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ The tool is deliberately minimal. It does one thing well: watch PRs and tell you - **`prw flaky`**: find a repository's flaky jobs (failed, then passed on re-run of the same commit) - **`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-cleanup**: merged/closed PRs are notified once and dropped from the watch list by `prw run` - **Watch multiple PRs** from different repositories simultaneously @@ -351,6 +352,32 @@ Tools: Results include a one-line summary for the model plus the full data as JSON (and as `structuredContent`). Authentication works like every other command — with `gh auth login` there's nothing to configure. All tools share one GitHub client, so the ETag cache keeps repeated polling free. +## Bitbucket Cloud + +prw also watches **Bitbucket Cloud** pull requests: pass their URL wherever a PR URL goes. + +```bash +prw watch https://bitbucket.org/acme/api/pull-requests/42 +prw wait https://bitbucket.org/acme/api/pull-requests/42 && ./deploy.sh +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`. + +**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: + +```bash +prw config set bitbucket_email you@example.com +prw config set bitbucket_token +# or: export BITBUCKET_EMAIL=you@example.com BITBUCKET_TOKEN= +``` + +A workspace, project or repository access token also works: set only `bitbucket_token` (it's sent as a Bearer token). No GitHub token is needed if you only watch Bitbucket PRs. + ## Configuration Configuration is stored in `~/.prw/config.json`. You can manage settings via the `config` subcommand. @@ -402,6 +429,7 @@ prw config set webhook_url https://hooks.slack.com/services/YOUR/WEBHOOK/URL - **`notification_native`**: Enable native OS notifications (true/false, default: false) - **`pr_events`**: Also notify about reviews and merge conflicts / readiness (true/false, default: true) - **`github_token`**: GitHub Personal Access Token (prefer env var `GITHUB_TOKEN`) +- **`bitbucket_token`** / **`bitbucket_email`**: Bitbucket Cloud API token and its Atlassian email, or an access token alone (env `BITBUCKET_TOKEN` / `BITBUCKET_EMAIL`) ## Notifications diff --git a/TASKS.md b/TASKS.md index d260cd8..f21a115 100644 --- a/TASKS.md +++ b/TASKS.md @@ -97,6 +97,7 @@ Legend — Impact: 1–5 · Effort: S (≤½ day), M (1–2 days), L (3+ days) - ✅ #15 Failed job log excerpt — done (`--logs` on `checks`/`wait`, MCP `get_failed_logs`; parser tested on a real staticcheck failure log; verified live on cloudposse/atmos#3476, #3422 and #2199 — a commit status — and over MCP stdio) - ✅ #16 Flakiness detection — done as `prw flaky` + MCP `get_flaky_checks` (failed-then-passed across attempts of the same run; verified live: grafana/grafana 4 flaky jobs in 300 runs, hashicorp/terraform "Unit Tests"; parallel fetching cut grafana from 24s to 9s). The "status history" half was dropped: low value for its cost. - 🚀 v0.4.0 released with everything above (PRs #6–#22). +- 🆕 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. - Next: #17 packaging (Homebrew/Scoop) + keychain token storage, #8 `gh` extension publishing — both need owner decisions ## Planned diff --git a/cmd/prw/bitbucket_test.go b/cmd/prw/bitbucket_test.go new file mode 100644 index 0000000..660bf0e --- /dev/null +++ b/cmd/prw/bitbucket_test.go @@ -0,0 +1,172 @@ +package main + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/devblac/prw/internal/bitbucket" + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" +) + +const bbPRURL = "https://bitbucket.org/acme/api/pull-requests/42" + +// fakeBitbucketAPI serves PR acme/api#42 with the given state and build statuses. +type fakeBitbucketAPI struct { + state string + statuses string + auth string +} + +func (f *fakeBitbucketAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.auth = r.Header.Get("Authorization") + switch p := r.URL.Path; { + case strings.HasSuffix(p, "/repositories/acme/api/pullrequests/42"): + fmt.Fprintf(w, `{"id": 42, "title": "Add login", "state": %q, "updated_on": "2026-01-01T00:00:00+00:00", + "source": {"commit": {"hash": "abc123def456"}}, + "participants": [{"user": {"uuid": "{a}"}, "role": "REVIEWER", "approved": true, "state": "approved"}]}`, f.state) + case strings.HasSuffix(p, "/pullrequests/42/mergeability/checks"): + fmt.Fprint(w, `{"values": [{"type": "git_mergeability_check", "status": "PASSED"}]}`) + case strings.HasSuffix(p, "/commit/abc123def456/statuses"): + fmt.Fprintf(w, `{"values": [%s]}`, f.statuses) + default: + http.NotFound(w, r) + } +} + +// setupBitbucket points Bitbucket requests at a fake API; GitHub has no token. +func setupBitbucket(t *testing.T, f *fakeBitbucketAPI, mutate func(*config.Config)) { + t.Helper() + setupWaitCommand(t, func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected GitHub request %s", r.URL.Path) + http.NotFound(w, r) + }) + cfg, _ := config.Load() + cfg.GitHubToken = "" + if mutate != nil { + mutate(cfg) + } + if err := cfg.Save(); err != nil { + t.Fatal(err) + } + t.Setenv("GITHUB_TOKEN", "") + t.Setenv("GH_TOKEN", "") + t.Setenv("BITBUCKET_TOKEN", "") + server := httptest.NewServer(f) + t.Cleanup(server.Close) + old := newBitbucketClient + newBitbucketClient = func(email, token string) *bitbucket.Client { + c := bitbucket.NewClient(email, token) + c.SetBaseURL(server.URL) + return c + } + t.Cleanup(func() { newBitbucketClient = old }) +} + +func bbStatus(name, state string) string { + return fmt.Sprintf(`{"key": %q, "name": %q, "state": %q, "url": "https://bitbucket.org/acme/api/pipelines/results/7", + "created_on": "2026-01-01T00:00:00+00:00", "updated_on": "2026-01-01T00:04:00+00:00"}`, name, name, state) +} + +func TestBitbucket_WatchListAndUnwatch(t *testing.T) { + f := &fakeBitbucketAPI{state: "OPEN", statuses: bbStatus("Pipeline - default", "INPROGRESS")} + setupBitbucket(t, f, func(c *config.Config) { c.BitbucketEmail, c.BitbucketToken = "dev@example.com", "api-token" }) + + out, err := captureStdout(func() error { return watchCmd.RunE(watchCmd, []string{bbPRURL}) }) + if err != nil || !strings.Contains(out, "Now watching: acme/api#42 - Add login") { + t.Fatalf("watch: %v\n%s", err, out) + } + if !strings.HasPrefix(f.auth, "Basic ") { + t.Errorf("the API token must be sent: Authorization = %q", f.auth) + } + cfg, _ := config.Load() + if len(cfg.WatchedPRs) != 1 || cfg.WatchedPRs[0].Host != forge.Bitbucket { + t.Fatalf("watch list = %+v", cfg.WatchedPRs) + } + + listJSON = false + out, _ = captureStdout(func() error { return listCmd.RunE(listCmd, nil) }) + if !strings.Contains(out, "bitbucket:acme/api") { + t.Errorf("list must show the host:\n%s", out) + } + + // The same owner/repo#number on GitHub is a different PR. + captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, []string{"https://github.com/acme/api/pull/42"}) }) + if cfg, _ := config.Load(); len(cfg.WatchedPRs) != 1 { + t.Error("unwatching the GitHub PR must not remove the Bitbucket one") + } + captureStdout(func() error { return unwatchCmd.RunE(unwatchCmd, []string{bbPRURL}) }) + if cfg, _ := config.Load(); len(cfg.WatchedPRs) != 0 { + t.Errorf("watch list = %+v", cfg.WatchedPRs) + } +} + +func TestBitbucket_Wait(t *testing.T) { + f := &fakeBitbucketAPI{state: "OPEN", statuses: bbStatus("Pipeline - default", "FAILED") + "," + bbStatus("Jenkins", "SUCCESSFUL")} + setupBitbucket(t, f, nil) + waitJSON = true + + out, err := captureStdout(func() error { return waitCmd.RunE(waitCmd, []string{bbPRURL}) }) + if code := exitCode(err); code != exitWaitFailed { + t.Fatalf("exit code = %d (err=%v)\n%s", code, err, out) + } + var result waitResult + if err := json.Unmarshal([]byte(out), &result); err != nil { + t.Fatalf("%v\n%s", err, out) + } + if result.Host != forge.Bitbucket || result.URL != bbPRURL || result.State != "failure" || result.MergeState != "ready" || + len(result.FailedChecks) != 1 || result.FailedChecks[0].Name != "Pipeline - default" { + t.Errorf("result = %+v", result) + } +} + +func TestBitbucket_Checks(t *testing.T) { + f := &fakeBitbucketAPI{state: "OPEN", statuses: bbStatus("Pipeline - default", "SUCCESSFUL")} + setupBitbucket(t, f, nil) + checksJSON, checksLogs = false, false + + out, err := captureStdout(func() error { return checksCmd.RunE(checksCmd, []string{bbPRURL}) }) + if err != nil { + t.Fatalf("%v\n%s", err, out) + } + if !strings.Contains(out, "✅ pass Pipeline - default 4m00s https://bitbucket.org/acme/api/pipelines/results/7") { + t.Errorf("stdout:\n%s", out) + } +} + +func TestBitbucket_RunNotifiesWithBitbucketLinks(t *testing.T) { + f := &fakeBitbucketAPI{state: "OPEN", statuses: bbStatus("Pipeline - default", "FAILED")} + setupBitbucket(t, f, func(c *config.Config) { + c.WatchedPRs = []config.WatchedPR{{Host: forge.Bitbucket, Owner: "acme", Repo: "api", Number: 42, LastKnownState: "pending"}} + }) + runOnce = true + t.Cleanup(func() { runOnce = false }) + + out, err := captureStdout(func() error { return runCmd.RunE(runCmd, nil) }) + if err != nil { + t.Fatalf("run without a GitHub token must work for Bitbucket-only watch lists: %v", err) + } + if !strings.Contains(out, "Status: pending → failure") || !strings.Contains(out, "Link: "+bbPRURL) { + t.Errorf("stdout:\n%s", out) + } +} + +func TestBitbucket_PrivateRepoHint(t *testing.T) { + setupBitbucket(t, &fakeBitbucketAPI{state: "OPEN"}, nil) + _, err := captureStdout(func() error { + return watchCmd.RunE(watchCmd, []string{"https://bitbucket.org/acme/private/pull-requests/1"}) + }) + if err == nil || !strings.Contains(err.Error(), "prw config set bitbucket_token") { + t.Errorf("err = %v; a 404 on Bitbucket must explain how to authenticate", err) + } +} + +func TestBitbucket_FlakyIsGitHubOnly(t *testing.T) { + if _, _, err := parseRepoArg("https://bitbucket.org/acme/api/pull-requests/42"); err == nil || !strings.Contains(err.Error(), "GitHub Actions") { + t.Errorf("err = %v", err) + } +} diff --git a/cmd/prw/broadcast.go b/cmd/prw/broadcast.go index 312a463..f5a16f3 100644 --- a/cmd/prw/broadcast.go +++ b/cmd/prw/broadcast.go @@ -9,6 +9,7 @@ import ( "github.com/spf13/cobra" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" "github.com/devblac/prw/internal/notify" ) @@ -41,11 +42,6 @@ var broadcastCmd = &cobra.Command{ return nil } - token := cfg.GetToken() - if token == "" { - return errMissingToken - } - filter := strings.ToLower(strings.TrimSpace(broadcastFilter)) if filter == "" { filter = "all" @@ -54,7 +50,7 @@ var broadcastCmd = &cobra.Command{ return fmt.Errorf("invalid --filter value %q (expected all, changed, or failing)", broadcastFilter) } - client := newGitHubClient(token) + clients := map[string]prClient{} webhookURL := broadcastWebhook if webhookURL == "" { @@ -71,6 +67,16 @@ var broadcastCmd = &cobra.Command{ for i := range cfg.WatchedPRs { pr := &cfg.WatchedPRs[i] + host := forge.NormalizeHost(pr.Host) + client, ok := clients[host] + if !ok { + if client, err = clientFor(cfg, host); err != nil { + fmt.Printf("Skipping %s/%s#%d: %v\n", pr.Owner, pr.Repo, pr.Number, err) + continue + } + clients[host] = client + } + ghPR, err := client.GetPullRequest(context.Background(), pr.Owner, pr.Repo, pr.Number) if err != nil { fmt.Printf("Error fetching PR %s/%s#%d: %v\n", pr.Owner, pr.Repo, pr.Number, err) @@ -112,6 +118,7 @@ var broadcastCmd = &cobra.Command{ } event := ¬ify.StatusChangeEvent{ + Host: pr.Host, Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, diff --git a/cmd/prw/checks.go b/cmd/prw/checks.go index 23d15f2..b1b27b7 100644 --- a/cmd/prw/checks.go +++ b/cmd/prw/checks.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -72,7 +73,7 @@ Exit codes: return nil }, RunE: func(cmd *cobra.Command, args []string) error { - owner, repo, number, err := github.ParsePRURL(args[0]) + ref, err := forge.ParsePRURL(args[0]) if err != nil { return waitErrorf(exitWaitError, "invalid PR URL: %v", err) } @@ -80,16 +81,16 @@ Exit codes: if err != nil { return waitErrorf(exitWaitError, "failed to load config: %v", err) } - token := cfg.GetToken() - if token == "" { - return &exitError{code: exitWaitError, err: errMissingToken} + client, err := clientFor(cfg, ref.Host) + if err != nil { + return &exitError{code: exitWaitError, err: err} } + owner, repo := ref.Owner, ref.Repo ctx := context.Background() - client := newGitHubClient(token) - report, err := fetchChecks(ctx, client, owner, repo, number, time.Now()) + report, err := fetchChecks(ctx, client, ref, time.Now()) if err != nil { - return &exitError{code: exitWaitError, err: err} + return &exitError{code: exitWaitError, err: fmt.Errorf("%w%s", err, bitbucketAuthHint(ref.Host, err))} } if checksLogs { report.Logs, err = fetchFailedLogs(ctx, client, owner, repo, report.SHA, checksLogLines) @@ -124,6 +125,7 @@ type checkEntry struct { // checksReport is the output of `prw checks`. type checksReport struct { + Host string `json:"host,omitempty"` Owner string `json:"owner"` Repo string `json:"repo"` Number int `json:"number"` @@ -139,7 +141,8 @@ type checksReport struct { // fetchChecks gets the head commit's check runs and statuses. Superseded runs of the // same check are left out, as they don't count towards the result. -func fetchChecks(ctx context.Context, client ciClient, owner, repo string, number int, now time.Time) (*checksReport, error) { +func fetchChecks(ctx context.Context, client ciClient, ref forge.Ref, now time.Time) (*checksReport, error) { + owner, repo, number := ref.Owner, ref.Repo, ref.Number pr, err := client.GetPullRequest(ctx, owner, repo, number) if err != nil { return nil, fmt.Errorf("failed to fetch PR: %w", err) @@ -158,7 +161,7 @@ func fetchChecks(ctx context.Context, client ciClient, owner, repo string, numbe github.ApplyNoCI(ci, pr.UpdatedAt, now) report := &checksReport{ Owner: owner, Repo: repo, Number: number, Title: pr.Title, SHA: sha, - URL: github.FormatPRURL(owner, repo, number), State: ci.State, MergeState: github.MergeStatus(pr), + URL: ref.URL(), Host: forge.StorageHost(ref.Host), State: ci.State, MergeState: github.MergeStatus(pr), Checks: []checkEntry{}, } if pr.IsClosed() { diff --git a/cmd/prw/checks_test.go b/cmd/prw/checks_test.go index 4d343b3..8a27199 100644 --- a/cmd/prw/checks_test.go +++ b/cmd/prw/checks_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -47,7 +48,7 @@ func checksFixture() *scriptedClient { } func TestFetchChecks_BucketsAndOrder(t *testing.T) { - report, err := fetchChecks(context.Background(), checksFixture(), "o", "r", 1, checksNow) + report, err := fetchChecks(context.Background(), checksFixture(), forge.Ref{Owner: "o", Repo: "r", Number: 1}, checksNow) if err != nil { t.Fatal(err) } @@ -68,7 +69,7 @@ func TestFetchChecks_BucketsAndOrder(t *testing.T) { } func TestPrintChecks(t *testing.T) { - report, err := fetchChecks(context.Background(), checksFixture(), "o", "r", 1, checksNow) + report, err := fetchChecks(context.Background(), checksFixture(), forge.Ref{Owner: "o", Repo: "r", Number: 1}, checksNow) if err != nil { t.Fatal(err) } diff --git a/cmd/prw/clients.go b/cmd/prw/clients.go new file mode 100644 index 0000000..efb8021 --- /dev/null +++ b/cmd/prw/clients.go @@ -0,0 +1,67 @@ +package main + +import ( + "context" + "errors" + "net/http" + + "github.com/devblac/prw/internal/bitbucket" + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" + "github.com/devblac/prw/internal/github" +) + +// prClient is everything the commands use from a code host's client. Both the +// GitHub and the Bitbucket clients implement it. +type prClient interface { + ciClient + github.LogFetcher + github.FlakyFetcher + GetReviews(ctx context.Context, owner, repo string, number int) ([]github.Review, error) +} + +// newBitbucketClient allows tests to point Bitbucket requests at a fake server. +var newBitbucketClient = func(email, token string) *bitbucket.Client { + return bitbucket.NewClient(email, token) +} + +// clientFor returns the client for PRs on host. GitHub needs a token; Bitbucket +// works anonymously for public repositories. +func clientFor(cfg *config.Config, host string) (prClient, error) { + if forge.IsBitbucket(host) { + auth, _ := cfg.ResolveBitbucketAuth() + return newBitbucketClient(auth.Email, auth.Token), nil + } + token := cfg.GetToken() + if token == "" { + return nil, errMissingToken + } + return newGitHubClient(token), nil +} + +// 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 + } + for _, pr := range cfg.WatchedPRs { + if !forge.IsBitbucket(pr.Host) { + return true + } + } + return len(cfg.WatchedPRs) == 0 // nothing yet: GitHub is the default +} + +// 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 { + var apiErr *github.APIError + if !forge.IsBitbucket(host) || !errors.As(err, &apiErr) { + return "" + } + switch apiErr.StatusCode { + case http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound: + return " (for private repositories set a Bitbucket API token: 'prw config set bitbucket_email ' and 'prw config set bitbucket_token ', or BITBUCKET_EMAIL / BITBUCKET_TOKEN)" + } + return "" +} diff --git a/cmd/prw/flaky.go b/cmd/prw/flaky.go index 213084c..1a98b26 100644 --- a/cmd/prw/flaky.go +++ b/cmd/prw/flaky.go @@ -7,6 +7,7 @@ import ( "io" "os" "regexp" + "strings" "text/tabwriter" "github.com/spf13/cobra" @@ -88,6 +89,9 @@ var ( // parseRepoArg accepts "owner/repo", a repository URL or a PR URL. func parseRepoArg(arg string) (owner, repo string, err error) { + if strings.Contains(strings.ToLower(arg), "bitbucket.org") { + return "", "", fmt.Errorf("flaky checks are only found for GitHub Actions so far, not Bitbucket") + } if m := repoSlug.FindStringSubmatch(arg); m != nil { return m[1], m[2], nil } diff --git a/cmd/prw/main.go b/cmd/prw/main.go index 7719aa9..87b7984 100644 --- a/cmd/prw/main.go +++ b/cmd/prw/main.go @@ -16,6 +16,7 @@ import ( "github.com/spf13/cobra" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" "github.com/devblac/prw/internal/notify" "github.com/devblac/prw/internal/version" @@ -140,29 +141,30 @@ Undo with 'prw unwatch --mine' / 'prw unwatch --review-requested'.`, } func watchSinglePR(prURL string) error { - owner, repo, number, err := github.ParsePRURL(prURL) + ref, err := forge.ParsePRURL(prURL) if err != nil { return fmt.Errorf("invalid PR URL: %w", err) } + owner, repo, number := ref.Owner, ref.Repo, ref.Number cfg, err := config.Load() if err != nil { return fmt.Errorf("failed to load config: %w", err) } - token := cfg.GetToken() - if token == "" { - return errMissingToken + client, err := clientFor(cfg, ref.Host) + if err != nil { + return err } // Try to fetch the PR to validate it exists and get title - client := newGitHubClient(token) pr, err := client.GetPullRequest(context.Background(), owner, repo, number) if err != nil { - return fmt.Errorf("failed to fetch PR: %w", err) + return fmt.Errorf("failed to fetch PR: %w%s", err, bitbucketAuthHint(ref.Host, err)) } watchedPR := config.WatchedPR{ + Host: forge.StorageHost(ref.Host), Owner: owner, Repo: repo, Number: number, @@ -177,7 +179,7 @@ func watchSinglePR(prURL string) error { if !added { // 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) + claimed = c.MarkManual(ref.Host, owner, repo, number) } if !added && !claimed { return config.ErrNoChanges @@ -288,6 +290,9 @@ var listCmd = &cobra.Command{ for _, pr := range cfg.WatchedPRs { repo := fmt.Sprintf("%s/%s", pr.Owner, pr.Repo) + if forge.IsBitbucket(pr.Host) { + repo = "bitbucket:" + repo + } prNum := fmt.Sprintf("#%d", pr.Number) status := pr.LastKnownState if status == "" { @@ -340,14 +345,15 @@ finds it again; use --mine / --review-requested to stop following those PRs.`, } func unwatchSinglePR(prURL string) error { - owner, repo, number, err := github.ParsePRURL(prURL) + ref, err := forge.ParsePRURL(prURL) if err != nil { return fmt.Errorf("invalid PR URL: %w", err) } + owner, repo, number := ref.Owner, ref.Repo, ref.Number removed := false if err := config.Update(func(c *config.Config) error { - removed = c.RemovePR(owner, repo, number) + removed = c.RemovePR(ref.Host, owner, repo, number) if !removed { return config.ErrNoChanges } @@ -411,13 +417,14 @@ Press Ctrl+C to stop.`, return fmt.Errorf("failed to load config: %w", err) } - token := cfg.GetToken() - if token == "" { + // GitHub needs a token; without one, only Bitbucket PRs can be watched. + var client watcher.GitHubClient + if token := cfg.GetToken(); token != "" { + client = newGitHubClient(token) + } else if needsGitHub(cfg) { return errMissingToken } - client := newGitHubClient(token) - // Build notifier chain notifiers := []notify.Notifier{notify.NewConsoleNotifier()} if cfg.WebhookURL != "" { @@ -439,6 +446,8 @@ Press Ctrl+C to stop.`, cfg.NotificationFilter = filter w := watcher.New(client, cfg, notifier) + bbAuth, _ := cfg.ResolveBitbucketAuth() + w.SetClient(forge.Bitbucket, newBitbucketClient(bbAuth.Email, bbAuth.Token)) // Setup signal handling ctx, cancel := context.WithCancel(context.Background()) @@ -488,6 +497,15 @@ var configShowCmd = &cobra.Command{ _, tokenSource := cfg.ResolveToken() fmt.Printf("github_token: %s\n", tokenSource) + bbAuth, bbSource := cfg.ResolveBitbucketAuth() + switch { + case bbAuth.Token == "": + fmt.Printf("bitbucket_token: %s (public repositories only)\n", bbSource) + case bbAuth.Email != "": + fmt.Printf("bitbucket_token: %s, API token for %s\n", bbSource, bbAuth.Email) + default: + fmt.Printf("bitbucket_token: %s, access token\n", bbSource) + } autoWatch := "off" if len(cfg.AutoWatch) > 0 { @@ -510,6 +528,8 @@ Supported keys: - webhook_url: URL to POST notifications to - webhook_format: auto, slack, discord, or generic (default: auto, detected from the URL) - github_token: GitHub personal access token + - bitbucket_token: Bitbucket Cloud API token (with bitbucket_email), or an access token alone + - bitbucket_email: the Atlassian account email that goes with an API token - notification_filter: change, fail, or success - notification_native: enable native OS notifications (true/false) - pr_events: notify about reviews and merge conflicts / readiness, not just CI (true/false, default: true)`, @@ -524,7 +544,11 @@ Supported keys: return err } - fmt.Printf("Set %s = %s\n", key, value) + if strings.HasSuffix(key, "_token") { + fmt.Printf("Set %s\n", key) // don't echo secrets into terminal scrollback + } else { + fmt.Printf("Set %s = %s\n", key, value) + } return nil }, } @@ -547,6 +571,10 @@ func applyConfigSet(cfg *config.Config, key, value string) error { cfg.WebhookFormat = format case "github_token": cfg.GitHubToken = value + case "bitbucket_token": + cfg.BitbucketToken = value + case "bitbucket_email": + cfg.BitbucketEmail = value case "notification_filter": filter := config.NormalizeNotificationFilter(value) if !config.IsValidNotificationFilter(filter) { @@ -599,6 +627,10 @@ func applyConfigUnset(cfg *config.Config, key string) error { cfg.WebhookFormat = "" case "github_token": cfg.GitHubToken = "" + case "bitbucket_token": + cfg.BitbucketToken = "" + case "bitbucket_email": + cfg.BitbucketEmail = "" case "notification_filter": cfg.NotificationFilter = config.NotificationFilterChange case "notification_native": @@ -626,6 +658,7 @@ func init() { } type listPROutput struct { + Host string `json:"host,omitempty"` // "bitbucket.org" for Bitbucket; empty for GitHub Owner string `json:"owner"` Repo string `json:"repo"` Number int `json:"number"` @@ -660,6 +693,7 @@ func outputJSONList(prs []config.WatchedPR) error { lastChecked = &t } output = append(output, listPROutput{ + Host: forge.StorageHost(pr.Host), Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, diff --git a/cmd/prw/mcp.go b/cmd/prw/mcp.go index a88173a..92078b8 100644 --- a/cmd/prw/mcp.go +++ b/cmd/prw/mcp.go @@ -18,6 +18,7 @@ import ( "github.com/spf13/cobra" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" "github.com/devblac/prw/internal/version" ) @@ -76,23 +77,19 @@ Authentication is the same as for every other command (gh auth login, GITHUB_TOK }, } -// mcpClient is what the MCP tools need from GitHub. +// mcpClient is what the MCP tools need from a code host (GitHub or Bitbucket). type mcpClient interface { ciClient github.LogFetcher github.FlakyFetcher } -func defaultMCPClient() (mcpClient, error) { +func defaultMCPClient(host string) (mcpClient, error) { cfg, err := config.Load() if err != nil { return nil, fmt.Errorf("failed to load config: %w", err) } - token := cfg.GetToken() - if token == "" { - return nil, errMissingToken - } - return newGitHubClient(token), nil + return clientFor(cfg, host) } type rpcRequest struct { @@ -127,16 +124,16 @@ type mcpServer struct { writeMu sync.Mutex - clientFactory func() (mcpClient, error) + clientFactory func(host string) (mcpClient, error) clientMu sync.Mutex - client mcpClient // created on first use and shared, so the ETag cache is too + clients map[string]mcpClient // per host, created on first use and shared, so the ETag cache is too inflightMu sync.Mutex inflight map[string]context.CancelFunc calls sync.WaitGroup } -func newMCPServer(in io.Reader, out, logOut io.Writer, clientFactory func() (mcpClient, error)) *mcpServer { +func newMCPServer(in io.Reader, out, logOut io.Writer, clientFactory func(host string) (mcpClient, error)) *mcpServer { return &mcpServer{ in: in, out: out, @@ -320,17 +317,22 @@ func (s *mcpServer) startToolCall(ctx context.Context, req rpcRequest) { // githubClient returns the shared GitHub client, creating it on first use so a missing // token is reported as a tool error instead of preventing the server from starting. -func (s *mcpServer) githubClient() (mcpClient, error) { +// clientFor returns the shared client for PRs on host. +func (s *mcpServer) clientFor(host string) (mcpClient, error) { + host = forge.NormalizeHost(host) s.clientMu.Lock() defer s.clientMu.Unlock() - if s.client != nil { - return s.client, nil + if c, ok := s.clients[host]; ok { + return c, nil } - c, err := s.clientFactory() + c, err := s.clientFactory(host) if err != nil { return nil, err } - s.client = c + if s.clients == nil { + s.clients = map[string]mcpClient{} + } + s.clients[host] = c return c, nil } @@ -563,21 +565,22 @@ type prURLArgs struct { PRURL string `json:"pr_url"` } -func parsePRArg(args json.RawMessage) (owner, repo string, number int, err error) { +// parsePRArg reads the pr_url argument: a GitHub or Bitbucket Cloud PR URL. +func parsePRArg(args json.RawMessage) (host, owner, repo string, number int, err error) { var a prURLArgs if len(args) > 0 { if err := json.Unmarshal(args, &a); err != nil { - return "", "", 0, fmt.Errorf("invalid arguments: %v", err) + return "", "", "", 0, fmt.Errorf("invalid arguments: %v", err) } } if strings.TrimSpace(a.PRURL) == "" { - return "", "", 0, errors.New("pr_url is required") + return "", "", "", 0, errors.New("pr_url is required") } - owner, repo, number, err = github.ParsePRURL(a.PRURL) + ref, err := forge.ParsePRURL(a.PRURL) if err != nil { - return "", "", 0, fmt.Errorf("invalid pr_url %q: %v", a.PRURL, err) + return "", "", "", 0, fmt.Errorf("invalid pr_url %q: %v", a.PRURL, err) } - return owner, repo, number, nil + return ref.Host, ref.Owner, ref.Repo, ref.Number, nil } func ciSummary(r *waitResult) string { @@ -607,15 +610,16 @@ func ciSummary(r *waitResult) string { } func toolGetCIStatus(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { - owner, repo, number, err := parsePRArg(args) + host, owner, repo, number, err := parsePRArg(args) if err != nil { return toolError("%v", err) } - client, err := s.githubClient() + client, err := s.clientFor(host) if err != nil { return toolError("%v", err) } - result := &waitResult{Owner: owner, Repo: repo, Number: number, URL: github.FormatPRURL(owner, repo, number)} + ref := forge.Ref{Host: host, Owner: owner, Repo: repo, Number: number} + result := &waitResult{Host: forge.StorageHost(host), Owner: owner, Repo: repo, Number: number, URL: ref.URL()} if _, err := pollOnce(ctx, client, result); err != nil { return toolError("failed to get CI status of %s/%s#%d: %v", owner, repo, number, err) } @@ -623,7 +627,7 @@ func toolGetCIStatus(ctx context.Context, s *mcpServer, args json.RawMessage, _ } func toolWaitForCI(ctx context.Context, s *mcpServer, args json.RawMessage, progress io.Writer) mcpToolResult { - owner, repo, number, err := parsePRArg(args) + host, owner, repo, number, err := parsePRArg(args) if err != nil { return toolError("%v", err) } @@ -652,14 +656,14 @@ func toolWaitForCI(ctx context.Context, s *mcpServer, args json.RawMessage, prog interval = mcpMinWaitInterval } - client, err := s.githubClient() + client, err := s.clientFor(host) if err != nil { return toolError("%v", err) } waitCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - result, err := waitForCI(waitCtx, client, owner, repo, number, interval, progress) + result, err := waitForCI(waitCtx, client, forge.Ref{Host: host, Owner: owner, Repo: repo, Number: number}, interval, progress) if err != nil { return toolError("failed waiting for CI of %s/%s#%d: %v", owner, repo, number, err) } @@ -667,7 +671,7 @@ func toolWaitForCI(ctx context.Context, s *mcpServer, args json.RawMessage, prog } func toolGetFailedLogs(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { - owner, repo, number, err := parsePRArg(args) + host, owner, repo, number, err := parsePRArg(args) if err != nil { return toolError("%v", err) } @@ -685,7 +689,7 @@ func toolGetFailedLogs(ctx context.Context, s *mcpServer, args json.RawMessage, maxLines = maxMCPLogLines } - client, err := s.githubClient() + client, err := s.clientFor(host) if err != nil { return toolError("%v", err) } @@ -725,7 +729,7 @@ func toolGetFlakyChecks(ctx context.Context, s *mcpServer, args json.RawMessage, if err != nil { return toolError("%v", err) } - client, err := s.githubClient() + client, err := s.clientFor(forge.GitHub) if err != nil { return toolError("%v", err) } @@ -754,6 +758,7 @@ func flakySummary(r *github.FlakyReport) string { type watchedPROutput struct { URL string `json:"url"` + Host string `json:"host"` Owner string `json:"owner"` Repo string `json:"repo"` Number int `json:"number"` @@ -776,7 +781,7 @@ func toolListWatched(ctx context.Context, s *mcpServer, _ json.RawMessage, _ io. state = "unknown" } out := watchedPROutput{ - URL: github.FormatPRURL(pr.Owner, pr.Repo, pr.Number), Owner: pr.Owner, Repo: pr.Repo, + URL: forge.PRURL(pr.Host, pr.Owner, pr.Repo, pr.Number), Host: forge.NormalizeHost(pr.Host), Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, Title: pr.Title, LastState: state, Review: pr.LastReviewState, Merge: pr.LastMergeState, } if !pr.LastChecked.IsZero() { @@ -793,11 +798,11 @@ func toolListWatched(ctx context.Context, s *mcpServer, _ json.RawMessage, _ io. } func toolWatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { - owner, repo, number, err := parsePRArg(args) + host, owner, repo, number, err := parsePRArg(args) if err != nil { return toolError("%v", err) } - client, err := s.githubClient() + client, err := s.clientFor(host) if err != nil { return toolError("%v", err) } @@ -808,7 +813,7 @@ func toolWatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.W added := false if err := config.Update(func(c *config.Config) error { - added = c.AddPR(config.WatchedPR{Owner: owner, Repo: repo, Number: number, Title: pr.Title}) + added = c.AddPR(config.WatchedPR{Host: forge.StorageHost(host), Owner: owner, Repo: repo, Number: number, Title: pr.Title}) if !added { return config.ErrNoChanges } @@ -821,17 +826,17 @@ func toolWatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.W if !added { summary = fmt.Sprintf("%s/%s#%d was already being watched.", owner, repo, number) } - return toolOK(summary, map[string]interface{}{"url": github.FormatPRURL(owner, repo, number), "title": pr.Title, "added": added}) + return toolOK(summary, map[string]interface{}{"url": forge.PRURL(host, owner, repo, number), "title": pr.Title, "added": added}) } func toolUnwatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io.Writer) mcpToolResult { - owner, repo, number, err := parsePRArg(args) + host, owner, repo, number, err := parsePRArg(args) if err != nil { return toolError("%v", err) } removed := false if err := config.Update(func(c *config.Config) error { - removed = c.RemovePR(owner, repo, number) + removed = c.RemovePR(host, owner, repo, number) if !removed { return config.ErrNoChanges } @@ -844,5 +849,5 @@ func toolUnwatchPR(ctx context.Context, s *mcpServer, args json.RawMessage, _ io if !removed { summary = fmt.Sprintf("%s/%s#%d was not being watched.", owner, repo, number) } - return toolOK(summary, map[string]interface{}{"url": github.FormatPRURL(owner, repo, number), "removed": removed}) + return toolOK(summary, map[string]interface{}{"url": forge.PRURL(host, owner, repo, number), "removed": removed}) } diff --git a/cmd/prw/mcp_test.go b/cmd/prw/mcp_test.go index 35062cc..b1ff5cc 100644 --- a/cmd/prw/mcp_test.go +++ b/cmd/prw/mcp_test.go @@ -27,7 +27,7 @@ type mcpHarness struct { mu sync.Mutex } -func startMCP(t *testing.T, factory func() (mcpClient, error)) *mcpHarness { +func startMCP(t *testing.T, factory func(string) (mcpClient, error)) *mcpHarness { t.Helper() path := filepath.Join(t.TempDir(), ".prw", "config.json") oldPath := config.ConfigPath @@ -158,8 +158,8 @@ func setSleep(t *testing.T, fn func(context.Context, time.Duration)) { t.Cleanup(func() { sleepFunc = old }) } -func staticClient(c mcpClient) func() (mcpClient, error) { - return func() (mcpClient, error) { return c, nil } +func staticClient(c mcpClient) func(string) (mcpClient, error) { + return func(string) (mcpClient, error) { return c, nil } } func successClient() *scriptedClient { @@ -286,7 +286,7 @@ func TestMCP_GetCIStatus(t *testing.T) { func TestMCP_ToolErrorsAreReportedAsIsError(t *testing.T) { tests := []struct { name string - factory func() (mcpClient, error) + factory func(string) (mcpClient, error) tool string args interface{} want string @@ -294,7 +294,7 @@ func TestMCP_ToolErrorsAreReportedAsIsError(t *testing.T) { {"missing pr_url", staticClient(successClient()), "get_ci_status", map[string]string{}, "pr_url is required"}, {"invalid pr_url", staticClient(successClient()), "wait_for_ci", map[string]string{"pr_url": "nope"}, "invalid pr_url"}, {"wrong argument type", staticClient(successClient()), "wait_for_ci", map[string]interface{}{"pr_url": "https://github.com/o/r/pull/1", "timeout_seconds": "soon"}, "invalid arguments"}, - {"missing token", func() (mcpClient, error) { return nil, errMissingToken }, "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "gh auth login"}, + {"missing token", func(string) (mcpClient, error) { return nil, errMissingToken }, "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "gh auth login"}, {"API error", staticClient(&scriptedClient{steps: []pollStep{{prErr: &github.APIError{StatusCode: 404, Body: "Not Found"}}}}), "get_ci_status", map[string]string{"pr_url": "https://github.com/o/r/pull/1"}, "404"}, } for _, tt := range tests { @@ -487,7 +487,7 @@ func TestMCP_WatchListTools(t *testing.T) { func TestMCP_SharesOneGitHubClient(t *testing.T) { created := 0 - h := startMCP(t, func() (mcpClient, error) { + h := startMCP(t, func(string) (mcpClient, error) { created++ return successClient(), nil }) diff --git a/cmd/prw/noci_test.go b/cmd/prw/noci_test.go index 221fd01..e9dd02b 100644 --- a/cmd/prw/noci_test.go +++ b/cmd/prw/noci_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -22,7 +23,7 @@ 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{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -42,7 +43,7 @@ func TestWaitForCI_WaitsForCIToRegisterAfterAPush(t *testing.T) { {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{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } diff --git a/cmd/prw/wait.go b/cmd/prw/wait.go index f090916..3c7de8f 100644 --- a/cmd/prw/wait.go +++ b/cmd/prw/wait.go @@ -15,6 +15,7 @@ import ( "github.com/spf13/cobra" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -106,10 +107,11 @@ GitHub Actions job (failed_logs in --json).`, return nil }, RunE: func(cmd *cobra.Command, args []string) error { - owner, repo, number, err := github.ParsePRURL(args[0]) + ref, err := forge.ParsePRURL(args[0]) if err != nil { return waitErrorf(exitWaitError, "invalid PR URL: %v", err) } + owner, repo := ref.Owner, ref.Repo if waitInterval <= 0 { return waitErrorf(exitWaitError, "--interval must be positive") } @@ -121,9 +123,9 @@ GitHub Actions job (failed_logs in --json).`, if err != nil { return waitErrorf(exitWaitError, "failed to load config: %v", err) } - token := cfg.GetToken() - if token == "" { - return &exitError{code: exitWaitError, err: errMissingToken} + client, err := clientFor(cfg, ref.Host) + if err != nil { + return &exitError{code: exitWaitError, err: err} } ctx, cancel := context.WithCancel(context.Background()) @@ -143,10 +145,9 @@ GitHub Actions job (failed_logs in --json).`, } }() - client := newGitHubClient(token) - result, err := waitForCI(ctx, client, owner, repo, number, waitInterval, os.Stderr) + result, err := waitForCI(ctx, client, ref, waitInterval, os.Stderr) if err != nil { - return &exitError{code: exitWaitError, err: err} + return &exitError{code: exitWaitError, err: fmt.Errorf("%w%s", err, bitbucketAuthHint(ref.Host, err))} } if waitLogs && (result.State == "failure" || result.State == "error") { // A fresh context: the wait's may be about to time out. @@ -169,6 +170,7 @@ GitHub Actions job (failed_logs in --json).`, // waitResult is the outcome of waitForCI; it is also the --json output. type waitResult struct { + Host string `json:"host,omitempty"` // "bitbucket.org" for Bitbucket; empty for GitHub Owner string `json:"owner"` Repo string `json:"repo"` Number int `json:"number"` @@ -220,9 +222,10 @@ type ciClient interface { // waitForCI polls until the PR's CI finishes, the PR is merged or closed, or ctx is // done (timeout or interrupt). It only returns an error for non-transient failures. -func waitForCI(ctx context.Context, client ciClient, owner, repo string, number int, interval time.Duration, progress io.Writer) (*waitResult, error) { +func waitForCI(ctx context.Context, client ciClient, ref forge.Ref, interval time.Duration, progress io.Writer) (*waitResult, error) { + owner, repo, number := ref.Owner, ref.Repo, ref.Number start := time.Now() - result := &waitResult{Owner: owner, Repo: repo, Number: number, URL: github.FormatPRURL(owner, repo, number), State: "pending"} + result := &waitResult{Host: forge.StorageHost(ref.Host), Owner: owner, Repo: repo, Number: number, URL: ref.URL(), State: "pending"} finish := func() *waitResult { result.ElapsedSeconds = time.Since(start).Seconds() return result diff --git a/cmd/prw/wait_ctx_test.go b/cmd/prw/wait_ctx_test.go index e2db5a7..dcc5e9a 100644 --- a/cmd/prw/wait_ctx_test.go +++ b/cmd/prw/wait_ctx_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -22,7 +23,7 @@ func TestWaitForCI_FollowsPushThatLandsDuringThePoll(t *testing.T) { {pr: openPR("new"), runs: []github.CheckRun{run("Test", "completed", "failure")}}, }} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -35,7 +36,7 @@ func TestWaitForCI_ConfirmsHeadBeforeReturning(t *testing.T) { noSleep(t) client := &scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "completed", "success")}}}} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -55,7 +56,7 @@ func TestWaitForCI_RetriesWhenTheHeadCheckFails(t *testing.T) { {pr: openPR("new"), runs: []github.CheckRun{run("Test", "completed", "failure")}}, }} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -73,7 +74,7 @@ func TestWaitForCI_InterruptDuringTheHeadCheck(t *testing.T) { revalidate: func() error { cancel(); return ctx.Err() }, // Ctrl-C while it's in flight }}} - result, err := waitForCI(ctx, client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(ctx, client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -104,7 +105,7 @@ func TestWaitForCI_TimeoutCancelsTheRequestInFlight(t *testing.T) { defer cancel() start := time.Now() - result, err := waitForCI(ctx, hangingClient(t), "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(ctx, hangingClient(t), forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -122,7 +123,7 @@ func TestWaitForCI_InterruptCancelsTheRequestInFlight(t *testing.T) { time.AfterFunc(100*time.Millisecond, cancel) // Ctrl-C start := time.Now() - result, err := waitForCI(ctx, hangingClient(t), "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(ctx, hangingClient(t), forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } diff --git a/cmd/prw/wait_test.go b/cmd/prw/wait_test.go index ba46fc3..8dafede 100644 --- a/cmd/prw/wait_test.go +++ b/cmd/prw/wait_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -149,7 +150,7 @@ func TestWaitForCI_PendingThenSuccess(t *testing.T) { }} var progress bytes.Buffer - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &progress) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &progress) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -178,7 +179,7 @@ func TestWaitForCI_FailsFast(t *testing.T) { {pr: openPR("sha"), runs: []github.CheckRun{run("Lint", "completed", "failure"), run("E2E", "in_progress", "")}}, }} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -199,7 +200,7 @@ func TestWaitForCI_LegacyErrorExitsAsFailure(t *testing.T) { {pr: openPR("sha"), status: &github.CombinedStatus{State: "error", TotalCount: 1, Statuses: []github.CommitStatus{{Context: "ci/jenkins", State: "error"}}}}, }} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -217,7 +218,7 @@ func TestWaitForCI_MergedWhileWaiting(t *testing.T) { {pr: merged}, }} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -235,7 +236,7 @@ func TestWaitForCI_ClosedWithoutMerge(t *testing.T) { closed.State = "closed" client := &scriptedClient{steps: []pollStep{{pr: closed}}} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -251,7 +252,7 @@ func TestWaitForCI_FollowsNewHeadCommit(t *testing.T) { {pr: openPR("new1111"), runs: []github.CheckRun{run("Test", "completed", "success")}}, }} - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -269,7 +270,7 @@ func TestWaitForCI_Timeout(t *testing.T) { defer cancel() client := &scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "in_progress", "")}}}} - result, err := waitForCI(ctx, client, "o", "r", 1, time.Hour, &bytes.Buffer{}) + result, err := waitForCI(ctx, client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Hour, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -288,7 +289,7 @@ func TestWaitForCI_Interrupted(t *testing.T) { defer func() { sleepFunc = old }() client := &scriptedClient{steps: []pollStep{{pr: openPR("sha"), runs: []github.CheckRun{run("Test", "queued", "")}}}} - result, err := waitForCI(ctx, client, "o", "r", 1, time.Hour, &bytes.Buffer{}) + result, err := waitForCI(ctx, client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Hour, &bytes.Buffer{}) if err != nil { t.Fatal(err) } @@ -306,7 +307,7 @@ func TestWaitForCI_RetriesTransientErrors(t *testing.T) { }} var progress bytes.Buffer - result, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &progress) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &progress) if err != nil { t.Fatalf("transient errors should be retried, got %v", err) } @@ -332,7 +333,7 @@ func TestWaitForCI_WaitsForRateLimitResetWithoutGivingUp(t *testing.T) { }} var progress bytes.Buffer - result, err := waitForCI(context.Background(), client, "o", "r", 1, 15*time.Second, &progress) + result, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, 15*time.Second, &progress) if err != nil { t.Fatalf("rate limits must not count as errors, got %v", err) } @@ -357,7 +358,7 @@ func TestWaitForCI_RateLimitWithoutResetUsesInterval(t *testing.T) { {prErr: &github.APIError{StatusCode: 429, RateLimited: true}}, {pr: openPR("sha"), runs: []github.CheckRun{run("Test", "completed", "success")}}, }} - if _, err := waitForCI(context.Background(), client, "o", "r", 1, 15*time.Second, &bytes.Buffer{}); err != nil { + if _, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, 15*time.Second, &bytes.Buffer{}); err != nil { t.Fatal(err) } if len(delays) != 1 || delays[0] != 15*time.Second { @@ -369,7 +370,7 @@ func TestWaitForCI_GivesUpAfterConsecutiveTransientErrors(t *testing.T) { noSleep(t) client := &scriptedClient{steps: []pollStep{{prErr: errors.New("no route to host")}}} - _, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}) + _, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}) if err == nil || !strings.Contains(err.Error(), "3 consecutive errors") { t.Errorf("expected to give up after 3 errors, got %v", err) } @@ -388,7 +389,7 @@ func TestWaitForCI_PermanentErrorsAreNotRetried(t *testing.T) { t.Run(tt.name, func(t *testing.T) { noSleep(t) client := &scriptedClient{steps: []pollStep{tt.step}} - if _, err := waitForCI(context.Background(), client, "o", "r", 1, time.Second, &bytes.Buffer{}); err == nil { + if _, err := waitForCI(context.Background(), client, forge.Ref{Owner: "o", Repo: "r", Number: 1}, time.Second, &bytes.Buffer{}); err == nil { t.Fatal("expected an error") } if client.polls != 1 { diff --git a/internal/bitbucket/client.go b/internal/bitbucket/client.go new file mode 100644 index 0000000..449c1c2 --- /dev/null +++ b/internal/bitbucket/client.go @@ -0,0 +1,309 @@ +// Package bitbucket is prw's Bitbucket Cloud client. It answers in the types of the +// github package (pull requests, check runs, reviews), so the watcher, `prw wait`, +// `prw checks` and the MCP server work on Bitbucket pull requests unchanged: +// +// - a pull request's source commit is its head; OPEN is open, MERGED is merged, +// DECLINED and SUPERSEDED are closed; +// - build statuses (Bitbucket Pipelines, Jenkins, ...) become check runs, with +// their link and duration; +// - participants who approved or requested changes become reviews; +// - the merge checks become the merge state (conflict, blocked, ready). +package bitbucket + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "net/url" + "strings" + "sync" + "time" + + "github.com/devblac/prw/internal/github" +) + +// DefaultBaseURL is the Bitbucket Cloud REST API. +const DefaultBaseURL = "https://api.bitbucket.org/2.0" + +const ( + statusesPageLen = 100 + maxStatusPages = 5 + // mergeabilityTTL is how long a PR's merge checks are reused while the PR itself + // is unchanged. Bitbucket allows 1,000 requests per hour, and the base branch can + // move (creating conflicts) without the PR being updated, so they're re-read + // periodically rather than on every poll. + mergeabilityTTL = 2 * time.Minute + // participantsTTL bounds how old the PR read by GetPullRequest may be for + // GetReviews to reuse its participants instead of fetching the PR again. + participantsTTL = time.Minute +) + +// ErrUnsupported is returned for features prw only has for GitHub so far. +var ErrUnsupported = errors.New("not supported for Bitbucket yet") + +// Client talks to the Bitbucket Cloud API. It is safe for concurrent use. +type Client struct { + http *github.Client + now func() time.Time + + mu sync.Mutex + prs map[string]cachedPR + mergeability map[string]cachedMergeability +} + +type cachedPR struct { + pr *pullRequest + fetchedAt time.Time +} + +type cachedMergeability struct { + state string + updatedOn time.Time // the PR's updated_on when the checks were read + fetchedAt time.Time +} + +// NewClient returns a client authenticated with an Atlassian API token and the +// account's email (Basic auth), or, without an email, with a workspace / project / +// repository access token (Bearer). With no token it is anonymous, which only works +// for public repositories. +func NewClient(email, token string) *Client { + h := github.NewClient("") + h.BaseURL = DefaultBaseURL + h.Accept = "application/json" + h.Service = "Bitbucket" + switch { + case token != "" && email != "": + h.AuthHeader = "Basic " + base64.StdEncoding.EncodeToString([]byte(email+":"+token)) + case token != "": + h.AuthHeader = "Bearer " + token + } + return &Client{http: h, now: time.Now, prs: map[string]cachedPR{}, mergeability: map[string]cachedMergeability{}} +} + +// SetBaseURL points the client at another server (tests). +func (c *Client) SetBaseURL(base string) { c.http.BaseURL = strings.TrimRight(base, "/") } + +// RateLimit returns the quota reported by the most recent response, if any. +func (c *Client) RateLimit() github.RateLimit { return c.http.RateLimit() } + +type pullRequest struct { + ID int `json:"id"` + Title string `json:"title"` + State string `json:"state"` // OPEN, MERGED, DECLINED, SUPERSEDED + UpdatedOn time.Time `json:"updated_on"` + Source struct { + Commit struct { + Hash string `json:"hash"` + } `json:"commit"` + } `json:"source"` + Participants []struct { + User struct { + UUID string `json:"uuid"` + DisplayName string `json:"display_name"` + Nickname string `json:"nickname"` + } `json:"user"` + Role string `json:"role"` + Approved bool `json:"approved"` + State string `json:"state"` // approved, changes_requested, or empty + } `json:"participants"` +} + +func repoPath(workspace, repo string) string { + return "/repositories/" + url.PathEscape(workspace) + "/" + url.PathEscape(repo) +} + +func prKey(workspace, repo string, number int) string { + return strings.ToLower(workspace) + "/" + strings.ToLower(repo) + "#" + fmt.Sprint(number) +} + +func (c *Client) fetchPR(ctx context.Context, workspace, repo string, number int) (*pullRequest, error) { + var pr pullRequest + if err := c.http.GetJSON(ctx, fmt.Sprintf("%s/pullrequests/%d", repoPath(workspace, repo), number), &pr); err != nil { + return nil, err + } + c.mu.Lock() + c.prs[prKey(workspace, repo, number)] = cachedPR{pr: &pr, fetchedAt: c.now()} + c.mu.Unlock() + return &pr, nil +} + +// GetPullRequest fetches a pull request, with its merge state. +func (c *Client) GetPullRequest(ctx context.Context, workspace, repo string, number int) (*github.PullRequest, error) { + bb, err := c.fetchPR(ctx, workspace, repo, number) + if err != nil { + return nil, err + } + pr := &github.PullRequest{Number: bb.ID, Title: bb.Title, UpdatedAt: bb.UpdatedOn} + pr.Head.SHA = bb.Source.Commit.Hash + switch strings.ToUpper(bb.State) { + case "OPEN": + pr.State = "open" + pr.MergeableState = c.mergeState(ctx, workspace, repo, number, bb.UpdatedOn) + case "MERGED": + pr.State, pr.Merged = "closed", true + default: // DECLINED, SUPERSEDED + pr.State = "closed" + } + return pr, nil +} + +type mergeCheck struct { + Type string `json:"type"` + Status string `json:"status"` // PASSED, FAILED, ... + Blocking bool `json:"blocking"` + Reason string `json:"reason"` +} + +// mergeState maps the PR's merge checks to a GitHub mergeable_state (see +// github.MergeStatus): "dirty" when git reports conflicts, "blocked" while a blocking +// merge check (approvals, successful builds, open tasks, ...) fails, "clean" otherwise. +// Whether the caller may merge (current_user_permission_check) is ignored: it's about +// who asks, not about the PR. Errors leave it unknown (""). +func (c *Client) mergeState(ctx context.Context, workspace, repo string, number int, updatedOn time.Time) string { + key := prKey(workspace, repo, number) + c.mu.Lock() + cached, ok := c.mergeability[key] + c.mu.Unlock() + if ok && cached.updatedOn.Equal(updatedOn) && c.now().Sub(cached.fetchedAt) < mergeabilityTTL { + return cached.state + } + + var resp struct { + Values []mergeCheck `json:"values"` + } + path := fmt.Sprintf("%s/pullrequests/%d/mergeability/checks", repoPath(workspace, repo), number) + if err := c.http.GetJSON(ctx, path, &resp); err != nil { + return "" + } + state := "clean" + for _, check := range resp.Values { + failed := strings.EqualFold(check.Status, "FAILED") + switch { + case check.Type == "git_mergeability_check" && failed: + state = "dirty" + case check.Type == "current_user_permission_check": + case failed && check.Blocking && state != "dirty": + state = "blocked" + } + } + c.mu.Lock() + c.mergeability[key] = cachedMergeability{state: state, updatedOn: updatedOn, fetchedAt: c.now()} + c.mu.Unlock() + return state +} + +// GetCombinedStatus returns no legacy statuses: Bitbucket's build statuses are all +// reported as check runs by GetCheckRuns. It makes no request. +func (c *Client) GetCombinedStatus(ctx context.Context, workspace, repo, ref string) (*github.CombinedStatus, error) { + return &github.CombinedStatus{State: "pending", SHA: ref}, nil +} + +type buildStatus struct { + Key string `json:"key"` + Name string `json:"name"` + State string `json:"state"` // SUCCESSFUL, FAILED, INPROGRESS, STOPPED + URL string `json:"url"` + CreatedOn time.Time `json:"created_on"` + UpdatedOn time.Time `json:"updated_on"` +} + +// GetCheckRuns returns the commit's build statuses (Bitbucket Pipelines, Jenkins, ...) +// as check runs. A status is updated in place when its build reruns, so each key +// appears once; IDs grow with the last update so the newest wins on duplicate names. +func (c *Client) GetCheckRuns(ctx context.Context, workspace, repo, ref string) ([]github.CheckRun, error) { + var runs []github.CheckRun + path := fmt.Sprintf("%s/commit/%s/statuses?pagelen=%d", repoPath(workspace, repo), url.PathEscape(ref), statusesPageLen) + for page := 0; page < maxStatusPages && path != ""; page++ { + var resp struct { + Values []buildStatus `json:"values"` + Next string `json:"next"` + } + if err := c.http.GetJSON(ctx, path, &resp); err != nil { + return nil, err + } + for _, s := range resp.Values { + runs = append(runs, toCheckRun(s)) + } + path = c.relative(resp.Next) + } + return runs, nil +} + +func toCheckRun(s buildStatus) github.CheckRun { + name := s.Name + if name == "" { + name = s.Key + } + run := github.CheckRun{ID: s.UpdatedOn.UnixNano(), Name: name, HTMLURL: s.URL, StartedAt: s.CreatedOn} + switch strings.ToUpper(s.State) { + case "INPROGRESS": + run.Status = "in_progress" + case "SUCCESSFUL": + run.Status, run.Conclusion, run.CompletedAt = "completed", "success", s.UpdatedOn + case "STOPPED": + run.Status, run.Conclusion, run.CompletedAt = "completed", "cancelled", s.UpdatedOn + default: // FAILED, or anything new: not a pass + run.Status, run.Conclusion, run.CompletedAt = "completed", "failure", s.UpdatedOn + } + return run +} + +// relative turns a "next" page URL into a path under the base URL ("" when done). +func (c *Client) relative(next string) string { + if next == "" { + return "" + } + if rest, ok := strings.CutPrefix(next, c.http.BaseURL); ok { + return rest + } + return "" // a link to another host: don't follow it with our credentials +} + +// GetReviews returns the current verdict of each participant who approved or +// requested changes, as reviews. It reuses the PR just read by GetPullRequest. +func (c *Client) GetReviews(ctx context.Context, workspace, repo string, number int) ([]github.Review, error) { + c.mu.Lock() + cached, ok := c.prs[prKey(workspace, repo, number)] + c.mu.Unlock() + pr := cached.pr + if !ok || c.now().Sub(cached.fetchedAt) > participantsTTL { + var err error + if pr, err = c.fetchPR(ctx, workspace, repo, number); err != nil { + return nil, err + } + } + var reviews []github.Review + for _, p := range pr.Participants { + var r github.Review + r.User.Login = p.User.UUID + if r.User.Login == "" { + r.User.Login = p.User.DisplayName + } + switch { + case strings.EqualFold(p.State, "changes_requested"): + r.State = "CHANGES_REQUESTED" + case p.Approved || strings.EqualFold(p.State, "approved"): + r.State = "APPROVED" + default: + continue + } + reviews = append(reviews, r) + } + return reviews, nil +} + +// GetJobLog is not available for Bitbucket yet. +func (c *Client) GetJobLog(ctx context.Context, workspace, repo string, jobID int64) (string, error) { + return "", ErrUnsupported +} + +// ListWorkflowRuns is GitHub Actions only (prw flaky). +func (c *Client) ListWorkflowRuns(ctx context.Context, workspace, repo string, max int) ([]github.WorkflowRun, error) { + return nil, ErrUnsupported +} + +// GetAttemptJobs is GitHub Actions only (prw flaky). +func (c *Client) GetAttemptJobs(ctx context.Context, workspace, repo string, runID int64, attempt int) ([]github.Job, error) { + return nil, ErrUnsupported +} diff --git a/internal/bitbucket/client_test.go b/internal/bitbucket/client_test.go new file mode 100644 index 0000000..9422732 --- /dev/null +++ b/internal/bitbucket/client_test.go @@ -0,0 +1,253 @@ +package bitbucket + +import ( + "context" + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/devblac/prw/internal/github" +) + +const prJSON = `{ + "id": 42, "title": "Add login", "state": %q, "updated_on": "2026-10-24T10:00:00.123456+00:00", + "source": {"commit": {"hash": "abc123def456"}}, + "participants": [ + {"user": {"uuid": "{a}", "display_name": "Ana"}, "role": "REVIEWER", "approved": true, "state": "approved"}, + {"user": {"uuid": "{b}", "display_name": "Bob"}, "role": "REVIEWER", "approved": false, "state": "changes_requested"}, + {"user": {"uuid": "{c}", "display_name": "Cid"}, "role": "PARTICIPANT", "approved": false, "state": null} + ] +}` + +type fakeBitbucket struct { + *httptest.Server + prState string + mergeChecks string + statuses []string // pages + requests map[string]*int32 + lastAuth string +} + +func newFake(t *testing.T) *fakeBitbucket { + f := &fakeBitbucket{prState: "OPEN", requests: map[string]*int32{}} + for _, k := range []string{"pr", "merge", "statuses"} { + f.requests[k] = new(int32) + } + f.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.lastAuth = r.Header.Get("Authorization") + p := r.URL.Path + switch { + case strings.HasSuffix(p, "/repositories/acme/api/pullrequests/42"): + atomic.AddInt32(f.requests["pr"], 1) + fmt.Fprintf(w, prJSON, f.prState) + case strings.HasSuffix(p, "/pullrequests/42/mergeability/checks"): + atomic.AddInt32(f.requests["merge"], 1) + if f.mergeChecks == "" { + http.NotFound(w, r) + return + } + fmt.Fprintf(w, `{"values": [%s]}`, f.mergeChecks) + case strings.HasSuffix(p, "/commit/abc123def456/statuses"): + atomic.AddInt32(f.requests["statuses"], 1) + page := 0 + fmt.Sscan(r.URL.Query().Get("page"), &page) + body := f.statuses[page] + next := "" + if page+1 < len(f.statuses) { + next = fmt.Sprintf(`, "next": "%s/repositories/acme/api/commit/abc123def456/statuses?pagelen=100&page=%d"`, f.URL, page+1) + } + fmt.Fprintf(w, `{"values": [%s]%s}`, body, next) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(f.Close) + return f +} + +func (f *fakeBitbucket) client(email, token string) *Client { + c := NewClient(email, token) + c.SetBaseURL(f.URL) + return c +} + +func status(key, name, state, created, updated string) string { + return fmt.Sprintf(`{"key": %q, "name": %q, "state": %q, "url": "https://bitbucket.org/acme/api/pipelines/results/%s", + "created_on": "2026-10-24T%s+00:00", "updated_on": "2026-10-24T%s+00:00"}`, key, name, state, key, created, updated) +} + +func TestGetPullRequest_States(t *testing.T) { + f := newFake(t) + c := f.client("", "") + for state, want := range map[string]struct { + state string + merged bool + }{ + "OPEN": {"open", false}, "MERGED": {"closed", true}, "DECLINED": {"closed", false}, "SUPERSEDED": {"closed", false}, + } { + f.prState = state + pr, err := c.GetPullRequest(context.Background(), "acme", "api", 42) + if err != nil { + t.Fatal(err) + } + if pr.State != want.state || pr.Merged != want.merged || pr.Head.SHA != "abc123def456" || pr.Title != "Add login" || pr.Number != 42 { + t.Errorf("%s → %+v", state, pr) + } + if pr.UpdatedAt.IsZero() { + t.Errorf("%s: updated_on not parsed", state) + } + } +} + +func TestGetPullRequest_MergeState(t *testing.T) { + tests := []struct { + name, checks, want string + }{ + {"clean", `{"type":"git_mergeability_check","status":"PASSED","blocking":false,"reason":"clean"}, + {"type":"current_user_permission_check","status":"FAILED","blocking":true}`, "clean"}, + {"conflict", `{"type":"git_mergeability_check","status":"FAILED","blocking":true,"reason":"conflict"}, + {"type":"standard_merge_check","status":"FAILED","blocking":true}`, "dirty"}, + {"needs approval", `{"type":"git_mergeability_check","status":"PASSED","blocking":false}, + {"type":"standard_merge_check","status":"FAILED","blocking":true}`, "blocked"}, + {"non-blocking failure", `{"type":"standard_merge_check","status":"FAILED","blocking":false}`, "clean"}, + {"unavailable", "", ""}, + } + for _, tt := range tests { + f := newFake(t) + f.mergeChecks = tt.checks + pr, err := f.client("", "").GetPullRequest(context.Background(), "acme", "api", 42) + if err != nil { + t.Fatal(err) + } + if pr.MergeableState != tt.want { + t.Errorf("%s: mergeable_state = %q, want %q", tt.name, pr.MergeableState, tt.want) + } + } +} + +func TestGetPullRequest_ReusesMergeChecksWhileThePRIsUnchanged(t *testing.T) { + f := newFake(t) + f.mergeChecks = `{"type":"git_mergeability_check","status":"PASSED"}` + c := f.client("", "") + now := time.Date(2026, 10, 24, 12, 0, 0, 0, time.UTC) + c.now = func() time.Time { return now } + + for i := 0; i < 3; i++ { + c.GetPullRequest(context.Background(), "acme", "api", 42) + } + if n := atomic.LoadInt32(f.requests["merge"]); n != 1 { + t.Errorf("merge checks read %d times in a row, want 1 (Bitbucket allows 1,000 requests/hour)", n) + } + now = now.Add(mergeabilityTTL + time.Second) + c.GetPullRequest(context.Background(), "acme", "api", 42) + if n := atomic.LoadInt32(f.requests["merge"]); n != 2 { + t.Errorf("merge checks must be re-read after %s (the base branch may have moved): %d reads", mergeabilityTTL, n) + } + f.prState = "MERGED" + c.GetPullRequest(context.Background(), "acme", "api", 42) + if n := atomic.LoadInt32(f.requests["merge"]); n != 2 { + t.Errorf("closed PRs have no merge checks to read: %d reads", n) + } +} + +func TestGetCheckRuns(t *testing.T) { + f := newFake(t) + f.statuses = []string{ + status("1", "Pipeline - default", "FAILED", "10:00:00", "10:08:30") + "," + status("2", "", "INPROGRESS", "10:05:00", "10:05:00"), + status("3", "Jenkins build", "SUCCESSFUL", "09:00:00", "09:01:00") + "," + status("4", "Pipeline - deploy", "STOPPED", "10:00:00", "10:02:00"), + } + c := f.client("", "") + runs, err := c.GetCheckRuns(context.Background(), "acme", "api", "abc123def456") + if err != nil { + t.Fatal(err) + } + var got []string + for _, r := range runs { + got = append(got, fmt.Sprintf("%s=%s/%s", r.Name, r.Status, r.Conclusion)) + } + want := "Pipeline - default=completed/failure 2=in_progress/ Jenkins build=completed/success Pipeline - deploy=completed/cancelled" + if strings.Join(got, " ") != want { + t.Errorf("runs = %s\nwant %s", strings.Join(got, " "), want) + } + if d := runs[0].CompletedAt.Sub(runs[0].StartedAt); d != 8*time.Minute+30*time.Second { + t.Errorf("duration = %s", d) + } + if !runs[1].CompletedAt.IsZero() || runs[0].HTMLURL != "https://bitbucket.org/acme/api/pipelines/results/1" { + t.Errorf("runs = %+v", runs[:2]) + } + + status, _ := c.GetCombinedStatus(context.Background(), "acme", "api", "abc123def456") + ci := github.SummarizeCI(status, runs) + if ci.State != "failure" || ci.Total != 4 || ci.Pending != 1 || len(ci.FailedChecks) != 2 { + t.Errorf("summary = %+v, want failure with the failed and the stopped build", ci) + } +} + +func TestGetCheckRuns_NoBuilds(t *testing.T) { + f := newFake(t) + f.statuses = []string{""} + c := f.client("", "") + runs, _ := c.GetCheckRuns(context.Background(), "acme", "api", "abc123def456") + status, _ := c.GetCombinedStatus(context.Background(), "acme", "api", "abc123def456") + if ci := github.SummarizeCI(status, runs); ci.State != "pending" || ci.Total != 0 { + t.Errorf("summary = %+v; no builds must look like no CI (pending with 0 checks)", ci) + } +} + +func TestGetReviews(t *testing.T) { + f := newFake(t) + c := f.client("", "") + c.GetPullRequest(context.Background(), "acme", "api", 42) + reviews, err := c.GetReviews(context.Background(), "acme", "api", 42) + if err != nil { + t.Fatal(err) + } + if len(reviews) != 2 || reviews[0].State != "APPROVED" || reviews[1].State != "CHANGES_REQUESTED" { + t.Errorf("reviews = %+v", reviews) + } + if github.ReviewDecision(reviews) != github.ReviewChangesRequested { + t.Error("a change request must win over an approval") + } + if n := atomic.LoadInt32(f.requests["pr"]); n != 1 { + t.Errorf("GetReviews must reuse the PR just read: %d PR requests", n) + } +} + +func TestAuthentication(t *testing.T) { + f := newFake(t) + f.client("dev@example.com", "api-token").GetPullRequest(context.Background(), "acme", "api", 42) + if want := "Basic " + base64.StdEncoding.EncodeToString([]byte("dev@example.com:api-token")); f.lastAuth != want { + t.Errorf("API token: Authorization = %q", f.lastAuth) + } + f.client("", "access-token").GetPullRequest(context.Background(), "acme", "api", 42) + if f.lastAuth != "Bearer access-token" { + t.Errorf("access token: Authorization = %q", f.lastAuth) + } + f.client("", "").GetPullRequest(context.Background(), "acme", "api", 42) + if f.lastAuth != "" { + t.Errorf("anonymous: Authorization = %q", f.lastAuth) + } +} + +func TestErrorsNameBitbucket(t *testing.T) { + f := newFake(t) + _, err := f.client("", "").GetPullRequest(context.Background(), "acme", "missing", 1) + if err == nil || !strings.Contains(err.Error(), "Bitbucket API returned 404") { + t.Errorf("err = %v", err) + } +} + +func TestRelativeDoesNotFollowOtherHosts(t *testing.T) { + c := NewClient("", "token") + if got := c.relative("https://evil.example.com/steal?page=2"); got != "" { + t.Errorf("relative = %q; credentials must not be sent to another host", got) + } + if got := c.relative(DefaultBaseURL + "/repositories/a/b/commit/c/statuses?page=2"); got != "/repositories/a/b/commit/c/statuses?page=2" { + t.Errorf("relative = %q", got) + } +} diff --git a/internal/config/autowatch_test.go b/internal/config/autowatch_test.go index 5c75e96..cba89fe 100644 --- a/internal/config/autowatch_test.go +++ b/internal/config/autowatch_test.go @@ -44,10 +44,10 @@ func TestEnableAndDisableAutoWatch(t *testing.T) { 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 != "" { + 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) { + 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") } } @@ -61,7 +61,7 @@ func TestPRMatchingIsCaseInsensitive(t *testing.T) { 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 { + if !c.RemovePR("", "DEVBLAC", "prw", 5) || len(c.WatchedPRs) != 0 { t.Error("RemovePR must match case-insensitively") } } diff --git a/internal/config/config.go b/internal/config/config.go index b722054..c7fa999 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,8 @@ import ( "path/filepath" "strings" "time" + + "github.com/devblac/prw/internal/forge" ) const ( @@ -25,8 +27,13 @@ type Config struct { WebhookURL string `json:"webhook_url,omitempty"` WebhookFormat string `json:"webhook_format,omitempty"` GitHubToken string `json:"github_token,omitempty"` - NotificationFilter string `json:"notification_filter,omitempty"` - NotificationNative bool `json:"notification_native,omitempty"` + // BitbucketToken and BitbucketEmail authenticate against Bitbucket Cloud: an + // Atlassian API token with the account's email (Basic auth), or a workspace / + // project / repository access token alone (Bearer). See ResolveBitbucketAuth. + BitbucketToken string `json:"bitbucket_token,omitempty"` + BitbucketEmail string `json:"bitbucket_email,omitempty"` + NotificationFilter string `json:"notification_filter,omitempty"` + NotificationNative bool `json:"notification_native,omitempty"` // DisablePREvents turns off review (approved / changes requested) and merge // (conflict / ready to merge) notifications; `pr_events` in `prw config`. DisablePREvents bool `json:"disable_pr_events,omitempty"` @@ -97,9 +104,12 @@ func (c *Config) DisableAutoWatch(source string) (wasOn bool, removed []WatchedP // WatchedPR represents a pull request being watched. type WatchedPR struct { - Owner string `json:"owner"` - Repo string `json:"repo"` - Number int `json:"number"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Number int `json:"number"` + // Host is where the PR lives: empty for GitHub, "bitbucket.org" for Bitbucket + // Cloud (see forge.StorageHost). + Host string `json:"host,omitempty"` LastKnownSHA string `json:"last_known_sha,omitempty"` LastKnownState string `json:"last_known_state,omitempty"` LastChecked time.Time `json:"last_checked,omitempty"` @@ -283,7 +293,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 samePR(*w, pr.Owner, pr.Repo, pr.Number) { + if samePR(*w, pr.Host, pr.Owner, pr.Repo, pr.Number) { w.LastKnownSHA = pr.LastKnownSHA w.LastKnownState = pr.LastKnownState w.LastChecked = pr.LastChecked @@ -323,7 +333,7 @@ func RemovePRs(prs []WatchedPR) error { return Update(func(c *Config) error { changed := false for _, pr := range prs { - if c.RemovePR(pr.Owner, pr.Repo, pr.Number) { + if c.RemovePR(pr.Host, pr.Owner, pr.Repo, pr.Number) { changed = true } } @@ -334,17 +344,19 @@ 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) +// samePR reports whether pr is owner/repo#number on host. Owner and repository names +// are case-insensitive on GitHub and Bitbucket, so "DevBlac/PRW" and "devblac/prw" are +// the same repository. An empty host means GitHub. +func samePR(pr WatchedPR, host, owner, repo string, number int) bool { + return pr.Number == number && forge.NormalizeHost(pr.Host) == forge.NormalizeHost(host) && + 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 { +func (c *Config) MarkManual(host, owner, repo string, number int) bool { for i := range c.WatchedPRs { - if samePR(c.WatchedPRs[i], owner, repo, number) && c.WatchedPRs[i].Source != "" { + if samePR(c.WatchedPRs[i], host, owner, repo, number) && c.WatchedPRs[i].Source != "" { c.WatchedPRs[i].Source = "" return true } @@ -355,7 +367,7 @@ func (c *Config) MarkManual(owner, repo string, number int) bool { // 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 samePR(existing, pr.Owner, pr.Repo, pr.Number) { + if samePR(existing, pr.Host, pr.Owner, pr.Repo, pr.Number) { return false } } @@ -364,9 +376,9 @@ 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 { +func (c *Config) RemovePR(host, owner, repo string, number int) bool { for i, pr := range c.WatchedPRs { - if samePR(pr, owner, repo, number) { + if samePR(pr, host, owner, repo, number) { c.WatchedPRs = append(c.WatchedPRs[:i], c.WatchedPRs[i+1:]...) return true } @@ -374,10 +386,10 @@ func (c *Config) RemovePR(owner, repo string, number int) bool { return false } -// UpdatePR updates the state of a watched PR. +// UpdatePR updates the state of a watched GitHub PR. func (c *Config) UpdatePR(owner, repo string, number int, sha, state string) { for i := range c.WatchedPRs { - if samePR(c.WatchedPRs[i], owner, repo, number) { + if samePR(c.WatchedPRs[i], forge.GitHub, owner, repo, number) { c.WatchedPRs[i].LastKnownSHA = sha c.WatchedPRs[i].LastKnownState = state c.WatchedPRs[i].LastChecked = time.Now() @@ -441,6 +453,31 @@ func (c *Config) GetToken() string { return token } +// BitbucketAuth is how to authenticate against Bitbucket Cloud. With an Email, Token +// is an Atlassian API token used with Basic auth; without one, Token is an access +// token sent as Bearer. A zero BitbucketAuth means anonymous (public repositories). +type BitbucketAuth struct { + Email string + Token string +} + +// ResolveBitbucketAuth returns the Bitbucket credentials and where they came from: +// the config file (bitbucket_token / bitbucket_email) or the BITBUCKET_TOKEN / +// BITBUCKET_EMAIL environment variables. +func (c *Config) ResolveBitbucketAuth() (BitbucketAuth, string) { + if c.BitbucketToken != "" { + return BitbucketAuth{Email: c.BitbucketEmail, Token: c.BitbucketToken}, TokenSourceConfig + } + if t := os.Getenv("BITBUCKET_TOKEN"); t != "" { + email := os.Getenv("BITBUCKET_EMAIL") + if email == "" { + email = c.BitbucketEmail + } + return BitbucketAuth{Email: email, Token: t}, "environment variable (BITBUCKET_TOKEN)" + } + return BitbucketAuth{}, TokenSourceNone +} + // normalizeNotificationFilter applies defaults and validation for the notification filter. func normalizeNotificationFilter(value string) string { filter := strings.ToLower(strings.TrimSpace(value)) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c08a012..020bb6d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -169,7 +169,7 @@ func TestRemovePR(t *testing.T) { }, } - if !cfg.RemovePR("owner", "repo", 1) { + if !cfg.RemovePR("", "owner", "repo", 1) { t.Error("expected RemovePR to return true") } if len(cfg.WatchedPRs) != 1 { @@ -179,7 +179,7 @@ func TestRemovePR(t *testing.T) { t.Errorf("wrong PR remaining: %+v", cfg.WatchedPRs[0]) } - if cfg.RemovePR("owner", "repo", 99) { + if cfg.RemovePR("", "owner", "repo", 99) { t.Error("expected RemovePR to return false for non-existent PR") } } @@ -516,7 +516,7 @@ func TestRemovePR_LastItem(t *testing.T) { }, } - if !cfg.RemovePR("owner", "repo", 1) { + if !cfg.RemovePR("", "owner", "repo", 1) { t.Error("expected RemovePR to return true") } if len(cfg.WatchedPRs) != 0 { diff --git a/internal/config/update_test.go b/internal/config/update_test.go index ff1bbb5..da6830e 100644 --- a/internal/config/update_test.go +++ b/internal/config/update_test.go @@ -265,7 +265,7 @@ func TestSavePRStates_PreservesConcurrentChanges(t *testing.T) { // Meanwhile, other commands remove #2, add #3 and change a setting. if err := Update(func(c *Config) error { - c.RemovePR("owner", "repo", 2) + c.RemovePR("", "owner", "repo", 2) c.AddPR(pr(3)) c.WebhookURL = "https://hooks.example.com/x" return nil diff --git a/internal/forge/forge.go b/internal/forge/forge.go new file mode 100644 index 0000000..26458ca --- /dev/null +++ b/internal/forge/forge.go @@ -0,0 +1,83 @@ +// Package forge identifies pull requests across the code hosting services prw +// supports (GitHub and Bitbucket Cloud): parsing their URLs and building links. +package forge + +import ( + "fmt" + "regexp" + "strconv" + "strings" +) + +// Hosts prw supports. +const ( + GitHub = "github.com" + Bitbucket = "bitbucket.org" +) + +// Ref identifies a pull request. Host is GitHub or Bitbucket; Owner is the GitHub +// owner or the Bitbucket workspace. +type Ref struct { + Host string + Owner string + Repo string + Number int +} + +var ( + githubPRURL = regexp.MustCompile(`^(?:https?://)?(?:www\.)?github\.com/([^/]+)/([^/]+)/pull/(\d+)`) + bitbucketPRURL = regexp.MustCompile(`^(?:https?://)?(?:www\.)?bitbucket\.org/([^/]+)/([^/]+)/pull-requests/(\d+)`) +) + +// ParsePRURL parses a GitHub (github.com/owner/repo/pull/N) or Bitbucket Cloud +// (bitbucket.org/workspace/repo/pull-requests/N) pull request URL. +func ParsePRURL(url string) (Ref, error) { + url = strings.TrimSpace(url) + for host, re := range map[string]*regexp.Regexp{GitHub: githubPRURL, Bitbucket: bitbucketPRURL} { + if m := re.FindStringSubmatch(url); m != nil { + n, err := strconv.Atoi(m[3]) + if err != nil { + return Ref{}, fmt.Errorf("invalid PR number: %w", err) + } + return Ref{Host: host, Owner: m[1], Repo: m[2], Number: n}, nil + } + } + return Ref{}, fmt.Errorf("not a GitHub (github.com/owner/repo/pull/N) or Bitbucket (bitbucket.org/workspace/repo/pull-requests/N) pull request URL") +} + +// NormalizeHost maps the empty host of watch-list entries created before Bitbucket +// support (and any casing) to a known host. +func NormalizeHost(host string) string { + if strings.EqualFold(strings.TrimSpace(host), Bitbucket) { + return Bitbucket + } + return GitHub +} + +// IsBitbucket reports whether host is Bitbucket Cloud. +func IsBitbucket(host string) bool { + return NormalizeHost(host) == Bitbucket +} + +// PRURL is the web URL of a pull request. +func PRURL(host, owner, repo string, number int) string { + if IsBitbucket(host) { + return fmt.Sprintf("https://bitbucket.org/%s/%s/pull-requests/%d", owner, repo, number) + } + return fmt.Sprintf("https://github.com/%s/%s/pull/%d", owner, repo, number) +} + +// URL is the web URL of the pull request. +func (r Ref) URL() string { return PRURL(r.Host, r.Owner, r.Repo, r.Number) } + +// String is the short form "owner/repo#N". +func (r Ref) String() string { return fmt.Sprintf("%s/%s#%d", r.Owner, r.Repo, r.Number) } + +// StorageHost is how a host is stored in the watch list: empty for GitHub, so that +// existing config files (which predate the field) keep meaning GitHub. +func StorageHost(host string) string { + if IsBitbucket(host) { + return Bitbucket + } + return "" +} diff --git a/internal/forge/forge_test.go b/internal/forge/forge_test.go new file mode 100644 index 0000000..81c49cc --- /dev/null +++ b/internal/forge/forge_test.go @@ -0,0 +1,39 @@ +package forge + +import "testing" + +func TestParsePRURL(t *testing.T) { + tests := []struct { + url string + want Ref + }{ + {"https://github.com/owner/repo/pull/123", Ref{GitHub, "owner", "repo", 123}}, + {"github.com/owner/repo/pull/7/files", Ref{GitHub, "owner", "repo", 7}}, + {"https://bitbucket.org/acme/api/pull-requests/42", Ref{Bitbucket, "acme", "api", 42}}, + {"https://bitbucket.org/acme/api/pull-requests/42/diff", Ref{Bitbucket, "acme", "api", 42}}, + {"bitbucket.org/acme/web-app/pull-requests/1", Ref{Bitbucket, "acme", "web-app", 1}}, + } + for _, tt := range tests { + got, err := ParsePRURL(tt.url) + if err != nil || got != tt.want { + t.Errorf("ParsePRURL(%q) = %+v, %v; want %+v", tt.url, got, err, tt.want) + } + } + for _, bad := range []string{"", "https://gitlab.com/o/r/-/merge_requests/1", "https://bitbucket.org/acme/api/pull/42", "https://github.com/o/r/pull-requests/1", "https://evil.com/github.com/o/r/pull/1"} { + if _, err := ParsePRURL(bad); err == nil { + t.Errorf("ParsePRURL(%q) should fail", bad) + } + } +} + +func TestPRURLAndHosts(t *testing.T) { + if got := PRURL("", "o", "r", 1); got != "https://github.com/o/r/pull/1" { + t.Errorf("empty host (old watch-list entries) must mean GitHub: %s", got) + } + if got := (Ref{Bitbucket, "acme", "api", 42}).URL(); got != "https://bitbucket.org/acme/api/pull-requests/42" { + t.Errorf("bitbucket URL = %s", got) + } + if StorageHost(GitHub) != "" || StorageHost("Bitbucket.org") != Bitbucket || !IsBitbucket("BITBUCKET.ORG") || IsBitbucket("") { + t.Error("host normalization") + } +} diff --git a/internal/github/client.go b/internal/github/client.go index 87ba773..c084cbe 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -26,6 +26,14 @@ type Client struct { Token string HTTPClient *http.Client + // The client doubles as the HTTP transport of other services' clients (see + // internal/bitbucket). AuthHeader, if set, is sent as Authorization instead of + // "Bearer "; Accept replaces GitHub's media type; Service names the API + // in errors ("GitHub" when empty). + AuthHeader string + Accept string + Service string + // mu guards cache and rateLimit. cache holds the last ETag and body per request // path: a long-running watcher re-asks for the same resources every poll, and a // conditional request answered with 304 Not Modified does not count against the @@ -78,16 +86,22 @@ type APIError struct { // secondary rate limit. ResetAt is when it is worth retrying (zero if unknown). RateLimited bool ResetAt time.Time + // Service is the API that answered ("GitHub" when empty). + Service string } func (e *APIError) Error() string { + service := e.Service + if service == "" { + service = "GitHub" + } if e.RateLimited { if !e.ResetAt.IsZero() { - return fmt.Sprintf("GitHub API rate limit exceeded (%d); resets at %s", e.StatusCode, e.ResetAt.Local().Format("15:04:05")) + return fmt.Sprintf("%s API rate limit exceeded (%d); resets at %s", service, e.StatusCode, e.ResetAt.Local().Format("15:04:05")) } - return fmt.Sprintf("GitHub API rate limit exceeded (%d)", e.StatusCode) + return fmt.Sprintf("%s API rate limit exceeded (%d)", service, e.StatusCode) } - return fmt.Sprintf("GitHub API returned %d: %s", e.StatusCode, e.Body) + return fmt.Sprintf("%s API returned %d: %s", service, e.StatusCode, e.Body) } // IsRateLimited reports whether err is (or wraps) a rate-limit APIError, and when to @@ -460,6 +474,12 @@ func hasCISignal(status *CombinedStatus, runs []CheckRun) bool { } // getJSON performs an authenticated GET request and decodes the JSON response into v. +// GetJSON GETs BaseURL+path and decodes the JSON response into v, with the client's +// authentication, ETag cache and rate-limit handling. +func (c *Client) GetJSON(ctx context.Context, path string, v interface{}) error { + return c.getJSON(ctx, path, v) +} + func (c *Client) getJSON(ctx context.Context, path string, v interface{}) error { req, err := http.NewRequestWithContext(ctx, "GET", c.BaseURL+path, nil) if err != nil { @@ -468,10 +488,17 @@ func (c *Client) getJSON(ctx context.Context, path string, v interface{}) error // GitHub rejects an empty "Bearer " with 401 even for public repos, so only send the // header when there is a token. - if c.Token != "" { + switch { + case c.AuthHeader != "": + req.Header.Set("Authorization", c.AuthHeader) + case c.Token != "": req.Header.Set("Authorization", "Bearer "+c.Token) } - req.Header.Set("Accept", "application/vnd.github.v3+json") + accept := c.Accept + if accept == "" { + accept = "application/vnd.github.v3+json" + } + req.Header.Set("Accept", accept) c.mu.Lock() cached, haveCached := c.cache[path] @@ -501,7 +528,9 @@ func (c *Client) getJSON(ctx context.Context, path string, v interface{}) error } default: errBody, _ := io.ReadAll(resp.Body) - return newAPIError(resp, errBody) + apiErr := newAPIError(resp, errBody) + apiErr.Service = c.Service + return apiErr } if err := json.Unmarshal(body, v); err != nil { diff --git a/internal/github/logs.go b/internal/github/logs.go index af88887..6017239 100644 --- a/internal/github/logs.go +++ b/internal/github/logs.go @@ -230,7 +230,7 @@ func FailedJobLogs(ctx context.Context, c LogFetcher, owner, repo string, runs [ entry := FailedJobLog{Name: run.Name, URL: run.HTMLURL} switch { case !IsActionsJob(run): - entry.Error = "not a GitHub Actions job; its log isn't available through the API" + entry.Error = "not a GitHub Actions job; prw only reads GitHub Actions logs so far" case fetched >= maxLogFetches: entry.Error = fmt.Sprintf("skipped: only the first %d failed jobs' logs are fetched", maxLogFetches) default: diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 4196a9f..66ff675 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -22,7 +23,9 @@ const ( // StatusChangeEvent represents a change of a PR: its CI status by default, or its // review decision or merge state, depending on Kind. type StatusChangeEvent struct { - Kind string + Kind string + // Host is where the PR lives (forge.GitHub when empty). + Host string Owner string Repo string Number int @@ -72,7 +75,7 @@ func NewConsoleNotifier() *ConsoleNotifier { // Notify prints the status change to console. func (c *ConsoleNotifier) Notify(event *StatusChangeEvent) error { - prURL := github.FormatPRURL(event.Owner, event.Repo, event.Number) + prURL := forge.PRURL(event.Host, event.Owner, event.Repo, event.Number) switch event.Kind { case EventKindReview: @@ -129,6 +132,7 @@ func NewWebhookNotifierWithFormat(url, format string) *WebhookNotifier { // WebhookPayload is the JSON structure sent to the webhook. type WebhookPayload struct { Type string `json:"type"` + Host string `json:"host,omitempty"` // "bitbucket.org" for Bitbucket; empty for GitHub Owner string `json:"owner"` Repo string `json:"repo"` PRNumber int `json:"pr_number"` diff --git a/internal/notify/webhook_format.go b/internal/notify/webhook_format.go index 731d6e9..010201a 100644 --- a/internal/notify/webhook_format.go +++ b/internal/notify/webhook_format.go @@ -6,6 +6,7 @@ import ( "net/url" "strings" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -93,6 +94,7 @@ func buildWebhookBody(format string, event *StatusChangeEvent) ([]byte, error) { func genericPayload(event *StatusChangeEvent) WebhookPayload { return WebhookPayload{ Type: payloadType(event.Kind), + Host: forge.StorageHost(event.Host), Owner: event.Owner, Repo: event.Repo, PRNumber: event.Number, @@ -101,7 +103,7 @@ func genericPayload(event *StatusChangeEvent) WebhookPayload { CurrentState: event.CurrentState, SHA: event.SHA, FailedChecks: event.FailedChecks, - URL: github.FormatPRURL(event.Owner, event.Repo, event.Number), + URL: forge.PRURL(event.Host, event.Owner, event.Repo, event.Number), Timestamp: event.Timestamp, } } @@ -182,7 +184,7 @@ func slackLinkText(s string) string { } func slackPayload(event *StatusChangeEvent) map[string]string { - prURL := github.FormatPRURL(event.Owner, event.Repo, event.Number) + prURL := forge.PRURL(event.Host, event.Owner, event.Repo, event.Number) var b strings.Builder fmt.Fprintf(&b, "%s *<%s|%s/%s#%d>*", stateEmoji(event.CurrentState), prURL, slackLinkText(event.Owner), slackLinkText(event.Repo), event.Number) @@ -237,7 +239,7 @@ func discordPayload(event *StatusChangeEvent) map[string]interface{} { embed := discordEmbed{ Title: truncate(title, discordTitleLimit), - URL: github.FormatPRURL(event.Owner, event.Repo, event.Number), + URL: forge.PRURL(event.Host, event.Owner, event.Repo, event.Number), Description: truncate(desc.String(), discordDescriptionMax), Color: stateColor(event.CurrentState), } diff --git a/internal/watcher/hosts_test.go b/internal/watcher/hosts_test.go new file mode 100644 index 0000000..43ef628 --- /dev/null +++ b/internal/watcher/hosts_test.go @@ -0,0 +1,96 @@ +package watcher + +import ( + "context" + "testing" + "time" + + "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" + "github.com/devblac/prw/internal/github" +) + +func bitbucketPR(number int, state string) config.WatchedPR { + pr := watched(number, state) + pr.Owner, pr.Repo, pr.Host = "acme", "api", forge.Bitbucket + return pr +} + +func TestCheckAllPRs_UsesEachHostsClient(t *testing.T) { + useTempConfig(t) + gh := &countingClient{mockGitHubClient: mockGitHubClient{prs: map[string]*github.PullRequest{"owner/repo/1": newPR(1, "sha1")}}} + bb := &countingClient{mockGitHubClient: mockGitHubClient{prs: map[string]*github.PullRequest{"acme/api/2": newPR(2, "sha2")}}} + cfg := &config.Config{WatchedPRs: []config.WatchedPR{watched(1, "pending"), bitbucketPR(2, "pending")}} + cfg.Save() + w := New(gh, cfg, &mockNotifier{}) + w.SetClient(forge.Bitbucket, bb) + + w.checkAllPRs(context.Background()) + if gh.calls != 1 || bb.calls != 1 { + t.Errorf("github calls=%d bitbucket calls=%d, want one each", gh.calls, bb.calls) + } +} + +func TestCheckAllPRs_RateLimitPausesOnlyThatHost(t *testing.T) { + useTempConfig(t) + gh := &countingClient{mockGitHubClient: mockGitHubClient{prs: map[string]*github.PullRequest{ + "owner/repo/1": newPR(1, "sha1"), "owner/repo/3": newPR(3, "sha3")}}} + bb := &countingClient{prErr: &github.APIError{StatusCode: 429, RateLimited: true}} + cfg := &config.Config{WatchedPRs: []config.WatchedPR{bitbucketPR(2, "success"), watched(1, ""), bitbucketPR(4, "success"), watched(3, "")}} + cfg.Save() + w := New(gh, cfg, &mockNotifier{}) + w.SetClient(forge.Bitbucket, bb) + + w.checkAllPRs(context.Background()) + if bb.calls != 1 { + t.Errorf("Bitbucket must stop after its first rate-limited request, made %d", bb.calls) + } + if gh.calls != 2 { + t.Errorf("GitHub PRs must still be checked while Bitbucket is limited, made %d", gh.calls) + } + if w.pausedUntil[forge.Bitbucket].IsZero() || !w.pausedUntil[forge.GitHub].IsZero() { + t.Errorf("pausedUntil = %v", w.pausedUntil) + } + if len(w.config.WatchedPRs) != 4 { + t.Errorf("no PR may be dropped: %+v", w.config.WatchedPRs) + } +} + +func TestCheckAllPRs_BitbucketMinInterval(t *testing.T) { + useTempConfig(t) + now := time.Date(2026, 10, 25, 12, 0, 0, 0, time.UTC) + bb := &countingClient{mockGitHubClient: mockGitHubClient{prs: map[string]*github.PullRequest{"acme/api/2": newPR(2, "sha2")}}} + recent, stale := bitbucketPR(2, "success"), bitbucketPR(2, "success") + recent.LastChecked = now.Add(-20 * time.Second) + stale.LastChecked = now.Add(-BitbucketMinInterval) + + for _, tt := range []struct { + pr config.WatchedPR + want int + }{{recent, 0}, {stale, 1}, {bitbucketPR(2, ""), 1}} { + bb.calls = 0 + cfg := &config.Config{WatchedPRs: []config.WatchedPR{tt.pr}} + cfg.Save() + w := New(nil, cfg, &mockNotifier{}) + w.SetClient(forge.Bitbucket, bb) + w.now = func() time.Time { return now } + w.checkAllPRs(context.Background()) + if bb.calls != tt.want { + t.Errorf("last checked %v ago: %d calls, want %d", now.Sub(tt.pr.LastChecked), bb.calls, tt.want) + } + } +} + +func TestCheckAllPRs_NoClientForAHostKeepsItsPRs(t *testing.T) { + useTempConfig(t) + cfg := &config.Config{WatchedPRs: []config.WatchedPR{watched(1, "success")}} + cfg.Save() + var nilClient *github.Client // no GitHub token: run passes a nil client + w := New(nilClient, cfg, &mockNotifier{}) + + w.checkAllPRs(context.Background()) + w.checkAllPRs(context.Background()) + if len(w.config.WatchedPRs) != 1 || w.config.WatchedPRs[0].LastKnownState != "success" { + t.Errorf("watch list = %+v", w.config.WatchedPRs) + } +} diff --git a/internal/watcher/ratelimit_test.go b/internal/watcher/ratelimit_test.go index d15e962..4e27429 100644 --- a/internal/watcher/ratelimit_test.go +++ b/internal/watcher/ratelimit_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" ) @@ -42,7 +43,7 @@ func TestCheckAllPRs_StopsAndPausesOnRateLimit(t *testing.T) { if client.calls != 1 { t.Errorf("must stop after the first rate-limited request, made %d", client.calls) } - if !w.pausedUntil.Equal(reset) { + if !w.pausedUntil[forge.GitHub].Equal(reset) { t.Errorf("pausedUntil = %v, want %v", w.pausedUntil, reset) } if len(w.config.WatchedPRs) != 3 { @@ -60,8 +61,8 @@ func TestPauseForRateLimit_DefaultsWhenResetUnknown(t *testing.T) { w := New(&mockGitHubClient{}, &config.Config{}, &mockNotifier{}) w.now = func() time.Time { return now } - w.pauseForRateLimit(time.Time{}) - if !w.pausedUntil.Equal(now.Add(defaultRateLimitPause)) { + w.pauseForRateLimit("", time.Time{}) + if !w.pausedUntil[forge.GitHub].Equal(now.Add(defaultRateLimitPause)) { t.Errorf("pausedUntil = %v, want now+%s", w.pausedUntil, defaultRateLimitPause) } } @@ -100,7 +101,7 @@ func TestCheckAllPRs_OtherErrorsDoNotPause(t *testing.T) { if client.calls != 2 { t.Errorf("a non-rate-limit error must not stop the round, made %d calls", client.calls) } - if !w.pausedUntil.IsZero() { + if len(w.pausedUntil) != 0 { t.Errorf("must not pause on a 404, pausedUntil = %v", w.pausedUntil) } } diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 70d572f..fbb41bd 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -2,15 +2,18 @@ package watcher import ( "context" + "errors" "fmt" "time" "github.com/devblac/prw/internal/config" + "github.com/devblac/prw/internal/forge" "github.com/devblac/prw/internal/github" "github.com/devblac/prw/internal/notify" ) -// GitHubClient defines the interface for GitHub operations. +// GitHubClient is what the watcher needs from a code host's client. The Bitbucket +// client implements it too, answering in the github package's types. type GitHubClient interface { GetPullRequest(ctx context.Context, owner, repo string, number int) (*github.PullRequest, error) GetCombinedStatus(ctx context.Context, owner, repo, ref string) (*github.CombinedStatus, error) @@ -26,13 +29,17 @@ type ConfigStore interface { // Watcher polls GitHub PRs and triggers notifications on status changes. type Watcher struct { - client GitHubClient + client GitHubClient // GitHub; nil without a token config *config.Config notifier notify.Notifier - - // pausedUntil is set when GitHub reports a rate limit; polls are skipped until then - // so the watcher stops spending requests that are bound to fail. - pausedUntil time.Time + // clients holds the clients of other hosts (Bitbucket), set with SetClient. + clients map[string]GitHubClient + + // pausedUntil is set per host when it reports a rate limit; that host's PRs are + // skipped until then so the watcher stops spending requests that are bound to fail. + pausedUntil map[string]time.Time + // warnedNoClient remembers hosts already reported as having no client. + warnedNoClient map[string]bool // now is the clock; tests replace it. now func() time.Time // lastAutoWatch is when the auto-watch searches last ran. @@ -41,17 +48,53 @@ type Watcher struct { 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. +// defaultRateLimitPause is used when the API doesn't say when the limit resets. 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. +const BitbucketMinInterval = time.Minute + // New creates a new Watcher. func New(client GitHubClient, cfg *config.Config, notifier notify.Notifier) *Watcher { return &Watcher{ - client: client, - config: cfg, - notifier: notifier, - now: time.Now, + client: client, + config: cfg, + notifier: notifier, + now: time.Now, + clients: map[string]GitHubClient{}, + pausedUntil: map[string]time.Time{}, + warnedNoClient: map[string]bool{}, + } +} + +// SetClient sets the client used for PRs on host (forge.Bitbucket). +func (w *Watcher) SetClient(host string, client GitHubClient) { + w.clients[forge.NormalizeHost(host)] = client +} + +// clientFor returns the client for PRs on host, or nil if there is none. +func (w *Watcher) clientFor(host string) GitHubClient { + host = forge.NormalizeHost(host) + if host == forge.GitHub { + if w.client == nil || isNilClient(w.client) { + return nil + } + return w.client } + return w.clients[host] +} + +// isNilClient catches a typed nil (e.g. a nil *github.Client) stored in the interface. +func isNilClient(c GitHubClient) bool { + if c == nil { + return true + } + if gc, ok := c.(*github.Client); ok && gc == nil { + return true + } + return false } // Run starts the watcher loop and runs until context is cancelled. @@ -82,9 +125,6 @@ func (w *Watcher) Run(ctx context.Context) error { fmt.Println("\nWatcher stopped.") return ctx.Err() case <-ticker.C: - if w.now().Before(w.pausedUntil) { - 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(ctx) @@ -131,6 +171,10 @@ func (w *Watcher) checkAllPRs(ctx context.Context) { for i := range w.config.WatchedPRs { pr := w.config.WatchedPRs[i] + if !w.due(pr) { + stillWatched = append(stillWatched, pr) // paused or checked too recently + continue + } closed, err := w.checkPR(ctx, &pr) if err != nil && ctx.Err() != nil { // Stopped mid-round (Ctrl-C): keep this and the remaining PRs unchanged. @@ -139,12 +183,13 @@ func (w *Watcher) checkAllPRs(ctx context.Context) { } if err != nil { stillWatched = append(stillWatched, pr) // keep watching; preserve any partial update (e.g. title) + if errors.Is(err, errNoClient) { + continue // reported once by checkPR + } if limited, resetAt := github.IsRateLimited(err); limited { - // Every remaining request this round would fail too: keep the rest of - // the list unchanged and pause until the limit resets. - stillWatched = append(stillWatched, w.config.WatchedPRs[i+1:]...) - w.pauseForRateLimit(resetAt) - break + // The host's other PRs would fail too: skip them until the limit resets. + w.pauseForRateLimit(pr.Host, resetAt) + continue } fmt.Printf("Error checking PR %s/%s#%d: %v\n", pr.Owner, pr.Repo, pr.Number, err) continue @@ -170,21 +215,53 @@ func (w *Watcher) checkAllPRs(ctx context.Context) { } } -func (w *Watcher) pauseForRateLimit(resetAt time.Time) { +// due reports whether pr should be checked this round: not while its host is rate +// limited, and Bitbucket PRs at most every BitbucketMinInterval. +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 { + return false + } + return true +} + +func (w *Watcher) pauseForRateLimit(host string, resetAt time.Time) { + host = forge.NormalizeHost(host) now := w.now() if resetAt.Before(now) { resetAt = now.Add(defaultRateLimitPause) } - w.pausedUntil = resetAt - fmt.Printf("⚠️ GitHub API rate limit reached; pausing checks until %s (%s).\n", - resetAt.Local().Format("15:04:05"), resetAt.Sub(now).Round(time.Second)) + w.pausedUntil[host] = resetAt + fmt.Printf("⚠️ %s API rate limit reached; pausing checks of its PRs until %s (%s).\n", + serviceName(host), resetAt.Local().Format("15:04:05"), resetAt.Sub(now).Round(time.Second)) +} + +func serviceName(host string) string { + if forge.IsBitbucket(host) { + return "Bitbucket" + } + return "GitHub" } +// errNoClient is returned for PRs on a host prw has no client for (no token). +var errNoClient = errors.New("no client for this host") + // checkPR checks one PR's CI status and reports whether it is now closed (merged or // not), in which case the caller should stop watching it. func (w *Watcher) checkPR(ctx context.Context, pr *config.WatchedPR) (closed bool, err error) { + client := w.clientFor(pr.Host) + if client == nil { + if host := forge.NormalizeHost(pr.Host); !w.warnedNoClient[host] { + w.warnedNoClient[host] = true + fmt.Printf("Warning: skipping %s PRs: no %s token configured.\n", serviceName(host), serviceName(host)) + } + return false, errNoClient + } // Fetch the PR to get its state and current head SHA - ghPR, err := w.client.GetPullRequest(ctx, pr.Owner, pr.Repo, pr.Number) + ghPR, err := client.GetPullRequest(ctx, pr.Owner, pr.Repo, pr.Number) if err != nil { return false, fmt.Errorf("failed to fetch PR: %w", err) } @@ -203,6 +280,7 @@ func (w *Watcher) checkPR(ctx context.Context, pr *config.WatchedPR) (closed boo // is a lifecycle event, not a CI result, and this is the user's only chance to // see it before prw stops watching the PR. event := ¬ify.StatusChangeEvent{ + Host: pr.Host, Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, @@ -221,7 +299,7 @@ func (w *Watcher) checkPR(ctx context.Context, pr *config.WatchedPR) (closed boo currentSHA := ghPR.Head.SHA // Aggregate commit statuses and check runs (GitHub Actions) for the head commit - ci, err := github.FetchCIStatus(ctx, w.client, pr.Owner, pr.Repo, currentSHA) + ci, err := github.FetchCIStatus(ctx, client, pr.Owner, pr.Repo, currentSHA) if err != nil { return false, fmt.Errorf("failed to fetch status: %w", err) } @@ -242,6 +320,7 @@ func (w *Watcher) checkPR(ctx context.Context, pr *config.WatchedPR) (closed boo shouldNotify(w.config.NotificationFilter, currentState) { notifiedCI = true event := ¬ify.StatusChangeEvent{ + Host: pr.Host, Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, @@ -275,7 +354,7 @@ func (w *Watcher) checkPR(ctx context.Context, pr *config.WatchedPR) (closed boo // notifiedCI tells whether a CI notification was just sent for this PR. Nothing is // announced the first time a PR is seen (the previous state is empty). func (w *Watcher) checkPREvents(ctx context.Context, pr *config.WatchedPR, ghPR *github.PullRequest, notifiedCI bool) error { - reviews, err := w.client.GetReviews(ctx, pr.Owner, pr.Repo, pr.Number) + reviews, err := w.clientFor(pr.Host).GetReviews(ctx, pr.Owner, pr.Repo, pr.Number) if err != nil { return fmt.Errorf("failed to fetch reviews: %w", err) } @@ -320,6 +399,7 @@ func mergeWorthNotifying(prev, merge string, notified bool) bool { func (w *Watcher) notifyPREvent(pr *config.WatchedPR, ghPR *github.PullRequest, kind, prev, current string) { event := ¬ify.StatusChangeEvent{ Kind: kind, + Host: pr.Host, Owner: pr.Owner, Repo: pr.Repo, Number: pr.Number, diff --git a/internal/watcher/watcher_persist_test.go b/internal/watcher/watcher_persist_test.go index 80b057e..ac20af5 100644 --- a/internal/watcher/watcher_persist_test.go +++ b/internal/watcher/watcher_persist_test.go @@ -52,7 +52,7 @@ func TestWatcherReloadsWatchListEachTick(t *testing.T) { // In another terminal: `prw unwatch` #1 and `prw watch` #2. if err := config.Update(func(c *config.Config) error { - c.RemovePR("owner", "repo", 1) + c.RemovePR("", "owner", "repo", 1) c.AddPR(watched(2, "")) return nil }); err != nil { @@ -101,7 +101,7 @@ func TestWatcherDoesNotResurrectUnwatchedPR(t *testing.T) { // The user unwatches #2 while a poll is in flight (after the watcher's last reload). if err := config.Update(func(c *config.Config) error { - c.RemovePR("owner", "repo", 2) + c.RemovePR("", "owner", "repo", 2) return nil }); err != nil { t.Fatal(err)