Skip to content

fix(work): settled cards (Review/Merged/Closed) refuse claims — CardState gates the claim path - #1291

Open
joelteply wants to merge 1 commit into
canaryfrom
fix/settled-cards-not-claimable
Open

fix(work): settled cards (Review/Merged/Closed) refuse claims — CardState gates the claim path#1291
joelteply wants to merge 1 commit into
canaryfrom
fix/settled-cards-not-claimable

Conversation

@joelteply

Copy link
Copy Markdown
Contributor

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

…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 joelteply left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant