Skip to content

Extract TicketBackend protocol — decouple gh/glab shell-outs from call sites #410

@dgershman

Description

@dgershman

Crow TicketBackend Abstraction Layer

Context

Crow today integrates with GitHub (gh) and GitLab (glab) for issue/PR tracking. The user wants to eventually swap in Corveil as a third backend. Step zero — and the only step in scope here — is to build the abstraction layer so that "which provider" becomes a single decision at the edge, not a switch statement (or worse) at every call site.

The audit found three coexisting patterns in the current code:

  1. Switch-at-callsite (clean): e.g. updateLabels at IssueTracker.swift:493 switches on provider and shells gh vs glab.
  2. Parallel implementations (most of the behavior): one fat GitHub function and one fat GitLab function for the same logical operation. Example: gh api graphql batched fetch for assigned issues at IssueTracker.swift:703, parallel glab api issues?scope=assigned_to_me at line 2509. Conceptually one method, two implementations sitting in the same file.
  3. GitHub-only paths that don't branch at all: project-board status mutation at line 2584, gh label create at 2038, guard session.provider == .github else { return } at 2542. These silently no-op for GitLab today; they'd silently no-op for Corveil too unless the abstraction surfaces the gap.

The data layer is already clean — Provider enum, TicketInfo struct, ProviderManager.detectProvider(from:) and parseTicketURLComponents. The work is to extend that clean data layer with a clean behavior layer, then move the existing shell-outs behind it.

Recommended Approach

Define the protocol

In crow/Packages/CrowProvider/Sources/CrowProvider/TicketBackend.swift (new file):

public protocol TicketBackend: Sendable {
    var provider: Provider { get }

    // Read
    func fetchTicket(url: String) async throws -> TicketInfo
    func listAssigned() async throws -> [AssignedIssue]
    func linkedPR(branch: String, repo: String) async throws -> PRInfo?
    func stalePRStates(refs: [PRRef]) async throws -> [PRRef: PRState]

    // Write
    func setStatus(url: String, status: TicketStatus) async throws
    func setLabels(url: String, labels: [String]) async throws
    func assign(url: String, to: String) async throws
    func createTicket(repo: String, title: String, body: String, labels: [String]) async throws -> TicketInfo

    // Capabilities — let callers branch on what a backend *can* do
    // instead of hard-coding `if provider == .github`.
    var capabilities: Set<TicketCapability> { get }
}

public enum TicketCapability: Sendable {
    case projectBoardStatus   // GitHub Projects v2 — gated today at IssueTracker.swift:2542
    case autoMergeLabel       // gh label create at 2038
    case batchedQuery         // gh api graphql batched fetch
}

Surfacing capabilities is the antidote to the third pattern (GitHub-only escapes). Today IssueTracker.markInReview silently does nothing for GitLab; tomorrow it asks backend.capabilities.contains(.projectBoardStatus) and gets a real answer for every backend without a switch.

Three implementations

All in crow/Packages/CrowProvider/Sources/CrowProvider/Backends/:

  • GitHubBackend.swift — owns every gh … shell-out. Capabilities: [projectBoardStatus, autoMergeLabel, batchedQuery].
  • GitLabBackend.swift — owns every glab … shell-out. Capabilities: [] for v1 (matches today's gated-off behavior).
  • StubCorveilBackend.swift — throws unimplemented on every method, declares provider = .corveil, capabilities []. Exists only to prove the protocol can carry a third case; real impl is out of scope here.

Each backend takes its dependencies (shell runner, host config) at init so unit tests can inject a fake runner instead of really spawning gh.

Factory

ProviderManager becomes the factory and keeps its URL parsing:

public actor ProviderManager {
    public func backend(for url: String) -> TicketBackend {  }
    public func backend(for provider: Provider) -> TicketBackend {  }
    // existing parseTicketURLComponents stays
}

Add .corveil to the Provider enum (CrowCore/Models/Enums.swift:22) and to host detection (ProviderManager.swift:17) — even though the stub backend is the only consumer.

Move the shell-outs behind it (the actual work)

This is the bulk of the diff. By call site:

Site Today After
IssueTracker.swift:493 updateLabels switch switch provider { gh / glab } backend.setLabels(...)
IssueTracker.swift:703 assigned issues (gh batched) one big GitHub function GitHubBackend.listAssigned
IssueTracker.swift:2509 assigned issues (glab) parallel GitLab function GitLabBackend.listAssigned
IssueTracker.swift:860 stale PR states (gh) GitHub batched GitHubBackend.stalePRStates
IssueTracker.swift:916 stale MR states (glab) per-MR loop GitLabBackend.stalePRStates
IssueTracker.swift:2584 markInReview (gh project) GitHub-only, gated if backend.capabilities.contains(.projectBoardStatus) { backend.setStatus(...) }
IssueTracker.swift:2038 autoMergeLabel create GitHub-only capability-gated equivalent
IssueTracker.swift:2015 gh api ad-hoc GitHub-only inline → backend method or remove
SessionService.swift:1124 gh issue view recovery hardcoded gh backend.fetchTicket(url:)
SessionService.swift:1139 gh pr list --head hardcoded gh backend.linkedPR(branch:repo:)
ProviderManager.swift:73 fetchTicket switch inside actor delegate to backend
ProviderManager.swift:144 listRepos switch inside actor new listRepos method on backend

The two listAssigned implementations stop sitting in the same file as two halves of one function and live in their respective backend files. That's the single biggest win — the "provider per operation" inversion.

setup.sh and skills are out of scope: they shell gh/glab directly and don't go through Swift. Keep that as-is for now; revisit once a Corveil backend is real.

Critical Files

Purpose Path
New protocol + capability enum crow/Packages/CrowProvider/Sources/CrowProvider/TicketBackend.swift
New backends crow/Packages/CrowProvider/Sources/CrowProvider/Backends/{GitHub,GitLab,StubCorveil}Backend.swift
Factory (refactor existing actor) crow/Packages/CrowProvider/Sources/CrowProvider/ProviderManager.swift
Add .corveil case crow/Packages/CrowCore/Sources/CrowCore/Models/Enums.swift:22
Migrate ~70 shell-outs crow/Sources/Crow/App/IssueTracker.swift (see call-site table)
Migrate 2 shell-outs crow/Sources/Crow/App/SessionService.swift:1124, 1139

Open Decisions

  1. Capability flags vs throws .unsupportedOperation. Both express "this backend can't do X." Capability flags let callers branch before calling; throwing lets them try-and-recover. Recommend capabilities — they're the direct replacement for today's guard session.provider == .github and they document the matrix in one place.
  2. ProviderManager actor boundary. Today it's an actor. Backends being Sendable protocol existentials lets the factory be sync; the actor isolation only matters for the shell runner. Recommend keeping ProviderManager non-actor (struct or class) and pushing isolation into each backend's shell runner.
  3. Batched-vs-per-item operations. GitHub fetches assigned issues + PRs + reviews in one gh api graphql call (line 703). GitLab does them serially. The protocol's listAssigned() -> [AssignedIssue] flattens that — fine for callers, but GitHubBackend internally still wants the batched query, so it keeps its current implementation behind the method. No protocol change needed.

Verification

This is a pure refactor — behavior should not change for GitHub or GitLab users.

  1. Build green with .corveil added to enum and StubCorveilBackend registered.
  2. Existing manual test paths still work: open a GitHub-backed session and a GitLab-backed session, verify ticket title fetches, assigned-issue panel, stale PR detection, and markInReview all behave as before.
  3. Unit-test each backend in isolation by injecting a fake shell runner that returns canned JSON. Asserts the right gh/glab command got built — the regression net the codebase currently lacks.
  4. Stub test: instantiate StubCorveilBackend via the factory using a corveil.io URL, confirm every call throws unimplemented cleanly (no crashes, no silent no-ops). This is the "the abstraction is real" check.
  5. Capability audit: grep for the old session.provider == .github / provider == .gitlab guards. After the refactor, every one of those should be either gone or rewritten as a capabilities.contains(...) check.

🐦‍⬛ Created with Crow via Claude Code

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions