fix(unifi): reuse recycled NFC cards (free on deactivate, reclaim from disabled holder)

CLAUDE.md

@@ -32,6 +32,7 @@ All tooling goes through `uv run` — the venv is managed by uv, not pip.
 - **Frozen dataclasses.** All domain models in `models.py` are `@dataclass(frozen=True)`. Never mutate; construct a new instance.
 - **Strict layering.** Nothing imports `orchestrator` except `scheduler` and (future) `webhook`. See dependency table in architecture.md §4.
 - **Card ID redaction.** Logs show last-4 only. Never log a full card ID at any level (architecture.md §11).
+- **No member names in logs/alerts.** Operational and audit log streams identify members by `contact_id` (and, for unmanaged UniFi accounts, user id) — never by name. CodeQL's `py/clear-text-logging-sensitive-data` flags `display_name` as PII. The interactive `show-diff` CLI may print names (direct operator output, not a log). See architecture/conventions.rst.
 - **Dry-run is sacred.** Dry-run flips a flag inside `UnifiClient` that turns writes into no-ops. Pure modules behave identically in dry-run and live — do not branch on dry-run in pure code.
 - **Fail-secure on safety guards.** Any guard firing means zero writes that cycle. No partial application.
 - **Crash logging uses `_logger.error("...", exc_info=exc)`, not `_logger.exception(...)`.** `exception()` reads `sys.exc_info()`, which is `(None, None, None)` outside an active `except` clause — the traceback would be silently dropped. Python 3.5+ accepts an exception instance directly via `exc_info=`. See `orchestrator.handle_crash` for the reference impl.

docs/architecture.md

@@ -252,6 +252,13 @@ config.load()
    **Per-user isolation:** a single contact's `UnifiClientError` is recorded and the cycle continues with the remaining contacts; `apply()` then raises one summary error so the orchestrator alerts. One bad record never blocks the rest. (The batch card pre-import is a shared prerequisite and still fails fast.)
 
    **Email is best-effort:** UniFi requires globally-unique emails across users *and* admins. When a member's CiviCRM email is already registered to another account — commonly a staff member who is also a UniFi admin — the write is retried without the `user_email` field (so name/card/policy still apply) and a warning is logged. The member keeps door access; only the conflicting email isn't synced. This is *not* counted as a per-user failure, so it doesn't alert.
+
+   **Card reuse and bind conflicts.** A recycled physical card can be assigned to a new member while it's still bound to an old UniFi user. **Key constraint:** UniFi rejects NFC-card mutations on a *deactivated* user (HTTP 404 "no-man zone") — a card cannot be unbound from a disabled account. door-sync works within that at two points so `CODE_CREDS_NFC_HAS_BIND_USER` doesn't strand a member:
+
+   - **On deactivation** (proactive): a departing user's NFC card(s) are deleted *before* the status is set to `DEACTIVATED`, i.e. while the account is still active and the delete is permitted. This is best-effort — a cleanup failure is logged but never blocks cutting access — so the next member assigned that number binds cleanly.
+   - **On bind conflict** (reactive): if a bind is still rejected because the card is bound elsewhere, door-sync inspects the current holder. If that holder is a **disabled** UniFi user (the card is dead state — e.g. a member deactivated before cards were freed on deactivation, or where the proactive cleanup didn't run), door-sync **force-reassigns** the card to the new member (`force_add: true`) and logs a redacted warning; the reclaim is not a failure. Forcing the bind is the only path here — the card can't be unbound from the disabled holder first. If the holder is **active** (or not in the fetched snapshot), the card is left alone and the bind fails as a per-user error (it does not block the rest of the cycle) whose message *identifies the current holder*: contact id for a sync-managed user, otherwise the UniFi user id, plus status. The member's **name is never logged** (PII stays out of logs/alerts, consistent with the rest of door-sync); the contact id resolves to the member in CiviCRM if an operator needs it.
+
+   `force_add` is `false` for every normal bind — door-sync never displaces an *active* user. `force_add: true` is used only to reclaim a card from a confirmed-**disabled** holder. The card number stays redacted everywhere (§11).
 7. **Log + persist state** — write audit entries; write last-success timestamp atomically (write to temp, fsync, rename).
 
 ---

docs/architecture/conventions.rst

@@ -66,6 +66,14 @@ Card IDs are security-sensitive. They appear in audit logs and operational logs
 as last-4-digits only (e.g., ``****1234``). Full card IDs are never logged at
 any level.
 
+Member names are likewise kept out of the operational and audit log streams.
+Log records and alerts identify a member by CiviCRM ``contact_id`` (and, for an
+unmanaged UniFi account, its user id) — never by name — so member PII does not
+accumulate in the journal. A ``contact_id`` resolves back to the member in
+CiviCRM when an operator needs the name. (The interactive ``show-diff`` CLI does
+print names to the operator's terminal on demand; that is direct operator
+output, not a persisted log.)
+
 
 Error Handling
 --------------

docs/superpowers/specs/2026-05-22-unifi-client-design.md

@@ -149,12 +149,12 @@ Pagination follows the same shape as `CivicrmClient`: a `_MAX_PAGES = 1_000` cei
    - If empty: skip.
    - Else: assemble a CSV in memory (one row per missing card_id, no header row — see §9 for the exact format), `POST /api/v1/developer/credentials/nfc_cards/import` (multipart, field name `file`).
    - On success, parse the response's `data` array of `{nfc_id, token}` entries and merge into the token map.
-3. **Deactivate** (`PUT /users/:id` with body `{"status": "DEACTIVATED"}` for each entry in `diff.to_deactivate`).
+3. **Deactivate** — for each entry in `diff.to_deactivate`: first DELETE the user's cached NFC card(s) (`/users/:id/nfc_cards/delete`) to free the number for reuse, *then* `PUT /users/:id` with body `{"status": "DEACTIVATED"}`. Card removal must precede the status change because UniFi rejects card mutations on a deactivated user (HTTP 404 "no-man zone"). The card delete is best-effort — a failure is logged but does not block the status change (access-cutting wins); any card left behind is reclaimed on its next assignment (step 4).
 4. **Update credential** — for each `(resolved, unifi_user)` in `diff.to_update_credential`:
    - If `display_name` changed: `PUT /users/:id` body `{first_name, last_name}` (split via `_split_name`).
    - If `card_id` changed:
      - If the user has any existing card: `DELETE /users/:id/nfc_cards/delete` with body `{"token": <old_token>}` (resolves old token from the cached `nfc_cards` map populated by `fetch_users()`).
-     - `PUT /users/:id/nfc_cards` with body `{"token": <new_token>, "force_add": false}`. Token is looked up in the NFC-token map (post-import).
+     - `PUT /users/:id/nfc_cards` with body `{"token": <new_token>, "force_add": false}`. Token is looked up in the NFC-token map (post-import). On a `CODE_CREDS_NFC_HAS_BIND_USER` conflict the card is held by another user: if that holder is *disabled* it is reclaimed with a forced re-bind, otherwise the holder is identified (by id) in a per-user error — see the `force_add` note in "Open questions / decisions". The same bind path (and conflict handling) is used in step 6.
 5. **Update policy** — for each `(resolved, unifi_user)` in `diff.to_update_policy`:
    - `PUT /users/:id/access_policies` with body `{"access_policy_ids": [<resolved.target_policy>]}`.
 6. **Add** — for each `resolved` in `diff.to_add`:
@@ -398,5 +398,5 @@ Test changes: extend `_write_minimal_valid` in `tests/test_config.py` to include
 - **Retry budget of 3 / inter-call delay of 75ms.** Both are constants in `client.py`. Promotable to config fields if real-world tuning demands.
 - **Whether to compress the CSV upload.** Not done; CSVs are kilobyte-scale.
 - **First-name placeholder for single-word display names.** `"—"` (em-dash) — chosen to be visibly distinct in the UniFi UI so an operator notices and edits the CiviCRM record. Open to bikeshed.
-- **Whether `force_add: true` should ever be used.** Default `false` (don't steal a card from another user). If we ever need to forcibly re-bind, that's a separate operational flow with explicit human authorization.
+- **When `force_add: true` is used.** Never for a normal bind — door-sync doesn't displace an *active* user. It is used in exactly one case: reclaiming a card from a confirmed-**disabled** holder on a `CODE_CREDS_NFC_HAS_BIND_USER` conflict. The card can't be unbound from the disabled holder first (UniFi 404s card mutations on a deactivated user), so forcing the new bind is the only way to move it; door-sync gates this on having checked the holder's status in the fetch snapshot. An **active** holder (or one not in the snapshot) is left untouched and the bind fails as a per-user error that identifies the holder by id (contact id, else UniFi user id — never the member's name; PII stays out of logs), so an operator resolves it deliberately. A *blanket* `force_add: true` remains rejected: it would clobber active admin-managed bindings.
 - **Audit-log entries for UniFi calls.** Out of scope; `audit.py` (a separate slice) owns audit logging. The orchestrator passes the diff and the result there.

docs/superpowers/specs/2026-05-27-manual-test-plan.md

@@ -176,9 +176,32 @@
 
 1. In CiviCRM, expire or remove a test member's active membership (or clear their card ID).
 2. Run `show-diff` — confirm `to_deactivate`.
-3. Run `--once` — verify in UniFi admin: user is deactivated (not deleted, just inactive).
+3. Run `--once` — verify in UniFi admin: the user's NFC card is **removed** (freed for reuse) and the user is deactivated (not deleted, just inactive). The card delete is issued *before* the status change.
 
-### 4f. Full Sync
+### 4f. Card Reuse and Reclaim
+
+Validates that a recycled physical card can move to a new member — the path that
+previously crashed every cycle with `CODE_CREDS_NFC_HAS_BIND_USER`.
+
+**Recycled card, holder freed on deactivation:**
+
+1. Take member A holding card X in UniFi. Deactivate A (4e) and confirm card X was removed.
+2. In CiviCRM, assign card X to a different member B. Run `show-diff` (B in `to_add` or `to_update_credential`), then `--once`.
+3. Verify in UniFi admin: card X is bound to B, and B is active.
+
+**Reclaim from an already-disabled holder** (the production case — a card still
+stuck on a user that was deactivated *before* this behavior shipped):
+
+4. Arrange member C **deactivated** in UniFi but still holding card Y (set this up in the UniFi admin UI if needed).
+5. In CiviCRM, assign card Y to member D. Run `--once`.
+6. Verify the operational log shows a single `WARNING ... reclaiming it for this member` line — redacted card (`****NNNN`) and `contact=<id>`, **no member name** — and that UniFi admin now shows card Y bound to D. No alert/crash for D.
+
+**Active-holder guard** (door-sync must NOT displace an active user):
+
+7. Arrange card Z bound to an *active* UniFi account (e.g. a manually-enrolled admin). Assign card Z to a member in CiviCRM and run `--once`.
+8. Verify the cycle records a per-user failure that identifies the holder by id (not name) and leaves card Z on the active account untouched.
+
+### 4g. Full Sync
 
 1. Remove the tier rule restriction from step 4a — allow all membership types.
 2. Run `show-diff` to review the full diff.
@@ -188,6 +211,8 @@
 ### Pass Criteria
 
 - All five write operations work correctly: add user, bind NFC card, assign policy, update credential, deactivate user.
+- Deactivation frees the member's NFC card (the number becomes reusable).
+- A recycled card moves to its new member; a card stuck on a disabled holder is reclaimed (single redacted warning, no alert); a card on an *active* holder is left untouched with an actionable per-user error.
 - Idempotency holds after each operation (re-run produces empty diff).
 - Audit log and state file reflect each cycle accurately.
 - No duplicate users or credentials created.