From 8b65014bfbf19f2ed9aed0fc58c92922edd0841b Mon Sep 17 00:00:00 2001 From: Dustin Hilgaertner Date: Fri, 12 Jun 2026 11:46:53 -0500 Subject: [PATCH 1/2] Add sidebar "Add label crow:merge to PR" context-menu action (CROW-502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right-clicking a session row that has a PR now offers "Add label crow:merge to PR", which ensures the label exists in the repo and adds it to the PR so the existing auto-merge watcher can pick it up — no need to leave Crow. - CodeBackend: new addMergeLabel(prURL:) (GitHub: `gh pr edit --add-label`), with a protocol-default throw so non-capable backends inherit it. - AppState: onAddMergeLabel closure, isAddingMergeLabel in-flight flag, and canAddMergeLabel(for:) capability resolver (ADR 0005 injection so CrowUI stays decoupled from CrowProvider). - IssueTracker.addMergeLabel(sessionID:) mirrors markInReview's capability gate / in-flight / error handling; repoSlug(fromPRURL:) parses owner/repo from a PR web URL. - AppDelegate wires the resolver + closure; SessionService clears the in-flight flag on delete. - SessionListView: self-gating addMergeLabelButton shown in the shared row menu and the inline Reviews menu; hidden when no PR or capability absent. 🐦‍⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude Crow-Session: 7CD33545-55C4-4C1D-8904-1126C7A16D9E --- .../CrowCore/Sources/CrowCore/AppState.swift | 22 +++++++++ .../Backends/GitHubCodeBackend.swift | 10 +++++ .../Sources/CrowProvider/CodeBackend.swift | 12 +++++ .../Sources/CrowUI/SessionListView.swift | 20 +++++++++ Sources/Crow/App/AppDelegate.swift | 12 +++++ Sources/Crow/App/IssueTracker.swift | 45 +++++++++++++++++++ Sources/Crow/App/SessionService.swift | 1 + 7 files changed, 122 insertions(+) diff --git a/Packages/CrowCore/Sources/CrowCore/AppState.swift b/Packages/CrowCore/Sources/CrowCore/AppState.swift index 8950ab2..50df763 100644 --- a/Packages/CrowCore/Sources/CrowCore/AppState.swift +++ b/Packages/CrowCore/Sources/CrowCore/AppState.swift @@ -347,6 +347,9 @@ public final class AppState { /// Called to mark a session's ticket as "In Review" on the GitHub Project board. public var onMarkInReview: ((UUID) -> Void)? + /// Called to add the `crow:merge` auto-merge label to a session's PR. + public var onAddMergeLabel: ((UUID) -> Void)? + /// Called to update session status to .inReview (persists to store). public var onSetSessionInReview: ((UUID) -> Void)? @@ -357,6 +360,10 @@ public final class AppState { /// Must be cleaned up when a session is deleted (see `SessionService.deleteSession`). public var isMarkingInReview: [UUID: Bool] = [:] + /// Whether a session's PR is currently being labeled with `crow:merge` (loading state). + /// Must be cleaned up when a session is deleted (see `SessionService.deleteSession`). + public var isAddingMergeLabel: [UUID: Bool] = [:] + /// Sessions whose async deletion (worktree teardown, branch removal, persistence) /// is currently in progress. Set on the main actor at the start of /// `SessionService.deleteSession` and cleared when the session is fully removed. @@ -464,6 +471,21 @@ public final class AppState { canSetProjectStatusResolver?(session) ?? false } + /// Resolves whether a session's code backend declares the `.autoMergeLabel` + /// capability (i.e. supports adding `crow:merge` to a PR). Wired by + /// `AppDelegate` using `ProviderManager.codeBackend(for:)`. CrowUI does not + /// depend on CrowProvider, so the capability lookup is injected as a closure + /// (same pattern as `canSetProjectStatusResolver`). Defaults to `nil` so + /// unwired contexts (tests, previews) treat the capability as absent. + public var canAddMergeLabelResolver: ((Session) -> Bool)? + + /// Whether the session's provider supports adding the `crow:merge` label to + /// its PR, based on the `CodeBackend` capability set. Used to gate the + /// "Add label crow:merge to PR" sidebar context-menu item. + public func canAddMergeLabel(for session: Session) -> Bool { + canAddMergeLabelResolver?(session) ?? false + } + public func primaryWorktree(for sessionID: UUID) -> SessionWorktree? { worktrees[sessionID]?.first(where: { $0.isPrimary }) ?? worktrees[sessionID]?.first } diff --git a/Packages/CrowProvider/Sources/CrowProvider/Backends/GitHubCodeBackend.swift b/Packages/CrowProvider/Sources/CrowProvider/Backends/GitHubCodeBackend.swift index 3699ae2..7b3cd12 100644 --- a/Packages/CrowProvider/Sources/CrowProvider/Backends/GitHubCodeBackend.swift +++ b/Packages/CrowProvider/Sources/CrowProvider/Backends/GitHubCodeBackend.swift @@ -252,6 +252,16 @@ public struct GitHubCodeBackend: CodeBackend { // MARK: - enableAutoMerge / updateBranch + public func addMergeLabel(prURL: String) async throws { + // Direct argv (not `sh -c`) eliminates shell interpolation around + // `prURL`; $TMPDIR cwd so gh doesn't infer the repo from the cwd. + _ = try await shellRunner.run( + args: ["gh", "pr", "edit", prURL, "--add-label", "crow:merge"], + env: [:], + cwd: NSTemporaryDirectory() + ) + } + public func enableAutoMerge(prURL: String) async throws { // Run inside $TMPDIR so gh doesn't pick up the cwd's git config when // detecting the repo. Direct argv (not `sh -c`) eliminates any shell diff --git a/Packages/CrowProvider/Sources/CrowProvider/CodeBackend.swift b/Packages/CrowProvider/Sources/CrowProvider/CodeBackend.swift index 3d610a7..2465b56 100644 --- a/Packages/CrowProvider/Sources/CrowProvider/CodeBackend.swift +++ b/Packages/CrowProvider/Sources/CrowProvider/CodeBackend.swift @@ -65,6 +65,11 @@ public protocol CodeBackend: Sendable { /// implementation returns `[]` (no key search); GitHub overrides it. func findPRsMatchingKeys(_ candidates: [KeyCandidate]) async throws -> [KeyPRMatch] + /// Add the `crow:merge` auto-merge label to the PR at `prURL`. + /// Capability-gated on `.autoMergeLabel`. Backends without the capability + /// inherit the default no-op-throw in the protocol extension below. + func addMergeLabel(prURL: String) async throws + /// Enable auto-merge on the PR at `prURL` (squash + delete branch). /// Capability-gated on `.autoMerge`. Backends without the capability throw /// `ProviderError.unimplemented`. @@ -87,6 +92,13 @@ public extension CodeBackend { /// no-op so a Jira-key reconcile pass degrades to "no matches" rather than /// forcing every conformer to implement it. func findPRsMatchingKeys(_ candidates: [KeyCandidate]) async throws -> [KeyPRMatch] { [] } + + /// Default: backends without `.autoMergeLabel` can't add the merge label. + /// GitHub overrides this; others inherit the throw so a capability-gated + /// caller that slips through degrades to an error rather than a silent no-op. + func addMergeLabel(prURL: String) async throws { + throw ProviderError.unimplemented("addMergeLabel not supported by \(provider)") + } } /// Optional capabilities a `CodeBackend` may declare. diff --git a/Packages/CrowUI/Sources/CrowUI/SessionListView.swift b/Packages/CrowUI/Sources/CrowUI/SessionListView.swift index a9a9962..d501e4a 100644 --- a/Packages/CrowUI/Sources/CrowUI/SessionListView.swift +++ b/Packages/CrowUI/Sources/CrowUI/SessionListView.swift @@ -167,6 +167,7 @@ public struct SessionListView: View { .listRowSeparator(.hidden) .listRowBackground(Color.clear) .contextMenu { + addMergeLabelButton(session) Button(role: .destructive) { sessionToDelete = session } label: { @@ -318,6 +319,8 @@ public struct SessionListView: View { .disabled(deleting) } + addMergeLabelButton(session) + Button(role: .destructive) { sessionToDelete = session } label: { @@ -326,6 +329,23 @@ public struct SessionListView: View { .disabled(deleting) } + /// "Add label crow:merge to PR" — shown only when the session has a PR link + /// and its code backend supports the auto-merge label (capability-gated via + /// `canAddMergeLabel`). Self-gating so callers can include it unconditionally. + @ViewBuilder + private func addMergeLabelButton(_ session: Session) -> some View { + let hasPR = appState.links(for: session.id).contains(where: { $0.linkType == .pr }) + if hasPR, appState.canAddMergeLabel(for: session) { + Button { + appState.onAddMergeLabel?(session.id) + } label: { + Label("Add label crow:merge to PR", systemImage: "arrow.triangle.merge") + } + .disabled(appState.isAddingMergeLabel[session.id] == true + || appState.isDeletingSession[session.id] == true) + } + } + private func filteredSessions(_ sessions: [Session]) -> [Session] { if searchText.isEmpty { return sessions } return sessions.filter { $0.name.localizedCaseInsensitiveContains(searchText) } diff --git a/Sources/Crow/App/AppDelegate.swift b/Sources/Crow/App/AppDelegate.swift index a1bc963..ee4a7bf 100644 --- a/Sources/Crow/App/AppDelegate.swift +++ b/Sources/Crow/App/AppDelegate.swift @@ -845,6 +845,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate { Task { await tracker?.markInReview(sessionID: id) } } + appState.canAddMergeLabelResolver = { [providerManager] session in + guard let provider = session.provider else { return false } + return providerManager + .codeBackend(for: provider)? + .capabilities + .contains(.autoMergeLabel) ?? false + } + + appState.onAddMergeLabel = { [weak tracker] id in + Task { await tracker?.addMergeLabel(sessionID: id) } + } + appState.onManualRefresh = { [weak tracker] in Task { await tracker?.refresh() } } diff --git a/Sources/Crow/App/IssueTracker.swift b/Sources/Crow/App/IssueTracker.swift index 14fb4c8..02cb3d6 100644 --- a/Sources/Crow/App/IssueTracker.swift +++ b/Sources/Crow/App/IssueTracker.swift @@ -1422,6 +1422,26 @@ final class IssueTracker { return "" } + /// Parse the `owner/repo` (or `group/sub/repo`) slug from a PR/MR *web* URL + /// such as `https://github.com/owner/repo/pull/123` or + /// `https://gitlab.com/group/sub/repo/-/merge_requests/12`. Returns the path + /// segments before the `pull` / `merge_requests` / `-` marker, or "" when the + /// URL can't be parsed. Distinct from `extractSlug(fromRemote:)`, which + /// parses git *remote* URLs (no `/pull/...` suffix). + nonisolated static func repoSlug(fromPRURL url: String) -> String { + let trimmed = url.trimmingCharacters(in: .whitespacesAndNewlines) + guard let range = trimmed.range(of: #"^https?://[^/]+/"#, options: .regularExpression) else { + return "" + } + let path = String(trimmed[range.upperBound...]) + var segments: [String] = [] + for segment in path.split(separator: "/").map(String.init) { + if segment == "pull" || segment == "merge_requests" || segment == "-" { break } + segments.append(segment) + } + return segments.joined(separator: "/") + } + private func shellSync(_ args: String...) throws -> String { let process = Process() let outPipe = Pipe() @@ -2306,4 +2326,29 @@ final class IssueTracker { // Update local session status to .inReview appState.onSetSessionInReview?(sessionID) } + + /// Add the `crow:merge` auto-merge label to a session's PR, ensuring the + /// label exists in the repo first. Capability-gated on `.autoMergeLabel` + /// (GitHub only today). Mirrors `markInReview`'s in-flight/error handling. + func addMergeLabel(sessionID: UUID) async { + guard let session = appState.sessions.first(where: { $0.id == sessionID }), + let prLink = appState.links(for: sessionID).first(where: { $0.linkType == .pr }), + let provider = session.provider, + let backend = providerManager.codeBackend(for: provider), + backend.capabilities.contains(.autoMergeLabel) else { return } + + appState.isAddingMergeLabel[sessionID] = true + defer { appState.isAddingMergeLabel[sessionID] = false } + + let repo = Self.repoSlug(fromPRURL: prLink.url) + do { + if !repo.isEmpty { try await backend.ensureMergeLabel(repo: repo) } + try await backend.addMergeLabel(prURL: prLink.url) + NSLog("[Crow] Added crow:merge to %@", prLink.url as NSString) + } catch ProviderError.insufficientScope { + reportScopeWarning("merge label") + } catch { + print("[IssueTracker] addMergeLabel failed for \(prLink.url): \(error.localizedDescription.prefix(200))") + } + } } diff --git a/Sources/Crow/App/SessionService.swift b/Sources/Crow/App/SessionService.swift index 808a118..fc478a8 100644 --- a/Sources/Crow/App/SessionService.swift +++ b/Sources/Crow/App/SessionService.swift @@ -975,6 +975,7 @@ final class SessionService { appState.removeHookState(for: id) appState.prStatus.removeValue(forKey: id) appState.isMarkingInReview.removeValue(forKey: id) + appState.isAddingMergeLabel.removeValue(forKey: id) appState.isDeletingSession.removeValue(forKey: id) store.mutate { data in From d9c31c4181abe51297b43f2a1805bd1174cbc152 Mon Sep 17 00:00:00 2001 From: Dustin Hilgaertner Date: Fri, 12 Jun 2026 11:56:22 -0500 Subject: [PATCH 2/2] Address review: test repoSlug, bail on empty slug, drop dead catch (CROW-502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add IssueTrackerPRSlugTests covering repoSlug(fromPRURL:) for the GitHub PR / GitLab MR shapes, query strings, fragments, http scheme, missing scheme, and empty input (Yellow). - addMergeLabel now bails with a warning when the repo slug can't be parsed, instead of calling addMergeLabel with no ensureMergeLabel and half-doing the action (Green #3). - Drop the unreachable `catch ProviderError.insufficientScope` — the gh code backend only throws ShellRunnerError, so the generic catch covers it (Green #2). 🐦‍⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude Crow-Session: 7CD33545-55C4-4C1D-8904-1126C7A16D9E --- Sources/Crow/App/IssueTracker.swift | 11 ++++-- .../IssueTrackerReconcileTests.swift | 34 +++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/Sources/Crow/App/IssueTracker.swift b/Sources/Crow/App/IssueTracker.swift index 02cb3d6..9ff4d3e 100644 --- a/Sources/Crow/App/IssueTracker.swift +++ b/Sources/Crow/App/IssueTracker.swift @@ -2341,12 +2341,17 @@ final class IssueTracker { defer { appState.isAddingMergeLabel[sessionID] = false } let repo = Self.repoSlug(fromPRURL: prLink.url) + guard !repo.isEmpty else { + // Without a repo slug we can't `ensureMergeLabel`, and the bare + // `gh pr edit --add-label` would fail if the label doesn't already + // exist. Bail loudly rather than silently half-doing the action. + print("[IssueTracker] addMergeLabel: could not parse repo slug from \(prLink.url)") + return + } do { - if !repo.isEmpty { try await backend.ensureMergeLabel(repo: repo) } + try await backend.ensureMergeLabel(repo: repo) try await backend.addMergeLabel(prURL: prLink.url) NSLog("[Crow] Added crow:merge to %@", prLink.url as NSString) - } catch ProviderError.insufficientScope { - reportScopeWarning("merge label") } catch { print("[IssueTracker] addMergeLabel failed for \(prLink.url): \(error.localizedDescription.prefix(200))") } diff --git a/Tests/CrowTests/IssueTrackerReconcileTests.swift b/Tests/CrowTests/IssueTrackerReconcileTests.swift index 312ad2a..9b82d2e 100644 --- a/Tests/CrowTests/IssueTrackerReconcileTests.swift +++ b/Tests/CrowTests/IssueTrackerReconcileTests.swift @@ -217,6 +217,40 @@ struct IssueTrackerRemoteSlugTests { } } +@Suite("IssueTracker PR-URL slug extraction") +struct IssueTrackerPRSlugTests { + @Test func parsesGitHubPRURL() { + #expect(IssueTracker.repoSlug(fromPRURL: "https://github.com/owner/repo/pull/123") == "owner/repo") + } + + @Test func parsesGitLabMRURL() { + // GitLab MR URLs interpose `/-/` before `merge_requests`; the slug is + // everything up to that marker, including nested groups. + #expect( + IssueTracker.repoSlug(fromPRURL: "https://gitlab.com/group/sub/repo/-/merge_requests/12") + == "group/sub/repo" + ) + } + + @Test func ignoresQueryStringAndFragment() { + #expect(IssueTracker.repoSlug(fromPRURL: "https://github.com/owner/repo/pull/123?w=1") == "owner/repo") + #expect(IssueTracker.repoSlug(fromPRURL: "https://github.com/owner/repo/pull/123#discussion_r1") == "owner/repo") + } + + @Test func parsesHTTPScheme() { + #expect(IssueTracker.repoSlug(fromPRURL: "http://github.com/owner/repo/pull/9") == "owner/repo") + } + + @Test func returnsEmptyForMissingScheme() { + #expect(IssueTracker.repoSlug(fromPRURL: "github.com/owner/repo/pull/1") == "") + } + + @Test func returnsEmptyForEmptyInput() { + #expect(IssueTracker.repoSlug(fromPRURL: "") == "") + #expect(IssueTracker.repoSlug(fromPRURL: " ") == "") + } +} + @Suite("IssueTracker reconcile fan-out") struct IssueTrackerReconcileFanOutTests {