From 80a6f289c2c6a463b2b98b4a570dd26e14c4150c Mon Sep 17 00:00:00 2001 From: Danny Gershman Date: Mon, 15 Jun 2026 00:19:47 -0400 Subject: [PATCH] needsRefine: accept .done as idle state for refine dispatch (CROW-510) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stateless needsRefine gate from #509 only accepted .idle, but a Claude Code agent's lifecycle is .idle → .working → … → .done. Once the first top-level Stop hook fires, the state stays at .done until the next prompt arrives — exactly when refine should dispatch. As a result, refine never fired after an agent's first task. Two PRs were observed stuck on this gate: corveil#1427 and shell-crm#202, both with hookState.activityState = .done and no commit since the reviewer's CHANGES_REQUESTED. Fix: accept both .idle (fresh agent) and .done (finished, waiting) in isManagedTerminalIdle. .working and .waiting still gate. Tests: doneAgentDispatches (regression for the bug), waitingAgentSuppressesDispatch (negative coverage for the remaining gated state). Updated busyAgentSuppressesDispatch and the makeTracker helper to take an AgentActivityState directly rather than a Bool. 🐦‍⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude Crow-Session: 5ABBAED5-0083-4F1C-BFB0-2EAF3019771D --- Sources/Crow/App/IssueTracker.swift | 12 ++++--- .../IssueTrackerNeedsRefineTests.swift | 35 +++++++++++++++---- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/Sources/Crow/App/IssueTracker.swift b/Sources/Crow/App/IssueTracker.swift index 3047e56..b988d83 100644 --- a/Sources/Crow/App/IssueTracker.swift +++ b/Sources/Crow/App/IssueTracker.swift @@ -1626,16 +1626,18 @@ final class IssueTracker { } /// True when the managed terminal for the session is at agent-launched - /// readiness with the agent in `.idle` (not working, waiting on input, - /// or already done). Both gates are required: a pre-launch terminal - /// can't be "stalled" because the agent never had a chance to run, and - /// firing into a busy agent would just interrupt productive work. + /// readiness with the agent available to accept a prompt — either + /// `.idle` (fresh, never run) or `.done` (finished a top-level task and + /// waiting). `.working` and `.waiting` still gate: firing into a busy + /// or blocked agent would interrupt it. A pre-launch terminal also + /// gates, because the agent never had a chance to run. private func isManagedTerminalIdle(sessionID: UUID) -> Bool { guard let managedTerminal = appState.terminals(for: sessionID).first(where: { $0.isManaged }) else { return false } guard appState.terminalReadiness[managedTerminal.id] == .agentLaunched else { return false } - return appState.hookState(for: sessionID).activityState == .idle + let state = appState.hookState(for: sessionID).activityState + return state == .idle || state == .done } /// True when no prior dispatch is recorded for this PR or the cooldown diff --git a/Tests/CrowTests/IssueTrackerNeedsRefineTests.swift b/Tests/CrowTests/IssueTrackerNeedsRefineTests.swift index c03ccc7..e98f4ab 100644 --- a/Tests/CrowTests/IssueTrackerNeedsRefineTests.swift +++ b/Tests/CrowTests/IssueTrackerNeedsRefineTests.swift @@ -33,7 +33,7 @@ struct IssueTrackerNeedsRefineTests { private func makeTracker( respondToChangesRequested: Bool = true, readiness: TerminalReadiness = .agentLaunched, - agentIdle: Bool = true + activityState: AgentActivityState = .idle ) -> (tracker: IssueTracker, sessionID: UUID, captured: TransitionCapture) { let state = AppState() let session = Session(name: "feature/stateless-test", kind: .work) @@ -50,11 +50,7 @@ struct IssueTrackerNeedsRefineTests { ) state.terminals[session.id] = [terminal] state.terminalReadiness[terminal.id] = readiness - // Hook state activity defaults to .idle from AppState — set - // explicitly when the test wants it busy. - if !agentIdle { - state.hookState(for: session.id).activityState = .working - } + state.hookState(for: session.id).activityState = activityState let tracker = IssueTracker(appState: state, providerManager: ProviderManager()) tracker.respondToChangesRequestedProvider = { respondToChangesRequested } @@ -185,13 +181,38 @@ struct IssueTrackerNeedsRefineTests { @Test func busyAgentSuppressesDispatch() { - let (tracker, _, captured) = makeTracker(agentIdle: false) + let (tracker, _, captured) = makeTracker(activityState: .working) + tracker.seenPRs.insert(prURL) + let pr = makeViewerPR(lastChangesRequestedAt: reviewAt, lastSubstantiveCommitAt: beforeReview) + tracker.applyPRStatuses(viewerPRs: [pr]) + #expect(captured.changesRequestedCount == 0) + } + + @Test + func waitingAgentSuppressesDispatch() { + // `.waiting` means the agent is blocked on input (e.g. a permission + // prompt). Refine must not fire — same reasoning as `.working`. + let (tracker, _, captured) = makeTracker(activityState: .waiting) tracker.seenPRs.insert(prURL) let pr = makeViewerPR(lastChangesRequestedAt: reviewAt, lastSubstantiveCommitAt: beforeReview) tracker.applyPRStatuses(viewerPRs: [pr]) #expect(captured.changesRequestedCount == 0) } + @Test + func doneAgentDispatches() { + // CROW-510 regression: after an agent finishes its first top-level + // task, the hook state lands on `.done` and stays there until the + // next prompt. Refine must dispatch in that state — historically the + // gate only accepted `.idle`, so the rule never fired after the + // agent's first task. + let (tracker, _, captured) = makeTracker(activityState: .done) + tracker.seenPRs.insert(prURL) + let pr = makeViewerPR(lastChangesRequestedAt: reviewAt, lastSubstantiveCommitAt: beforeReview) + tracker.applyPRStatuses(viewerPRs: [pr]) + #expect(captured.changesRequestedCount == 1) + } + @Test func preLaunchTerminalSuppressesDispatch() { let (tracker, _, captured) = makeTracker(readiness: .uninitialized)