ReviewOS

stacks/buddy

Extract GitProvider abstraction (unblocks GitLab #115, Bitbucket #116)

#1390
Closed chris opened this 8 days ago ·
8 days ago

Part of #1367 (Phase 4 — Platform Reach). Unblocks #115 (GitLab) and #116 (Bitbucket) — CodeRabbit supports GitHub/GitLab/Azure DevOps/Bitbucket; Pullfrog is GitHub-only (a reach advantage for us over Pullfrog if we ship it).

Problem

GitProviderName is a single-member union — literally 'github' (src/types.ts:10) — and src/git/github-provider.ts (1566 lines) is the only implementation, but GitHub-isms leak everywhere:

  • src/buddy.ts calls GitHub-shaped methods and embeds GitHub URL/API assumptions in PR lifecycle logic
  • src/setup.ts generates GitHub Actions workflows only; token guidance is PAT/GITHUB_TOKEN-specific
  • src/dashboard/ uses GitHub issues; bin/cli.ts reads GITHUB_TOKEN/BUDDY_BOT_TOKEN
  • Endpoints resolution (src/utils/endpoints.ts) handles GHES but nothing else

Adding GitLab today would mean forking half the codebase. This issue extracts the seam without implementing GitLab/Bitbucket (those remain #115/#116).

Spec

1. Interface extraction

Define a complete GitProvider interface in src/git/provider.ts from the actual call sites (audit every githubProvider. call in buddy.ts, cli.ts, dashboard, gates):

  • Branch ops: create/delete/list, stale detection
  • Commit ops: commit files (both local-git and API-tree paths), file contents at ref
  • PR ops: create/update/close/reopen/merge/list, reviews, review comments, comments, labels, reviewers, draft state, auto-merge enablement (#1369)
  • Issue ops: create/update/list, comments, labels, pin/unpin (GraphQL on GitHub; GitLab has native pinning differences — keep pin optional-capability)
  • Checks/statuses: check-run create/update (#1382), combined status read (#1369, #1381)
  • Reactions, collaborator-permission lookup (#1376)
  • Capability flags: capabilities(): { pinIssues, checkRuns, suggestions, autoMergeNative, ... } so features degrade per-platform instead of assuming

2. Terminology & URL abstraction

  • Neutral core vocabulary (PR vs MR handled at the provider edge)
  • All web/API URL construction moves behind the provider (prUrl(n), compareUrl(...) — audit src/pr/pr-generator.ts and release-notes-fetcher.ts for hardcoded github.com links; release-notes fetching of dependency repos stays GitHub-centric and is fine)
  • CI generation: src/setup.ts workflow generation gains a provider dimension (GitHub Actions now; GitLab CI templates arrive with #115)

3. Config & auth

  • Widen GitProviderName to 'github' | 'gitlab' | 'bitbucket' with validation that rejects not-yet-implemented providers with a "tracked in #115/#116" message
  • Token resolution becomes provider-owned (GITHUB_TOKEN / GITLAB_TOKEN / BITBUCKET_TOKEN conventions)

Tasks

  • Call-site audit → GitProvider interface + capability flags (document each method's contract)
  • Refactor GitHubProvider to implement it; move GitHub-only helpers behind the interface
  • Provider factory (createProvider(config)); thread through Buddy, CLI, dashboard, setup
  • URL/terminology sweep with tests asserting no github.com literals outside the GitHub provider + release-notes fetcher
  • Conformance test suite (test/git/provider-conformance.ts) any future provider must pass — this is the real deliverable for #115/#116 implementers
  • Docs: docs/advanced/providers.md describing the interface and how to add a provider

Acceptance criteria

  • Zero behavior change on GitHub (full regression suite green)
  • A mock provider passing the conformance suite can run scan→PR→dashboard end-to-end in tests
  • #115/#116 become "implement the interface + CI templates" tasks with a checklist, not archaeology

Dependencies

  • None hard; do after Phase 0 lands to avoid refactoring under churn. Unblocks #115, #116.
8 days ago

Landed in 6517353.

GitProvider now lives in src/git/provider.ts, split into members every platform has and members gated by declared ProviderCapabilities. supports() checks the flag and the method's presence, so a provider that declares a capability it did not implement is treated as lacking it rather than throwing at the call site; assertSupports() fails loudly where the capability is the whole command.

The deliverable for #115/#116 is test/git/provider-conformance.ts — a behavioural contract testing what callers may assume, never URL shapes or status codes — plus test/git/in-memory-provider.ts as a complete reference implementation. Implementing GitLab is now 'make the suite pass'.

gitlab and bitbucket type-check but are rejected at config validation with a link to their tracking issue, so a typo cannot reach a workflow run.

Also threaded createProvider through Buddy and seven CLI commands that each rebuilt the provider by hand, and added a test asserting github.com appears only where the platform is genuinely the subject.

Sign in to comment on this issue.