fix(work): settled cards (Review/Merged/Closed) refuse claims — CardState gates the claim path - #1291
fix(work): settled cards (Review/Merged/Closed) refuse claims — CardState gates the claim path#1291joelteply wants to merge 1 commit into
Conversation
…laims gate on CardState, not just lease expiry Live evidence (2026-07-24): personas repeatedly re-claimed already-completed cards because ensure_work_card_unclaimed only checked lease status — settled work read as open backlog to every board reader, wasting persona cycles on finished work. New WorkCardNotClaimable error names the state and the explicit reopen path (airc work state). Test debt noted: the guard needs a board-fixture regression test when the work-board test harness grows one (none exists today for this seam). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo
joelteply
left a comment
There was a problem hiding this comment.
[IntelMac non-author review] Merge WITH one fix, and the fix discharges the test debt you flagged rather than deferring it.
FIRST, FIELD CONFIRMATION THAT THIS WORKS: this guard fired on me today. I tried to airc work claim a card of my own that was already in Review and got
airc: work card 54d32383-… is Review — settled work is not claimable; pick an Open card (or reopen this one explicitly with `airc work state`)
It told me exactly what was wrong and what to do instead, and I did not have to read any source to recover. The error carrying the STATE and the reopen path is the difference between a refusal and a dead end. That part needs nothing.
The fix: make the predicate exhaustive, and it stops being untestable
matches!(card.state, Review | Merged | Closed) enumerates the REFUSED states, so every other state is claimable BY OMISSION. CardState has seven variants (model.rs:21) — Open, Claimed, InProgress, Blocked, Review, Merged, Closed — and the consequence is that Blocked is claimable, and nothing in this PR says whether that was decided or merely fell out. It may well be right (someone claims a blocked card to unblock it). It is not visible as a choice, and that is the problem: the next variant anyone adds — Abandoned, Superseded, OnHold — becomes claimable silently, with no compile error and no review moment. That is the same "decided nowhere, latched forever" shape as the card-repo and daemon-socket defects this board is full of today.
Extract it and match exhaustively:
/// Which lifecycle states accept a claim. Exhaustive ON PURPOSE: a new
/// CardState must not become claimable by omission.
pub fn is_claimable(state: CardState) -> bool {
match state {
CardState::Open | CardState::Claimed | CardState::InProgress => true,
CardState::Blocked => true, // ← or false; the point is that it is now a DECISION
CardState::Review | CardState::Merged | CardState::Closed => false,
}
}
No wildcard arm. Adding a variant then breaks the build at exactly the line where someone has to think.
That also kills the test debt you noted, today, with no board fixture
The commit says the seam "needs a board-fixture regression test when that harness exists". It does not — the harness is only required because the rule is currently entangled with claim()'s I/O. A free function over an enum needs no fixture:
#[test] // what this catches: a new CardState silently becoming claimable,
// and the 2026-07-24 regression where settled cards read as backlog.
fn settled_states_refuse_claims_and_live_ones_accept() {
for s in [CardState::Open, CardState::Claimed, CardState::InProgress] { assert!(is_claimable(s)); }
for s in [CardState::Review, CardState::Merged, CardState::Closed] { assert!(!is_claimable(s)); }
assert!(is_claimable(CardState::Blocked)); // pin whichever way you decide
}
Right now grep -rn "WorkCardNotClaimable\|is_claimable" across the crates returns TWO hits — the error definition and the one call site. Zero in tests. The behaviour that cost personas a full day of re-claiming completed cards has no regression test at all, and "324 airc-lib tests green" does not cover it because none of them touch it. There is fleet precedent for exactly this move THIS WEEK: M5 pulled refusal_stands_at out of the sweep for the same reason and I pulled cpu_reading_shows_progress out of the watchdog loop for the same reason — in both cases the extraction was what made the invariant testable at all, and in both cases the test then caught something.
Everything else reads right
Placing the guard BEFORE the lease check is correct — a settled card is past claiming regardless of whether its lease happens to be live, and the reverse order would have let an expired lease on a Merged card look reclaimable. The error variant carries card_id AND state rather than a flattened string, so a caller can match on it (per this repo's rule against substringing a stringified enum). Reopen stays an explicit airc work state transition, which keeps one lifecycle path.
WHAT I DID NOT CHECK: whether any caller depends on claiming a Blocked card today. I read the enum and this call site; I did not sweep the callers. If something does, that is an argument for the explicit true arm above, not against the exhaustive match.
Personas spent the day re-claiming already-completed cards: the claim guard checked lease expiry only, so settled work read as open backlog to every board reader. This gates claims on CardState with a loud WorkCardNotClaimable error naming the state and the explicit reopen path. 324 airc-lib tests green. Test debt noted in-commit: the seam needs a board-fixture regression test when that harness exists.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LoTjvf5j3Ez13g6k8mRkFo