Skip to content

needsRefine: accept .done as idle state for refine dispatch#511

Merged
dgershman merged 1 commit into
mainfrom
feature/crow-510-idle-gate-done
Jun 15, 2026
Merged

needsRefine: accept .done as idle state for refine dispatch#511
dgershman merged 1 commit into
mainfrom
feature/crow-510-idle-gate-done

Conversation

@dgershman

Copy link
Copy Markdown
Collaborator

Closes #510. Patches #509.

Summary

The stateless needsRefine gate landed in #509 only accepted AgentActivityState.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 today, both with hookState.activityState = .done and no commit since the reviewer's CHANGES_REQUESTED:

Change

One-line predicate change in isManagedTerminalIdle:

let state = appState.hookState(for: sessionID).activityState
return state == .idle || state == .done

.working and .waiting still gate. Doc comment updated to match. No other code moves.

Tests

In Tests/CrowTests/IssueTrackerNeedsRefineTests.swift:

  • doneAgentDispatches — new, regression for the bug. .done + needs-refine PR → one dispatch.
  • waitingAgentSuppressesDispatch — new, negative coverage for the remaining gated state.
  • busyAgentSuppressesDispatch — existing test, updated to pass activityState: .working instead of agentIdle: false.
  • makeTracker helper signature: agentIdle: BoolactivityState: AgentActivityState.

All 14 needsRefine tests pass; all 141 IssueTracker-suite tests pass.

Smoke test

Not yet run against the live store — recommend restarting the deployed Crow build against the same store after this lands and watching for corveil#1427 and shell-crm#202 to dispatch on the next poll without manual intervention.

Test plan

  • swift test --filter IssueTrackerNeedsRefineTests — all 14 pass
  • swift test --filter IssueTracker — all 141 pass
  • swift build --arch arm64 — clean
  • Restart Crow against the live store; observe corveil#1427 and shell-crm#202 dispatch on next poll

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
Crow-Session: 5ABBAED5-0083-4F1C-BFB0-2EAF3019771D
@dgershman dgershman requested a review from dhilgaertner as a code owner June 15, 2026 04:20
@dgershman dgershman added the crow:merge Crow auto-merge on green label Jun 15, 2026

@dhilgaertner dhilgaertner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Critical Issues

None.

Security Review

Strengths:

  • No security surface touched. The change is a pure in-memory predicate over local AppState enum values — no I/O, no auth, no user input, no external data. The new test helper is test-only.

Concerns:

  • None.

Code Quality

  • Correctness (Green): The new predicate state == .idle || state == .done matches the real agent lifecycle. Verified against ClaudeHookSignalSource: the top-level Stop event sets .done (ClaudeHookSignalSource.swift:92), leaving the agent at the prompt and able to accept the next prompt — precisely when refine should dispatch. The previously-only-accepted .idle state never recurs after the agent's first task, which is the exact bug described (CROW-510).
  • Gating still sound (Green): .working (active) and .waiting (blocked on a permission prompt / question / StopFailure, see :65,82,97) continue to gate, so refine won't interrupt a busy or blocked agent. The .agentLaunched readiness pre-check is preserved.
  • No false positives from .done (Green): .done is also reached via SessionStart source=resume — likewise a legitimately-available-at-prompt state, so accepting it does not introduce spurious dispatches.
  • Tests (Green): Adds doneAgentDispatches (regression for the bug) and waitingAgentSuppressesDispatch (negative coverage for the remaining gated state), and migrates busyAgentSuppressesDispatch to the new helper. The makeTracker signature change from agentIdle: Bool to activityState: AgentActivityState is a strict improvement — all four states are now expressible.
  • Docs (Green): The doc comment on isManagedTerminalIdle was updated to match the new behavior.

Note: I could not execute the test suite in this review checkout — swift test fails at dependency resolution because the local GhosttyKit.xcframework binary artifact is not present in the worktree (an environment limitation, unrelated to this change). The logic was verified by reading the state machine directly.

Summary Table

Color Meaning Verdict effect
Red Must fix Request changes
Yellow Should fix Request changes
Green Consider Approve allowed

Recommendation: Approve — driven by [0 Red, 0 Yellow, 5 Green] findings. A minimal, correct, well-tested one-line fix with matching test and doc updates.


🐦‍⬛ Reviewed by Crow via Claude Code

@dgershman dgershman merged commit ece78ae into main Jun 15, 2026
2 checks passed
@dgershman dgershman deleted the feature/crow-510-idle-gate-done branch June 15, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crow:merge Crow auto-merge on green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

needsRefine idle gate excludes .done — refine never dispatches after an agent's first task

2 participants