radiusmethod · dgershman · Jun 14, 2026 · Jun 14, 2026 · Jun 14, 2026 · Jun 14, 2026
@@ -232,8 +232,16 @@ extension WorkspaceGateway {
 
 /// 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.
+/// usually requires action.
+///
+/// `respondToChangesRequested` defaults **on** as of CROW-505 — auto-refine
+/// is the answer to the user's complaint that a PR sitting in
+/// CHANGES_REQUESTED with an idle agent never re-prompts. Existing users'
+/// explicit choices stay sticky: `decodeIfPresent` returns whatever was
+/// previously written, so a user who turned this off keeps it off across
+/// the upgrade. `respondToFailedChecks` still defaults off — typing into a
+/// terminal unprompted is intrusive, and CI flakes shouldn't auto-trigger
+/// a fix-attempt.
 public struct AutoRespondSettings: Codable, Sendable, Equatable {
     /// Inject a "fix the review feedback" prompt when a PR transitions into
     /// `reviewStatus == .changesRequested`.
@@ -244,7 +252,7 @@ public struct AutoRespondSettings: Codable, Sendable, Equatable {
     public var respondToFailedChecks: Bool
 
     public init(
-        respondToChangesRequested: Bool = false,
+        respondToChangesRequested: Bool = true,
         respondToFailedChecks: Bool = false
     ) {
         self.respondToChangesRequested = respondToChangesRequested
@@ -253,7 +261,7 @@ public struct AutoRespondSettings: Codable, Sendable, Equatable {
 
     public init(from decoder: Decoder) throws {
         let c = try decoder.container(keyedBy: CodingKeys.self)
-        respondToChangesRequested = try c.decodeIfPresent(Bool.self, forKey: .respondToChangesRequested) ?? false
+        respondToChangesRequested = try c.decodeIfPresent(Bool.self, forKey: .respondToChangesRequested) ?? true
         respondToFailedChecks = try c.decodeIfPresent(Bool.self, forKey: .respondToFailedChecks) ?? false
     }
 

@@ -16,21 +16,38 @@ public struct PRStatus: Codable, Sendable, Equatable {
     /// CHANGES_REQUESTED review is currently visible, or on providers that
     /// don't expose review IDs in the monitored-PR query (e.g. GitLab).
     public var latestReviewID: String?
+    /// Whether the underlying PR is currently OPEN (as opposed to MERGED or
+    /// closed-unmerged). Distinct from `mergeable == .merged`: a CLOSED PR
+    /// has `mergeable == .unknown` but is still not actionable.
+    ///
+    /// Required by the stalled-`.changesRequested` re-fire pass (CROW-505):
+    /// the standing-state predicate reads `previousPRStatus[sid]` directly
+    /// rather than firing on transition edges, so without this flag a PR
+    /// that merged or closed while in `CHANGES_REQUESTED` (its
+    /// `reviewDecision` doesn't reset on close) would keep re-prompting the
+    /// agent every quiet window to "address review feedback" on a dead PR.
+    /// Defaults to `true` for backward decode — pre-CROW-505 stores have no
+    /// `isOpen` field, and the very next poll rewrites the persisted status
+    /// from the live viewer payload before re-fire is consulted, so the
+    /// transient default never affects an actual re-fire decision.
+    public var isOpen: Bool
 
     public init(
         checksPass: CheckStatus = .unknown,
         reviewStatus: ReviewStatus = .unknown,
         mergeable: MergeStatus = .unknown,
         failedCheckNames: [String] = [],
         headSha: String? = nil,
-        latestReviewID: String? = nil
+        latestReviewID: String? = nil,
+        isOpen: Bool = true
     ) {
         self.checksPass = checksPass
         self.reviewStatus = reviewStatus
         self.mergeable = mergeable
         self.failedCheckNames = failedCheckNames
         self.headSha = headSha
         self.latestReviewID = latestReviewID
+        self.isOpen = isOpen
     }
 
     public init(from decoder: Decoder) throws {
@@ -41,10 +58,11 @@ public struct PRStatus: Codable, Sendable, Equatable {
         failedCheckNames = try c.decodeIfPresent([String].self, forKey: .failedCheckNames) ?? []
         headSha = try c.decodeIfPresent(String.self, forKey: .headSha)
         latestReviewID = try c.decodeIfPresent(String.self, forKey: .latestReviewID)
+        isOpen = try c.decodeIfPresent(Bool.self, forKey: .isOpen) ?? true
     }
 
     private enum CodingKeys: String, CodingKey {
-        case checksPass, reviewStatus, mergeable, failedCheckNames, headSha, latestReviewID
+        case checksPass, reviewStatus, mergeable, failedCheckNames, headSha, latestReviewID, isOpen
     }
 
     public enum CheckStatus: String, Codable, Sendable {

@@ -35,6 +35,15 @@ public struct PRStatusTransition: Sendable, Equatable {
     public let latestReviewID: String?
     /// Names of failing checks (empty for `.changesRequested`).
     public let failedCheckNames: [String]
+    /// True when this transition is a synthetic re-emission produced by the
+    /// stalled-`.changesRequested` re-fire pass (CROW-505), not a fresh
+    /// edge-detected transition. Consumers route on this to suppress noisy
+    /// downstream effects on re-fires (e.g. AppDelegate skips
+    /// `notifyPRTransition` so the user doesn't get a fresh macOS
+    /// notification every quiet window for the same review). The
+    /// `AutoRespondCoordinator` dispatch still runs — re-prompting the
+    /// stalled agent is the whole point.
+    public let isReFire: Bool
 
     public init(
         kind: Kind,
@@ -43,7 +52,8 @@ public struct PRStatusTransition: Sendable, Equatable {
         prNumber: Int? = nil,
         headSha: String? = nil,
         latestReviewID: String? = nil,
-        failedCheckNames: [String] = []
+        failedCheckNames: [String] = [],
+        isReFire: Bool = false
     ) {
         self.kind = kind
         self.sessionID = sessionID
@@ -52,6 +62,7 @@ public struct PRStatusTransition: Sendable, Equatable {
         self.headSha = headSha
         self.latestReviewID = latestReviewID
         self.failedCheckNames = failedCheckNames
+        self.isReFire = isReFire
     }
 
     /// Stable key used to suppress duplicate fires across polling cycles.
@@ -74,6 +85,46 @@ public struct PRStatusTransition: Sendable, Equatable {
     }
 }
 
+/// Per-emitted-transition metadata persisted alongside the dedup key.
+///
+/// `emittedAt` lets the stalled-re-fire pass (CROW-505) enforce a quiet
+/// window before re-prompting; `headShaAtEmit` is the head SHA observed at
+/// dispatch, which preserves the anti-loop guarantee — a real agent push
+/// advances the head SHA, so re-fire only triggers when the author has not
+/// pushed since the original prompt fired.
+///
+/// `reFireCount` bounds the number of times the same emission may re-fire
+/// before requiring a real edge event (reviewer rotates `latestReviewID`,
+/// author pushes, bucket exits) to reset. Without this cap the re-fire would
+/// loop forever when the agent legitimately *replies* on the PR instead of
+/// pushing — a behavior the auto-respond prompt itself encourages ("If a
+/// comment is unclear or you disagree, leave a reply…"). The reply leaves
+/// the SHA unchanged but is a valid terminal state, not a stall.
+public struct EmittedTransitionMeta: Codable, Sendable, Equatable {
+    public var emittedAt: Date
+    public var headShaAtEmit: String?
+    public var reFireCount: Int
+
+    public init(emittedAt: Date, headShaAtEmit: String?, reFireCount: Int = 0) {
+        self.emittedAt = emittedAt
+        self.headShaAtEmit = headShaAtEmit
+        self.reFireCount = reFireCount
+    }
+
+    public init(from decoder: Decoder) throws {
+        let c = try decoder.container(keyedBy: CodingKeys.self)
+        emittedAt = try c.decode(Date.self, forKey: .emittedAt)
+        headShaAtEmit = try c.decodeIfPresent(String.self, forKey: .headShaAtEmit)
+        // Backward decode for pre-cap stores: a missing field means the entry
+        // has never been re-fired yet, so 0 is the safe default.
+        reFireCount = try c.decodeIfPresent(Int.self, forKey: .reFireCount) ?? 0
+    }
+
+    private enum CodingKeys: String, CodingKey {
+        case emittedAt, headShaAtEmit, reFireCount
+    }
+}
+
 extension PRStatus {
     /// Compute the transitions implied by moving from `old` to `new`.
     ///

@@ -0,0 +1,47 @@
+import Foundation
+import Testing
+@testable import CrowCore
+
+/// CROW-505: AutoRespondSettings default flip and decoder behavior.
+@Suite("AutoRespondSettings defaults")
+struct AutoRespondSettingsDefaultsTests {
+    @Test func defaultInitializerDefaultsRespondToChangesRequestedOn() {
+        // The whole point of CROW-505: a fresh install gets auto-refine on by
+        // default. Old default was off; users assumed it worked but the toggle
+        // was silently false.
+        let settings = AutoRespondSettings()
+        #expect(settings.respondToChangesRequested == true)
+        // Failed-checks stays off — separate decision; intrusive prompts on
+        // every CI flake aren't what most users want.
+        #expect(settings.respondToFailedChecks == false)
+    }
+
+    @Test func decoderFallbackForMissingKeyMatchesNewDefault() throws {
+        // Older configs that never wrote the field at all (key absent) should
+        // pick up the new default — this is the upgrade path for users who
+        // never touched the setting.
+        let json = "{}".data(using: .utf8)!
+        let decoded = try JSONDecoder().decode(AutoRespondSettings.self, from: json)
+        #expect(decoded.respondToChangesRequested == true)
+        #expect(decoded.respondToFailedChecks == false)
+    }
+
+    @Test func decoderPreservesExplicitFalseChoice() throws {
+        // Existing users who explicitly toggled the setting OFF in UI have the
+        // key written as `false` in their JSON. `decodeIfPresent` returns the
+        // written value, so their choice survives the default flip. This is
+        // the "existing choices stay sticky" guarantee.
+        let json = #"{"respondToChangesRequested": false, "respondToFailedChecks": false}"#
+            .data(using: .utf8)!
+        let decoded = try JSONDecoder().decode(AutoRespondSettings.self, from: json)
+        #expect(decoded.respondToChangesRequested == false)
+    }
+
+    @Test func decoderHonorsExplicitTrueChoice() throws {
+        let json = #"{"respondToChangesRequested": true, "respondToFailedChecks": true}"#
+            .data(using: .utf8)!
+        let decoded = try JSONDecoder().decode(AutoRespondSettings.self, from: json)
+        #expect(decoded.respondToChangesRequested == true)
+        #expect(decoded.respondToFailedChecks == true)
+    }
+}
@@ -5,18 +5,27 @@ import CrowCore
 /// without re-firing already-handled PR transitions (CROW-456).
 ///
 /// `previousPRStatus` is keyed by session UUID string so the on-disk shape
-/// stays plain JSON. `emittedTransitionKeys` is serialized as an array because
-/// `Set` is not a stable JSON type — the in-memory load converts back.
+/// stays plain JSON. `emittedTransitionKeys` is the legacy array of dedup
+/// keys retained for backward read; CROW-505 introduced the richer
+/// `emittedTransitionMeta` map, which is now the source of truth on write.
+/// Older `store.json` files lacking the map are migrated lazily in
+/// `IssueTracker.hydratePersistedState`.
 public struct PersistedIssueTrackerState: Codable, Sendable, Equatable {
     public var previousPRStatus: [String: PRStatus]
     public var emittedTransitionKeys: [String]
+    /// Optional for backward compat: pre-CROW-505 stores only wrote
+    /// `emittedTransitionKeys`. Newer stores populate this map and leave
+    /// `emittedTransitionKeys` empty.
+    public var emittedTransitionMeta: [String: EmittedTransitionMeta]?
 
     public init(
         previousPRStatus: [String: PRStatus] = [:],
-        emittedTransitionKeys: [String] = []
+        emittedTransitionKeys: [String] = [],
+        emittedTransitionMeta: [String: EmittedTransitionMeta]? = nil
     ) {
         self.previousPRStatus = previousPRStatus
         self.emittedTransitionKeys = emittedTransitionKeys
+        self.emittedTransitionMeta = emittedTransitionMeta
     }
 }
 

@@ -214,9 +214,11 @@ import Testing
         latestReviewID: "R_kgD0_1"
     )
     let dedupKey = "\(sessionID.uuidString)|changesRequested|R_kgD0_1"
+    let emittedAt = Date(timeIntervalSince1970: 1_700_000_000)
     let state = PersistedIssueTrackerState(
         previousPRStatus: [sessionID.uuidString: pr],
-        emittedTransitionKeys: [dedupKey]
+        emittedTransitionKeys: [dedupKey],
+        emittedTransitionMeta: [dedupKey: EmittedTransitionMeta(emittedAt: emittedAt, headShaAtEmit: "abc123", reFireCount: 2)]
     )
 
     let store = JSONStore(directory: dir)
@@ -226,6 +228,26 @@ import Testing
     #expect(reloaded == state)
     #expect(reloaded?.previousPRStatus[sessionID.uuidString]?.latestReviewID == "R_kgD0_1")
     #expect(reloaded?.emittedTransitionKeys == [dedupKey])
+    #expect(reloaded?.emittedTransitionMeta?[dedupKey]?.headShaAtEmit == "abc123")
+    #expect(reloaded?.emittedTransitionMeta?[dedupKey]?.emittedAt == emittedAt)
+    // CROW-505 review #3: reFireCount survives the round trip so the
+    // per-emission cap persists across Crow restarts.
+    #expect(reloaded?.emittedTransitionMeta?[dedupKey]?.reFireCount == 2)
+}
+
+@Test func emittedTransitionMetaDecodesLegacyEntryWithoutReFireCount() throws {
+    // Backward compat: pre-cap stores wrote `EmittedTransitionMeta` without
+    // the `reFireCount` field. The decode must default it to 0 so an
+    // upgrade doesn't wedge the re-fire path by inheriting a stale count.
+    // Uses the same `.iso8601` date strategy as `JSONStore` so the test
+    // matches the real on-disk shape.
+    let legacyJSON = #"{"emittedAt": "2023-11-14T22:13:20Z", "headShaAtEmit": "abc123"}"#
+        .data(using: .utf8)!
+    let decoder = JSONDecoder()
+    decoder.dateDecodingStrategy = .iso8601
+    let decoded = try decoder.decode(EmittedTransitionMeta.self, from: legacyJSON)
+    #expect(decoded.headShaAtEmit == "abc123")
+    #expect(decoded.reFireCount == 0)
 }
 
 @Test func storeDataDecodesWithoutIssueTrackerStateField() throws {

@@ -778,6 +778,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
         tracker.onPRStatusTransitions = { [weak self] transitions in
             guard let self else { return }
             for transition in transitions {
+                // CROW-505 review #3: re-fire transitions skip the macOS
+                // notification so a stalled review doesn't generate a fresh
+                // banner every quiet window for the same review id. The
+                // re-prompt itself (below) is the useful effect; the
+                // notification was pure noise.
+                if transition.isReFire { continue }
                 if let session = self.appState.sessions.first(where: { $0.id == transition.sessionID }) {
                     self.notificationManager?.notifyPRTransition(transition, session: session)
                 }
@@ -803,6 +809,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
         tracker.autoRebaseWatcherEnabledProvider = { [weak self] in
             self?.appConfig?.autoRebaseWatcherEnabled ?? false
         }
+        tracker.respondToChangesRequestedProvider = { [weak self] in
+            self?.appConfig?.autoRespond.respondToChangesRequested ?? false
+        }
         tracker.onAutoRebasePushed = { [weak self] sessionID, _, number in
             self?.notificationManager?.notifyAutoRebasePushed(number: number, sessionID: sessionID)
         }