Replace edge-triggered changesRequested detection with a stateless 'last PR event was a CHANGES_REQUESTED review and no substantive commit since' check

[dgershman]

Context

Asking the user's question — "is it possible to just monitor the last thing that happened on a PR to see if it was a changes-requested review with no new SHA besides a rebase?" — and you get a much simpler design than what crow#505 / merged PR #507 actually shipped.

What's deployed today is edge-triggered with bookkeeping: per-session previousPRStatus + emittedTransitionMeta, dedupeKey keyed on latestReviewID, a stalled-re-fire pass with a 10-minute quiet window and headShaAtEmit to avoid loops. It works when everything stays consistent. It silently fails when the tracker is paused, the poll loop is starved (we just observed this on shell-crm#202 — Crow alive over RPC but [IssueTracker] log lines went silent for 30+ minutes), previousPRStatus doesn't get persisted, or the dedup map gets out of sync with reality after a restart.

Real-world gap: shell-crm#202 sat in CHANGES_REQUESTED from Review #3 at 23:41 UTC to ~00:17 UTC without Crow re-prompting, even though #507 was deployed at 22:46 UTC and Crow restarted at 23:34 UTC.

This proposal removes the bookkeeping by deriving the answer from the PR alone, every poll, no memory.

Proposal: stateless "needs refine" check on the PR itself

For every session that has a .pr link, on each tick:

1. Fetch the PR (already done by IssueTracker).
2. Compute last_changes_requested_at = max submitted_at across reviews where state == CHANGES_REQUESTED AND the reviewer hasn't been superseded by a newer non-pending review from the same user (use GitHub's "latest review per reviewer" rule — same one reviewDecision already uses).
3. Compute last_substantive_commit_at = max committer.date across PR commits where the commit is not a rebase/merge. A commit counts as non-substantive when any of:
   • parents.length > 1 (merge commit — covers git merge origin/main, GitHub's "Update branch" button)
   • commit message matches ^Merge (branch|remote-tracking|pull request)\b
   • tree equals one of the parents' trees (pure rebase / cherry-pick that didn't actually change files — optional, costs another API call)
4. If reviewDecision == "CHANGES_REQUESTED" and last_substantive_commit_at < last_changes_requested_at and the managed terminal is idle: emit .changesRequested.

That's the whole rule. No in-memory previousPRStatus, no emittedTransitionMeta, no dedup keys, no quiet-window enforcement (idle-terminal already covers it), no "did we already fire for this review id" bookkeeping.

Why this is enough

• Loop prevention is automatic. The moment the agent makes any substantive commit, last_substantive_commit_at advances past last_changes_requested_at and the rule stops firing. No headShaAtEmit snapshot needed.
• Round-2/round-3 handled for free. A new reviewer-submitted CHANGES_REQUESTED advances last_changes_requested_at; if the agent already committed in round-2 the rule won't fire (commit > review time), and when round-3 lands it will (new review time > commit time).
• Rebases and merge-from-main don't trick it. Filtering on commit shape excludes them from last_substantive_commit_at, so a Merge branch 'main' commit doesn't reset the "needs refine" flag the way it would in a naive headSha != headShaAtEmit check.
• Restart-safe with no persistence. The answer derives from PR data on every poll. A Crow restart, a missed poll, or a corrupted store.json doesn't matter — the next successful poll re-derives the truth.
• No false fires during reviewer time. If the reviewer leaves CHANGES_REQUESTED and then immediately follows up with another formal review (round-2 within seconds), the rule fires once for whichever is the latest CHANGES_REQUESTED — naturally deduped without keys.
• First-observation cases collapse into the same code path as round-N — no special branch like PRStatusTransition.swift:108–136 needed.

Where the current design beats this

The one case CROW-505/#507's design covers that the stateless rule does not: the agent's terminal is busy when the prompt is first dispatched, then goes idle without producing a commit, and there is no new reviewer activity. The stateless rule, by checking "managed terminal idle," will catch this on the next idle tick — so actually, it does cover this, just without per-session bookkeeping. The CROW-505 design pays for the bookkeeping to enforce maxStalledRefires: 1 and stalledRefireQuietWindow: 10min — both worthwhile guardrails the stateless rule needs to add back. See "Add to make this safe" below.

Add to make this safe

• Idle-terminal check (already present in dispatch/canSend): keep it. Prevents firing into a working agent.
• Per-PR cooldown (small, ephemeral): a Map<PR-URL → lastDispatchAt> with a 5–10 min minimum gap between fires for the same PR. Survives restart loss with no real damage — worst case is one duplicate prompt right after restart, which is fine. Replaces the maxStalledRefires: 1 guard with simpler "don't spam."
• First-observation skip: on the very first poll for a session (the in-memory state is empty), skip emitting until the second poll. Cheap startup-flood guard equivalent to what transitions(from: nil, …) had pre-CROW-477; small cost vs. catching false fires when GitHub returns stale data on the first tick.

What deletes if this lands

previousPRStatus, emittedTransitionMeta, dedupeKey, EmittedTransitionMeta.headShaAtEmit, stalledRefireQuietWindow, maxStalledRefires, and hydratePersistedState on the tracker can all be deleted — they were carrying weight specifically for the edge-triggered design. The persisted issueTrackerState blob in store.json becomes obsolete; hydration becomes a no-op.

Open question: tear the old machinery out in the same PR, or in a follow-up? Same PR makes the diff easier to reason about; follow-up makes the new rule easier to revert if it misfires.

Acceptance

• Simulate Review Native macOS IDE with Corveil branding and GitHub integration #1 → no-commit pause → Review Integrate Claude Code hooks and event system for session activity tracking #2 → still no commit → Crow re-prompts on the next idle poll without needing a reviewer-side rotation.
• Simulate Merge branch main after CHANGES_REQUESTED — needs_refine stays true (does NOT get reset by the merge commit).
• Simulate a real fix push — needs_refine flips to false on the next poll.
• Simulate a Crow restart mid-stall — next poll picks up the standing CHANGES_REQUESTED and fires once (with the first-observation skip + cooldown together preventing double-fires).

Relationship to prior tickets

Follow-up to crow#505 / merged crow#507 ("Auto-refine: re-fire stalled rounds when agent is idle + head SHA unchanged"). This isn't a revert — #507 fixed the original "edge-triggered + dedup leaves agent stranded" complaint. This proposes replacing the whole edge-triggered model with a stateless one after #507 was observed still missing fires in the wild.

🐦‍⬛ Created with Crow via Claude Code