Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Packages/CrowCore/Sources/CrowCore/AppState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,22 @@ public final class AppState {
terminals(for: sessionID).contains(where: { $0.isManaged })
}

/// Resolves whether a session's task backend declares the
/// `.projectBoardStatus` capability. Wired by `AppDelegate` using
/// `ProviderManager.taskBackend(for:)`. CrowUI does not depend on
/// CrowProvider, so the capability lookup is injected as a closure
/// (same pattern as `onMarkInReview`, `onListWorkspaceRepos`).
/// Defaults to `nil` so unwired contexts (tests, previews) treat the
/// capability as absent. See ADR 0005.
public var canSetProjectStatusResolver: ((Session) -> Bool)?

/// Whether the session's provider supports setting project-board status,
/// based on the `TaskBackend` capability set. Replaces the previous
/// `session.provider == .github` UI guards. See ADR 0005.
public func canSetProjectStatus(for session: Session) -> Bool {
canSetProjectStatusResolver?(session) ?? false
}

public func primaryWorktree(for sessionID: UUID) -> SessionWorktree? {
worktrees[sessionID]?.first(where: { $0.isPrimary }) ?? worktrees[sessionID]?.first
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import Foundation
import Testing
@testable import CrowCore

// MARK: - AppState Capability Tests
//
// Cover the capability accessor that replaces UI provider guards
// (`session.provider == .github`). The accessor delegates to a resolver
// closure wired by AppDelegate using `ProviderManager.taskBackend(for:)` —
// these tests use stub closures to verify the wiring contract rather than
// the live capability lookup, which is covered separately by
// `BackendsTests.testGitHubTaskBackendDeclaresCapabilities`. See ADR 0005.

@MainActor @Test func canSetProjectStatusReturnsFalseWhenResolverUnset() {
let appState = AppState()
let session = Session(name: "no-resolver", provider: .github)
#expect(appState.canSetProjectStatus(for: session) == false)
}

@MainActor @Test func canSetProjectStatusReturnsResolverFalse() {
let appState = AppState()
appState.canSetProjectStatusResolver = { _ in false }
let session = Session(name: "resolver-false", provider: .github)
#expect(appState.canSetProjectStatus(for: session) == false)
}

@MainActor @Test func canSetProjectStatusReturnsResolverTrue() {
let appState = AppState()
appState.canSetProjectStatusResolver = { _ in true }
let session = Session(name: "resolver-true", provider: .github)
#expect(appState.canSetProjectStatus(for: session) == true)
}

@MainActor @Test func canSetProjectStatusPassesSessionToResolver() {
let appState = AppState()
let target = Session(name: "passed-through", provider: .gitlab)
var observed: Session?
appState.canSetProjectStatusResolver = { session in
observed = session
return session.provider == .gitlab
}
let result = appState.canSetProjectStatus(for: target)
#expect(observed?.id == target.id)
#expect(result == true)
}

@MainActor @Test func canSetProjectStatusDelegatesForNilProvider() {
// The accessor itself does not short-circuit on `provider == nil`;
// that lives in the resolver body wired by AppDelegate. This documents
// the contract so a future refactor doesn't quietly move the gating.
let appState = AppState()
var called = false
appState.canSetProjectStatusResolver = { _ in
called = true
return false
}
let session = Session(name: "no-provider")
_ = appState.canSetProjectStatus(for: session)
#expect(called == true)
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@ import CrowCore
///
/// Capabilities declared:
/// - `.batchedQuery` — `listAssigned`-style fetches use one GraphQL call.
/// - `.projectBoardStatus` — GitHub Projects v2 is the UI surface this
/// capability gates. The `setTaskStatus` GraphQL migration is still
/// pending; the legacy `IssueTracker.markInReview` path (see
/// `IssueTracker.swift:2549`) executes the mutation today via the
/// `onMarkInReview` closure, so calling `setTaskStatus` on this backend
/// directly still throws `.unimplemented`. UI guards branch on this
/// capability instead of `provider == .github` (see ADR 0005); execution
/// falls through to the legacy path until the migration lands.
///
/// Note: `.projectBoardStatus` is intentionally *not* declared until the
/// `markInReview` GraphQL migration lands (see `setTaskStatus` below and
/// IssueTracker.swift:2539). The flag is contract: declaring it means a
/// capability-gated caller can call `setTaskStatus` and expect success.
/// We add the flag when the implementation arrives, not before.
///
/// See ADR 0005 for the protocol contract and ADR 0005's Context section for why
/// See ADR 0005 for the protocol contract and its Context section for why
/// task ops are separate from code ops.
public struct GitHubTaskBackend: TaskBackend {
public let provider: Provider = .github
public let capabilities: Set<TaskCapability> = [.batchedQuery]
public let capabilities: Set<TaskCapability> = [.batchedQuery, .projectBoardStatus]

private let shellRunner: ShellRunner

Expand Down Expand Up @@ -65,10 +67,12 @@ public struct GitHubTaskBackend: TaskBackend {

public func setTaskStatus(url: String, status: TicketStatus) async throws {
// Real implementation requires the GraphQL project-item lookup + mutation
// sequence currently inlined at IssueTracker.swift:2539. That migration is
// sequence currently inlined at IssueTracker.swift:2549. That migration is
// deferred to a follow-up PR — see ADR 0005 references.
// The capability is declared so callers don't fall into the throw path
// before the migration lands.
// UI guards key off `.projectBoardStatus` (declared above); execution
// currently routes through `IssueTracker.markInReview` via the
// `onMarkInReview` closure rather than through this method, so the
// throw is unreached on the live path until the migration lands.
throw ProviderError.unimplemented(
"GitHubTaskBackend.setTaskStatus: migration of markInReview project-board mutation pending"
)
Expand Down
17 changes: 13 additions & 4 deletions Packages/CrowProvider/Sources/CrowProvider/TaskBackend.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,14 @@ public protocol TaskBackend: Sendable {
func setLabels(url: String, add: [String], remove: [String]) async throws

/// Set the project-board status for a task.
/// Capability-gated: only call when `capabilities.contains(.projectBoardStatus)`.
/// Calling without the capability throws `ProviderError.unimplemented`.
/// Capability-gated for **UI surfacing**: callers gate the affordance on
/// `capabilities.contains(.projectBoardStatus)` (see ADR 0005). The
/// capability does *not* guarantee this method is implemented — a backend
/// may declare it to surface the UI while routing execution through a
/// legacy path (e.g. `GitHubTaskBackend` declares the capability but
/// throws here; `IssueTracker.markInReview` performs the mutation until
/// the `setTaskStatus` GraphQL migration lands). Calling without the
/// capability also throws `ProviderError.unimplemented`.
func setTaskStatus(url: String, status: TicketStatus) async throws
}

Expand All @@ -42,8 +48,11 @@ public protocol TaskBackend: Sendable {
/// becomes `if taskBackend.capabilities.contains(.projectBoardStatus)`, so adding
/// a third provider (or extending an existing one) doesn't ripple through call sites.
public enum TaskCapability: Sendable, Hashable {
/// Can set project-board status (GitHub Projects v2 today).
/// Gates `setTaskStatus`. Without this capability that method throws.
/// Declares that the provider exposes a project-board status concept the
/// UI should surface (GitHub Projects v2 today). Gates UI affordances such
/// as the "Mark as In Review" button. Does **not** guarantee that calling
/// `setTaskStatus` succeeds — see `setTaskStatus` for the implementation
/// caveat. Backends without this capability hide the affordance entirely.
case projectBoardStatus

/// Can fulfill `listAssigned`-style queries in a single batched call rather
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,14 @@ final class BackendsTests: XCTestCase {
func testGitHubTaskBackendDeclaresCapabilities() {
let backend = GitHubTaskBackend(shellRunner: FakeShellRunner())
XCTAssertEqual(backend.provider, .github)
// .projectBoardStatus intentionally NOT declared until the markInReview
// GraphQL migration lands. Declaring it while setTaskStatus throws would
// lie to capability-gated callers. See ADR 0005.
XCTAssertFalse(backend.capabilities.contains(.projectBoardStatus))
// `.projectBoardStatus` declares the UI surface (GitHub Projects v2).
// The `setTaskStatus` GraphQL migration is still pending — execution
// currently routes through IssueTracker.markInReview via the
// onMarkInReview closure, so direct `setTaskStatus` calls on this
// backend still throw .unimplemented (asserted by
// testGitHubTaskBackendSetTaskStatusThrowsUnimplemented). See ADR 0005
// and issue crow#413 for the UI guard migration.
XCTAssertTrue(backend.capabilities.contains(.projectBoardStatus))
XCTAssertTrue(backend.capabilities.contains(.batchedQuery))
}

Expand Down
2 changes: 1 addition & 1 deletion Packages/CrowUI/Sources/CrowUI/SessionDetailView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public struct SessionDetailView: View {

if session.status == .active,
session.ticketURL != nil,
session.provider == .github {
appState.canSetProjectStatus(for: session) {
if appState.isMarkingInReview[session.id] == true {
ProgressView()
.controlSize(.small)
Expand Down
2 changes: 1 addition & 1 deletion Packages/CrowUI/Sources/CrowUI/SessionListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ public struct SessionListView: View {

if session.status == .active,
session.ticketURL != nil,
session.provider == .github {
appState.canSetProjectStatus(for: session) {
Button {
appState.onMarkInReview?(session.id)
} label: {
Expand Down
8 changes: 8 additions & 0 deletions Sources/Crow/App/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
self?.jobScheduler?.runNow(jobID)
}

appState.canSetProjectStatusResolver = { [providerManager] session in
guard let provider = session.provider else { return false }
return providerManager
.taskBackend(for: provider)
.capabilities
.contains(.projectBoardStatus)
}

appState.onMarkInReview = { [weak tracker] id in
Task { await tracker?.markInReview(sessionID: id) }
}
Expand Down
13 changes: 11 additions & 2 deletions docs/adr/0005-task-and-code-backend-protocols.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,24 @@ Crow exposes two independent protocols under `CrowProvider`:
- **`TaskBackend`** — tracker side. Owns issue/task lifecycle: `fetchTask`, `listAssigned`, `setLabels`, `setTaskStatus`, `assign`, `createTask`.
- **`CodeBackend`** — VCS side. Owns PR lifecycle: `linkedPR`, `prStates`, `ensureMergeLabel`, `fetchCrowAuthoredCommits`.

Each backend declares its **capabilities** as a `Set` (`TaskCapability` and `CodeCapability`). Callers branch on capabilities, not on provider identity. Examples:
Each backend declares its **capabilities** as a `Set` (`TaskCapability` and `CodeCapability`). Callers branch on capabilities, not on provider identity. Capabilities gate UI affordances and call-site decisions; they do not by themselves guarantee a method's implementation is wired (a backend may declare a UI-facing capability while routing execution through a legacy path — see `.projectBoardStatus` on `GitHubTaskBackend`, where `IssueTracker.markInReview` performs the mutation until the `setTaskStatus` GraphQL migration lands). Examples:

```swift
// UI gating — the direct replacement for `if session.provider == .github { ... }`.
if taskBackend.capabilities.contains(.projectBoardStatus) {
showInReviewButton()
}

// Calling a capability-gated method. Backends that declare the capability may
// still throw `.unimplemented` if their implementation is pending; callers
// that exercise the method directly must be prepared for that and either fall
// back to a legacy path or surface the error.
if taskBackend.capabilities.contains(.projectBoardStatus) {
try await taskBackend.setTaskStatus(url: url, status: .inReview)
}
```

This is the direct replacement for `if session.provider == .github { ... }`. New backends declare what they support and the call site asks; no `switch` over a closed set of providers.
New backends declare what they support and the call site asks; no `switch` over a closed set of providers.

Backends take a `ShellRunner` (a thin `protocol` over `Process()` + `ShellEnvironment.shared`) at init, so unit tests inject a `FakeShellRunner` that records command vectors and returns canned JSON. The three near-duplicate `private func shell` implementations in `ProviderManager`, `IssueTracker`, and `SessionService` collapse into one `ProcessShellRunner`.

Expand Down
Loading