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,