docs: record #579 as won't-fix — suspension is immediate for use, not issuance

AGENTS.md

@@ -8,12 +8,16 @@ the **canonical rule set** for the repo. Humans usually want the short path firs
 [`docs/`](docs/README.md) (runbooks, decision records, releasing),
 [`SECURITY.md`](SECURITY.md).
 
-**Every rule here is one paragraph.** A rule that carries a `→` link was earned
-by a defect that shipped, and the narrative — what broke, which review round
-found it, what the wrong fix was — is behind that link in
-[`docs/decisions/`](docs/decisions/): **follow it before changing the rule.** A
-rule with no link is a plain convention that has not yet cost anything; it needs
-consistency, not archaeology.
+**Every rule here is one paragraph.** A rule that carries a `→` link records
+what stands behind it, and the narrative is behind that link in
+[`docs/decisions/`](docs/decisions/): **follow it before changing the rule.**
+Two kinds, and the link alone does not say which: an **earned rule** was
+earned by a defect that shipped (what broke, which review round found it, what
+the wrong fix was), and an **accepted-risk rule** records a deliberate
+declination — no incident, the record says `No incident` and the rule is
+*load-bearing* (break it and the accepted risk returns). A rule with no link
+is a plain convention that has not yet cost anything; it needs consistency, not
+archaeology.
 
 - [Communicating](#communicating) · [Layout](#layout) · [Build / test / run](#build--test--run)
 - [Conventions](#conventions-follow-these) — the rules that break things when ignored
@@ -72,6 +76,7 @@ dotnet test  Cluckwork.sln                 # 688 tests as of 2026-07; integratio
 - **Multi-tenancy:** every tenant-owned entity has `AccountId`, enforced by an EF **global query filter** plus a **`TenantStampInterceptor`** (stamps on insert). `TenantContext` resolves per-request from the JWT `account_id` claim; at startup it is unresolved, so seeders use `IgnoreQueryFilters()`.
 - **Transient-DB retry stops at unreplayable work (#269).** `EnableRetryOnFailure` covers self-contained EF units only; an automatic replay above a stateful detector (a counter, a CAS stamp, a single-use claim) cannot tell "this request racing itself" from the signal the detector exists to catch. Two cures, and picking wrong ships the bug: `SingleAttemptExecution` when the replay is itself observable, a durability probe on a self-minted token when the replay writes nothing. → [`269-transient-db-retry-boundary.md`](docs/decisions/269-transient-db-retry-boundary.md)
 - **`AuditEvents` is not time-partitioned, on purpose (#505).** The dominant read filters on `AccountId`+`EntityType`+`EntityId` with no date predicate, so monthly partitions would turn one index lookup into one per partition for no pruning benefit. If it is ever needed, partition by `AccountId`. → [`505-audit-events-no-time-partition.md`](docs/decisions/505-audit-events-no-time-partition.md)
+- **Suspension is immediate for *use*, not for *issuance* (#579).** Login checks `Account.IsActive` and mints in two steps; a suspension committing between them returns 200 with an inert credential, and the `FOR SHARE` lock that would close the window was declined twice on hot-path cost. #530's "race-safe" means use, not issuance — the inertness rests on four named premises (live middleware read, `RefreshAsync`'s suspended check, suspension's same-transaction revocation of credentials existing when the sweep executes, reactivation's revoke), each pinned by a guard that fails when that premise alone breaks; break any one and #579 reopens. → [`579-suspension-issuance-window.md`](docs/decisions/579-suspension-issuance-window.md)
 
 ### Migrations and schema
 

docs/decisions/579-suspension-issuance-window.md

@@ -0,0 +1,148 @@
+# Suspension is immediate for use, not for issuance — the check-then-mint window stays open (#579)
+
+> **Rule** — the one-paragraph version lives in [`AGENTS.md`](../../AGENTS.md);
+> this file is the relocated rationale (what shipped, why the short version was
+> insufficient, what not to break).
+
+**Status:** accepted (won't-fix, 2026-08-21, owner decision on #579)
+**Date:** 2026-08-21
+**No incident** — accepted-risk record (no defect shipped; deliberate declination).
+
+## What happened
+
+There was no defect to fix — this is a deliberate declination, recorded so the
+next reader does not re-derive it. It is an accepted-risk decision, not an
+earned rule: the `AGENTS.md` bullet carries the same link shape as the
+incident-earned rules because the decision record is its rationale home, but the
+rule was not earned by a shipped defect.
+
+Login does two things in sequence, in separate steps: `AuthEndpoints` checks
+`Account.IsActive` on a plain read, then `IdentityProvider.LoginAsync` mints the
+token pair. A `suspend-account` that commits between them leaves login returning
+**200 with a freshly minted refresh-token row that post-dates the suspension's
+revocation sweep**. `AccountSuspensionService`'s header states this plainly.
+
+The gap was first weighed in #532, which declined the fix on cost; #579 was
+filed (split from #534) so the gap would be tracked rather than dropped when
+#534 closed, and re-decided here.
+
+## The rule
+
+**Suspension is immediate for *use*, not for *issuance*.** Do not add a
+`FOR SHARE` lock on the account row inside login's issuance transaction to
+close the window, and do not read #530's finish line — "immediate, race-safe
+suspension" — as covering issuance. *Race-safe* means use: a suspended farm's
+existing credentials die on their next presentation.
+
+**The inertness of a window-minted credential rests on exactly four premises,
+each pinned by name, each enforced by a test that fails when that premise alone
+is broken — not just when the whole service is broken. A change that breaks any
+one of them must reopen #579 rather than ship:**
+
+1. **`CredentialEpochMiddleware` re-reads `Account.IsActive` live on every
+   authenticated request** (it already joins user and account in the per-request
+   read that #364 made fail-closed — the round trip *is* the guarantee, so it is
+   never cached). A window-minted access token is therefore refused on its very
+   next request, with the epoch already bumped.
+2. **`RefreshAsync`'s own suspended-farm check** (`IdentityProvider.cs`, the
+   `#532 — a suspended farm cannot rotate a session` block) refuses the
+   window-minted refresh token on its first rotation attempt.
+3. **`AccountSuspensionService` revokes every user's `CredentialEpoch` and
+   `SecurityStamp` in the same transaction as `IsActive`** (one
+   `AmbientTransaction`, one `SaveChanges`), for every credential that exists
+   **when the sweep executes**. Note the scope: a refresh row minted *after*
+   the sweep — precisely the window artifact this record exists for — is not
+   destroyed by this premise; it survives the suspension transaction and is
+   inert for the other three reasons (the live account checks) until
+   reactivation's sweep destroys it for good (premise 4). Enforced by
+   `SuspendAsync_SuspendedPremiseIsAtomic_RollbacksWithTheEpochBump`, whose
+   static half bounds each revocation's `ExecuteUpdateAsync` call — both the
+   user epoch/stamp sweep and the refresh-token sweep — to sit between the
+   suspension transaction's begin and commit, and whose runtime half faults
+   the audit write (the last in-transaction write) and asserts the account
+   reverts to active **and** every epoch/stamp reverts and the token stays
+   live. The static check is the load-bearing half for the split-transaction
+   regression: it also verifies the tracked `account.Suspend()`/`Reactivate()`
+   mutation sits before the in-transaction flush and that NO `SaveChangesAsync`
+   runs after the commit, so the account row is dirty when the suspension
+   transaction's save runs and is not re-flushed in a second transaction. A
+   split that still commits in one Postgres transaction (an extra `SaveChanges`,
+   a deferred `ExecuteUpdateAsync` awaited after the commit) is invisible to the
+   runtime fault, which rolls back whatever transaction it is in — verified by
+   mutation on this slice. The variants that leave `IsActive` false with live
+   credentials (a sweep moved to a second transaction, the two revocations split
+   across transactions, or the account mutation deferred to a post-commit save)
+   are the ones the static check reddens on. **Honest limitation:** the static
+   check is a *textual-position* guard — it equates where the call text sits
+   with when it executes. A refactor that wraps each sweep in a local lambda
+   defined before the commit and invoked after it evades the check (the offsets
+   stay in-span while the revocations execute post-commit); closing that
+   evasion needs a production test seam so a runtime guard can observe execution
+   order, which is out of scope for a won't-fix record. The guard is therefore
+   best-effort against the realistic refactor shapes (moving a statement, each
+   verified by mutation); a lambda-wrap refactor leaves the premise resting on
+   the runtime half + code review, and the guard's comment must be revisited.
+4. **Reactivation revokes again** — `ReactivationRevokesTheSessionsMintedBetweenSuspendAndReactivate`
+   (`tests/Cluckwork.Api.IntegrationTests/AccountSuspensionTests.cs`) inserts
+   the window artifact on purpose and proves it is destroyed for good at
+   reactivation. It fabricates the row because the race is not reproducible on
+   demand; a future interleaving test must drive the real window, not reuse the
+   fabricated row.
+
+The observable symptom is a user who appears to sign in and is bounced on their
+first action — **not access to a suspended farm**.
+
+## Why not the obvious alternative
+
+Closing the window requires login to take a `FOR SHARE` lock on the account row
+**before** minting, inside its issuance transaction — matching the account-first
+ordering `AccountSuspensionService` and `AdminRecoveryService` already use.
+
+It is a **cost question, not a deadlock question**. Every *suspension-side*
+locking path takes the account row first (`AccountSuspensionService`,
+`AdminRecoveryService`), so a login lock taken before the mint would serialise
+against them, never deadlock. (Not every locking path in the repo is
+account-first — the currency-lock protocol, e.g. `UpdateInventoryItemHandler`,
+takes the item row first and the account `FOR SHARE` second, on its own
+cycle-freedom argument — but suspension and that protocol never take locks
+concurrently with each other, so the two orderings never meet.) The cost is a
+lock plus an extra round trip on the login hot path, paid by every farm on every login, in order to
+prevent a credential that is inert by the four premises above. #532 weighed
+that and declined on cost, not on difficulty; #579 re-weighed it at Phase 1.6
+scale (a handful of farms) and declined again.
+
+If suspension ever gains user-facing frequency — many farms, frequent
+operator-driven suspensions — the cost argument weakens and this decision
+should be revisited. That revisit is a cost decision for the owner, with this
+file as the starting point.
+
+## What this does NOT cover
+
+- It does not weaken *use*-side immediacy. The four premises are load-bearing:
+  a refactor that moves the login account read into the issuance transaction,
+  caches `CredentialEpochMiddleware`'s read, relaxes `RefreshAsync`'s suspended
+  check, or splits suspension's three (now four) statements out of one
+  transaction re-opens the window's consequences and must re-decide this.
+- It does not bless a *wider* window. If a future change makes the minted
+  credential usable (breaking any premise), the lock is no longer "prevent a
+  credential that can never be used" — it is required, and this record no
+  longer applies.
+- It says nothing about `provision-account` / `bootstrap-admin` issuance paths,
+  which run at boot time, not on the login hot path.
+
+## How it is enforced
+
+Nothing enforces the *declination* — it is a decision, and it relies on review:
+the four premises are the tripwires, and each is pinned by the named test or
+named code path above, each chosen to fail when its premise alone is broken:
+
+| Premise | Guard | Fails when the premise alone breaks |
+|---|---|---|
+| 1 — live middleware read | `ABearerWhoseEpochStillMatches_IsRejected_WhenTheFarmIsInactive` (plus the #364 no-cache tests) | middleware stops reading `IsActive` (or caches it) while login still checks — the epoch still matches, so only the middleware's `AccountIsActive` clause can reject |
+| 2 — `RefreshAsync`'s suspended check | `ARefreshTokenWhoseEpochStillMatches_IsRejected_WhenTheFarmIsInactive` | that block is deleted while the middleware stays live — the epoch still matches, so only the suspended-farm check can refuse the rotation |
+| 3 — suspension's same-transaction revocation | `SuspendAsync_SuspendedPremiseIsAtomic_RollbacksWithTheEpochBump` | the account row and the revocations land in different transactions (a sweep deferred to a post-commit `ExecuteUpdateAsync`, the two revocations in separate transactions, or the tracked account mutation deferred to a post-commit save) |
+| 4 — reactivation's revoke | `ReactivationRevokesTheSessionsMintedBetweenSuspendAndReactivate` | reactivation stops revoking, while a suspend→reactivate cycle still destroys the artifact |
+
+The window *itself* is unguarded by design (that is the declination) — none of
+these four tests can reproduce the race on demand; premise 4's test fabricates
+the window artifact on purpose because it cannot drive the real window.

docs/decisions/README.md

@@ -13,6 +13,15 @@ nearly every long bullet encodes a defect that actually shipped plus the reasoni
 that stops it recurring, so the rule is compressed but **nothing is deleted** —
 follow the `→` link from the `AGENTS.md` bullet to get here.
 
+There are two kinds of record, and the `AGENTS.md` bullet's link does not by
+itself say which: an **earned** record was earned by a defect that shipped (the
+narrative names what broke, which review round found it, and the wrong fix), and
+an **accepted-risk** record (e.g. the [suspension issuance window (#579)](579-suspension-issuance-window.md))
+records a deliberate declination — no incident, the record says `No incident`,
+and the rule is load-bearing (break it and the accepted risk returns). Both are
+indexed here; the footer below is what distinguishes them from the plain
+conventions that carry no link at all.
+
 Starting a new record: copy [`TEMPLATE.md`](TEMPLATE.md).
 
 ## Index
@@ -36,6 +45,7 @@ Starting a new record: copy [`TEMPLATE.md`](TEMPLATE.md).
 | [Production logs: compact JSON on stdout (#404)](404-production-logs.md) | AGENTS · Conventions |
 | [Generated PostgreSQL schema documentation (#417)](417-schema-docs.md) | AGENTS · Conventions |
 | [`AuditEvents` is not time-partitioned (#505)](505-audit-events-no-time-partition.md) | AGENTS · Conventions |
+| [Suspension is immediate for use, not issuance (#579)](579-suspension-issuance-window.md) | AGENTS · Conventions |
 | [Writing a guard (a test that asserts an invariant)](407-writing-a-guard.md) | AGENTS · Writing a guard |
 | [CI security gates, lock-file healing, Dependabot, action pinning (#146)](146-ci-security-gates.md) | AGENTS · CI security gates |
 | [Releases and image publishing — internals (#351)](351-releases.md) | AGENTS · Releases · and [`docs/releasing.md`](../releasing.md) |
@@ -51,9 +61,16 @@ Starting a new record: copy [`TEMPLATE.md`](TEMPLATE.md).
 **Every bullet that cites an issue has a record here; the plain conventions do
 not, and should not.** The Result pattern, handler-per-feature, FluentValidation,
 the endpoint shape, `Version++`, multi-tenancy and nullable-enabled are house
-style — consistency rules with no incident behind them, so there is nothing to
-relocate and no `→` link to follow. `AGENTS.md` says which kind a bullet is by
-whether it carries one.
+style — consistency rules with no incident behind them and no deliberate
+declination to record, so there is nothing to relocate and no `→` link to
+follow. A linked bullet is one of two kinds: an **earned** record (a defect that
+shipped — the record's narrative names the incident and the review round that
+found it) or an **accepted-risk** record (no incident — the record says `No
+incident` and the rule is load-bearing, e.g. the suspension issuance window
+(#579)). `AGENTS.md` says a bullet is linked by whether it carries one; the
+record itself says which of the two linked kinds it is — an accepted-risk record
+carries a `No incident` marker (e.g. #579), and an earned record's narrative
+names the shipped defect and the review round that found it.
 
 The last eight rows were added when `AGENTS.md` was compressed to one paragraph
 per rule: #265, #264, #260, #318 and #267 had kept their rationale inline until