From a13cd5487b50221f6994a4b060284ebf372a0e23 Mon Sep 17 00:00:00 2001 From: Dustin Hilgaertner Date: Fri, 24 Apr 2026 17:41:03 -0500 Subject: [PATCH] Auto-respond to PR status changes (#201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a watched PR transitions into 'changes requested' or 'CI failing', type a short prompt into the session's managed Claude Code terminal so Claude can read the review or logs and address it without the user having to switch sessions and dictate the fix. Both behaviors are off by default and toggle from Settings → Notifications. macOS notifications fire on every transition; the terminal injection only fires when opted in. Detection piggybacks on the existing 60s GitHub poll — no new API calls, no new scopes — and dedupes per (session, kind, head SHA) so re-runs on the same commit don't re-fire. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Sources/CrowCore/Models/AppConfig.swift | 40 ++++- .../CrowCore/Models/NotificationEvent.swift | 8 + .../Sources/CrowCore/Models/PRStatus.swift | 22 ++- .../CrowCore/Models/PRStatusTransition.swift | 102 +++++++++++++ .../NotificationEventTests.swift | 14 +- .../PRStatusTransitionTests.swift | 142 ++++++++++++++++++ .../CrowUI/NotificationSettingsView.swift | 30 +++- .../CrowUI/Sources/CrowUI/SettingsView.swift | 6 +- Sources/Crow/App/AppDelegate.swift | 19 +++ Sources/Crow/App/AutoRespondCoordinator.swift | 112 ++++++++++++++ Sources/Crow/App/IssueTracker.swift | 67 ++++++++- Sources/Crow/App/NotificationManager.swift | 51 +++++++ .../IssueTrackerCompletionTests.swift | 1 + Tests/CrowTests/IssueTrackerDedupTests.swift | 2 + 14 files changed, 605 insertions(+), 11 deletions(-) create mode 100644 Packages/CrowCore/Sources/CrowCore/Models/PRStatusTransition.swift create mode 100644 Packages/CrowCore/Tests/CrowCoreTests/PRStatusTransitionTests.swift create mode 100644 Sources/Crow/App/AutoRespondCoordinator.swift diff --git a/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift b/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift index 4d11316..730d56d 100644 --- a/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift +++ b/Packages/CrowCore/Sources/CrowCore/Models/AppConfig.swift @@ -13,6 +13,7 @@ public struct AppConfig: Codable, Sendable, Equatable { public var remoteControlEnabled: Bool public var managerAutoPermissionMode: Bool public var telemetry: TelemetryConfig + public var autoRespond: AutoRespondSettings public init( workspaces: [WorkspaceInfo] = [], @@ -21,7 +22,8 @@ public struct AppConfig: Codable, Sendable, Equatable { sidebar: SidebarSettings = SidebarSettings(), remoteControlEnabled: Bool = false, managerAutoPermissionMode: Bool = true, - telemetry: TelemetryConfig = TelemetryConfig() + telemetry: TelemetryConfig = TelemetryConfig(), + autoRespond: AutoRespondSettings = AutoRespondSettings() ) { self.workspaces = workspaces self.defaults = defaults @@ -30,6 +32,7 @@ public struct AppConfig: Codable, Sendable, Equatable { self.remoteControlEnabled = remoteControlEnabled self.managerAutoPermissionMode = managerAutoPermissionMode self.telemetry = telemetry + self.autoRespond = autoRespond } public init(from decoder: Decoder) throws { @@ -41,10 +44,43 @@ public struct AppConfig: Codable, Sendable, Equatable { remoteControlEnabled = try container.decodeIfPresent(Bool.self, forKey: .remoteControlEnabled) ?? false managerAutoPermissionMode = try container.decodeIfPresent(Bool.self, forKey: .managerAutoPermissionMode) ?? true telemetry = try container.decodeIfPresent(TelemetryConfig.self, forKey: .telemetry) ?? TelemetryConfig() + autoRespond = try container.decodeIfPresent(AutoRespondSettings.self, forKey: .autoRespond) ?? AutoRespondSettings() } private enum CodingKeys: String, CodingKey { - case workspaces, defaults, notifications, sidebar, remoteControlEnabled, managerAutoPermissionMode, telemetry + case workspaces, defaults, notifications, sidebar, remoteControlEnabled, managerAutoPermissionMode, telemetry, autoRespond + } +} + +/// Opt-in settings that let Crow type instructions into a session's managed +/// Claude Code terminal when a watched PR transitions into a state that +/// usually requires action. Both flags default off — typing into a terminal +/// unprompted is intrusive, so the user must explicitly enable each. +public struct AutoRespondSettings: Codable, Sendable, Equatable { + /// Inject a "fix the review feedback" prompt when a PR transitions into + /// `reviewStatus == .changesRequested`. + public var respondToChangesRequested: Bool + /// Inject a "fix the failing checks" prompt when a PR transitions into + /// `checksPass == .failing` (keyed on the head SHA, so re-runs of the + /// same commit don't re-fire). + public var respondToFailedChecks: Bool + + public init( + respondToChangesRequested: Bool = false, + respondToFailedChecks: Bool = false + ) { + self.respondToChangesRequested = respondToChangesRequested + self.respondToFailedChecks = respondToFailedChecks + } + + public init(from decoder: Decoder) throws { + let c = try decoder.container(keyedBy: CodingKeys.self) + respondToChangesRequested = try c.decodeIfPresent(Bool.self, forKey: .respondToChangesRequested) ?? false + respondToFailedChecks = try c.decodeIfPresent(Bool.self, forKey: .respondToFailedChecks) ?? false + } + + private enum CodingKeys: String, CodingKey { + case respondToChangesRequested, respondToFailedChecks } } diff --git a/Packages/CrowCore/Sources/CrowCore/Models/NotificationEvent.swift b/Packages/CrowCore/Sources/CrowCore/Models/NotificationEvent.swift index 708a30a..1c767a1 100644 --- a/Packages/CrowCore/Sources/CrowCore/Models/NotificationEvent.swift +++ b/Packages/CrowCore/Sources/CrowCore/Models/NotificationEvent.swift @@ -9,6 +9,8 @@ public enum NotificationEvent: String, Codable, Sendable, CaseIterable, Identifi case taskComplete case agentWaiting case reviewRequested + case changesRequested + case checksFailing public var id: String { rawValue } @@ -17,6 +19,8 @@ public enum NotificationEvent: String, Codable, Sendable, CaseIterable, Identifi case .taskComplete: "Task Complete" case .agentWaiting: "Agent Waiting" case .reviewRequested: "Review Requested" + case .changesRequested: "Changes Requested" + case .checksFailing: "CI Failing" } } @@ -25,6 +29,8 @@ public enum NotificationEvent: String, Codable, Sendable, CaseIterable, Identifi case .taskComplete: "Claude finished responding" case .agentWaiting: "Claude needs your input or permission" case .reviewRequested: "Someone requested your review on a PR" + case .changesRequested: "A reviewer requested changes on your PR" + case .checksFailing: "CI checks started failing on your PR" } } @@ -33,6 +39,8 @@ public enum NotificationEvent: String, Codable, Sendable, CaseIterable, Identifi case .taskComplete: "Glass" case .agentWaiting: "Funk" case .reviewRequested: "Glass" + case .changesRequested: "Funk" + case .checksFailing: "Sosumi" } } diff --git a/Packages/CrowCore/Sources/CrowCore/Models/PRStatus.swift b/Packages/CrowCore/Sources/CrowCore/Models/PRStatus.swift index a21b059..a2c97eb 100644 --- a/Packages/CrowCore/Sources/CrowCore/Models/PRStatus.swift +++ b/Packages/CrowCore/Sources/CrowCore/Models/PRStatus.swift @@ -1,22 +1,40 @@ import Foundation /// Status of a pull request associated with a session. -public struct PRStatus: Codable, Sendable { +public struct PRStatus: Codable, Sendable, Equatable { public var checksPass: CheckStatus public var reviewStatus: ReviewStatus public var mergeable: MergeStatus public var failedCheckNames: [String] + /// Head commit SHA. Used to dedupe per-commit transition events + /// (e.g. don't re-fire "checks failing" when the same commit is re-run). + public var headSha: String? public init( checksPass: CheckStatus = .unknown, reviewStatus: ReviewStatus = .unknown, mergeable: MergeStatus = .unknown, - failedCheckNames: [String] = [] + failedCheckNames: [String] = [], + headSha: String? = nil ) { self.checksPass = checksPass self.reviewStatus = reviewStatus self.mergeable = mergeable self.failedCheckNames = failedCheckNames + self.headSha = headSha + } + + public init(from decoder: Decoder) throws { + let c = try decoder.container(keyedBy: CodingKeys.self) + checksPass = try c.decodeIfPresent(CheckStatus.self, forKey: .checksPass) ?? .unknown + reviewStatus = try c.decodeIfPresent(ReviewStatus.self, forKey: .reviewStatus) ?? .unknown + mergeable = try c.decodeIfPresent(MergeStatus.self, forKey: .mergeable) ?? .unknown + failedCheckNames = try c.decodeIfPresent([String].self, forKey: .failedCheckNames) ?? [] + headSha = try c.decodeIfPresent(String.self, forKey: .headSha) + } + + private enum CodingKeys: String, CodingKey { + case checksPass, reviewStatus, mergeable, failedCheckNames, headSha } public enum CheckStatus: String, Codable, Sendable { diff --git a/Packages/CrowCore/Sources/CrowCore/Models/PRStatusTransition.swift b/Packages/CrowCore/Sources/CrowCore/Models/PRStatusTransition.swift new file mode 100644 index 0000000..74904a2 --- /dev/null +++ b/Packages/CrowCore/Sources/CrowCore/Models/PRStatusTransition.swift @@ -0,0 +1,102 @@ +import Foundation + +/// A detected change in a session's `PRStatus` that warrants user attention. +/// +/// Computed by `IssueTracker` once per polling cycle by comparing the new +/// `PRStatus` against the previous one. Pure value type — no UI/AppState deps — +/// so the comparison logic stays unit-testable. +public struct PRStatusTransition: Sendable, Equatable { + public enum Kind: String, Sendable, Equatable { + /// A reviewer transitioned the PR into "changes requested". + case changesRequested + /// At least one CI/CD check newly transitioned to failing on the + /// current head commit. + case checksFailing + } + + public let kind: Kind + public let sessionID: UUID + public let prURL: String + public let prNumber: Int? + /// Head commit SHA at the moment of the transition. Used as part of the + /// dedupe key for `.checksFailing` so re-runs on the same commit don't + /// re-fire. + public let headSha: String? + /// Names of failing checks (empty for `.changesRequested`). + public let failedCheckNames: [String] + + public init( + kind: Kind, + sessionID: UUID, + prURL: String, + prNumber: Int? = nil, + headSha: String? = nil, + failedCheckNames: [String] = [] + ) { + self.kind = kind + self.sessionID = sessionID + self.prURL = prURL + self.prNumber = prNumber + self.headSha = headSha + self.failedCheckNames = failedCheckNames + } + + /// Stable key used to suppress duplicate fires across polling cycles. + /// `.changesRequested` keys on `(session, kind)` — the rule re-arms when + /// the status moves away from `.changesRequested`. `.checksFailing` keys + /// on `(session, kind, headSha)` so a new commit can re-fire. + public var dedupeKey: String { + switch kind { + case .changesRequested: + return "\(sessionID.uuidString)|changesRequested" + case .checksFailing: + return "\(sessionID.uuidString)|checksFailing|\(headSha ?? "")" + } + } +} + +extension PRStatus { + /// Compute the transitions implied by moving from `old` to `new`. + /// + /// Returns an empty array on the first observation (`old == nil`) so + /// existing PR state never triggers a fire-on-startup. Otherwise emits + /// at most one `.changesRequested` and one `.checksFailing`, in that order. + /// + /// - Note: This is the pure piece of the transition pipeline; the + /// stateful dedupe (across polls) lives in `IssueTracker`. + public static func transitions( + from old: PRStatus?, + to new: PRStatus, + sessionID: UUID, + prURL: String, + prNumber: Int? + ) -> [PRStatusTransition] { + guard let old else { return [] } // first observation — never fire + + var out: [PRStatusTransition] = [] + + if old.reviewStatus != .changesRequested && new.reviewStatus == .changesRequested { + out.append(PRStatusTransition( + kind: .changesRequested, + sessionID: sessionID, + prURL: prURL, + prNumber: prNumber, + headSha: new.headSha, + failedCheckNames: [] + )) + } + + if old.checksPass != .failing && new.checksPass == .failing { + out.append(PRStatusTransition( + kind: .checksFailing, + sessionID: sessionID, + prURL: prURL, + prNumber: prNumber, + headSha: new.headSha, + failedCheckNames: new.failedCheckNames + )) + } + + return out + } +} diff --git a/Packages/CrowCore/Tests/CrowCoreTests/NotificationEventTests.swift b/Packages/CrowCore/Tests/CrowCoreTests/NotificationEventTests.swift index 11cc0df..4e1bee4 100644 --- a/Packages/CrowCore/Tests/CrowCoreTests/NotificationEventTests.swift +++ b/Packages/CrowCore/Tests/CrowCoreTests/NotificationEventTests.swift @@ -3,7 +3,7 @@ import Testing @testable import CrowCore @Test func notificationEventAllCasesCount() { - #expect(NotificationEvent.allCases.count == 3) + #expect(NotificationEvent.allCases.count == 5) } @Test func notificationEventDefaultSoundsNonEmpty() { @@ -62,3 +62,15 @@ import Testing #expect(decoded == event) } } + +// MARK: - PR transition events + +@Test func changesRequestedAndChecksFailingArePresent() { + #expect(NotificationEvent.allCases.contains(.changesRequested)) + #expect(NotificationEvent.allCases.contains(.checksFailing)) +} + +@Test func prTransitionEventsHaveDistinctDefaultSounds() { + // Different default sounds so the two events are audibly distinct. + #expect(NotificationEvent.changesRequested.defaultSound != NotificationEvent.checksFailing.defaultSound) +} diff --git a/Packages/CrowCore/Tests/CrowCoreTests/PRStatusTransitionTests.swift b/Packages/CrowCore/Tests/CrowCoreTests/PRStatusTransitionTests.swift new file mode 100644 index 0000000..027281b --- /dev/null +++ b/Packages/CrowCore/Tests/CrowCoreTests/PRStatusTransitionTests.swift @@ -0,0 +1,142 @@ +import Foundation +import Testing +@testable import CrowCore + +@Suite("PRStatus transitions") +struct PRStatusTransitionTests { + private let sessionID = UUID() + private let prURL = "https://github.com/foo/bar/pull/1" + private let shaA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + private let shaB = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + private func status( + review: PRStatus.ReviewStatus = .reviewRequired, + checks: PRStatus.CheckStatus = .pending, + sha: String? = nil, + failed: [String] = [] + ) -> PRStatus { + PRStatus( + checksPass: checks, + reviewStatus: review, + mergeable: .unknown, + failedCheckNames: failed, + headSha: sha + ) + } + + private func compute(from old: PRStatus?, to new: PRStatus) -> [PRStatusTransition] { + PRStatus.transitions(from: old, to: new, sessionID: sessionID, prURL: prURL, prNumber: 1) + } + + // MARK: - First-observation gate + + @Test + func firstObservationNeverFires() { + // No prior status — even if the PR is already in a transition state, + // we must not fire (it'd flood the user with notifications on every + // app launch). + #expect(compute(from: nil, to: status(review: .changesRequested)).isEmpty) + #expect(compute(from: nil, to: status(checks: .failing, sha: shaA)).isEmpty) + #expect(compute(from: nil, to: status(review: .changesRequested, checks: .failing, sha: shaA)).isEmpty) + } + + // MARK: - Changes requested + + @Test + func reviewRequiredToChangesRequestedFires() { + let old = status(review: .reviewRequired) + let new = status(review: .changesRequested) + let ts = compute(from: old, to: new) + #expect(ts.count == 1) + #expect(ts.first?.kind == .changesRequested) + #expect(ts.first?.sessionID == sessionID) + #expect(ts.first?.prURL == prURL) + } + + @Test + func approvedToChangesRequestedFires() { + let ts = compute(from: status(review: .approved), to: status(review: .changesRequested)) + #expect(ts.count == 1) + #expect(ts.first?.kind == .changesRequested) + } + + @Test + func changesRequestedToChangesRequestedDoesNotFire() { + // Status hasn't transitioned — every poll observes the same state. + let ts = compute(from: status(review: .changesRequested), to: status(review: .changesRequested)) + #expect(ts.isEmpty) + } + + @Test + func changesRequestedToApprovedDoesNotFire() { + // Reviewer approved — that's the rule clearing, not a new event to act on. + let ts = compute(from: status(review: .changesRequested), to: status(review: .approved)) + #expect(ts.isEmpty) + } + + // MARK: - Checks failing + + @Test + func passingToFailingFires() { + let old = status(checks: .passing, sha: shaA) + let new = status(checks: .failing, sha: shaA, failed: ["lint"]) + let ts = compute(from: old, to: new) + #expect(ts.count == 1) + #expect(ts.first?.kind == .checksFailing) + #expect(ts.first?.headSha == shaA) + #expect(ts.first?.failedCheckNames == ["lint"]) + } + + @Test + func pendingToFailingFires() { + let ts = compute(from: status(checks: .pending, sha: shaA), to: status(checks: .failing, sha: shaA)) + #expect(ts.count == 1) + #expect(ts.first?.kind == .checksFailing) + } + + @Test + func failingToFailingDoesNotFire() { + // Pure transitions (no dedupe yet) treats this as no transition because + // `old.checksPass == .failing` already. + let ts = compute(from: status(checks: .failing, sha: shaA), to: status(checks: .failing, sha: shaA)) + #expect(ts.isEmpty) + } + + @Test + func failingToPassingDoesNotFire() { + let ts = compute(from: status(checks: .failing, sha: shaA), to: status(checks: .passing, sha: shaA)) + #expect(ts.isEmpty) + } + + // MARK: - Both transitions in one cycle + + @Test + func simultaneousTransitionsFireBoth() { + let old = status(review: .reviewRequired, checks: .pending, sha: shaA) + let new = status(review: .changesRequested, checks: .failing, sha: shaA) + let ts = compute(from: old, to: new) + #expect(ts.count == 2) + #expect(ts.contains(where: { $0.kind == .changesRequested })) + #expect(ts.contains(where: { $0.kind == .checksFailing })) + } + + // MARK: - Dedupe key + + @Test + func dedupeKeyForChangesRequestedDoesNotIncludeSha() { + // Once we've seen a changesRequested for a session, subsequent + // re-observations of the same state must dedupe regardless of which + // commit happens to be HEAD at the moment. + let t1 = PRStatusTransition(kind: .changesRequested, sessionID: sessionID, prURL: prURL, prNumber: 1, headSha: shaA) + let t2 = PRStatusTransition(kind: .changesRequested, sessionID: sessionID, prURL: prURL, prNumber: 1, headSha: shaB) + #expect(t1.dedupeKey == t2.dedupeKey) + } + + @Test + func dedupeKeyForChecksFailingIncludesSha() { + // A new commit that also fails CI must be allowed to re-fire. + let t1 = PRStatusTransition(kind: .checksFailing, sessionID: sessionID, prURL: prURL, prNumber: 1, headSha: shaA) + let t2 = PRStatusTransition(kind: .checksFailing, sessionID: sessionID, prURL: prURL, prNumber: 1, headSha: shaB) + #expect(t1.dedupeKey != t2.dedupeKey) + } +} diff --git a/Packages/CrowUI/Sources/CrowUI/NotificationSettingsView.swift b/Packages/CrowUI/Sources/CrowUI/NotificationSettingsView.swift index 96b9345..6cf9aaf 100644 --- a/Packages/CrowUI/Sources/CrowUI/NotificationSettingsView.swift +++ b/Packages/CrowUI/Sources/CrowUI/NotificationSettingsView.swift @@ -5,13 +5,19 @@ import CrowCore /// Settings view for configuring notification sounds and macOS system notifications. public struct NotificationSettingsView: View { @Binding var settings: NotificationSettings + @Binding var autoRespond: AutoRespondSettings var onSave: (() -> Void)? /// Built-in macOS system sounds available for selection. private static let builtInSounds = NotificationSettings.builtInSounds - public init(settings: Binding, onSave: (() -> Void)? = nil) { + public init( + settings: Binding, + autoRespond: Binding, + onSave: (() -> Void)? = nil + ) { self._settings = settings + self._autoRespond = autoRespond self.onSave = onSave } @@ -33,10 +39,32 @@ public struct NotificationSettingsView: View { ForEach(NotificationEvent.allCases) { event in eventSection(for: event) } + + autoRespondSection } .formStyle(.grouped) } + @ViewBuilder + private var autoRespondSection: some View { + Section { + Toggle("Respond to 'changes requested' reviews", isOn: $autoRespond.respondToChangesRequested) + .onChange(of: autoRespond.respondToChangesRequested) { _, _ in onSave?() } + Toggle("Respond to failed CI checks", isOn: $autoRespond.respondToFailedChecks) + .onChange(of: autoRespond.respondToFailedChecks) { _, _ in onSave?() } + Text("When enabled, Crow types an instruction into the session's Claude Code terminal asking Claude to read the review or CI logs and address the issue. Off by default — typing into a terminal unprompted is intrusive.") + .font(.caption) + .foregroundStyle(.secondary) + } header: { + VStack(alignment: .leading, spacing: 2) { + Text("Auto-respond") + Text("Automatically prompt Claude to fix PR feedback") + .font(.caption) + .foregroundStyle(.secondary) + } + } + } + @ViewBuilder private func eventSection(for event: NotificationEvent) -> some View { let config = bindingForEvent(event) diff --git a/Packages/CrowUI/Sources/CrowUI/SettingsView.swift b/Packages/CrowUI/Sources/CrowUI/SettingsView.swift index ac97e89..229fcda 100644 --- a/Packages/CrowUI/Sources/CrowUI/SettingsView.swift +++ b/Packages/CrowUI/Sources/CrowUI/SettingsView.swift @@ -41,7 +41,11 @@ public struct SettingsView: View { .tabItem { Label("General", systemImage: "gearshape") } workspacesTab .tabItem { Label("Workspaces", systemImage: "rectangle.stack") } - NotificationSettingsView(settings: $config.notifications, onSave: { save() }) + NotificationSettingsView( + settings: $config.notifications, + autoRespond: $config.autoRespond, + onSave: { save() } + ) .tabItem { Label("Notifications", systemImage: "bell") } } .frame(width: 520, height: 480) diff --git a/Sources/Crow/App/AppDelegate.swift b/Sources/Crow/App/AppDelegate.swift index edb1b00..3259e41 100644 --- a/Sources/Crow/App/AppDelegate.swift +++ b/Sources/Crow/App/AppDelegate.swift @@ -18,6 +18,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate { private var socketServer: SocketServer? private var issueTracker: IssueTracker? private var notificationManager: NotificationManager? + private var autoRespondCoordinator: AutoRespondCoordinator? private var allowListService: AllowListService? private var telemetryService: TelemetryService? private var devRoot: String? @@ -235,6 +236,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate { self.appState.onWorkOnIssue?(issue.url) self.notificationManager?.notifyAutoWorkspaceCreated(issue) } + tracker.onPRStatusTransitions = { [weak self] transitions in + guard let self else { return } + for transition in transitions { + if let session = self.appState.sessions.first(where: { $0.id == transition.sessionID }) { + self.notificationManager?.notifyPRTransition(transition, session: session) + } + } + self.autoRespondCoordinator?.handle(transitions) + } tracker.start() self.issueTracker = tracker @@ -246,6 +256,15 @@ final class AppDelegate: NSObject, NSApplicationDelegate { let notifManager = NotificationManager(appState: appState, settings: config.notifications) self.notificationManager = notifManager + // Initialize auto-respond coordinator. Reads `autoRespond` lazily from + // `self.appConfig` so toggles take effect on the next transition. + self.autoRespondCoordinator = AutoRespondCoordinator( + appState: appState, + settingsProvider: { [weak self] in + self?.appConfig?.autoRespond ?? AutoRespondSettings() + } + ) + // Initialize allow list service let allowList = AllowListService(appState: appState, devRoot: devRoot) self.allowListService = allowList diff --git a/Sources/Crow/App/AutoRespondCoordinator.swift b/Sources/Crow/App/AutoRespondCoordinator.swift new file mode 100644 index 0000000..396039f --- /dev/null +++ b/Sources/Crow/App/AutoRespondCoordinator.swift @@ -0,0 +1,112 @@ +import Foundation +import CrowCore +import CrowTerminal + +/// Handles auto-respond to PR status transitions: when CrowConfig opts in +/// for a transition kind, find the session's managed terminal and inject a +/// short prompt asking Claude Code to investigate and address the issue. +/// +/// Crow does not fetch review bodies or CI logs itself — the prompt asks +/// Claude to do that via `gh`/`glab`. This keeps Crow simple and avoids new +/// API scopes / rate-limit pressure. +/// +/// If a transition's toggle is off, or the session has no managed terminal +/// surface, the coordinator silently skips. The caller still fires the +/// macOS notification regardless, so the user can act manually. +@MainActor +final class AutoRespondCoordinator { + private let appState: AppState + /// Closure that returns the current `AutoRespondSettings`. Closure rather + /// than a stored value so updates from Settings UI take effect on the + /// next transition without explicit wiring. + private let settingsProvider: () -> AutoRespondSettings + + init(appState: AppState, settingsProvider: @escaping () -> AutoRespondSettings) { + self.appState = appState + self.settingsProvider = settingsProvider + } + + func handle(_ transitions: [PRStatusTransition]) { + let cfg = settingsProvider() + for t in transitions { + switch t.kind { + case .changesRequested where cfg.respondToChangesRequested: + dispatch(t) + case .checksFailing where cfg.respondToFailedChecks: + dispatch(t) + default: + continue + } + } + } + + private func dispatch(_ transition: PRStatusTransition) { + let terminals = appState.terminals(for: transition.sessionID) + guard let terminal = terminals.first(where: { $0.isManaged }) else { + NSLog("[AutoRespond] Skipping %@ for session %@: no managed terminal", + transition.kind.rawValue, transition.sessionID.uuidString) + return + } + guard TerminalManager.shared.existingSurface(for: terminal.id) != nil else { + NSLog("[AutoRespond] Skipping %@ for session %@: terminal surface not initialized", + transition.kind.rawValue, transition.sessionID.uuidString) + return + } + + let provider = appState.sessions.first(where: { $0.id == transition.sessionID })?.provider ?? .github + let prompt = AutoRespondPrompts.build(for: transition, provider: provider) + NSLog("[AutoRespond] Sending %@ prompt to terminal %@ (%d chars)", + transition.kind.rawValue, terminal.id.uuidString, prompt.count) + TerminalManager.shared.send(id: terminal.id, text: prompt) + } +} + +/// Builds the deterministic prompt text injected into a session's managed +/// terminal when an auto-respond transition fires. Each prompt: +/// 1. States what just happened (and links to the PR). +/// 2. Tells Claude how to fetch the relevant context via `gh`/`glab`. +/// 3. Asks Claude to make local changes and push to update the PR. +/// +/// Always ends with a trailing newline so the terminal submits it as Enter. +enum AutoRespondPrompts { + static func build(for transition: PRStatusTransition, provider: Provider) -> String { + let prRef = transition.prNumber.map { "PR #\($0)" } ?? "the PR" + let cli = provider == .gitlab ? "glab" : "gh" + + switch transition.kind { + case .changesRequested: + let fetchHint: String + if provider == .gitlab { + fetchHint = "Run `glab mr view \(transition.prURL) --comments` to read the review feedback." + } else { + fetchHint = "Run `gh pr view \(transition.prURL) --json reviews,comments` (and `gh api repos/{owner}/{repo}/pulls/\(transition.prNumber.map(String.init) ?? "")/comments` for inline comments) to read the full review feedback." + } + return """ + Crow detected a 'changes requested' review on \(prRef) (\(transition.prURL)). + \(fetchHint) Address every reviewer comment in code, commit the fix, and push so the PR updates. If a comment is unclear or you disagree, leave a reply explaining your reasoning instead of changing the code. + + """ + + case .checksFailing: + let failedSummary: String + if transition.failedCheckNames.isEmpty { + failedSummary = "" + } else { + let names = transition.failedCheckNames.prefix(5).joined(separator: ", ") + let extra = transition.failedCheckNames.count > 5 ? " (+\(transition.failedCheckNames.count - 5) more)" : "" + failedSummary = " Failing checks: \(names)\(extra)." + } + let logHint: String + if provider == .gitlab { + logHint = "Run `glab ci view` / `glab ci trace` on the failing pipeline to read the logs." + } else { + logHint = "Run `\(cli) pr checks \(transition.prURL)` to list the failing checks, then `\(cli) run view --log-failed ` to read the failure output." + } + return """ + Crow detected failing CI checks on \(prRef) (\(transition.prURL)).\(failedSummary) + \(logHint) Identify the root cause, fix it locally, run the relevant tests, then commit and push so CI re-runs. + + """ + } + } +} diff --git a/Sources/Crow/App/IssueTracker.swift b/Sources/Crow/App/IssueTracker.swift index 390e6ee..bc275dc 100644 --- a/Sources/Crow/App/IssueTracker.swift +++ b/Sources/Crow/App/IssueTracker.swift @@ -25,6 +25,11 @@ final class IssueTracker { /// `onWorkOnIssue` flow and post a notification. var onAutoCreateRequest: ((AssignedIssue) -> Void)? + /// Callback for detected PR status transitions — fires once per + /// transition, after dedupe. Wired in AppDelegate to drive notifications + /// and the auto-respond coordinator. + var onPRStatusTransitions: (([PRStatusTransition]) -> Void)? + /// Previously seen review request IDs for delta detection. private var previousReviewRequestIDs: Set = [] private var isFirstFetch = true @@ -39,6 +44,18 @@ final class IssueTracker { /// the trigger is one-shot and visible across machines. static let autoCreateLabel = "crow:auto" + /// Last observed `PRStatus` per session, used to compute transitions. + /// Populated lazily on first observation; that first poll never fires + /// transitions (matches the `previousReviewRequestIDs` first-fetch + /// behavior so existing PR state isn't replayed at startup). + private var previousPRStatus: [UUID: PRStatus] = [:] + + /// Stable keys of transitions we've already emitted, used to suppress + /// duplicates across the in-process lifetime. See `PRStatusTransition.dedupeKey`. + /// Cleared per-session when we observe the rule "re-arm" (e.g. checks + /// move back to passing/pending) so subsequent transitions still fire. + private var emittedTransitionKeys: Set = [] + /// Guards the GitHub-scope console warning so it fires once per session. private var didLogGitHubScopeWarning = false @@ -409,6 +426,7 @@ final class IssueTracker { let reviewDecision: String // APPROVED / CHANGES_REQUESTED / REVIEW_REQUIRED / "" let isDraft: Bool let headRefName: String + let headRefOid: String // Head commit SHA (empty if unavailable) let baseRefName: String let repoNameWithOwner: String let linkedIssueReferences: [LinkedIssue] @@ -452,6 +470,7 @@ final class IssueTracker { reviewDecision: winner.reviewDecision.isEmpty ? loser.reviewDecision : winner.reviewDecision, isDraft: winner.isDraft, headRefName: winner.headRefName.isEmpty ? loser.headRefName : winner.headRefName, + headRefOid: winner.headRefOid.isEmpty ? loser.headRefOid : winner.headRefOid, baseRefName: winner.baseRefName.isEmpty ? loser.baseRefName : winner.baseRefName, repoNameWithOwner: winner.repoNameWithOwner.isEmpty ? loser.repoNameWithOwner : winner.repoNameWithOwner, linkedIssueReferences: winner.linkedIssueReferences.isEmpty ? loser.linkedIssueReferences : winner.linkedIssueReferences, @@ -498,7 +517,7 @@ final class IssueTracker { viewerPRs: viewer { pullRequests(first: 50, states: [OPEN], orderBy: {field: UPDATED_AT, direction: DESC}) { nodes { - number url state mergeable reviewDecision isDraft headRefName baseRefName + number url state mergeable reviewDecision isDraft headRefName headRefOid baseRefName repository { nameWithOwner } closingIssuesReferences(first: 5) { nodes { number repository { nameWithOwner } } } statusCheckRollup { @@ -658,7 +677,7 @@ final class IssueTracker { pr\(i): repository(owner: $owner\(i), name: $repo\(i)) { pullRequest(number: $num\(i)) { number url state mergeable reviewDecision isDraft - headRefName baseRefName + headRefName headRefOid baseRefName repository { nameWithOwner } } } @@ -709,6 +728,7 @@ final class IssueTracker { let reviewDecision = prObj["reviewDecision"] as? String ?? "" let isDraft = prObj["isDraft"] as? Bool ?? false let headRefName = prObj["headRefName"] as? String ?? "" + let headRefOid = prObj["headRefOid"] as? String ?? "" let baseRefName = prObj["baseRefName"] as? String ?? "" let repoName = (prObj["repository"] as? [String: Any])?["nameWithOwner"] as? String ?? "" @@ -720,6 +740,7 @@ final class IssueTracker { reviewDecision: reviewDecision, isDraft: isDraft, headRefName: headRefName, + headRefOid: headRefOid, baseRefName: baseRefName, repoNameWithOwner: repoName, linkedIssueReferences: [], @@ -832,6 +853,7 @@ final class IssueTracker { let reviewDecision = node["reviewDecision"] as? String ?? "" let isDraft = node["isDraft"] as? Bool ?? false let headRefName = node["headRefName"] as? String ?? "" + let headRefOid = node["headRefOid"] as? String ?? "" let baseRefName = node["baseRefName"] as? String ?? "" let repoName = (node["repository"] as? [String: Any])?["nameWithOwner"] as? String ?? "" @@ -867,6 +889,7 @@ final class IssueTracker { reviewDecision: reviewDecision, isDraft: isDraft, headRefName: headRefName, + headRefOid: headRefOid, baseRefName: baseRefName, repoNameWithOwner: repoName, linkedIssueReferences: linkedRefs, @@ -1370,13 +1393,48 @@ final class IssueTracker { guard !viewerPRs.isEmpty else { return } let byURL = Dictionary(viewerPRs.map { ($0.url, $0) }, uniquingKeysWith: Self.mergePRRecords) + var transitions: [PRStatusTransition] = [] let sessionsWithPRs = appState.sessions.filter { $0.id != AppState.managerSessionID } for session in sessionsWithPRs { let links = appState.links(for: session.id) guard let prLink = links.first(where: { $0.linkType == .pr }) else { continue } guard let pr = byURL[prLink.url] else { continue } - appState.prStatus[session.id] = buildPRStatus(from: pr) + let newStatus = buildPRStatus(from: pr) + let oldStatus = previousPRStatus[session.id] + + // Re-arm rules whose triggering condition has cleared, so a future + // re-entry (approved → changesRequested again, passing → failing on + // a new commit) can fire even if we previously emitted. + if let old = oldStatus { + if old.reviewStatus == .changesRequested && newStatus.reviewStatus != .changesRequested { + emittedTransitionKeys.remove("\(session.id.uuidString)|changesRequested") + } + if old.checksPass == .failing && newStatus.checksPass != .failing { + if let sha = old.headSha { + emittedTransitionKeys.remove("\(session.id.uuidString)|checksFailing|\(sha)") + } + } + } + + let candidates = PRStatus.transitions( + from: oldStatus, + to: newStatus, + sessionID: session.id, + prURL: prLink.url, + prNumber: pr.number + ) + for t in candidates where !emittedTransitionKeys.contains(t.dedupeKey) { + emittedTransitionKeys.insert(t.dedupeKey) + transitions.append(t) + } + + previousPRStatus[session.id] = newStatus + appState.prStatus[session.id] = newStatus + } + + if !transitions.isEmpty { + onPRStatusTransitions?(transitions) } } @@ -1430,7 +1488,8 @@ final class IssueTracker { checksPass: checksPass, reviewStatus: reviewStatus, mergeable: mergeStatus, - failedCheckNames: failedChecks + failedCheckNames: failedChecks, + headSha: pr.headRefOid.isEmpty ? nil : pr.headRefOid ) } diff --git a/Sources/Crow/App/NotificationManager.swift b/Sources/Crow/App/NotificationManager.swift index 7306c29..6b19772 100644 --- a/Sources/Crow/App/NotificationManager.swift +++ b/Sources/Crow/App/NotificationManager.swift @@ -140,6 +140,57 @@ final class NotificationManager: NSObject, UNUserNotificationCenterDelegate { ) } + // MARK: - PR Status Transition Notifications + + /// Notify the user about a detected PR status transition (changes + /// requested or CI failing). Honors the same gating as `handleEvent`: + /// `globalMute` → category toggles → per-event toggles. Always posts a + /// system notification when allowed (no foreground/visible suppression + /// — the user typically wants to know about these even when looking at + /// the session). + func notifyPRTransition(_ transition: PRStatusTransition, session: Session) { + guard !settings.globalMute else { return } + + let event: NotificationEvent + let title: String + let body: String + let prRef = transition.prNumber.map { "PR #\($0)" } ?? "PR" + let suffix = session.ticketTitle.map { ": \($0)" } ?? "" + + switch transition.kind { + case .changesRequested: + event = .changesRequested + title = "Changes Requested \u{2014} \(session.name)" + body = "\(prRef)\(suffix) received a 'changes requested' review." + case .checksFailing: + event = .checksFailing + title = "CI Failing \u{2014} \(session.name)" + if transition.failedCheckNames.isEmpty { + body = "\(prRef)\(suffix) has failing CI checks." + } else { + let names = transition.failedCheckNames.prefix(3).joined(separator: ", ") + let extra = transition.failedCheckNames.count > 3 ? " (+\(transition.failedCheckNames.count - 3) more)" : "" + body = "\(prRef)\(suffix) failing: \(names)\(extra)" + } + } + + let config = settings.config(for: event) + guard config.enabled else { return } + + if settings.soundEnabled && config.soundEnabled { + playSound(named: config.soundName) + } + + if settings.systemNotificationsEnabled && config.systemNotificationEnabled { + postSystemNotification( + title: title, + body: body, + sessionID: transition.sessionID, + eventName: "PRTransition.\(transition.kind.rawValue)" + ) + } + } + // MARK: - Sound Playback private func playSound(named name: String) { diff --git a/Tests/CrowTests/IssueTrackerCompletionTests.swift b/Tests/CrowTests/IssueTrackerCompletionTests.swift index e8f0336..6bc03a4 100644 --- a/Tests/CrowTests/IssueTrackerCompletionTests.swift +++ b/Tests/CrowTests/IssueTrackerCompletionTests.swift @@ -43,6 +43,7 @@ struct IssueTrackerCompletionTests { reviewDecision: "", isDraft: false, headRefName: "", + headRefOid: "", baseRefName: "", repoNameWithOwner: "", linkedIssueReferences: [], diff --git a/Tests/CrowTests/IssueTrackerDedupTests.swift b/Tests/CrowTests/IssueTrackerDedupTests.swift index 88d818b..b4d959a 100644 --- a/Tests/CrowTests/IssueTrackerDedupTests.swift +++ b/Tests/CrowTests/IssueTrackerDedupTests.swift @@ -13,6 +13,7 @@ struct IssueTrackerDedupTests { reviewDecision: String = "", isDraft: Bool = false, headRefName: String = "", + headRefOid: String = "", baseRefName: String = "", repoNameWithOwner: String = "", linkedIssueReferences: [IssueTracker.ViewerPR.LinkedIssue] = [], @@ -28,6 +29,7 @@ struct IssueTrackerDedupTests { reviewDecision: reviewDecision, isDraft: isDraft, headRefName: headRefName, + headRefOid: headRefOid, baseRefName: baseRefName, repoNameWithOwner: repoNameWithOwner, linkedIssueReferences: linkedIssueReferences,