# Roster Data Reconciliation Plan

> ### 📁 [Aug 5] Doc reset — paths in this file have moved
>
> **Status claims in this document are no longer authoritative.** `OUTSTANDING_ISSUES.md` is the only
> file that says what is open; this one is kept for its **implementation briefs** (CF3, CF7, CF9, CF11
> in §10.3) and its **decision record** (D-1…D-14 in §7).
>
> Four documents it cites moved to `archive/` on Aug 5 — prefix the path when following a reference:
> `GROUP_CLEANUP_BRIEF.md`, `GROUP_ID_AT_ACCEPT_BRIEF.md`, `D8_OPTIONS_COMPARISON.md`,
> `SEASON_TOURNAMENT_DRIFT_AUDIT.md`, `DOC_CLEANUP_CONSOLIDATION_WIP.md`.
>
> **Known stale here:** the START HERE block and §10.3 still describe D-8 commit 3 as uncommitted and
> untested. It merged as `c7251c6` via PR #129, followed by `7a5f57b`. ⚠ **Line numbers throughout this
> file have drifted — re-locate by method name.**

> ### 🚚 [Aug 23] Second correction pass — three claims below are now false
>
> 1. **"It is still not on PROD."** It is. PR #124 and PR #129 were deployed, and update `9073` ran on
>    TEST and PROD. The pre-deploy checklist in *Do these first* (steps 3–6) was **retired unrun** —
>    `OUTSTANDING_ISSUES.md` §0 records that as a decision, not an oversight.
> 2. **"D-8 commit 3 is written but unlinted and untested."** Merged as `c7251c6`. Two places below
>    still say otherwise: *The single most important fact*, and row 8 of the commit table in §9.
> 3. **The audit script moved** to `archive/ROSTER_DATA_AUDIT.sql`, and query B was never added to it.
>    Item 0.1, which was the plan to add it, is retired — so anything below that treats query B as the
>    safety net is describing something that does not exist.
>
> **Still true and still the point of this file:** the CF3, CF7, CF9 and CF11 briefs in §10.3 and the
> D-1…D-14 decision record in §7. **CF3, CF7 and CF9 were re-verified against the code on Aug 23 and
> are all genuinely unstarted** — see `OUTSTANDING_ISSUES.md` P1 for the current line numbers, which
> have drifted again since these briefs were written.

**Date:** July 28, 2026 · updated July 29 (CF10, CF11, decisions D-7…D-9) · July 30 (CF12, D-10, CF9
scope) · **August 4 morning** (full re-verification — archived banner below) · **August 4 evening**
(CF8, CF4, CF5 implemented; decisions D-12, D-13) · **August 5** (§10.8 passed on LOCAL; D-8 decided
and CF10 + B + E implemented)
**Status:** **PARTIALLY IMPLEMENTED — no data repair, code fixes only** (Andrew, July 28).
**Planned commits 1, 2, 3 and 5 are committed, plus CF5 (half of commit 6) and now CF10 (commit 8's
first half). Commits 4, 7 and 9's remainder are not started.**

> ### ✅ [Aug 5] The "none of it has ever run" era is over. ✅ [Aug 23] And it is now on PROD.
>
> **§10.8 was worked through on LOCAL and passed** (Andrew, Aug 5). The six fixes merged in PR #124 —
> CF1, CF1b, CF2, CF12, CF4, CF8, CF5 — have been linted and exercised. That was this document's
> single largest open risk for a week and it is closed.
>
> **[Aug 23] This block used to end "nothing here has been deployed to TEST or PROD."** That was true
> when written and stopped being true around Aug 9. PR #124 and PR #129 are deployed; update `9073`
> ran on both boxes. **Operating rule 1 (admin Group Invitations read-only for tournament groups) is
> LIFTED.** Operating rule 2 (`schedule_visible` OFF while iterating schedules) is **not** — it never
> depended on this deploy, it guards TS2 and S2, and as of Aug 22 it covers seasons as well as
> tournaments. See `SESSION_HANDOFF.md`.

> ### ⚠ HOW TO READ THIS DOCUMENT — line numbers are not trustworthy
>
> Nine commits have landed since most of the briefs below were written, and **several briefs carry
> line numbers that were already wrong when written or have drifted since — including some tagged
> "Aug 4" that look current.** An Aug 4 evening audit found bad citations in the CF3, CF8, CF10, CF11
> and D-9 sections specifically.
>
> **Re-locate everything by method name before trusting it.** The prose, the reasoning and the
> decisions are sound; the coordinates are not. Briefs with a ✅ or 🔎 banner at their head have been
> reviewed most recently — **CF1, CF2, CF7, CF10, CF11 and CF12 have no banner and are the least
> reviewed.**
**Audit script:** `archive/ROSTER_DATA_AUDIT.sql` rev 2 (read-only; run on TEST first) — **still not
run, and moved out of the repo root on Aug 23**
**Covers:** 2026 Early Fall Coed, 2026 Early Fall Mens, SLO Friendly 2026
**Sources:** `OUTSTANDING_ISSUES.md` P1 · `SEASON_TOURNAMENT_DRIFT_AUDIT.md` D6/D7/D13, **D15/D19** · `ADMIN_TOOLING_REVIEW.md` T2/T3/T5, R4, R10, **T8** · `INVITATION_FLOW_ANALYSIS.md` E4/E5

---

> # ▶ START HERE — state as of August 5
>
> ## What exists right now
>
> **PR #124 is merged to `main`** (merge commit `1eb1a15`) and **has now been linted and exercised on
> LOCAL** — §10.8 passed, Aug 5:
>
> | Commit | Contents | Session |
> |---|---|---|
> | `e5317f8` | CF1 + CF12 display half | Aug 4 morning |
> | `59c12ec` | CF2 + D2 + CF12 write half | Aug 4 morning |
> | `8156bc1` | CF1b — reconcile across every team on tournament cancel | Aug 4 morning |
> | `b46c28b` | **CF4** — `pickLiveRegistration` trait + 21 call sites | Aug 4 evening |
> | `b22734f` | **CF8** — dissolve service, `sendGroupDissolved()`, cancel cleanup | Aug 4 evening |
> | `7c9a7c5` | **CF5** — roster builder reconciles against every team | Aug 4 evening |
>
> **On top of that, the D-8 package is being built on branch `fix/solo_group_auto_delete`:**
>
> | Commit | Contents | State |
> |---|---|---|
> | `dde90b5` | **D-8/B commit 1** — `clearGroupIfOrphaned()` + `clearInviterGroupIfOrphaned()` on `GroupDissolveService`; four injections; `services.yml` | committed, linted |
> | `995e286` | **D-8 commit 2 (CF10)** — mint moved below the validation checks; `getGroupSize()` off-by-one fix; three paste artifacts deleted | committed, linted |
> | ~~*(uncommitted)*~~ **`c7251c6`** | **D-8 commit 3 (B + E)** — the thirteen call sites, six files | ⚠ **[CORRECTED Aug 8] This row said "uncommitted — written, NOT yet linted or tested." It merged as `c7251c6` via PR #129 on Aug 5**, followed by `7a5f57b` fixing a dead-end admin page that LOCAL testing surfaced. All thirteen call sites and both service methods verified present |
>
> **The branch is `fix/solo_group_auto_delete`.** `GROUP_CLEANUP_BRIEF.md` §14 proposed
> `fix/D8_group_cleanup`; that name was not used. Recorded here because a wrong branch name in these
> documents is a mistake this project has already made once — `fix/roster_sync` never existed and is
> still cited wrongly in several files.
>
> ## ⚠ Operating rules — one lifted, one widened *(updated Aug 23)*
>
> - ~~**The admin Group Invitations page is read-only for tournament groups**~~ — **LIFTED.** The D2
>   and D19/CF12 fixes (`59c12ec`, `e5317f8`) were verified on LOCAL and are **on PROD**. The accept
>   dropdown is safe to use on tournaments again. Kept struck through rather than deleted because this
>   rule was in force for about three weeks and someone may still be avoiding the page.
> - **Keep `schedule_visible` OFF** while iterating schedule candidates. **Still in force**, and since
>   Aug 22 it covers **seasons as well as tournaments** — nothing reads per-game `published` any more,
>   so a regenerate on a visible season or tournament goes live to players immediately (TS2 **and**
>   S2, both unrelated to this cluster, both still open).
>
> ## The single most important fact
>
> ~~Roughly 1,100 lines of PHP across six code fixes have never been executed.~~ **Resolved Aug 5 —
> §10.8 passed on LOCAL.**
>
> ~~**The risk has moved, not disappeared.** It is now: (a) none of it is on PROD, and (b) D-8 commit 3
> is written but unlinted and untested.~~ **[Aug 23] Both halves are spent:** it is on PROD, and commit
> 3 merged as `c7251c6` plus the follow-up `7a5f57b`.
>
> **What the residual risk actually is now:** the 29-case test plan in `archive/GROUP_CLEANUP_BRIEF.md`
> §9 was only partly run and no per-case record exists, and **query B — the one diagnostic that would
> reveal a missed fourteenth call site — was never added anywhere and now never will be.** So the
> assurance for D-8 is "two weeks live, no reported symptom." That is real evidence, but it is not the
> evidence this document planned on. If a group refuses to dissolve, or an invitation points at a
> group that no longer exists, run query B out of the brief by hand before assuming it is new.
>
> ## Do these first, before writing any new code
>
> 1. ~~`ddev php -l` on the 12 files~~ · ~~`ddev drush cr`~~ · ~~the §10.8 click-throughs~~ —
>    **all done Aug 5.**
> 2. ~~**Lint D-8 commit 3** (six files, listed in the table above) and **`ddev drush cr`**.~~
>    **[Aug 8] Done — it merged as `c7251c6`.** Following this step as written would send you looking
>    for uncommitted work that does not exist.
> 3. ~~**Run `GROUP_CLEANUP_BRIEF.md` §9.2 and §9.3** — 29 cases.~~ **[Aug 23] Retired unrun.** Which
>    of the 29 cases actually ran is now unknown and will stay unknown; the deploy went out without
>    the record. The cases themselves are still the right ones if you ever need to test this area.
> 4. ~~**Run §9.4 query A and B and record the counts before deploying.**~~ **Retired unrun** — the
>    "before" window closed at deploy and is unrecoverable.
> 5. ~~**Add query B to the audit script.**~~ **Retired.** This was the entire safety net for the B
>    design, and closing it was **Andrew's explicit call on Aug 23**, on the strength of two weeks live
>    with no symptom. `OUTSTANDING_ISSUES.md` §0 records the trade. Query B still exists in
>    `archive/GROUP_CLEANUP_BRIEF.md` §9.4 as a hand-run diagnostic.
> 6. ~~Then deploy LOCAL → TEST → PROD.~~ **Done, around Aug 9.** One config file from PR #131 rode
>    along and **its import on TEST/PROD was never confirmed** — see `SESSION_HANDOFF.md`, Aug 7
>    entry, which is now the only copy of those instructions.
>
> ## What shipped, what has not
>
> | Fix | State | Where |
> |---|---|---|
> | **CF1 / CF1b** | **LOCAL-tested** (§10.8) | `e5317f8`, `8156bc1` |
> | **CF2 + D2** | **LOCAL-tested** (§10.8 item 4). Was the least-validated code in the set — the Aug 4 reproduction never touched the admin page — so this was the highest-value item in that checklist | `59c12ec` |
> | **CF12** | **LOCAL-tested** (§10.8 item 4, incl. the Save trap and promote-to-captain) | `e5317f8` (display) + `59c12ec` (write) |
> | **CF8** | **LOCAL-tested** (§10.8 item 2) | `b22734f` |
> | **CF4** | **LOCAL-tested** (§10.8 item 1, the original reproduction). Scope grew to 21 sites / 8 classes | `b46c28b` |
> | **CF5** | **LOCAL-tested** (§10.8 item 3). Three call sites, not the one the brief scoped | `7c9a7c5` |
> | **CF10** | **IMPLEMENTED Aug 5, linted.** Not yet click-tested — that is `GROUP_CLEANUP_BRIEF.md` §9.2 cases 1-4. **Implemented from `GROUP_CLEANUP_BRIEF.md` §5, not from the CF10 brief below** | `995e286` |
> | **B + E** (D-8) | ⚠ **[CORRECTED Aug 8] Was "WRITTEN Aug 5, not linted, not tested."** It is **merged and linted**, thirteen call sites across six files, plus the follow-up `7a5f57b`. What is still open is the *per-case test record* (`OUTSTANDING_ISSUES.md` 0.3) and the CF10 failed-invite click-through (0.4) | `c7251c6` via PR #129 |
> | **CF3** | **not started.** Brief is STALE — re-verify before use, see its section | — |
> | **CF9** | **not started.** Researched Aug 4 evening; findings folded into its brief | — |
> | **CF7** | **not started.** Its ⚠ box asks you to decide on two hooks `e5317f8` already deleted | — |
> | **CF11** | **not started, and now OPTIONAL** — route 4 clears itself under B. Decide after four weeks of §9.4 query B. Its brief's `notify => TRUE` is wrong; it should be `FALSE` | — |
> | **CF6** | resolved, no code | — |
>
> ## Open decisions blocking work
>
> - ~~**D-8** — should a group auto-clear the manager's `group_id` when it drops to one live member?~~
>   **DECIDED — Andrew, August 5: CF10 + B + E.** Clean up the group when it empties, and move the mint
>   below the validation checks. Caleb's option D (no `group_id` until an invitation is accepted) is
>   **HELD, not rejected**. **The implementation spec is `GROUP_CLEANUP_BRIEF.md`** (~9-11 hrs, thirteen
>   call sites); `D8_OPTIONS_COMPARISON.md` is why B+E beat D; `GROUP_ID_AT_ACCEPT_BRIEF.md` is option D
>   in full, held. **Consequences for this document: CF10 is reinstated** (D had superseded it — it is
>   the only thing that closes route 1), **CF11 becomes optional** (route 4 clears itself under B; decide
>   after four weeks of `GROUP_CLEANUP_BRIEF.md` §9.4 query B), and **D-9 mostly resolves itself**.
>   §7's D-8 row and §10.3's CF10/CF11 briefs are updated to match; **`GROUP_CLEANUP_BRIEF.md` §5 is
>   authoritative for CF10**, whose brief below carries stale line numbers.
>
> **No open decision now blocks this work.** `GROUP_CLEANUP_BRIEF.md` §11 carries nine implementation
> questions (Q-B1…Q-B9); Q-B9, the only one that had to be settled before shipping, was **answered
> August 5** — see that section. **The blocker is testing, not deciding:** §10.8 below.
>
> ## Decided since
>
> - **CF9 is its own commit — DECIDED (Andrew, Aug 4).** Commit 6 was CF5 + CF9. CF5 shipped alone in
>   `7c9a7c5`; **CF9 becomes commit 6b, standalone.** They share no files and CF9 carries all the risk
>   (three constructor changes, a services.yml edit, and a branch deletion). See decision **D-14**.
>
> ## Two live bugs found by testing, both now fixed in code but unverified
>
> - **The Aug 4 reproduction** (register → cancel → re-register → accept invitation) produced a
>   three-way split-brain roster. Root cause was CF4's `reset()`; **CF1 unmasked it** by correctly
>   clearing `Registration.team`, which removed the guard that had been accidentally blocking the write.
>   Fixed by CF4. **Re-run this exact sequence as the primary regression test.**
> - **Phantom captain view.** A tournament registration with no team renders as a manager with an
>   Invite a Player form (`GroupController:417`), then errors with "No team found for this
>   registration". Pre-existing, cosmetic, **not fixed** — no brief covers it. See §11.
>
> ## Before re-testing: clean up the LOCAL data the bug created
>
> Registration **5088** (testuser524, SLO tournament) is cancelled but still carries `team`,
> `group_id` and `invitation_status = accepted` from the bad accept. It will confound the next run.

---

> ### 🗄 ARCHIVED — August 4 *morning* verification pass
>
> Kept for provenance. Its corrections are already folded into the briefs below, and the work it was
> written to guide has since been implemented. **The "read this before implementing anything"
> instruction it opens with is superseded by START HERE above.**
>
> Every line reference and factual claim in §5 and §10.3 was re-checked against the working tree at
> `509a6f2`. **Most held exactly** — none of the six main files had changed since July 28, so the
> "line numbers shift" warning in §10.1 had not yet bitten. Six briefs carried errors material enough
> to cost implementation time. All are corrected in place below and marked **`[CORRECTED Aug 4]`**.
>
> **The four that change the shape of the work:**
>
> 1. **CF12 half (b) rested on a false premise.** The member-row `<select>` is *not* branched by
>    `$context['type']`, and dropping an option does not close the Save trap. Rewritten in §10.3.
> 2. **CF8's notification will silently send no SMS** unless it passes an explicit `$sms_body`.
>    Caleb's Aug 2 rewrite of `NotificationService::send()` removed the derive-from-body fallback.
> 3. **CF3 site 10 conflicts with CF8.** `GroupInvitationsForm:769` is the dissolve's *write* query
>    and must be exempted from the status filter, or the dissolve stops cleaning class-SB rows.
> 4. **CF4's site list is wrong in both directions.** Two listed sites are not bugs, four real ones
>    were missed, and the true count is **21**, not "roughly fifteen".
>
> **Smaller corrections:** CF10's `save()` is line 934 and sits *outside* the quoted 929-933 range;
> CF11 cites an `OrderCompleteSubscriber` branch that does not exist; `dissolveGroupConfirmed()` runs
> to 810, not 800; `manage()`'s gate is at 600, not 598.
>
> **Verified accurate, no changes needed: CF2, D2, CF5, CF7, CF9.** CF1 is accurate as far as it goes
> but was missing the co-captain case — see §10.3 CF1 part 4.
>
> **Two findings not in any document**, both recorded in §10.3: the entire cancellation surface is
> exactly two code paths (CF1 + CF8 cover it — there is no player-facing cancel route), and
> `ccsoccer_registration_delete()` is a **misnamed hook that has never fired** and would throw if it did
> — which bears directly on CF7's own new hook. See the CF7 brief.

---

## 1. What this document is

Part 2 of the investigate-then-repair pattern. Part 1 is the audit script, which reports counts and
sample rows and changes nothing. This document says, for each mismatch class the audit finds:

- which side is the **source of truth**,
- what a **fix would do** to those rows,
- whether it is a **one-time data repair**, a **code fix** so it stops recurring, or **both**.

Nothing here should be executed until the TEST numbers have been reviewed. Several rules below are
marked **DECISION** because the data cannot tell us the intent — those are listed together in §7.

---

## 2. The model: what is authoritative where

This is the root of the whole problem — the two flows have different models and the shared code
does not know which one it is in.

### Season — `Registration` is the only source of truth

| Fact | Lives in |
|---|---|
| Is this player in the season? | `Registration.status` |
| Which team are they on? | `Registration.team` |
| Which friend-group are they in? | `Registration.group_id` |
| Who manages that group? | the row in the group with `invited_by` empty |
| Roster of a season team | `Team.players` — **must be empty; the season flow never writes it** |

That last row is already a recorded decision (`TeamListBuilder.php:359-365`, from the R1 fix):
*"Deliberately not dual-writing `Team.players` from the season flow — dual writes are what produced
the tournament ghost-member desyncs."* So for seasons, reconciliation means cleaning registration
rows and asserting `Team.players` is empty. Class **SA** exists only to prove that assertion.

### Tournament — split brain, and that is the bug

| Fact | Written by | Read by |
|---|---|---|
| Roster / capacity (`isFull()`, `getRosterStats()`, team page, public Teams list) | `Team.players` | everything player-facing |
| Team assignment | `Registration.team` | Tournament Roster Builder display, cancel forms, checkout |
| Group pointer | `Registration.group_id` → `Team.group_id` | invitation plumbing |

`Team.players` and `Registration.team` are written by different code paths, several of which write
only one side. **Proposed rule, and the rule the repair would encode:**

> **Registration governs membership. `Team.players` governs placement.**
>
> - A uid may appear in a tournament team's `Team.players` **only if** that player holds a live
>   registration for that tournament. No live registration ⇒ remove from the roster.
> - For a player who does hold a live registration, `Registration.team` must name the team whose
>   `Team.players` contains them.
> - Where only one side has a claim, that side wins (backfill the other), rather than deleting data.

**"Live" below means a registration whose `status` is NOT `cancelled` or `expired`.**

> **[CORRECTED Aug 5] This line used to read *"live means `status IN ('paid','active')`"* — the
> allowlist — which contradicted **D-4** and **D-13** in §7 of this same document, and contradicted the
> code. The exclude-list is what was decided and what shipped:
> `LiveRegistrationTrait::isDeadRegistrationStatus()` (CF4, `b46c28b`, 8 classes) excludes `cancelled`
> and `expired` and treats `waitlist` as **live**. `GroupDissolveService` uses the same trait.
>
> On today's data the two definitions agree on 100% of rows — every registration is `paid` or
> `cancelled` (§3) — so nothing downstream in this document changes. It is corrected because a stray
> allowlist inside the model section is precisely the "quietly different definition of the same
> concept" drift this document exists to stop, and because §3, §7 and the audit script all assume the
> exclude-list. **If you are writing new code, use the trait rather than either literal.**
>
> Separately, and still open: `SESSION_HANDOFF.md`'s **Decision 2** asks whether the module should
> standardise on the exclude-list at all, given 46 sites use the allowlist and 9 use `paid`-only. That
> is a wider question than D-4 settled. This line follows the decision as recorded; it does not
> pre-empt Decision 2.

### A wrinkle worth knowing before reading the numbers

`Registration.group_id` being NULL on a tournament player is **legitimate**, not drift. The Roster
Builder deliberately leaves it NULL when an admin drags a free agent onto a captained team
(`TournamentRosterBuilderController.php:164-170`) — that is exactly the Yong Pong case from the
July 27 session. **Any repair must not "backfill" group_id for those players.** Only classes TG and
TH touch group_id, and both are keyed on it being set and wrong, never on it being absent.

---

## 3. Three competing definitions of "who counts"

### First, what the status field actually holds

`Registration.status` declares six allowed values, but a grep of every write site shows only **two
are ever written in production**:

| Value | Written by | Reality |
|---|---|---|
| `paid` | `OrderCompleteSubscriber.php:553` (season) and `:855` (tournament) — the only two registration-creating code paths in the app. Also the drush seeder and the D7 migration. | **Every registration is born `paid`.** |
| `cancelled` | `CancelRegistrationForm.php:349`, `TournamentCancelRegistrationForm.php:271` | The only other value written anywhere. |
| `active` | **nothing** | Vestigial. Every `'status' => 'active'` hit in the module belongs to the Override or Credits entity, not Registration. |
| `pending` | **nothing** (it is the field default, but both create sites pass `paid` explicitly) | Vestigial. Every `'status' => 'pending'` hit belongs to the Invitation entity. |
| `waitlist` | **nothing** — the waitlist is a separate `Waitlist` entity | Vestigial. |
| `expired` | **nothing** on Registration (`Override::expire()` sets it on Override) | Vestigial — same finding as E5 on the Invitation side. |

So `Registration.status` is a two-value field in practice: **paid or cancelled**. The only route to
any other value is an admin hand-editing a registration at
`/admin/ccsoccer/registration/{id}/edit`, where `status` renders as an options_select.

**This makes D-4 nearly free**, and it also means the `paid`-only balancer cannot currently be
dropping anyone. Section 0.3 of the audit confirms it empirically in one glance — but the code
already says it.

### The inconsistency that remains

Even though the three definitions agree on today's data, they are still three definitions, and the
next hand-edited row or new status value makes them diverge:

| Definition | Where | Effect |
|---|---|---|
| **No status filter at all** | `GroupController::getGroupSize()` (1943), `myRegistrations()` (145, 253), `manage()` season roster (475), `invitationHasMatchingRegistration()` (836), `RosterBuilderController::mergeToGroup()` (318), `GroupInvitationsForm` (88, 130, 769) | Cancelled players occupy group slots and render on rosters — D13/R4 |
| **`paid` + `active`** | `OrderCompleteSubscriber` capacity guard (508), `Season` capacity (354), `TeamListBuilder` season mode | The module-wide convention |
| **`paid` only** | *All* of `TeamBalancerService` (345, 273, 1351) | An `active` registration is invisible to Suggest Rosters and to `syncGroupToRoster()` |

**Resolved (D-4):** standardise on **excluding `cancelled` and `expired`** rather than allowlisting
`paid`/`active`. That is `pickLiveRegistration()`'s own stated reasoning — *"'cancelled' is excluded
rather than allowlisting live statuses because the full status set isn't confidently known, and an
allowlist that misses one fails silently"* (`GroupController.php:1914`) — and it is what the D14
nudge fix already adopted. An allowlist would silently drop a hand-set `active` row; the exclude
list would not.

---

## 4. Mismatch classes → resolution

Class codes match the audit script's summary table (section 1) and detail sections (3, 4, 5).

**The summary counts overlap and must not be summed** — one bad row can appear in several classes.
The one exception is TA / TB / TC / T-OK, which are mutually exclusive and together account for every
distinct (team, player) entry in `Team.players` across tournament teams, so those four should
reconcile against the roster-entry total.

### 4.1 Tournament

| Class | Symptom | Source of truth / resolution | Type |
|---|---|---|---|
| **TA1** | On `Team.players`, holds a cancelled/expired registration. The classic ghost — D6/T2. | Registration wins. Remove uid from `Team.players`. If they are `Team.captain`, clear the captain ref too (D-5 note below). | **Both** — repair + CF1 |
| **TA2** | On `Team.players`, only a `pending`/`waitlist` registration | Registration wins, but **DECISION D-5**: a pending registration may be mid-checkout. Recommend: leave, report, re-check after registration closes. | Repair (deferred) |
| **TA3** | On `Team.players`, no registration at all for the tournament | Almost certainly legacy/seed. Recommend eyeball each before removal — the count should be tiny. | Repair (reviewed) |
| **TB** | On team X's `Team.players`, no live registration points at X, and at least one live registration has `team` NULL | Signature of D7 (admin decline cleared the reg, never touched the roster). **DECISION D-1**: roster wins (backfill `Registration.team`) or registration wins (remove from roster)? Recommend roster wins — the team page draws from `Team.players`, so it is what the board has actually been looking at. | **Both** — repair + CF2 |
| **TC** | On team X's `Team.players`, but *every* live registration points at some other team Y | T5. If the player is *also* in Y's `Team.players`, the move completed and X was never cleaned → remove from X. If not, the move half-completed → registration wins: add to Y, remove from X. | **Both** — repair + CF5 |
| **TD** | Live `Registration.team` = X (X in the same tournament), player absent from X's `Team.players` | D7 admin-accept. Registration wins → append to `Team.players`. Warn if the team goes over `max_roster_size`. | **Both** — repair + CF2 |
| **TL** | Tournament registration whose `Registration.team` is a team in a *different* tournament, or a season team | Cross-container corruption, not a roster drift. NULL the reference and let the roster builder re-place them. | Repair (reviewed) |
| **TE** | Same uid appears twice in one team's `Team.players` | Dedupe to a single delta. | **Both** — repair + CF9 |
| **TF** | Cancelled/expired registration still carrying `team` and/or `group_id` | Clear `team`, `group_id`, `invited_by`; set `invitation_status = 'none'`. (Removal from `Team.players` is TA1.) | **Both** — repair + CF1 |
| **TG** | `group_id` set, matches no `team.group_id` | If `Registration.team` is set and that team has a `group_id`, adopt it; otherwise NULL it. Never invent one. | Repair |
| **TH** | `group_id` resolves to a team ≠ `Registration.team` | `Registration.team` wins → set `group_id` to that team's `group_id`. | Repair |
| **TI** | `Registration.team` points at a deleted team | NULL the reference. | **Both** — repair + CF7 |
| **TJ** | `Team.captain`/`co_captain` not in own `Team.players`, or their registration is not live | Live registration → append to roster. Not live → clear the ref; the team needs a new captain before roster publish. | **Both** — repair + CF1 |
| **TK** | Player holds >1 live registration for the same tournament | **Not automatable.** Money is involved — one of them was probably paid for. Admin review, then cancel/refund one. | Review only |

### 4.2 Season

| Class | Symptom | Source of truth / resolution | Type |
|---|---|---|---|
| **SA** | A season team has a non-empty `Team.players` | Model violation. Expect 0. If non-zero, delete those rows after confirming nothing reads them. | Repair (if any) |
| **SB** | Cancelled/expired season registration still carrying `group_id` / `invited_by` / `team` | E4/D13/R4 — the direct cause of inflated group counts and blocked re-invites. Clear `group_id`, `invited_by`, `team`; set `invitation_status = 'none'`. **Order matters**: if the row is the group's manager, resolve SD first. | **Both** — repair + CF8 |
| **SC** | A `group_id` with zero live members | The group no longer exists in any meaningful sense. Clear `group_id` on the remaining dead rows; decline any pending invitations for it (IA). | Repair |
| **SD** | Group has live members but its manager row is dead | R4's "a cancelled reg can be picked as group manager", plus: nobody can manage the group. **DECISION D-2** — recommend promoting the earliest live member (lowest registration id): set their `invited_by` to NULL and repoint the other members' `invited_by` at them. | Repair |
| **SE** | Group has zero manager rows, or more than one | Zero → promote as SD. More than one → a roster-builder merge joined two groups; keep the earliest as manager, repoint the others. | Repair |
| **SF** | Group counts as full only because of dead rows | No separate action — **fixed for free by SB**. Listed so the "team is full with room" count is visible before and after. | (consequence of SB) |
| **SG** | `Registration.team` points at a team from another season | Stale assignment. NULL it — the roster builder will place them. | Repair |
| **SH** | Player holds >1 live registration for the same season | Same as TK — money involved, admin review. | Review only |
| **SI** | Legacy `team_…` group_id, or a season group_id colliding with a `team.group_id` | `_ccsoccer_repair_legacy_team_group_ids()` already exists (`ccsoccer.install:4572`). If rows appear, re-run it rather than writing something new. | Repair (existing tool) |

### 4.3 Invitations (both flows)

Pending invitations reserve capacity in `getGroupSize()` and `Team::getRosterStats()`, so a stale one
holds a spot indefinitely (E5 — `expired` is defined but never set anywhere).

| Class | Symptom | Resolution | Type |
|---|---|---|---|
| **IA** | Pending invite whose season group has no live member | Set status `declined` (**DECISION D-3**: `declined` vs `expired`). | Repair |
| **IB** | Pending team invite whose team no longer exists | Same. | Repair |
| **IC** | Pending invite where the invitee already holds a live registration there | They are already in; the invite double-reserves a slot. Decline it. | **Both** — repair + a guard at accept time |

---

## 5. Code fixes — so it stops recurring

Ordered by how much new drift each one prevents. CF1, CF2 and CF8 are the three that dirty the data
on **every** cancel and **every** admin accept/decline; without them a repair starts rotting the same day.

> **[Aug 4 evening] Implementation status — this table describes the ORIGINAL analysis, not current
> state.** The descriptions below are the July 28 problem statements and several are now historical.
> For what actually shipped and how the scope changed, read the ✅ banner on each brief in §10.3.
>
> | Written (untested) | Not started |
> |---|---|
> | CF1, CF1b, CF2, CF12, CF8, CF4, CF5 | CF3, CF9, CF7, CF10, CF11 |
>
> Two rows below are actively misleading and are corrected in their §10.3 briefs: **CF8's** row says it
> "must include the SD manager-promotion rule" — superseded by decision D-2, which chose dissolve, and
> the shipped code dissolves. **CF3's** row cites `GroupInvitationsForm:769` as the query to exempt;
> CF8 moved that query into `GroupDissolveService`, so the line no longer exists in that file.

| # | Fix | File / method | Class it stops |
|---|---|---|---|
| **CF1** | Tournament cancel must call `TournamentTeamManager::unassignPlayerFromTeam()` (`TournamentTeamManager.php:470`) — it already exists and does exactly this (removes from `Team.players` *and* clears `team`, `group_id`, `invited_by` at 493-495), it is simply never called. **Verified: zero callers anywhere in the repo**, including tests and drush. Two caveats found while auditing: (a) it guards on `if ($team)` reading `Registration.team`, so for a TB-shaped row it silently skips the roster cleanup — it needs a fallback that searches every team in the tournament for the uid; (b) it refuses when `is_captain` is set (479-484), while the cancel form handles captains by clearing `Team.captain` — the captain path has to be made explicit. **[ADDED Aug 4] (c) Co-captains are not handled anywhere on this path.** `promoteToCoCaptain()` sets `is_captain = TRUE` on the registration (`GroupInvitationsForm:1096`) *and* `Team.co_captain` (`:1101`), while the cancel form only queries `Team.captain` — so as originally specified, CF1 would pull a cancelling co-captain off `Team.players` and leave `Team.co_captain` pointing at them. `TournamentTeamManager` contains no `co_captain` reference at all. Audit class **TJ** counts exactly this. **[Aug 4] (d)** the form does not inject the service yet. | `TournamentCancelRegistrationForm::submitForm()` ~270-294 | TA1, TF, TJ (D6/T2) |
| **CF2** | Admin accept/decline must sync `Team.players`: decline → `removePlayerFromTeam()`, accept → `addPlayerToTeam()`. Currently sets/clears `Registration.team` only. | `GroupInvitationsForm::submitForm()` ~815, 924-936; `acceptInvitation()` ~1160, 1223-1229 | TB, TD (D7) |
| **CF8** | Season cancel must clear `group_id` / `invited_by` / `invitation_status`. Today `submitForm()` sets status and saves — that is the entire mutation *to the registration* (the method does also adjust `Season.reserved_spots`, credits and refunds). Must include the SD manager-promotion rule, or cancelling a manager silently orphans their group. | `CancelRegistrationForm::submitForm()` 349-350 | SB, SC, SD, SF (E4/D13) |
| **CF3** | Add a status filter to the unfiltered season group queries, and settle on one definition (D-4). Best done as one shared helper rather than ten copies. **Ten sites**, not the seven the earlier docs imply. **[CORRECTED Aug 4]** All ten verified present and still unfiltered at the stated lines, but two labelling errors: `GroupController` 827-830 is a **tournament/team** query, not a season group query (the §10.3 wording is right, this row's heading is not), and **`GroupInvitationsForm` 769 must be EXCLUDED from the filter** — it is the dissolve's write query and filtering it breaks CF8. Nine sites to filter, one to exempt. | `GroupController` 1943, 145, 253, 475, 836, and the team-invite branch at 827-830; `RosterBuilderController` 318; `GroupInvitationsForm` 88, 130, ~~769~~ | D13/R4 recurrence |
| **CF4** | Replace bare `reset()` with `pickLiveRegistration()`. It is `protected` on `GroupController` (verified — `:1927`), so it has to move to a trait or service first. This shape has produced **six confirmed bugs** so far (per the docblock at `GroupController.php:1914`). **[CORRECTED Aug 4 — the count and the list were both wrong.]** The true figure is **21 bare `reset()` calls on unfiltered registration collections** in production code (24 including three dev-seed sites in `CcsoccerCommands.php`), not "roughly fifteen". **Two listed sites are not bugs and must come off:** `TeamBalancerService` 281 is already status-filtered (`'status' => 'paid'` at :277), and `GroupController` 1315-1323 is not a `reset()` at all — it is the already-correct D14 inline copy of the helper, a *de-duplication* target rather than a fix. **Four real sites were missed**, two of them player-facing write paths — see the corrected list in §10.3. | as listed in §10.3 | T3/R4 |
| **CF5** | On move, reconcile against **all** teams in the tournament rather than only `reg.team` — a ghost has no `reg.team`, so the old team is never cleaned and the desync compounds per move. | `TournamentRosterBuilderController::move()` ~131 | TC (T5) |
| **CF7** | Deleting a Team should NULL every `Registration.team` pointing at it. There is no `hook_team_predelete()` today, and `clearTeamsForSeason()` deletes teams outright. | new hook in `ccsoccer.module`; `TeamManagerService::clearTeamsForSeason()` 234 | TI (R10) |
| **CF9** | Route the inline `Team.players` appends through `TournamentTeamManager::addPlayerToTeam()`, which already dedupes and capacity-checks. (`OrderCompleteSubscriber:707` is a team *create* seeded with its captain, not an append — it cannot produce a duplicate and is out of scope. `CcsoccerCommands.php:1501` also bypasses the manager but is dev-seed only.) **Scope corrected July 30 — three live sites, not four:** `OrderCompleteSubscriber:821` sits inside the `elseif ($action === 'join')` branch, which has been **dead since June 23** (`8483a20` reroutes every invited join to `token_accept`; a repo-wide grep finds no producer of `action = 'join'`). Prefer deleting the branch to refactoring it — see the CF12 brief in §10.3. | `OrderCompleteSubscriber` 773 (~~821 dead~~); `GroupController::acceptTeamInvitation()` 1549; `RegistrationController` 372 | TE |
| **CF6** | `dissolveGroupConfirmed()` clears `group_id`/`invited_by`/`invitation_status` but leaves `Registration.team` and `Team.players` intact. Decide the intended behavior before repairing rows it created. | `GroupInvitationsForm::dissolveGroupConfirmed()` 756, 772-774 | — (question, not yet a fix) |
| **CF10** | `invite()` generates and **saves** the season `group_id` before the invitee is validated, so a *rejected* invite still converts the inviter into a permanent group-of-one. Move the generate-and-save below the duplicate/eligibility checks so a group exists only once an invitation does. | `GroupController::invite()` 929-933 (write), 975-1030 (checks it precedes) | new-drift source: stranded solo managers |
| **CF11** | A season group manager has **no way out of their own group** — `leaveGroup()` refuses managers, and neither `removeMember()`, `leaveGroup()` nor `deleteInvitation()` clears the manager's own `group_id` when the group empties. Add a player-facing "Disband Group" for a manager who is the group's only live member. | `GroupController` (new `disbandGroup()`), `ccsoccer.routing.yml`, `ccsoccer-group-manage.html.twig`, shared dissolve service from CF8 | stranded solo managers (admin-ticket load) |
| **CF12** | Tournament registrations never receive an `invitation_status` (**D19**), so they store the field default `'none'` — which is **not among** `buildMembersTable()`'s three select options, so the browser renders the first one, **"Pending"**, for players who are paid, on a team, and in the captain's group. `buildStatsBar()`'s `match()` default counts those same rows as *accepted*, so two widgets on one page contradict each other. Worse, **Save writes the displayed value**: `original_status` is stored as `'none'`, the browser submits `'pending'`, the change-detector fires at 909 and a real `pending` is persisted at 919 — the page converts its own display fault into data. **Two halves:** (a) set `invitation_status` in `createTournamentRegistration()`'s create array, mirroring the season branch at :559; (b) fix the display side. **[CORRECTED Aug 4 — half (b) was described wrongly.]** The brief said "drop Pending from the tournament branch," but there **is no tournament branch** for member rows: the three-option array at `:548-552` is built unconditionally for both flows (only the *captain* row at 488/507 and co-captain at 519 branch on `$context['type']`). And dropping the option would not close the trap — `#default_value` at `:570` is still `'none'`, so the select still matches no option and the browser still renders the first one; Save then writes `accepted` instead of `pending`. **The real fix is normalising the default value on the tournament side.** See the rewritten §10.3 brief. | `OrderCompleteSubscriber::createTournamentRegistration()` ~850-864; `GroupInvitationsForm::buildMembersTable()` 545-577 | new-drift source: admin **Save** on the Group Invitations page. Retires **D19**, half of **D15** |

All of these are PHP. Per repo convention: archive the file to `archive/<Name>_<date>.php` before
editing (**check the filename is not already taken** — that bit us on July 27), `php -l`, LOCAL
click-through, then TEST, then PROD. There is no PHP in the authoring sandbox, so none of it can be
linted here.

---

## 6. Repair tooling — proposal, not built

A Drush command rather than raw SQL, for three reasons: it can use the entity API so `postSave()`
cache invalidation and hooks fire; it can be dry-run; and it becomes the reusable bulk sweep that
`OUTSTANDING_ISSUES.md` P5 item 5 says was never built.

```
drush ccsoccer:roster-reconcile --dry-run                 # default; prints the plan, writes nothing
drush ccsoccer:roster-reconcile --class=TA1,TF --dry-run  # one class at a time
drush ccsoccer:roster-reconcile --tournament=<id> --apply
```

Requirements:

- **`--dry-run` is the default.** `--apply` must be explicit.
- Every mutation logged with entity id, field, before value, after value.
- **Idempotent and re-runnable** — a second `--apply` must be a no-op.
- Class-scoped, so ambiguous classes (TB, TA2, TA3, TK, SH) can be held back while the unambiguous
  ones land.
- **Acceptance test = re-running `ROSTER_DATA_AUDIT.sql` and getting zeros** in every class that was
  in scope.

---

## 7. Decisions

**Andrew's call, July 28: no data repair this cycle.** The league is casual, cancellations and
declines are rare, teams are essentially never deleted. Existing rows stay as they are; the work is
code-only, aimed at the next season and the next tournament. That resolves or retires most of the
table below.

| # | Question | Status |
|---|---|---|
| **D-1** | **TB** — roster wins or registration wins? | **Moot** while there is no repair. Becomes live again only if a repair is ever run. |
| **D-2** | **SD** — a group's manager cancels. Promote, or dissolve? | **DECIDED — dissolve.** The group is cleared and the remaining members are notified they are no longer in a group; they can coordinate and form a new one. Encoded in **CF8**. |
| **D-3** | **IA/IB** — stale pending invitations: `declined` or `expired`? | **DECIDED — `declined`.** Nothing in the codebase ever sets or reads `expired` on an Invitation (E5); using it would create a state no code handles. Applies to the invitations CF8's dissolve declines. |
| **D-4** | **§3** — one canonical "live" definition. | **DECIDED — exclude `cancelled`/`expired`** rather than allowlist. See §3: `active`, `pending`, `waitlist` and `expired` are never written to a Registration by any code path, so this is behaviour-neutral today and robust to a hand-edited row tomorrow. |
| **D-5** | **TA2/TA3** — auto-remove roster entries with no live registration? | **Moot** — no repair. |
| **D-6** | **TK/SH** — players holding two *live* registrations for the same container. | **Effectively impossible by design.** `addSeasonToCart()` (`RegistrationController.php:960-977`) and `addTournamentToCart()` (`:1051-1068`) both refuse if a non-cancelled registration already exists. Only a race between two simultaneous checkouts, or an admin creating one by hand, could produce it. If the audit shows any, review individually — money is involved. |

### Added July 29

| # | Question | Status |
|---|---|---|
| **D-7** | **CF11** — should a manager be able to disband a group that still has other members? | **DECIDED — no, solo only.** Allow disband when the caller is the group's only live member. A populated group is dissolved by removing members first, or by an admin. Rationale in CF11: dissolving a populated group silently ejects people who chose that group and needs the D-2 notification path, which turns a small fix into CF8's scope. |
| **D-8** | Should a group **auto-clear** the manager's `group_id` when it drops to one live member (last member leaves / is removed / the only invitation is deleted)? | **DECIDED — Andrew, August 5: yes, as CF10 + B + E. Caleb still to review.** Auto-clear when a season group drops to one live member **and has no pending invitation** (the carve-out is what makes B safe — it is why this was rejected in July), *plus* **CF10**, moving the mint below the validation checks so route 1 never creates the state. **Implementation spec: `GROUP_CLEANUP_BRIEF.md`** — one predicate (`GroupDissolveService::clearGroupIfOrphaned()`), thirteen call sites, ~9-11 hrs. **Caleb's option D — do not mint a `group_id` until an invitation is accepted — is HELD, not rejected** (`GROUP_ID_AT_ACCEPT_BRIEF.md`); `D8_OPTIONS_COMPARISON.md` records why B+E won. **The two deciding arguments:** D re-keys ~20 of 29 `Invitation.group_id` sites and adds three concurrency races, while B touches none of them and adds no concurrency changes; and if B misses a call site someone is stranded in a group of one — today's behaviour, visible, fixed by Dissolve — whereas D's failure modes were a manager-less group or a player silently ungrouped after payment. **Knock-ons: CF10 reinstated, CF11 optional, D-9 mostly self-resolving.** Traced Aug 4 and still true: all three season accept paths already decline rival invitations, so the "A and C both invited B" case only leaves C's `group_id` behind — which is exactly what B clears. |
| **D-8 (original framing)** | *(superseded, kept for the reasoning)* | ~~**OPEN — Andrew's call.**~~ **Resolved by the row above.** It would close routes 2-4 in CF11's table so nobody ever gets stranded, rather than giving them a button. Against: a manager legitimately holding a group open for a slow invitee would have it silently dissolved under them, and re-inviting would mint a new `group_id` (harmless, but the pending invitation would need repointing — or deleting, which is the same stranded shape). CF11 does not depend on this either way. **That objection is what `GROUP_CLEANUP_BRIEF.md` §4.2's pending-invitation carve-out exists to answer** — the group is only dissolved when there is no invitation outstanding, so a manager waiting on a slow invitee keeps their group. |
| **D-9** | Should `manage()` show invitations addressed to a **solo manager** (today gated on `if (!$group_id)`, ~598)? | **MOSTLY RESOLVES ITSELF under D-8's answer** — under CF10 + B + E a player stops being *stranded* solo, so the gate rarely bites. What remains is the manager who is *legitimately* alone with an invitation outstanding: they still cannot see incoming ones. Widening the gate to *"no group, or manager of a group of one"* is a one-line display change. **Deferred, not in the D-8 commit** — it is a display gate rather than a data one, and the original caution stands: verify against the roster-builder screens first. `GROUP_CLEANUP_BRIEF.md` §10. |

### Added July 30

| # | Question | Status |
|---|---|---|
| **D-10** | **CF12** — for a tournament registration that is paid, on a team, and carrying the captain's `group_id`, should `invitation_status` be `accepted`, or a new value distinguishing "invited and accepted" from "self-joined / admin-placed"? | **DECIDED — `accepted`.** The codebase already made this call: update hook 9068 (`_ccsoccer_repair_legacy_team_group_ids()`, `ccsoccer.install:4572`) repoints legacy `team_*` group_ids **and sets `invitation_status = 'accepted'`** for precisely this population. CF12(a) makes the steady-state write path agree with the repair hook that shipped in June. Nothing reads the field as "responded to an invitation" — `submitForm()` treats `accepted` as *confirmed member of this group* and `declined` as *remove from group*, and that is the whole consumer set. A fourth enum value would cost an update hook plus every `match()`/comparison site revisited, to record a distinction the `Invitation` table already holds. **Free agents are not affected either way** — the page's own query (`GroupInvitationsForm:88`, `loadByProperties(['group_id' => …])`) excludes a NULL `group_id`, so an admin-dragged free agent never appears as a member row at all. |
| **D-11** | Should the **captain/manager** row also be written `accepted`? | **No — keep `'none'`.** Three paths deliberately write `'none'` for the group manager (`TournamentRosterBuilderController:505`, `RosterBuilderController:548`, `GroupInvitationsForm:774`); `'none'` means *in this group, not by invitation*. The `create` branch of `createTournamentRegistration()` should write `'none'` explicitly rather than relying on the field default. Display-neutral: `separateManagerAndMembers()` renders the captain via its own `captain_` row, which never reads the field. |

### Added August 4

| # | Question | Status |
|---|---|---|
| **D-14** | **CF9** — should CF5 and CF9 ship as one commit, as the plan scoped them? | **DECIDED — no, split them (Andrew, Aug 4).** They share no files: CF5 is three call sites in `TournamentRosterBuilderController`, CF9 touches `OrderCompleteSubscriber`, `GroupController` and `RegistrationController`. CF9 also carries all the risk — three constructor changes plus a `ccsoccer.services.yml` edit (none of the three sites currently inject `TournamentTeamManager`), a double-save interaction with `getOrCreateTeamGroupId()`, and the deletion of the dead `action = 'join'` branch. Pairing a low-risk three-line change with that made the combined commit unreviewable and un-revertable independently. **CF5 shipped alone as `7c9a7c5`; CF9 is now commit 6b.** |
| **D-13** | **CF4** — which registration statuses count as dead? | **DECIDED — `cancelled` and `expired`; `waitlist` is LIVE.** `Registration` enumerates six statuses (`pending`, `paid`, `active`, `cancelled`, `waitlist`, `expired`) and the module already treats those two as the dead pair: `GroupController::manage()` rejects both with "This registration has been cancelled", and the My Registrations list excludes both. The trait's first draft skipped only `cancelled`, which left the split-brain bug reachable through an expired row — no code path *writes* `expired` today, but the status is an `options_select` on the registration edit form and a filter option in `RegistrationListBuilder`, so such rows are expected to exist. `waitlist` counts as live deliberately: a waitlisted player holds a real uncancelled registration and is the right row to write against. Excluding the two dead values rather than allowlisting the four live ones follows **D-4**. Implemented as `LiveRegistrationTrait::isDeadRegistrationStatus()`, which `GroupDissolveService` also uses so there is one definition of "dead". |
| **D-12** | **CF8** — when a group is dissolved, which invitation statuses get declined? | **DECIDED — both `pending` and `accepted`**, scoped by `group_id` alone. The three copies this replaces each did something different: `dissolveGroupConfirmed()` declined only `pending`, `leaveGroup()` and `removeMember()` only `accepted`. Both matter and for different reasons — a stale **pending** invitation reserves capacity in `getGroupSize()` indefinitely (E5), and a stale **accepted** invitation blocks re-inviting that address (E4/Bug 3, the "already accepted" block the July 20 checklist section G was written to prove fixed). The union is the correct behaviour for a group that is ceasing to exist. Not exposed as an `$options` key: every caller would pass the same value, which is the signal it should not be a parameter. **Implemented in `GroupDissolveService::dissolveGroup()`.** Note this is a small behaviour change to the admin Dissolve button, which previously left accepted invitations alone. |

**The duplicate that *does* happen is register → cancel → register again.** That leaves a cancelled
row and a live row for the same container, which is exactly the population every bare `reset()` in
the module resolves wrongly (`loadByProperties()` keys by id, `reset()` takes the lowest, which is
the older cancelled row). Audit classes **TM** and **SJ** count it, and section 3.10 lists the rows
with both "what `reset()` takes" and "what it should take". **If TM and SJ both come back 0, CF4
cannot bite on current data** and can be scheduled purely as hardening.

---

## 8. Suggested sequencing (superseded — see §9 and §10)

> **This section assumed a data repair.** Andrew decided on July 28 not to repair data this cycle,
> which removes the "land code fixes first so the repair doesn't re-rot" rationale that ordered it.
> **Use §10 instead.** Kept for reference in case a repair is ever revisited.

1. **Run `ROSTER_DATA_AUDIT.sql` on TEST.** Review section 1 (summary), then 2.1 (which tournament
   teams falsely report full) and 4.2 (season group integrity). ← *we are here*
2. **Review the sample rows for the ambiguous classes** (TB especially) and settle D-1 … D-6.
3. **Land CF1, CF2, CF8 first.** Repairing before these ship means the data starts re-rotting on the
   next cancellation or admin decline. Normal path: archive → `php -l` → LOCAL → TEST → PROD.
4. **Build the reconcile command**, dry-run it on TEST, apply on TEST, re-run the audit, confirm zeros.
5. **PROD:** database backup → dry-run → apply → re-run the audit. ~200 players/season, so the row
   counts should be small enough to eyeball the whole dry-run output.
6. **Then** build rosters, with CF3/CF4/CF5 (read-side correctness) following behind — they don't
   create drift, they just misreport it.

There is a real tension in step 3: rosters are being built over the next few weeks and registration
is open now, so cancellations are actively creating new drift. If the code fixes can't land first,
the fallback is to repair now and repair again immediately before the roster build, accepting that
the window between them re-dirties.

---

## 9. Deferred: the existing drift

These fixes stop new drift. They do **not** clean what is already stored. If the audit shows a
tournament team whose `isfull_today` reads FULL while `isfull_after_cleanup` reads "has room", that
team will still misreport during the SLO Friendly roster build. Two ways to deal with it later,
neither in scope now:

- **Read-side fix** — make capacity count roster entries backed by a live registration, which is
  T2's own alternative ("capacity should count what's displayed, or reconcile ghosts on load"). Fixes
  the symptom everywhere without touching data. Touches a hot path, so it needs its own session.
- **Targeted repair** — §4/§6, scoped to the handful of affected rows.

---

## 10. Implementation brief — start here for a code-only session

**Goal:** make the code correct so the *next* season and the *next* tournament don't accumulate this
drift. **No data repair.** Existing rows stay exactly as they are.

### 10.0 Context in ten lines

- Season rosters live only on `Registration.team`; `Team.players` is empty for season teams and must
  stay that way. Season teams have no roster cap, so "team is full" is impossible there.
- Tournament rosters live on `Team.players` (capacity + display) *and* `Registration.team` (roster
  builder, cancel, checkout). Different code paths write different sides. That is the bug.
- `TournamentTeamManager::unassignPlayerFromTeam()` already does the right cleanup and **has zero
  callers**. Wiring it up is most of the tournament fix.
- `Registration.status` is a two-value field in practice: `paid` or `cancelled` (see §3).
- A tournament free agent legitimately has `group_id` NULL. Never "fix" that.
- Every bare `reset()` on a registration collection takes the *oldest* row, which is the cancelled
  one when a player registered → cancelled → re-registered. Six confirmed bugs so far.
- `invitation_status` is written by the **season** registration path and never by the tournament one, so
  every tournament registration stores the field default `'none'` — a value the admin Group Invitations
  page has no option for and therefore renders as "Pending". CF12. Do not confuse this with `status`.
- A season group is created **lazily, by `invite()`** — `group_id` is NULL until a player sends their
  first invitation. Registering alone leaves it NULL (`OrderCompleteSubscriber:481,557`), which is why
  a solo player's Manage Group page shows an invite form and an empty roster. That is correct. But
  `group_id` is currently saved *before* the invitee is validated, and a manager can never leave their
  own group — so a rejected invite strands the player permanently. CF10 + CF11.

### 10.1 Ground rules

- **Never run git write commands in this repo.** Hand Andrew the command. (A `git reset` in a past
  session left stale lock files that blocked his commit.)
- **No PHP in the authoring sandbox.** Every change needs `php -l` and a LOCAL click-through on
  Andrew's machine before commit. Do not commit unlinted code.
- **Archive before editing:** copy to `archive/<ClassName>_<YYYY-MM-DD>.php` first, and **check the
  filename isn't already taken** — a past session clobbered an existing archive.
- Line numbers in this document are from July 28 and **shift as soon as the first file is edited**.
  Re-locate everything by method name.
- Deploys here are code-only: `drush cr`. Run `drush config:status` before any `cex`.

### 10.2 Pre-flight — do these before writing code

> ### ✅ [Aug 4 evening] THIS SECTION IS SPENT — five of six items are done. Skip it.
>
> It was a pre-flight for commit 1. Nine commits have landed. Item 1 (clear the tree) is done; item 3
> (nudge fix on PROD) is done; item 5 (check for a stale `.git/index.lock`) applied to the first
> commit, long past; item 6 (archive filename collisions) applied to commit 1 specifically. Item 2 —
> **run `ROSTER_DATA_AUDIT.sql`** — is still not done, but CF4's own banner records that the question it
> was meant to answer is now **moot**: the write-path bug was reproduced by hand, so the audit no longer
> sizes CF4.
>
> **The live pre-flight is now the ▶ START HERE block at the top of this document: lint, `drush cr`,
> then §10.8.**

**[STATUS Aug 4 morning]** Items 1 and 3 are **satisfied**; item 2 is still open but does not block commits 1-3.

1. ~~**Clear the working tree.**~~ **✅ DONE.** The July 28 roster-builder changes and `fix/team_display`
   (`9dfe05f`) all landed via PR #119 (`7fd60bf`). Working tree is clean apart from two stale untracked
   drafts, `SESSION_HANDOFF-wip.md` (Jul 18) and `SEASON_TOURNAMENT_DRIFT_AUDIT-wip.md` (Jul 19) — both
   two weeks behind their committed counterparts and safe to delete.
2. **Run `ROSTER_DATA_AUDIT.sql` on TEST** and read classes **TM** and **SJ** (section 3.10). They
   size CF4: if both are 0, no player currently holds a cancelled + live pair, the `reset()` bug
   cannot fire on today's data, and CF4 is pure hardening that can be scoped small.
   **[Aug 4] Still not run — but it sizes CF4 (commit 5) only. It is not a blocker for commits 1-3,
   and since there is no data repair this cycle the other class counts are informational.**
3. ~~**Confirm `fix/tournament_nudge_500` reached PROD.**~~ **✅ DONE.** `fa76cfd` is on `main` and was
   PROD-verified Aug 2 (`DOC_CLEANUP_CONSOLIDATION_WIP.md` §6). One commit on that branch (`a6a4403`)
   is still unmerged — docs plus a drush seeder trim, nothing CF4 touches.
4. **[Aug 4, new] Nothing else is in flight.** Caleb's bulk-notification work is merged and deployed;
   the deploy queue is empty for the first time in weeks. No branch touches any file in commits 1-3.
5. **[Aug 4, new] Check for a stale `.git/index.lock`** before the first commit — one was present on
   Aug 4. This is the failure mode recorded in the July 27 process note.
6. **[Aug 4, new] No archive filename collisions** for commit 1: there is no existing
   `archive/TournamentCancelRegistrationForm_*.php` or `archive/TournamentTeamManager_*.php`.

### 10.3 The work, in order

Each item: what to change, which decision it encodes, how to test it, what could go wrong.

---

#### CF1 — Tournament cancel must clean the roster  *(highest value; stops new ghosts)*

> ### ✅ IMPLEMENTED — `e5317f8` (CF1) + `8156bc1` (CF1b). Untested. Line numbers below are stale.
>
> Read as the rationale record, not as instructions. The commit message on `e5317f8` lists what
> actually shipped, including the co-captain handling this brief was missing and the deletion of the
> two dead `ccsoccer_registration_*` hooks.
>
> **CF1 has a consequence recorded nowhere else in this brief:** by correctly clearing
> `Registration.team` on cancel, it removed the guard that had been accidentally masking CF4's
> `reset()` bug. That is how the Aug 4 reproduction became a silent data corruption rather than a
> confusing warning. CF1 is right; it simply exposed something underneath. See the CF4 brief.

**Files:** `Form/TournamentCancelRegistrationForm.php` (`submitForm()`),
`Service/TournamentTeamManager.php` (`unassignPlayerFromTeam()`, `removePlayerFromTeam()`)

**Today:** the cancel form sets `status`/`cancellation_date`, and clears `Team.captain` if the player
was captain. It never removes them from `Team.players` and never clears `team`/`group_id`/
`invited_by`. That single gap is the origin of every tournament ghost (D6/T2).

**[VERIFIED Aug 4]** Every line reference in this brief still resolves exactly. Neither file has been
touched since it was written — `TournamentCancelRegistrationForm.php` has **one commit in its entire
history** (`f475ff0`, June 3) and `TournamentTeamManager.php` last changed pre-July.
**`unassignPlayerFromTeam()` still has zero callers repo-wide.**

**[Aug 4] Blast radius is fully bounded, which the brief did not claim.** Only two code paths anywhere
write `status = 'cancelled'` to a Registration: this form (`:271`) and `CancelRegistrationForm` (`:349`).
There is no player-facing cancel route — both are `_admin_route` behind `manage tournaments` /
`manage seasons`. The deposit refund and forfeit forms write a separate `deposit_refund_status` field
and never touch `status`. **So CF1 + CF8 together cover the entire cancellation surface.**

**Change, in five parts** *(was three — parts 2 and 4 added Aug 4)*:

1. **Harden `unassignPlayerFromTeam()`.** It currently resolves the team from `Registration.team` and
   silently skips the roster cleanup when that is NULL — which is exactly the ghost shape it is
   supposed to clean. Add a fallback: when `Registration.team` is empty, scan every team in the
   registration's tournament and remove the uid from any `Team.players` containing it. Write this as
   a small public helper (`removePlayerFromAllTeamsInTournament()`) because **CF5 needs the same
   function** — one implementation, two callers.
   **[CORRECTED Aug 4, shipped as commit 1b] "When `Registration.team` is empty" is too narrow — scan
   unconditionally.** A player whose `reg.team` names team A while their uid is *also* stale on team
   B's `Team.players` is audit class **TC** / finding **T5**, left behind by a half-completed move.
   Cleaning only `reg.team`'s team handles the empty case and lets the second one compound. The scan
   covers `reg.team`'s own team anyway, so there is no second branch to write and the method gets
   simpler, not more complex. **This is the same correction already recorded in the CF5 brief below**
   ("the flaw is broader than 'the ghost has no `reg.team`'") — the two briefs disagreed, and CF5 was
   the one that was right.
2. **[NEW Aug 4] Inject the service into the form.** `TournamentCancelRegistrationForm` currently
   takes only `EntityTypeManagerInterface` (`__construct()` :43, `create()` :50). Add
   `ccsoccer.tournament_team_manager` to both. Small, but unmentioned in the original brief and the
   kind of omission that only surfaces as a container error on LOCAL.
3. **Add an `$allow_captain = FALSE` parameter.** `unassignPlayerFromTeam()` refuses when the
   registration's `is_captain` is set (479-484) and `removePlayerFromTeam()` refuses when the uid is
   `Team.captain` (177-183). The cancel form already clears `Team.captain` for a cancelling captain,
   so have it **clear `Team.captain` first, then call with `$allow_captain = TRUE`**. Order matters.
   **[CORRECTED Aug 4] Thread the parameter through BOTH methods, not just `unassignPlayerFromTeam()`.**
   The second guard lives in `removePlayerFromTeam()` and keys on `Team.captain`; relying on "we cleared
   it first, so that guard passes" makes correctness depend on save ordering and entity-cache freshness.
   Pass it explicitly. The three existing callers of `removePlayerFromTeam()`
   (`TournamentRosterBuilderController` 131, 342, 490) are unaffected by a defaulted parameter.
4. **[NEW Aug 4 — the gap that would have made CF1 a regression] Handle co-captains.**
   `promoteToCoCaptain()` sets `is_captain = TRUE` on the co-captain's *registration*
   (`GroupInvitationsForm:1096`) **and** `Team.co_captain` to their uid (`:1101`). So today, when a
   co-captain cancels: the form's captain block queries `Team.captain = player`, finds nothing, and
   **never clears `Team.co_captain`**; `unassignPlayerFromTeam()`'s `is_captain` guard then refuses the
   whole operation. With CF1 as originally written, `$is_captain` reads TRUE (same field), you pass
   `$allow_captain = TRUE`, `removePlayerFromTeam()`'s `Team.captain` guard passes because they are not
   the captain — and they come off `Team.players` **while `Team.co_captain` still points at them.**
   That is a dangling leader reference, exactly T9's shape, and exactly what audit class **TJ** counts
   (`ROSTER_DATA_AUDIT.sql:293` checks captain *and* co_captain).
   **Fix:** extend the captain block to also clear `Team.co_captain` where it names the cancelling
   player, and clear it inside `removePlayerFromTeam()` for the same uid. `Team.co_captain` is a real
   base field (`Team.php:133`); the service currently contains **no `co_captain` reference at all**.
   D4 fixed this in `GroupController::removeMember()` only — the service never got the same line.
5. **Call it from `submitForm()`** after the status write, before the refund branch. In practice that
   means immediately after the existing captain block (`:279-294`) and before the refund at `:296`.

**Also:** a cancelling captain leaves a team with no captain. Keep the current behaviour (just clear
it), but log a watchdog warning naming the team so the tournament director notices before publish.

**[Aug 4] Two more field decisions the original brief left implicit:**

- **`invitation_status`.** Class **TF** (§4.1) says cancel should set it to `'none'`, but
  `unassignPlayerFromTeam()` does not. It is a **no-op today** — every tournament registration already
  stores `'none'`, which is the CF12 bug — but the moment CF12(a) lands in commit 2, tournament
  registrations start storing `'accepted'` and a cancelled row would keep it. **Add it in commit 1**
  rather than circling back.
- **`is_captain`.** Left TRUE on the cancelled registration by every current path. Recommend clearing
  it **in the form, not the service** — only the form knows this is a cancellation rather than an
  unassignment. Leaving it set is what makes captain-vs-co-captain detection ambiguous downstream.

**[Aug 4] Accepted design note: the registration is saved twice** — once by the form for
`status`/`cancellation_date`, once inside `unassignPlayerFromTeam()`. Harmless (storage returns the same
statically-cached object), and the alternative is inlining the service's field clearing into the form,
which is the copy-paste failure mode this entire document exists to prevent. Take the two saves.

**Test on LOCAL:**

- Cancel a non-captain player who is on a team → disappears from the Manage Team roster and the
  Tournament Roster Builder; `Team.players` count drops by one; `team`/`group_id`/`invited_by` NULL.
- Cancel a captain → `Team.captain` cleared, captain removed from `Team.players`, team survives,
  co-captain untouched, warning logged.
- **[NEW Aug 4] Cancel a co-captain** → `Team.co_captain` cleared, removed from `Team.players`,
  **`Team.captain` untouched**, team survives.
- **[NEW Aug 4] Cancel a player on a team captained by someone else** → that captain is untouched on
  both `Team.captain` and `Team.players`.
- **Ghost case:** hand-set a test registration's `team` to NULL while leaving the uid in
  `Team.players`, then cancel → the fallback still removes them.
- Cancel a free agent with no team → no error, no warning.
- Cancel a season registration → behaviour unchanged.

**Risk:** touches the refund path's form. Keep the roster cleanup on its own line before the refund
so a refund failure can't leave the roster half-cleaned.

---

#### CF2 — Admin accept/decline must sync `Team.players`  *(carries a dependency)*

> ### ✅ IMPLEMENTED — `59c12ec`. Untested, and **never exercised even once.**
>
> Read as the rationale record, not as instructions; line numbers below are stale.
>
> **This is the least-validated code in the whole sequence.** The Aug 4 reproduction that exposed the
> CF4 bug went nowhere near the admin Group Invitations page, so nothing about CF2 or D2 has been
> observed working. It also closes a **live PROD bug** — admin accept of a tournament invitation
> fatals and half-writes — so the operating rule in `SESSION_HANDOFF.md` stands until it is tested and
> deployed: **the admin Group Invitations page is read-only for tournament groups.**
>
> Give it the most attention in §10.8's checklist.

**File:** `Form/GroupInvitationsForm.php` (`submitForm()` ~815 with the status flip at 924-936;
`acceptInvitation()` ~1160 with the writes at 1223-1229)

**Today:** decline clears `Registration.team` (927) and accept sets it (934); neither touches the
Team entity. Declined players stay on the roster as ghosts; accepted players are missing from it.

**Change:** in the tournament branch only — decline → `removePlayerFromTeam($team, $uid)`;
accept → `addPlayerToTeam($team, $uid, TRUE)`. Force is correct: admin placement past capacity is
already accepted as designed (T1). Seasons have no Team entity — leave that branch alone.

**⚠ Dependency: fix D2 in the same commit.** Admin-accepting a tournament invitation currently
*crashes* — `GroupInvitationsForm.php:1279` queries a `tournament` field that does not exist on the
Invitation entity, throwing after the registration was already saved. **You cannot test CF2's accept
half until D2 is fixed.** The correct pattern is in `GroupController::acceptTeamInvitation()`: filter
by `isTeamInvite()` plus the team's tournament.

**[VERIFIED Aug 4]** Every reference in this brief resolves exactly: `submitForm()` :815, decline clears
`Registration.team` at :927, accept sets it at :934, `acceptInvitation()` :1160 with writes at
:1223-1229, the phantom `tournament` condition at :1279 executing at :1283.
`Invitation::baseFieldDefinitions()` (:49-177) defines `inviter`, `invitee_email`, `invitee`, `season`,
`group_id`, `team`, `token`, `status`, `message`, `responded_at`, `notified`, `created`, `changed` —
**no `tournament`**, confirmed. `GroupInvitationsForm.php` contains **zero** reads or writes of
`Team.players` anywhere. `GroupController::acceptTeamInvitation()` (:1503) does it correctly at
:1547-1558 with the sibling-decline pattern at :1590-1605.

**[Aug 4] Four hazards neither this brief nor the handoff mentions — fold them into the same commit:**

1. **The half-write is wider than "registration saved, invitation not".** `submitForm()` saves **every
   changed member row** at :938 before the third pass starts, and saves each earlier `inv_` row at :973
   before reaching the one that throws. There is no transaction anywhere in the method, so a submit
   carrying several changes can commit an arbitrary *prefix* of them and abort. This upgrades the
   handoff's "consider reordering `submitForm()`" from a suggestion to a requirement.
2. **The PROD operating rule is broader than the bug.** Only the `inv_` pending-invitation rows (third
   pass, :963-969) reach `acceptInvitation()` and fatal. Accepting a `reg_` **member** row (second pass,
   :930-936) never calls it — it silently produces the `Team.players` drift instead. Two different
   failure modes on one screen; the test plan must separate them, because only one of them is D2.
3. **`acceptInvitation()`'s registration lookup (:1216-1221) has no status filter** and then `reset()`s,
   so a **cancelled** registration can be picked up and resurrected into the group.
   `createTournamentRegistration()` (:621) *does* exclude cancelled — the twin paths disagree. This is
   also a CF4 site (`GroupInvitationsForm` 1224); fixing it here is cheaper than deferring it.
4. **`responded_at` is never stamped** by :1229 or by `submitForm()` :972-973, while
   `GroupController` sets it at :1569. That is a **fourth** divergence between the twin accept paths,
   alongside the missing `sendPlayerJoined()`, the missing `Team.players` write, and the missing guards.

**[Aug 4] One-way door worth knowing before writing the decline test:** member rows are loaded by
`group_id` (:88) and decline nulls `group_id` at :924, so after Save the row **vanishes from the page**
and the accept branch can never be reached for it again. CF2's `removePlayerFromTeam()` therefore has to
run in the same submit — there is no second chance.

**Test on LOCAL:**

- Admin declines a tournament member → gone from `Team.players` *and* `Registration.team` cleared;
  the team page roster shrinks by one.
- Admin accepts a pending tournament invitation → **no error page** (D2), player appears in
  `Team.players`, `Registration.team` set, competing invitations declined, nothing half-committed.
- **[NEW Aug 4] Admin accepts a `reg_` member row** (not an `inv_` invitation row) → no fatal before or
  after, and `Team.players` now gains the player. This path never fataled; it silently drifted.
- **[NEW Aug 4] Submit several row changes at once, one of which would have thrown** → nothing is
  committed, or all of it is. Currently a prefix commits.
- **[NEW Aug 4] Accept an invitation for a player who also holds a cancelled registration** for the same
  tournament → the **live** row is updated, not the cancelled one.
- Season accept/decline → unchanged.

---

#### CF12 — tournament `invitation_status` is never written, and the admin page renders the gap as "Pending"

> ### ✅ IMPLEMENTED — display half `e5317f8`, write half `59c12ec`. Untested. Line numbers stale.
>
> Read as the rationale record. Closes **D19** and half of **D15**; both still need their entries
> updated in `SEASON_TOURNAMENT_DRIFT_AUDIT.md`, which is commit 9 and not done.

**Files:** `EventSubscriber/OrderCompleteSubscriber.php` (`createTournamentRegistration()`),
`Form/GroupInvitationsForm.php` (`buildMembersTable()`)

**Ships with CF2 + D2 in commit 2.** Same file, and the Save trap below lives inside the exact lines CF2
rewrites (`submitForm()` 909-919). Landing CF2 alone leaves its own test plan passing green while three
rows on the same screen still misreport — CF2's tests look at `Team.players`, not at the other members.

**The report (Andrew, July 30).** Lunch Crew, SLO Friendly 2026. The Tournament Roster Builder shows 8
players on the team. The Group Invitations page for the same team lists the last three to join as
**Pending** — while its own stats bar, four inches above, reads **Accepted: 7**.

**Two screens, two fields.** `TournamentRosterBuilderForm` never reads `invitation_status` — not once in
the file. Its query is `tournament` + `status IN ('paid','active')` (:335), column placement is
`registration.team` (:428), and the yellow captain-group box is plain `group_id` equality (:804, whose
comment claims "invited and accepted" but tests neither). `GroupInvitationsForm:545` reads
`invitation_status`. Neither screen is wrong about its own field; the field is simply unset.

**Why the widgets disagree with each other.** `'none'` is a legal stored value but is not among the three
options `buildMembersTable()` offers (:548). A `<select>` whose `#default_value` matches no option renders
nothing selected, and the browser displays the **first** option — "Pending". Meanwhile `buildStatsBar()`
(:363) buckets the same rows via `match($status)` with `default => $accepted_count++`. Hence
`3 explicit accepted + 3 'none' + 1 captain = Accepted: 7` alongside three dropdowns reading Pending.
**Two widgets on one page disagreeing about the same rows is the signature of an out-of-range value, not
of real pending state** — the same diagnostic shape as the July 27 Lunch Crew stats-box bug (`9dfe05f`),
where the data was right and the display was wrong.

*(The bar's separate `Pending: 8` is a different quantity entirely — `count($pending_invitations)`, i.e.
`ccsoccer_invitation` entities for the team. That is T8's population, not this one. Both render on one
page, which is worth knowing before reading any number on it.)*

**Why now — a June 26 regression, not a longstanding gap.** Two commits, three days apart:

| Commit | Date | What it did | Effect on this bug |
|---|---|---|---|
| `8483a20` | Jun 23 | Team-paid registration (Wayne's team fee). Removed the open team dropdown; joining is now invite-only. Reroutes `join:<team_id>` to emit `action = 'token_accept'` "so the downstream branch is reused verbatim" (:277-284). | **None.** Neither branch ever set `invitation_status`. Side effect: the `'join'` branch became dead code — see CF9. |
| `84dac45` | Jun 26 | Routed all three branches through `getOrCreateTeamGroupId()` so checkout registrations carry the team's real UUID. | **This is the trigger.** |

Before `84dac45`, checkout wrote `group_id = 'team_<teamID>_<captainID>'` while the Team entity's
`group_id` was a UUID — so a checkout registration matched no group and **never appeared on this page at
all**. Its own commit message says so: *"not counted as accepted on the Group Invitations page… This made
checkout-registered players look like free agents."* After it, they appear — carrying `'none'`.

The split is clean, and it explains why only *some* rows are affected. `84dac45` also shipped update hook
9068, whose repair loop (`ccsoccer.install:4572`) does:

```php
$registration->set('group_id', $team_uuid);
$registration->set('invitation_status', 'accepted');   // ← the hook knew the right answer
```

…gated on `condition('group_id', 'team_', 'STARTS_WITH')`. So:

- **Registered before Jun 26** → legacy `team_*` group_id → repaired by 9068 → `accepted`. *(Rod, Taylor, Raquel.)*
- **Registered after Jun 26** → correct UUID at creation → 9068's query never matches → `'none'` → "Pending". *(Dylan, Ryan, Eileen — the "last three to join".)*

The one-time repair encoded the correct rule a month ago; the permanent write path never got the same
line. That is also what settles **D-10**.

**⚠ The trap — do not click Save on that page before this ships.** `original_status` is stored as `'none'`
(:576), the browser submits `'pending'`, so `$new_status !== $original_status` at 909 and
`set('invitation_status', 'pending')` fires at 919. One Save turns a cosmetic fault into genuinely wrong
data, three rows per affected team. **This is why CF12 belongs with the drift-stoppers and not in Parked:
the Group Invitations page is currently a new-drift source, triggered by its most ordinary action — on the
screen the tournament director is being given access to.**

**Change, two halves.**

1. **Write side** — `createTournamentRegistration()`, in the `->create([…])` array alongside `group_id`
   and `invited_by` (~861-863). Mirror the season branch at :559, which has always passed the field:
   `token_accept` and `join` → `'accepted'` (D-10); `create` (captain) → `'none'` (D-11); pool and
   "no team yet" → `'none'`. Set it explicitly in every branch rather than leaning on the field default,
   so the next reader can see the intent.
2. **Display side** — **[REWRITTEN Aug 4. The original text below was based on a false premise; read
   this replacement, not it.]**

   > ~~`buildMembersTable()`, tournament branch only: offer **Accepted / Declined**, drop **Pending**…~~

   **What was wrong.** The brief claimed "the two branches are already separated by `$context['type']`
   throughout the method." **They are not, for member rows.** The three-option array at **:548-552** is
   built **unconditionally** and used by both flows. The only `$context['type'] === 'tournament'` test
   inside that loop is at **:555**, which merely *appends* the promote options. Branching on context
   exists for the **captain** row (`if` :488 / `else` :507) and the **co-captain** row (:519) — not the
   member rows. So there is no tournament branch to edit in place; the conditional has to be introduced,
   and "leave the season branch alone" cannot be satisfied by editing what is there.

   **Second thing wrong: dropping the option does not close the trap.** `#default_value` at **:570** is
   `$current_status`, which is still `'none'`. A `<select>` whose default matches no option renders the
   **first** option regardless of which options you offer. Remove `pending` and the browser shows
   `accepted` instead — better-looking, but the row is still out of range, the change detector at :909
   still fires (`'accepted' !== 'none'`), and **:919 still writes**, plus **:932 re-sets `group_id`**.
   The Save trap survives in mutated form: an unrequested write on a row nobody touched. Benign data,
   but the mechanism the brief set out to kill is still there.

   **What to do instead — normalise the default value.** In the member-row loop, on the tournament path
   only, map a stored `'none'` to `'accepted'` when computing `#default_value`. That single change:
   makes the select genuinely in range, so the browser shows what is stored; makes "Save with no edits"
   a true no-op at :909; and agrees with `buildStatsBar()`'s `match()` default (:363-367), which has been
   counting these rows as accepted all along. Dropping **Pending** from the tournament option list is
   still worth doing on top — pending is not a reachable state for a tournament *member* row (a genuinely
   pending invitee has no registration in the group yet; they are an `Invitation` entity rendering in the
   separate `inv_` rows via `loadPendingInvitations()` :260) — but it is the secondary half now, not the
   primary one. **Leave the season path alone** — pending *is* a real member state there.

   **[NEW Aug 4] You get a second live bug fixed for free.** The promote-to-captain and
   promote-to-co-captain options at **:555** are gated on `$current_status === 'accepted'`. Every
   post-June-26 checkout registrant carries `'none'`, so **those players cannot be promoted from this
   page at all** — the option is silently absent. Same root cause, same line, and normalising the default
   value fixes it without any further code. This is not recorded anywhere else and it is affecting the
   SLO Friendly screens right now.

   **[NEW Aug 4] Two decoys to be aware of while reading the file.** `:545` and `:362` both use
   `?? 'accepted'`, which only fires when the field is *empty*. Because the field default is `'none'`,
   affected rows store a real value and the fallback never runs — a quick read of :545 will wrongly
   suggest the display is already safe. Separately, the self-healing backfill at **:102-134** writes
   `invitation_status = 'accepted'` on load, but only when the group query returns zero registrations,
   so it never masks this on a populated team. It is a third place encoding update 9068's rule.

**Deliberately not doing: a data backfill.** §10.6 forbids data repair this cycle, and half (b) makes it
unnecessary — an existing `'none'` row renders as Accepted, which is what `buildStatsBar()` has been
counting all along, so the two widgets agree without a single write. If a repair is ever run, the rule is
9068's, already written and already idempotent.

**Test on LOCAL:**

- Register for a tournament via an invited join (`join:<team_id>` → `token_accept`) → registration stores
  `invitation_status = 'accepted'`; the Group Invitations page shows **Accepted**, stats bar agrees.
- Register as captain (`create`) → `'none'`; renders as the **Captain** row, unchanged.
- Register as free agent / CCSoccer pool → `'none'`, no `group_id`, **absent** from the page entirely
  (`:88` excludes NULL `group_id`) and present in the roster-builder workbench or pool. This is the
  Yong Po shape — confirm it is untouched.
- **The regression case:** hand-set a tournament member's `invitation_status` to `'none'`, load the page →
  renders **Accepted**, not Pending; stats bar unchanged. Click **Save** with no edits → *"No changes were
  made."* and the stored value is still `'none'`. *(Before the fix this wrote `pending`. This is the
  acceptance test.)*
- Admin sets a member to **Declined** → still removes from the group and clears `team` as today.
- **[NEW Aug 4] Promote a post-June-26 registrant** (one carrying `'none'`) **to Captain, then to
  Co-Captain** → the options are **present** in their dropdown and both promotions work. Before this
  fix the options were silently absent for every such player. This is a second acceptance test.
- Season group invitations page → **Pending still offered**, all three statuses behave as before,
  and a season member stored as `'none'` still renders however it renders today — **the default-value
  normalisation must not reach the season path.**

**Verify the timeline before writing code** (read-only; belongs with the §10.2 pre-flight):

```sql
SELECT id, player, created, invitation_status, group_id
FROM ccsoccer_registration
WHERE tournament = 5 AND registration_type = 'tournament'
  AND status = 'paid' AND team IS NOT NULL
  AND (invitation_status = 'none' OR invitation_status IS NULL)
ORDER BY created;
```

Every row should postdate the June 26 deploy of `84dac45`. If any predate it, there is a third write path
this brief has not found — stop and locate it before changing anything.

**Risk:** **[CORRECTED Aug 4 — was "low", now low-to-moderate.]** Half (a) is genuinely low: additive to a
create array. Half (b) is **no longer a one-line option removal** — it introduces a context conditional
into a loop that currently has none, and changes a `#default_value` computation that both the season and
tournament flows run through. The way to get it wrong is leaking either change into the season path,
where `pending` is a legitimate member state. **The two flows are NOT already separated here** (that was
the original brief's error), so the separation you rely on is the one you are adding — write it once,
around both the option list and the default value, and assert the season behaviour explicitly in test.

**Note for the audit:** `ROSTER_DATA_AUDIT.sql` has no class that counts this. TG and TH are `group_id`
classes; nothing inspects `invitation_status` on the tournament side. Running the audit will not surface
it, and re-running it after CF12 will not show a change. Add a class if a repair is ever revisited.

---

#### CF8 — Season cancel must clean the group, and dissolve it if the manager leaves

**Files:** `Form/CancelRegistrationForm.php` (`submitForm()`), `Service/NotificationService.php`,
`Form/GroupInvitationsForm.php` (`dissolveGroupConfirmed()` — extract, don't copy)

**Today:** the entire registration mutation is `set('status','cancelled')` + `save()` (349-350). The
row keeps its `group_id`, so it still counts toward the group cap and still renders on the group page.

**Change:**

1. Clear `group_id`, `invited_by`, and set `invitation_status = 'none'` on the cancelling registration.
2. **If the canceller was the group manager** (`invited_by` empty) **and other members remain →
   dissolve the group** (decision D-2): clear those three fields on every remaining member, set any
   pending invitations for the group to `declined` with `responded_at` (decision D-3), and notify
   each remaining member.
3. If the canceller was an ordinary member, clear only their own fields; the group survives.
4. **Do not touch `Registration.team`.** A friend group is not a roster — dissolving a group must not
   un-assign anyone from their season team.

> ### ⚠ [Aug 4] §5's CF8 row contradicts D-2 — D-2 wins.
>
> The §5 table says CF8 *"must include the SD manager-promotion rule."* That is stale: **D-2 was
> decided as dissolve, not promote** (§7). Promotion was the original recommendation for repair class
> SD and was superseded on July 28. Build the dissolve. Ignore the §5 wording.

---

#### CF8 — full implementation brief *(written Aug 4 morning — ✅ ALREADY IMPLEMENTED, see banner)*

> ### ✅ DONE — shipped in `b22734f`. Do not implement this again.
>
> The heading used to read *"ready to implement; all line numbers current"* and the completion banner
> sat 200 lines below, after eight sub-sections of imperative instructions. That is fixed; the full
> record of what landed, what deviated, and what was caught in review is in the ✅ banner further down
> this brief.
>
> **Every line number in the sub-sections below is PRE-implementation and now wrong** —
> `dissolveGroupConfirmed()` is no longer at 823-877 (its body moved to `GroupDissolveService`),
> `submitForm()`'s cancel write is no longer at 349-350, and the `sendGroupDissolved()` insertion slot
> no longer applies because the method exists. **Read this section as the rationale record, not as
> instructions.**

**Ordering note.** This is commit 3 and the last of the three drift-stoppers. It is season-side only and
touches nothing commits 1, 1b or 2 touched, except `GroupInvitationsForm::dissolveGroupConfirmed()`,
which those commits did not go near. **CF11 (commit 8) consumes the service this commit extracts.**

##### 1. The service to extract

There is **no group service today** — group logic lives entirely in `GroupController` and
`GroupInvitationsForm`. Create one.

**File:** `src/Service/GroupDissolveService.php`
**Service id:** `ccsoccer.group_dissolve`
**Definition** (matches the house pattern — `team_manager`, `team_balancer`,
`tournament_team_manager` and `credit_manager` all take exactly these two; `waitlist_manager` and
`order_complete_subscriber` both take `@ccsoccer.notification` as a plain argument, so there is
precedent and no circularity):

```yaml
  ccsoccer.group_dissolve:
    class: Drupal\ccsoccer\Service\GroupDissolveService
    arguments: ['@entity_type.manager', '@logger.factory', '@ccsoccer.notification']
```

**Method:**

```php
/**
 * @return array
 *   ['cleared' => int, 'declined' => int, 'notified' => int]
 */
public function dissolveGroup(string $group_id, array $options = []): array
```

`$options` keys, all optional: `notify` (bool, default FALSE), `except_registration_id` (int|null —
the canceller's own row, already cleared by the caller), `manager_name` (string), `season_name`
(string), `reason` (string for the watchdog line).

**Extract from `GroupInvitationsForm::dissolveGroupConfirmed()`, which is now lines 823-877.** The
reusable core is **832-863** — the two storages, the clear loop, the decline loop and the log. The
split point is **line 865**: everything from there down (865-868 status message, 871-876 redirect) is
caller-specific and stays in the form.

##### 2. The invitation-status question — the one real design decision here

The three existing copies do **not** agree on which invitations they decline, and a naive extraction
will silently drop half the behaviour:

| Copy | Invitations targeted | Scoped by |
|---|---|---|
| `dissolveGroupConfirmed()` 847-857 | `status = 'pending'` | `group_id` only |
| `leaveGroup()` 2010-2023 | `status = 'accepted'` | email + season + group_id |
| `removeMember()` 1832-1863 | `status = 'accepted'` | email + team, or email + season + group_id |

Both statuses matter and for different reasons. A stale **pending** invitation reserves capacity in
`getGroupSize()` indefinitely (E5). A stale **accepted** invitation blocks re-invites — that is
Bug 3 / E4, the "already accepted" block that the July 20 checklist section G was written to prove
fixed.

**Decide: the service declines both.** Query `status IN ('pending','accepted')` scoped by `group_id`,
which is the union of all three behaviours and is correct for a group that is ceasing to exist. Record
it as a new decision **D-12** in §7. If you disagree, the alternative is an `$options['statuses']` key
— but note that every caller would then pass the same value, which is the signal that it should not be
a parameter.

**Use `Invitation::decline()`** (`Invitation.php:260-263` — sets `status` and `responded_at`, does not
save) rather than inlining the two `set()` calls. All three current copies inline them; the entity
method already exists and only `OrderCompleteSubscriber` uses it.

##### 3. What the service must NOT do

- **Must not touch `Registration.team`** — CF6's recorded decision. A friend group is not a roster.
- **Must not check `groups_locked`.** `dissolveGroupConfirmed()` deliberately does not (admin path),
  while `leaveGroup()` and `removeMember()` do. Leave the check to the callers, exactly as today.
- **Must not filter the registration query to live rows.** This is the CF3 conflict recorded above:
  line 836 is a *write* query and cancelled rows are precisely what needs clearing (class SB).
- **Must not redirect or set messages.** Those are the caller's.

##### 4. `NotificationService::sendGroupDissolved()`

**Insert after `sendPlayerRemoved()` ends at :1408, before `sendNudge()`'s docblock at :1410.**

```php
public function sendGroupDissolved(
  UserInterface $player,
  string $season_name,
  string $manager_name
): array
```

> ### 🔴 The SMS body is mandatory, not optional.
>
> `send()` is `send(UserInterface $user, string $subject, string $body, string $sms_body = '')`
> (:391). Since Caleb's Aug 2 rewrite, **an empty `$sms_body` means "send no SMS"** — at :412-425 a
> user whose preference is `text` or `both` gets nothing, and it is deliberately **not logged per
> user** so the bulk logs stay readable. Before `ab57c21` it derived one from the email body.
>
> Pass an explicit `$sms_body` or every text-preference member of a dissolved group is silently
> dropped. Keep it **under 153 characters** or it bills as two SMS parts.
>
> Suggested: `"CCSoccer - Your group for $season_name was dissolved because the organiser cancelled.
> You are still registered."` — 108 characters plus the season name.

**Do not reuse `sendPlayerRemoved()`** (:1372-1408). Its subject is literally *"CCSoccer - You have
been removed from {team}"*, which misdescribes what happened, and its season branch tells the player to
contact the group manager — the person who just left. Copy its *shape*, not its text. (Note its
`$context_type` local at :1379 is dead — do not carry that over.)

**Wording must say:** the group organiser cancelled their registration; the group has been dissolved;
**they are still registered for the season**; they are free to join or form another group. That third
clause is the one that prevents support email.

**Display names:** `NotificationService::getUserDisplayName()` (:2058-2074) is `protected`, so the
service cannot call it. Either pass `$manager_name` in as a string from the caller — which is what
`removeMember()` does at :1873-1874 and :1891-1892 — or widen the helper to public in this commit and
retire the four inline duplicates (`CancelRegistrationForm` :142-150 and :365-373,
`GroupController` :1782, :1874, :1892). **Recommend widening it**; it is the same drift class as
`isPlayerWoman()` on July 29.

**Preference `none` suppresses this entirely** (`send()` :399-403, open item S1). Not CF8's problem —
do not try to solve it here.

##### 5. `CancelRegistrationForm::submitForm()` — ordering

Method is **322-456**. The cancel write is **349-350**. `$registration`, `$season`, `$player` are all
in scope from :340 onward, so nothing needs re-loading.

**Insert the group cleanup between :350 and :352** — after the status save, before the waitlist block.
Rationale is the same as CF1's: keep it ahead of the money. The `commerce` arm's failure branch
(:437-441) does **not** return, so anything placed before the switch runs unconditionally regardless of
refund outcome.

**Order within the block:**

```
1. read $group_id and $invited_by from the registration BEFORE clearing them
2. clear the canceller's own three fields + save        (steps 1 and 3 of the change list)
3. if ($group_id && $was_manager):
     dissolveGroup($group_id, [
       'notify' => TRUE,
       'except_registration_id' => $registration->id(),
       'manager_name' => $player_name,
       'season_name' => $season->label(),
     ])
4. elseif ($group_id):  // ordinary member — nothing further, group survives
```

**Step 1 is the trap.** `$was_manager = $registration->get('invited_by')->isEmpty()` must be evaluated
*before* step 2 nulls it, or every canceller looks like a member and no group is ever dissolved. This is
the single most likely way to get CF8 wrong and it will not throw — it will just quietly do nothing.

`$player_name` is built at :365-373, *after* the insertion point. Either move that block above :352 or
call the widened `getUserDisplayName()`.

**Wiring:** the class injects only `entity_type.manager` and `ccsoccer.credit_manager` (constructor
:47-53, `create()` :58-63) and pulls everything else with `\Drupal::service()` inline —
`ccsoccer.waitlist_manager` :353, `ccsoccer.notification` :392/:407/:428. Either match that local style
or inject properly. **Recommend injecting**, matching what
`TournamentCancelRegistrationForm` now does for `tournamentTeamManager` after commit 1.

##### 6. Retire the copies

Once the service exists, repoint **`dissolveGroupConfirmed()`** (823-877) at it immediately — that is
the proof the extraction is behaviour-preserving, and it is the only caller that can be tested against
known-good current behaviour. `leaveGroup()` (2004-2023) and `removeMember()` (1819-1863) are
*not* pure dissolves (they clear one row and decline that person's invitations, rather than clearing the
whole group) so they are **not** direct substitutions — leave them, and note in the service's docblock
that a future `clearMembership()` sibling would absorb them.

##### 7. Test on LOCAL

- Member of a 3-person group cancels → their group fields cleared; other two unaffected; group shows 2;
  **no notifications sent**.
- **Manager of a 3-person group cancels** → all three cleared; **2 notifications, each with an SMS
  actually delivered** (check the SMS, not just the email — that is the Caleb-rewrite regression);
  pending **and accepted** invitations for the group `declined` with `responded_at`;
  **nobody's `Registration.team` changed** — check before and after.
- Manager of a group of one cancels → clean, no notifications, no errors.
- Cancel with an outstanding pending invitation → `declined`. Then **re-invite that same address** →
  succeeds (this is the E4/Bug-3 regression guard, and it is why accepted invitations are in scope).
- Admin **Dissolve group** button → still works, same result, via the extracted service.
- Tournament cancel → unchanged (that path is CF1, already shipped).
- A member with **preference `none`** in a dissolved group → no notification, no error, group still
  cleared.

**Run the notification test on LOCAL, not TEST.** `sendEmail()`'s gating (:455-486) now treats
local/dev as unrestricted because DDEV routes everything to Mailpit, so the emails are directly
observable. On TEST only board members and beta testers would receive them.

##### 8. Risks

- **Medium overall**, higher than CF1. The service extraction changes a path that currently works
  (admin Dissolve), and the notification depends on `send()` semantics that changed four days ago.
- **Caleb should review the notification half** — it is his rewrite that makes the explicit SMS body
  load-bearing.
- The `$was_manager` ordering trap above fails silently. Assert it explicitly in test.
- No transaction here either. A dissolve that throws mid-loop leaves some members cleared and some not.
  Lower stakes than `submitForm()`'s (the fields are idempotent — re-running the dissolve finishes the
  job) but worth a note in the service docblock.

> ### ✅ [Aug 4 evening] CF8 IMPLEMENTED — committed as `b22734f`, never executed.
>
> **What landed:**
>
> - **`src/Service/GroupDissolveService.php`** (new), service id `ccsoccer.group_dissolve`, extracted
>   from `dissolveGroupConfirmed()` exactly at the split point this brief specified.
> - **`NotificationService::sendGroupDissolved()`** with an explicit `$sms_body` — 103 characters fixed
>   plus the season name, with a season-name-free fallback if the total would exceed 153.
> - **`CancelRegistrationForm::submitForm()`** — cleanup inserted between the status save and the
>   waitlist block; the service is injected rather than pulled with `\Drupal::service()`.
> - **`dissolveGroupConfirmed()`** repointed at the service, proving the extraction round-trips.
> - **Decision D-12** recorded: the dissolve declines both `pending` and `accepted` invitations.
>
> **Deviations from this brief, deliberate:**
>
> - **`getUserDisplayName()` was NOT widened to public.** The brief recommended it plus retiring four
>   inline duplicates; that pulls `GroupController` into an untested commit. The existing inline block
>   in `submitForm()` was moved above the insertion point instead — zero behaviour change. The widening
>   is still worth doing separately.
> - **A bug was caught in review and fixed:** the first draft notified from the unfiltered registration
>   query, so a class-SB member (cancelled row still carrying the `group_id`) would have received
>   "you are still registered", which is false for them. Notification is now gated on live status;
>   clearing deliberately is not.
> - The admin Dissolve button's status message now says "outstanding invitation(s)" rather than
>   "pending", since D-12 widened what the count includes.
>
> **Still open, flagged not fixed:** `send()`'s docblock at :385-386 still reads "If not provided, body
> will be stripped", which is wrong since Caleb's Aug 2 rewrite and documents the exact trap CF8 nearly
> fell into. Left for Caleb — it is his rewrite.

**[VERIFIED Aug 4]** The premise holds exactly: `submitForm()`'s **entire** registration mutation is still
`set('status','cancelled')` + `save()` at **349-350**, and nothing in the method or in
`processCommerceRefund()` touches `group_id`, `invited_by`, `invitation_status` or `team`. For the record,
everything else `submitForm()` (322-456) does — none of which CF8 should disturb: resolves player/season
(339-346); **bumps `Season.reserved_spots` if the waitlist is non-empty** (352-364); builds the display
name (366-374); then one of three refund arms — `credit` (377-397, issues a cancellation credit and
notifies), `none` (399-410, notifies), `commerce` (412-442, refunds and notifies, **and sends no
notification at all when the refund fails**); audit log (445-452); redirect (455).

> **[Aug 4 evening] The remainder of the original July 28 CF8 text was DELETED here.**
>
> It duplicated the full implementation brief above with a second, conflicting set of line numbers and
> a three-argument `sendGroupDissolved()` signature the brief had already corrected. Two conflicting
> sets of coordinates in one section is exactly the drift this document exists to prevent, and it cost
> real time in the Aug 4 evening session before it was resolved.
>
> **CF8 is implemented (`b22734f`).** The authoritative record is the ✅ banner at the head of this
> brief. Its LOCAL test list (§7 above, and the consolidated one in §10.8) requires checking that the
> **SMS** actually delivered, not just the email, and that **accepted** invitations are declined
> alongside pending ones.

---

#### CF3 — Status filters on the season group queries

> ### 🔴 [Aug 4 evening] THIS BRIEF IS STALE. Re-verify every line number before using it.
>
> Nothing below was invalidated in substance, but **five files moved underneath it** during the Aug 4
> sessions — commits 1, 2, CF8 and CF4 all edited `GroupInvitationsForm`, `GroupController` and
> `RosterBuilderController`. Do a locate-by-method-name pass first, exactly like the Aug 4 morning
> verification did. Known drift:
>
> | Brief says | Actual, re-verified against the tree at `918653d` |
> |---|---|
> | `GroupInvitationsForm` 88 | **131** — the `buildForm()` group query |
> | `GroupInvitationsForm` 130 | **173** — the re-load inside the self-healing backfill |
> | `GroupInvitationsForm` **769** | **GONE from this file.** CF8 moved the dissolve's write query to `GroupDissolveService.php:138` |
> | `GroupController` 1943 (`getGroupSize()`) | **1922** — CF4 deleted ~30 lines above it |
> | `RosterBuilderController` 318 (`mergeToGroup()`) | method now starts at **278**; re-locate the query inside it |
>
> *(An earlier draft of this table gave 125-128 and 128 for the two `GroupInvitationsForm` rows and
> 135 for the service. All three were wrong. These are verified.)*
>
> **The exemption rule below still holds, but its target moved.** The query that must NOT be filtered
> now lives in `GroupDissolveService::dissolveGroup()`, where it is already documented as such in the
> class docblock: *"Must NOT filter the registration query to live rows... The CF3 status-filter work
> must EXEMPT this query."* So CF3 no longer has to remember to skip a line in a shared file — the
> query is in its own service with the reason written next to it. **That is a simplification: nine
> sites to filter, and the tenth is no longer in the way.**
>
> **New consideration from CF4.** `LiveRegistrationTrait::isDeadRegistrationStatus()` now exists and
> encodes D-4's dead pair (`cancelled`, `expired`) in one place, used by five classes plus
> `GroupDissolveService`. **CF3's shared helper should build on that rather than introduce a second
> definition of "live"** — otherwise CF3 reintroduces exactly the duplication CF4 just removed. The
> natural shape is a query-level sibling on the same trait, e.g. `applyLiveRegistrationFilter($query)`,
> so the row-level and query-level tests cannot drift apart.

**Files:** `Controller/GroupController.php` (`getGroupSize()` 1943, `myRegistrations()` 145 and 253,
`manage()` 475, `invitationHasMatchingRegistration()` 836 **and its team-invite branch 827-830**),
`Controller/RosterBuilderController.php` (`mergeToGroup()` 318), `Form/GroupInvitationsForm.php`
(88, 130, 769) — **ten sites.** *(Line numbers as of July 28 — see the staleness banner above.)*

**Change:** exclude `cancelled` and `expired` (decision D-4 — exclude-list, not allowlist).

**[VERIFIED Aug 4 morning]** All ten sites still existed, still lacked a status filter, and were still at
exactly the stated lines. But three corrections:

> ### 🔴 [NEW Aug 4] `GroupInvitationsForm:769` must be EXEMPTED, not filtered — it conflicts with CF8.
>
> Site 10 is not a read. It is the **write** query inside `dissolveGroupConfirmed()` — the loop that
> clears `group_id` / `invited_by` / `invitation_status` on every row in the group. Applying the live
> filter there means the dissolve **stops cleaning cancelled rows**, which are precisely the rows class
> **SB** identifies as the cause of inflated group counts and blocked re-invites. Filtering it would make
> CF3 undo CF8.
>
> **Nine sites to filter, one to exempt.** State the rule explicitly in the shared helper's docblock:
> *read-side queries filter to live rows; write-side cleanup queries load everything.*

- **[CORRECTED Aug 4] Site 6 (`GroupController` 827-830) is not a season group query.** It is the
  team-invite branch of `invitationHasMatchingRegistration()` and queries `tournament` + `team`. Real,
  still unfiltered, still worth fixing — but it lands in tournament territory, so it interacts with
  commits 1, 2 and 6 rather than with the season group work. The §5 row's blanket "season group queries"
  label is wrong for this one.
- **[NEW Aug 4] Sites 8 and 9 are not season-only either.** `GroupInvitationsForm` serves both flows and
  does not determine context until :144, so a filter there changes the **tournament** admin page too.
  That file is already being rewritten by commit 2 — sequence accordingly or expect a conflict.
- **[NEW Aug 4] Two already-filtered group counts will contradict D-4 the moment this helper lands**, and
  neither is mentioned anywhere in this document: `OrderCompleteSubscriber:508-515` (the post-payment
  group capacity guard, `status IN ('paid','active')`) and `TeamBalancerService:1383-1388`
  (`syncGroupToRoster()`, `status = 'paid'`). Both are **allowlists**. If CF3's goal is one canonical
  definition, either convert them in this commit or record an explicit carve-out — otherwise the module
  ends up with a documented exclude-list helper sitting beside two undocumented allowlists, which is the
  same drift in a new place.

**Implementation note that will bite you:** most of these use `loadByProperties()`, which cannot
express `NOT IN`. Each site must either switch to `getQuery()->condition('status',
['cancelled','expired'], 'NOT IN')` or post-filter the loaded array. Put the filter in one shared
helper (a small trait is enough) so the sites can't drift again.

**Test on LOCAL:** put a cancelled member in a group, then confirm the count drops by one on the
Manage Group page, in "spots left", in the invite cap, on the admin group-invitations page, and in
the roster builder's group-size warning — and that an invite that previously failed with "Group is at
maximum size" now succeeds. Confirm a live member still counts everywhere.

---

#### CF4 — Replace bare `reset()` with `pickLiveRegistration()`

> ### ✅ [Aug 4, later] PARTIALLY IMPLEMENTED — write and decision paths only. Read this before continuing.
>
> **The audit-scoping question below is now moot for the sites that landed.** Andrew reproduced the
> bug on LOCAL against a live tournament, so the "schedule it as hardening if TM and SJ are 0" advice
> no longer applies to the write paths — they were shipping a confirmed data-corrupting bug.
>
> **Reproduction (Andrew, Aug 4).** Tournament *Andrew 20 teams*, team *Mac Attack*. Player registers,
> is invited, accepts; admin cancels the registration; player registers again; captain re-invites;
> player accepts. The accept resolved to the **cancelled** registration, appended the player to
> `Team.players`, and wrote the team linkage onto the dead row. Result: the captain's Manage Group and
> the Tournament Teams page showed the player on the team, while My Registrations and the Roster
> Builder showed them unassigned — and the player saw *"You have joined Mac Attack"* and
> *"This registration has been cancelled"* on the same screen.
>
> **CF1 did not cause this, but it unmasked it.** Before CF1, cancelling left `Registration.team` set,
> so `acceptTeamInvitationDirectly()`'s "already on a team?" guard fired and blocked the write.
> CF1 correctly clears `team`, which removed the guard that was accidentally masking `reset()`.
> **This is why the CF4 write-path slice had to ship with CF1 rather than after it.**
>
> **What landed:**
>
> - **`src/LiveRegistrationTrait.php`** (new) — `pickLiveRegistration()`, matching the
>   `DateRangeFormatTrait` house pattern (trait directly in `src/`, namespace `Drupal\ccsoccer`).
> - **Both existing private copies deleted** and repointed at the trait: `GroupController` (written for
>   the D14 nudge fix) and `GroupInvitationsForm` (added by CF2, its `@todo` pointed here). The inline
>   `foreach` in `nudge()` now calls the trait too. **There is now exactly one copy of this logic.**
> - **Four accept write paths:** `RegistrationController` season + tournament,
>   `GroupController::acceptSeasonInvitation()` + `acceptTeamInvitation()`. Note the site has **two
>   Accept buttons routing through different controllers** — My Registrations goes to
>   `GroupController`, the Register page's invite-token flow goes to `RegistrationController`. Both
>   were broken; fixing one would have left the bug reproducible via the other.
> - **Two `invite()` duplicate-check guards** in `GroupController` (tournament + season) — these are
>   the guards that should have caught the case above.
> - **`GroupInvitationsForm::determineContext()`** — decided the whole admin page's season-vs-tournament
>   context from the oldest row. Falls back to `reset()` when every row is cancelled, since the page
>   must still render; the member list itself stays unfiltered on purpose.
> - **`CreditManagerService::issueRainoutCredit()`** and **`TournamentTeamManager::getPlayerRegistration()`**.
>
> - **`GroupController::deleteInvitation()`** — the literal twin of `nudge()`'s redirect 50 lines
>   above, missed by the original sweep.
> - **All nine roster-builder sites** — `RosterBuilderController` 106, 177, 307, 520, 521 and
>   `TournamentRosterBuilderController` 118, 276, 453, 454. **These were mis-filed as display sites in
>   the list below; they are not.** `RosterBuilderController` 106/177 do `set('team', …)` + `save()`,
>   and `TournamentRosterBuilderController` 118 gates on `is_captain` before writing. A drag that
>   resolved to a cancelled row wrote the team assignment onto the dead row — the drag appeared to
>   succeed and the player stayed in the workbench. Same bug class, in the tool the board is about to
>   use to build fall rosters, so they shipped here rather than waiting on the audit.
>
> **Behaviour change to know about:** the trait returns **NULL** where `reset()` returned a dead
> row. Every touched call site handles NULL explicitly — the accept paths now fall through to
> "register first", which is the correct answer for a player holding only dead rows.
> `getPlayerRegistration()` is public; its single caller `getPlayerTeam()` already handled NULL.
>
> **`expired` is dead too — see decision D-13.** The first draft of the trait skipped only
> `cancelled`, which would have left the identical split-brain bug reachable via an expired row.
>
> **Nothing further is outstanding for CF4** beyond four sites confirmed safe: `TournamentController`
> 818, `TeamBalancerService` 281 and `SchedulePdfService` 142 all carry status conditions in the query
> itself, and `Team.php:557` is an unfiltered but read-only skill-average fallback — worth a comment,
> not a fix.
>
> **⚠ CF3's site list is now stale — re-verify before starting it.** Commits 1, 2 and CF8 all edited
> `GroupInvitationsForm`. The brief's CF3 sites in that file were 88, 130 and 769: **130 is now 128**,
> and **769 no longer exists in that file at all** — CF8 moved the dissolve's write query into
> `GroupDissolveService.php`. The "must EXEMPT `GroupInvitationsForm:769` or it undoes CF8" warning
> below still holds in substance, but the query now lives in the service and is documented there.

**Scope this from the audit.** TM = SJ = 0 → do the roster-builder sites only, as hardening.
Non-zero → sweep all of them.

**Change:** move `pickLiveRegistration()` off `GroupController` (it is `protected` at **:1927**, so no
other class can reach it) into a trait, and use it at the sites below.

**[CORRECTED Aug 4 — the original list was wrong in both directions. Use this one.]**

**Fix these (17 verified bare `reset()` calls on unfiltered registration collections):**

| File | Lines |
|---|---|
| `RosterBuilderController` | 106, 177, 307, 520, 521 |
| `TournamentRosterBuilderController` | 118, 276, 453, 454 |
| `GroupInvitationsForm` | 1189, 1224 |
| `GroupController` | 997, 1016, 1377, 1442, 1536 |
| `TournamentTeamManager` | 308 (`getPlayerRegistration()` — public, check its callers) |

**Remove these two from the list — they are not bugs:**

- **`TeamBalancerService` 281** — its query at **:273-278** already carries `'status' => 'paid'`. The
  cancelled-row bug cannot fire here. The only residual exposure is the class-SH "two live rows" case.
- **`GroupController` 1315-1323** — **not a `reset()` at all.** It is the already-correct inline copy of
  the helper written for the D14 nudge fix (a `foreach` skipping `cancelled`, keeping the highest id).
  It belongs under *de-duplicate into the trait*, not under *fix the bug*. §5's row omitted it entirely
  while §10.3 listed it as a bug site — the two sections disagreed with each other.

**Add these four — all missed by the original sweep, and two of them are write paths:**

| Site | Line | Why it matters |
|---|---|---|
| `RegistrationController` | **109** | `acceptSeasonInvitationDirectly()`. Query at 102-105 is `player` + `season`, no status. A returning player who cancelled once has their invitation **accepted onto the cancelled row**. Highest-impact of the four — player-facing write, not display |
| `RegistrationController` | **134** | Tournament twin of the above; query 126-129, no status |
| `GroupInvitationsForm` | **144** | `determineContext(reset($registrations))` over the unfiltered load at 88/130. **The entire admin page's season-vs-tournament context is decided from the oldest row in the group**, which may be cancelled. Simultaneously a CF3 and CF4 site, in a file commits 2 and 4 already open — cheapest possible addition |
| `CreditManagerService` | **173** | `issueRainoutCredit()`; query 169-172, no status. Links the credit to a possibly-cancelled registration id. Low harm (reporting), same shape |

**Do not touch (already status-filtered — useful as precedent for the helper's shape):**
`TournamentController:818` and `SchedulePdfService:142` (allowlists);
`RegistrationController:541, 812, 971, 1062` (**exclude-lists — these already match D-4**).
**Out of scope, dev-seed only:** `CcsoccerCommands.php:1444, 1488, 1559`.
**Judgment call, probably leave:** `CompletionPane.php:64` — `reset()` over `commerce_order` ids, scoped
to one order rather than player+season, so `pickLiveRegistration()` is not the right fix. Worth a comment.

**[Aug 4] Re-scope note.** With 21 real sites rather than "roughly fifteen", and with two of them being
player-facing write paths, the original advice ("scope to the two roster-builder sites as hardening if
TM and SJ are 0") may no longer be right. **`RegistrationController:109` and `:134` arguably belong in
the small scope regardless of the audit numbers**, because they write.

**Test on LOCAL:** create a player with a cancelled *and* a live registration for the same season,
then exercise each touched surface and confirm it resolves to the live row — the roster builder drag
lands, the manage page's Remove button works, the nudge redirect goes to the right page, **and an
invitation accepted via `RegistrationController` lands on the live registration.**

---

#### CF5 — `move()` must reconcile against every team in the tournament

> ### ✅ [Aug 4, later] IMPLEMENTED — and the brief undercounted the sites by two.
>
> **Three call sites, not one.** The brief scoped CF5 to `move()`. The identical
> read-`reg.team` → load-that-one-team → remove block appears **three times** in
> `TournamentRosterBuilderController`, with the same blind spot in each:
>
> | Method | What it does | Player reconciled |
> |---|---|---|
> | `move()` | drag / multi-drag between teams and the workbench | each dragged player |
> | `mergeToGroup()` | Shift+drag a player into a captain's group | the merged player |
> | `createGroup()` | pair a manager with a member, moving the manager to the member's team | the manager |
>
> All three now call `removePlayerFromAllTeamsInTournament()`. Leaving the latter two would have kept
> two admin write paths generating exactly the drift CF5 exists to stop.
>
> **Two behaviours the brief did not claim, both improvements:**
>
> - **Move-to-pool now cleans properly.** `$team_id` is NULL for a workbench/pool move, and a NULL
>   `$except_team_id` excludes nothing — so the player is cleared off *every* team. The old block could
>   only ever clear the one `reg.team` named.
> - **The `!= $team_id` guard is subsumed** by passing the destination as the exclusion, so no separate
>   same-team check is needed.
>
> **Captain safety verified.** All three sites are behind an `is_captain` guard that rejects the
> operation before this code runs, and `is_captain` is TRUE for co-captains too, so `$allow_captain`
> stays FALSE and a team leader can never reach the reconcile. This was worth checking — CF1 made
> `removePlayerFromTeam()` refuse leaders, so a site that *could* reach it with a co-captain would have
> silently stopped cleaning.
>
> **Not a CF5 site:** `GroupInvitationsForm:1048` also calls `removePlayerFromTeam()`, but it removes
> from a *specific named* team (the one being declined), not from "wherever `reg.team` says" — the
> semantics are different and correct as written.
>
> **Cost accepted:** the helper loads the tournament's teams once per player, inside `move()`'s
> multi-drag loop. ~20 teams and a handful of players per drag; correctness beats the queries. Hoist
> the team list if a drag ever feels slow.

**File:** `Controller/TournamentRosterBuilderController.php` (`move()`, `reg.team` read at 127,
`removePlayerFromTeam()` call at 131)

**Change:** don't trust `reg.team` to name the old team. Call the
`removePlayerFromAllTeamsInTournament()` helper written for CF1, excluding the destination team.

**[VERIFIED Aug 4]** Both line references exact; the flaw is live — the guard at :128 short-circuits on a
NULL or 0 `$old_team_id` and nothing else in `move()` (101-186) touches any other team. Two additions:

- **The flaw is broader than "the ghost has no `reg.team`".** Any *wrong* `reg.team` — pointing at C
  while the uid sits in A's `players` — leaves A dirty too. Reconciling against all teams covers both.
- **The destination side is already safe.** :139 calls `addPlayerToTeam($team, $player_id, TRUE)`, which
  dedupes at :133. So the only observable symptom is the stale **source** team.
- **CF5 is hard-blocked on CF1** — the helper does not exist in the tree yet. Note also that :118 is one
  of CF4's bare `reset()` sites, ten lines above the code CF5 edits.

---

#### CF9 — Route the inline `Team.players` appends through the manager

> ### 🔎 [Aug 4 evening] RESEARCHED, NOT IMPLEMENTED. Read this before starting.
>
> **CF9 is the highest-value item remaining, and bigger than this brief implies.**
>
> **Why it matters more than the brief says.** The three inline appends do not dedupe;
> `addPlayerToTeam()` does, via `isPlayerOnTeam()`. The Aug 4 reproduction appended a player to
> `Team.players` through one of these very sites — a second pass would have put them on the roster
> **twice**, which is audit class **TE**. CF4 stopped the wrong-row selection; **nothing currently
> stops a duplicate append.**
>
> **Three findings this brief does not contain:**
>
> 1. **None of the three sites can call the manager today.** `TournamentTeamManager` is not injected
>    into `GroupController`, `RegistrationController`, or `OrderCompleteSubscriber`. CF9 is therefore
>    **three constructor changes plus a `ccsoccer.services.yml` edit** for the subscriber — not three
>    block swaps. That means another mandatory `drush cr`.
> 2. **Double-save interaction.** At all three sites the append and a possible `set('group_id', …)`
>    currently share a single `$team->save()`. `addPlayerToTeam()` saves internally, so a naive swap
>    gives two saves per accept. On `OrderCompleteSubscriber` the `getOrCreateTeamGroupId()` helper
>    **also** saves, so that path could reach three. Restructure rather than let it accrete.
> 3. **The `:336` citation below is wrong.** Line 336 of `OrderCompleteSubscriber` is credit-shortfall
>    reconciliation. `processAction()` is in `TournamentTeamPane`, ~:317. The substance holds; the
>    pointer does not.
>
> **The dead branch really is dead — verified end to end, not taken on trust.**
> `TournamentTeamPane::buildActionOptions()` only ever emits `join:<tid>`, `create`, `none` and
> `ccsoccer_pool` — **no bare `join`** — and `submitPaneForm()` intercepts `join:<tid>` and rewrites it
> to `token_accept` before it is persisted. Nothing in the repo can produce `action = 'join'`.
> **Safe to delete.**
>
> **Line numbers have drifted ~10-15** since this brief was written (CF12 added to
> `createTournamentRegistration()`, CF4 added to both controllers):
>
> | Brief says | Actual (Aug 4 evening) |
> |---|---|
> | `OrderCompleteSubscriber` 773 (token_accept append) | **:783**; its `isFull()` guard **:776** |
> | `OrderCompleteSubscriber` 821 (dead branch append) | **:836**; branch runs **:816-838**; `isFullIncludingPending()` **:829** |
> | `GroupController` 1549 | **:1559** |
> | `RegistrationController` 372 | **:390** |
>
> **✅ DECIDED (Andrew, Aug 4) — CF9 is its own commit, 6b.** Recorded as **D-14** in §7. CF5 shipped
> alone as `7c9a7c5`. Do not fold CF9 in with anything else: it is three constructor changes, a
> `ccsoccer.services.yml` edit, a double-save restructure and a branch deletion, and it should be
> revertable on its own.
>
> **Suggested shape for commit 6b**, in this order so each step is separately checkable:
> 1. Inject `TournamentTeamManager` into the three classes (+ the services.yml arg for the subscriber).
>    **This alone needs `drush cr`.**
> 2. Swap the three appends to `addPlayerToTeam($team, $uid, TRUE)` — `force = TRUE` preserves the
>    existing `isFull()` / `isFullIncludingPending()` guards that drive the
>    `ccsoccer_team_capacity_exceeded` order flag. Letting the manager do the capacity check instead
>    would silently change the flagging semantics.
> 3. Restructure the group_id save so the append and the `set('group_id', …)` do not double-save.
> 4. Delete the dead `action = 'join'` branch (`OrderCompleteSubscriber` :816-838) and add the docblock
>    note on `createTournamentRegistration()` recording that invited joins arrive as `token_accept`.
>
> **One thing to check before step 4**, if you want certainty about pre-June-23 orders:
> `SELECT id FROM commerce_order WHERE state != 'completed' AND placed < UNIX_TIMESTAMP('2026-06-23');`

**Files:** `EventSubscriber/OrderCompleteSubscriber.php` 773 (~~821~~ — see below),
`Controller/GroupController.php` 1549, `Controller/RegistrationController.php` 372

**Change:** replace the read-append-set blocks with
`TournamentTeamManager::addPlayerToTeam($team, $uid, TRUE)`, which already dedupes.

**⚠ Scope correction, July 30 — three live sites, not four.** `OrderCompleteSubscriber:821` sits inside
`elseif ($action === 'join')` (800-838), which has been **unreachable since June 23**. `8483a20` rewrote
`TournamentTeamPane::submitPaneForm()` to encode an invited join as `join:<team_id>` and then emit
`'action' => 'token_accept'` (:277-284, with a comment saying the downstream branch is reused verbatim);
`processAction()` never sees a `join` and explicitly documents that (:336). A repo-wide grep finds **no
producer of `action = 'join'` anywhere**, including drush and tests.

**Recommend deleting the branch rather than refactoring it.** Refactoring dead code through the manager
adds a call site nothing exercises, and the branch is a decoy for the next reader — it is the only other
place that sets a tournament `group_id`, so anyone tracing CF12 lands in it. Delete it in this commit with
a docblock note on `createTournamentRegistration()` recording that invited joins arrive as `token_accept`.

**One caveat before deleting:** an order created before June 23 and completed afterward would still carry
`action = 'join'` in its order data. Vanishingly unlikely now (five weeks and a live tournament later), but
it argues for deleting deliberately rather than letting it rot. If you want certainty, check first:
`SELECT id FROM commerce_order WHERE state != 'completed' AND placed < UNIX_TIMESTAMP('2026-06-23');`

**Careful:** OrderCompleteSubscriber's existing `isFull()` / `isFullIncludingPending()` guards at 765
and 813 must stay exactly where they are — they drive the `ccsoccer_team_capacity_exceeded` order
flag. Passing `force = TRUE` preserves that; letting `addPlayerToTeam()` do the capacity check
instead would silently change the flagging semantics.

**Out of scope:** `OrderCompleteSubscriber:707` is a team *create* seeded with its captain, not an
append. `CcsoccerCommands.php:1501` is dev-seed only.

**Test:** token-accept and join checkout paths still add the player exactly once; replaying doesn't
duplicate.

---

#### CF7 — Deleting a team must null the registrations pointing at it

> ### ⬜ NOT STARTED — but part of its ⚠ box below is already done.
>
> The box asks the reader to *"decide in this commit whether to delete the two dead functions"*
> (`ccsoccer_registration_delete()` / `_presave()`). **`e5317f8` already deleted them** —
> `ccsoccer.module` now carries a comment recording it. That decision is made; do not re-litigate it.
>
> The rest of the brief stands, and its line references were **spot-checked Aug 4 evening and are
> still correct**: `clearTeamsForSeason()` at 228-238 with the delete at :234, no `hook_team_predelete`
> anywhere in the module, and `ccsoccer_team_predelete` is the right hook name.

**Files:** `ccsoccer.module` (new hook), `Service/TeamManagerService.php` (`clearTeamsForSeason()` 228-234)

**Change:** add `ccsoccer_team_predelete(EntityInterface $entity)` — query registrations with
`team = $entity->id()`, NULL the field, save. `clearTeamsForSeason()` gets this for free once the hook
exists, since it deletes through storage.

**[VERIFIED Aug 4]** Accurate throughout. No `predelete` hook of any kind exists in
`web/modules/custom`. `clearTeamsForSeason()` is at 228-238 with the storage delete at :234, so it does
inherit the hook. **The proposed name is correct** — the Team entity's id is `team` (`Team.php` annotation),
so `ccsoccer_team_predelete()` is the right spelling of `hook_ENTITY_TYPE_predelete`.

> ### ⚠ [NEW Aug 4] There is a misnamed, never-firing hook twelve lines away. Do not copy its pattern.
>
> `ccsoccer.module:607` defines `ccsoccer_registration_delete()`. The Registration entity's id is
> **`ccsoccer_registration`**, not `registration`, so the correct name would be
> `ccsoccer_ccsoccer_registration_delete()`. As written it implements the hook for an entity type that
> does not exist, and **has therefore never run.**
>
> That is lucky, because its body calls `\Drupal::service('ccsoccer.registration')` — **there is no such
> service** in `ccsoccer.services.yml` — and then `cancelRegistration()`, a method that exists nowhere in
> the module. Were the name ever corrected, every registration delete would throw
> `ServiceNotFoundException`. `ccsoccer_registration_presave()` (:595) is misnamed the same way; it is an
> empty TODO stub, so it costs nothing.
>
> **Consequence for CF7:** a hook in this file silently not firing is a demonstrated failure mode here,
> not a hypothetical. **Add a watchdog notice on hook entry and assert it in the test** — "delete a team,
> check the registrations" would pass identically if the hook never ran and the registrations happened to
> have no team. Also worth deciding in this commit whether to delete the two dead functions or fix their
> names; deleting is safer, since fixing `_delete` activates a call to a service that does not exist.

**Test:** delete a season team that has players assigned → their `Registration.team` goes NULL and
they appear in the roster-builder workbench rather than vanishing from the board.
**[Aug 4]** Confirm from watchdog that the hook actually fired — see the box above.

**Risk:** low — teams are essentially never deleted here. Make it defensive (`accessCheck(FALSE)`,
no exceptions escaping a delete hook).

---

#### CF6 — resolved, no code

Season: `dissolveGroupConfirmed()` leaving `Registration.team` intact is **correct** — the friend
group is not the roster. Tournament: dissolving a team's group leaves members on `Team.players` and
`Registration.team`, which is benign (they stay on the team, they just lose the invitation linkage).
No change. Recorded so a future review doesn't re-flag it.

---

#### CF10 — a group must not be created by a *failed* invitation

> ### ✅ IMPLEMENTED — `995e286`, August 5. Linted, not yet click-tested.
>
> **Implemented from `GROUP_CLEANUP_BRIEF.md` §5, NOT from this brief.** Read this section as the
> rationale record only; the line numbers below are stale and §5 of that document is authoritative.
>
> D-8 was decided as **CF10 + B + E** (§7), which reinstated CF10 after option D had briefly superseded
> it — it is the only thing that closes route 1, which B cannot reach.
>
> **What actually shipped, and the one thing this brief did not know about:** §5 carried a correction
> this section never had. `getGroupSize()` reads registrations *from the database*, so with the save
> moved below the checks a brand-new group returns **0** where it used to return **1**. The shipped code
> therefore counts the inviter explicitly (`$group_is_new ? 1 : $this->getGroupSize(...)`). Without that
> line the group cap silently tightens by one at the boundary — a change nobody would trace back here.
>
> Also shipped in the same commit: the three stray paste-artifact comment blocks inside `invite()` are
> deleted.
>
> **Still to do:** `GROUP_CLEANUP_BRIEF.md` §9.2 cases 1-4 (the three failed-invite paths leave
> `group_id` NULL; the happy path is unchanged and reuses an existing group_id).
>
> ### 🔴 Line numbers below are STALE. Re-locate by method name first.
>
> CF4 edited `GroupController` heavily (deleted a ~30-line method, added trait calls at six sites), so
> every citation in this brief and CF11's has moved. Verified against `918653d`:
>
> | Brief says | Actual |
> |---|---|
> | `invite()` write block **929-933/935**, `save()` on **934** | block **931-937**, `save()` **936**; `invite()` starts at **864** |
> | *"stray leftover instruction comment"* at **963-973** | **965-975** |
> | `manage()` gates on `if (!$group_id)` at **~598** | **602** |
> | `leaveGroup()` **1977-1981** | method **1947**, the manager refusal at **1958** |
> | `mergeToGroup()` **310**, `createGroup()` **524/531** | methods at **278** and **473** (`RosterBuilderController`) |
>
> The reasoning and the fix are unaffected — only the coordinates.

**File:** `Controller/GroupController.php` (`invite()`, season branch)

**[CORRECTED Aug 4] The block is 929-935 and the `save()` this whole brief is about is on line 934** —
one line *below* the range quoted here. Everything else verified exact: the tournament branch at :902,
the reconciliation write at :910-912, the `groups_locked` gate at :881-887, and every check below the
save returning without unwinding it.

**Today** (`invite()` ~929-935): the season branch mints the `group_id` and saves the registration
immediately, before the invitee has been resolved or validated:

```php
$group_id = $registration->get('group_id')->value;
if (!$group_id) {
  $group_id = $this->uuid->generate();
  $registration->set('group_id', $group_id);
  $registration->save();          // ← committed before any invitee check
}
```

Every downstream check — "please select a player or enter an email" (~955), "already a member of your
group", "already in another group for this season" (~1008-1030), the duplicate-invitation filter
(~1035-1075) — runs *after* that save and `return`s on failure. So a player who types one bad address
walks away holding a `group_id` with no invitation behind it, and that state is a **one-way door**:

- `invite()`'s own Check 1 now blocks anyone from inviting them ("already in another group").
- `RosterBuilderController::mergeToGroup()` (310) and `createGroup()` (524, 531) refuse to drag them
  into a group.
- `leaveGroup()` (1977-1981) refuses them by design: *"As the group manager, you cannot leave your own
  group."*
- `manage()` gates `$pending_invitations_to_me` on `if (!$group_id)` (~598), so they cannot even *see*
  an invitation that predates their accidental group.

Only an admin Dissolve Group clears it.

**Change:** the season branch needs `$group_id` as a *value* for the checks below it (Check 1 compares
the invitee's group to it, Check 2 keys the duplicate lookup on it), so keep generating it — just
don't persist it, and don't set it on the entity, until the invitation is about to be created.

1. Generate the UUID into a local **without** `set()`/`save()`, and track whether it is new
   (`$group_is_new = TRUE`).
2. Leave the `$current_size` / `$max_group_size` check as-is — `getGroupSize()` on a brand-new UUID
   returns 0, which is correct for a group that doesn't exist yet. **[Aug 4]** Note this is a **fifth**
   early return sitting between the save and the invitee checks (**941-944**); §10.3's prose only
   enumerates the ones from ~955 down.
3. Immediately before the invitation entity is created (after every check has passed), if
   `$group_is_new`, do the `set('group_id', …)` + `save()`.
4. Order the two saves so the invitation is created **after** the registration save succeeds — a
   pending invitation pointing at a `group_id` no registration holds is the IA/SC shape.

**Tournaments are already correct and must not be touched.** There the `group_id` comes from the Team
entity (`$team->getGroupId()`, ~902), the team is the real container, and the registration write at
910-912 is reconciliation with the team rather than group creation.

**Test on LOCAL:**

- Registered player with no group sends an invite to an email **already in another group** → error as
  today, and their own registration still has `group_id` NULL. *(This is the whole point of the fix.)*
- Same player, invite with an empty search box → error, `group_id` still NULL.
- Same player, valid invite → `group_id` set exactly as today, invitation created, invitee sees it.
- Second valid invite from the same manager → reuses the existing `group_id`, does not mint a new one.
- Invitee accepts → group of 2, manager badge on the right row.
- Group at max size → still blocked, and a player with no group who hits that error keeps
  `group_id` NULL.
- Tournament captain invites → unchanged, `Team.group_id` still authoritative.

**Risk:** low, but re-read the method end-to-end after moving the block — anything between the old and
new positions that reads `$registration->get('group_id')` from the *entity* rather than the local
`$group_id` would change behaviour. As of July 29 nothing does. **Re-confirmed Aug 4.**

**[NEW Aug 4] Housekeeping in the same commit:** lines **963-973** are a stray leftover instruction
comment — *"UPDATED CODE for GroupController.php — invite() method"*, referencing long-dead line numbers
around 583-605 — sitting **inside** `invite()` between 960 and 975, i.e. squarely in the region CF10
edits. Delete it while you are in there.

---

#### CF11 — a solo manager must be able to disband their own group

> ### ⬜ NOT STARTED — and now **OPTIONAL. Do not build it yet.**
>
> **[Aug 5] D-8 was decided as CF10 + B + E** (§7), and **under B route 4 clears itself** — the last
> member leaving, being removed, or being cancelled now dissolves the group automatically. **The Disband
> button is therefore no longer the fix for anything.** It survives only as insurance against a missed
> call site: if one of `GROUP_CLEANUP_BRIEF.md`'s thirteen sites is wrong, the button lets the player
> unstick themselves instead of emailing an admin.
>
> **Decide after four weeks of `GROUP_CLEANUP_BRIEF.md` §9.4 query B** (`GROUP_CLEANUP_BRIEF.md` §10,
> Q-B3). If that count settles at zero, this button would be dead code guarding a state that no longer
> occurs. **If it is built, use `GROUP_ID_AT_ACCEPT_BRIEF.md` §9, which is more current than this brief.**
>
> **Two corrections to the text below, both material:**
>
> - **`notify` should be `FALSE`, not `TRUE`.** By definition the caller is the group's only live member,
>   so there is nobody but the clicker to notify.
> - **It still consumes CF8's `GroupDissolveService`, which is committed but never executed** — and
>   after the D-8 package that service has fourteen callers rather than one. Test it via §10.8 first.
>
> **D-7 still stands: solo only**, not a general manager-dissolve.
>
> **Line numbers here are stale for the same reason as CF10's** — see that brief's table. The
> `GroupDissolveService` this brief anticipates now exists at `src/Service/GroupDissolveService.php`
> with method `dissolveGroup(string $group_id, array $options = [])`.
>
> This is the only item in the document currently generating admin email tickets, so it is a
> reasonable thing to promote once the testing backlog is cleared.

**Files:** `Controller/GroupController.php` (new `disbandGroup()`), `ccsoccer.routing.yml`,
`templates/ccsoccer-group-manage.html.twig`, plus the dissolve service extracted for CF8

**The scenario (Andrew, July 29).** Three players want to be together. P1 invites P2 → P1 is now a
manager. P3 also invites P2 → P3 is now a manager. P2 can only accept one. The loser is a manager of a
group containing only themselves, and:

- P1 cannot invite P3, because P3 has a `group_id` (Check 1).
- P3 cannot join P1, because `leaveGroup()` refuses managers.
- P3 cannot dissolve, because dissolve is admin-only (`GroupInvitationsForm`, permission
  `generate rosters`).

**So the only resolution is an email to the admin.** That is the bug — not a data problem, a missing
player-facing action.

**Four routes into the stranded state**, all confirmed in code — worth knowing, because CF10 only
closes the first:

| # | How | Where |
|---|---|---|
| 1 | Invite fails validation; `group_id` was already saved | `invite()` 929-933 — **closed by CF10** |
| 2 | Invite sent, invitee declines or accepts a rival group | nothing clears the inviter's `group_id` |
| 3 | Invite sent, never answered; manager deletes it | `deleteInvitation()` 1353 hard-deletes the invitation, never touches `group_id` |
| 4 | Group had members, they all left or were removed | `leaveGroup()` 2003-2006 and `removeMember()` 1818-1822 clear only the *departing* row |

**Recommended scope: "only live member" — not a general manager-dissolve.**

Allow it when the group's live-member count is exactly 1 (the caller). If other members are present,
keep today's behaviour: remove them first via `removeMember()`, then disband. Rationale — a
manager who dissolves a populated group silently strips 2-3 other people out of the group they chose,
which is precisely the notification-bearing event D-2 built `sendGroupDissolved()` for. Restricting
CF11 to the empty case means **no notification is needed at all** (there is nobody to notify), which
keeps it a genuinely small change. If a full manager-dissolve is wanted later it is the same service
call plus the CF8 notification loop.

**Change:**

1. **Reuse CF8's extracted service.** CF8 (D-2) already has to pull `dissolveGroupConfirmed()`'s body
   out of `GroupInvitationsForm` into something both the admin form and the season cancel path call.
   `disbandGroup()` is the third caller. **Do not write a fourth copy of "clear three fields, decline
   the pending invitations"** — four near-identical copies of this already exist.
   **[CORRECTED Aug 4] The fourth copy was misattributed.** Three are right:
   `dissolveGroupConfirmed()` **768-790**, `leaveGroup()` **2003-2022**, `removeMember()` **1818-1862**
   (declines in two arms — tournament 1832-1846, season 1847-1863). But
   **`OrderCompleteSubscriber` has no group-clearing branch** — it never clears `group_id`,
   `invited_by` or `invitation_status` on a registration; every occurrence there reads, assigns a local,
   or writes into a `create()` array. Its nearest analog,
   `declinePendingInvitationsForRegistration()` (976-1021), is decline-only and calls
   `Invitation::decline()` rather than the inline set-status-plus-`responded_at` pair, so it is not the
   same shape either. **The honest fourth copy is `GroupInvitationsForm::submitForm()` 919-929** — the
   admin per-row decline, which clears `group_id` (924), `invited_by` (925) and, for tournaments, `team`
   (927). That is the **CF2 site**, so commits 2, 3 and 8 all touch this pattern.
   The underlying argument is unchanged and arguably stronger: the clear-three-fields shape also appears
   at `TournamentTeamManager::unassignPlayerFromTeam()` 493-495,
   `TournamentRosterBuilderController` 174-176 and 504-505, `RosterBuilderController` 547-548, and
   `CcsoccerCommands` 1447 and 1952. That drift is what this whole document is about.
2. **New method `disbandGroup($registration)`** with these guards, in order:
   - registration loads, and `player` is the current user (copy `leaveGroup()`'s ownership check);
   - `registration_type === 'season'` — tournaments are teams, out of scope, hard-refuse;
   - status is live (not `cancelled`) — reuse the D-4 exclude rule;
   - `group_id` is non-empty, else "You are not in a group.";
   - caller is the manager (`invited_by` empty), else point them at Leave Group;
   - `groups_locked` on the season is FALSE — matches `leaveGroup()` (1996-1999) and `invite()`
     (881-886);
   - **live-member recount at submit time** — see the race note below.
3. **Call the service**, then redirect to `/my-registrations` with "Your group has been disbanded. You
   can now join another player's group or start a new one."
4. **Do not touch `Registration.team`** — CF6's recorded decision. Disbanding a friend group must not
   un-assign anyone from a season team.
5. **Watchdog notice** with registration id, uid, group_id, and how many invitations were declined.
   This is a new player-facing mutation; it should be traceable without a database dive.

**The race that must be handled.** The button renders from a page that may be minutes stale. If the
invitee accepts between render and click, a naive dissolve silently ejects them. So the handler must
**re-load the group's live members inside the request** and refuse if the count is > 1:

> "Someone joined your group since this page loaded. Refresh to see your group."

Count with the D-4 exclude rule (`status NOT IN ('cancelled','expired')`), by `getQuery()` — note
`loadByProperties()` cannot express `NOT IN`, the same wrinkle CF3 hits at ten sites. Ideally CF11
consumes CF3's shared helper, which argues for sequencing it after CF3.

**Pending invitations are declined, not deleted** — `declined` per D-3, with `responded_at` stamped,
exactly as the admin path does. Consequence worth knowing and **already safe**: if the invitee is
mid-checkout with that invitation selected, `OrderCompleteSubscriber`'s `isPending()` guard (~494)
sees it is no longer pending and creates the registration with `group_id` NULL rather than throwing.
The player gets no message about it — that is the pre-existing "missing user-facing messaging around
group-full" item from July 20, not something CF11 introduces or should try to fix.

**Route** — copy the neighbours exactly, both requirements matter:

```yaml
ccsoccer.group.disband:
  path: '/my-group/{registration}/disband'
  defaults:
    _controller: '\Drupal\ccsoccer\Controller\GroupController::disbandGroup'
  requirements:
    _user_is_logged_in: 'TRUE'
    _csrf_token: 'TRUE'
  methods: [POST]
```

**Template.** `ccsoccer-group-manage.html.twig` currently renders Leave Group only for
`not is_manager and member.is_self` (84-90), and the roster table itself only renders when
`group_roster is not empty` — which for a solo manager is a one-row table. Add the Disband button in
the `is_manager and member.is_self` slot, gated on a new `can_disband` flag computed in `manage()`
(manager + season + exactly one live member + not locked) rather than re-deriving the rule in Twig.
Confirm-dialog wording should name the consequence: *"Disband your group? Any pending invitations will
be cancelled. You'll be able to join someone else's group."*

**Also worth fixing in the same commit (one line each, same root cause):**

- `manage()`'s `if (!$group_id)` gate on `$pending_invitations_to_me` (**line 600** — 598 is its
  comment; **[corrected Aug 4]**) — a solo manager cannot
  see invitations addressed to them, so after disbanding they must reload the page to discover one
  already waiting. Widening it to *"no group, or manager of a group of one"* makes the Manage Group
  page tell the truth. **Check this against the roster-builder screens before changing it** — it is a
  display gate, not a data one.
- Routes 2-4 above still strand people even with CF11 shipped; CF11 just gives them a way out. A
  tighter fix is to clear the manager's own `group_id` when a group drops to one live member, inside
  the same service (`leaveGroup()`, `removeMember()`, `deleteInvitation()` all call it). ~~**Deferred
  deliberately** — it changes behaviour for managers who are deliberately holding a group open for a
  slow invitee, and needs Andrew's call.~~ **[Aug 5] This is exactly what was chosen — D-8 decided as
  CF10 + B + E.** The "manager holding a group open for a slow invitee" objection is answered by
  `GROUP_CLEANUP_BRIEF.md` §4.2's carve-out: a group with any pending invitation is never dissolved.
  The real site count is **thirteen**, not the three guessed here. See **D-8**.

**Test on LOCAL:**

- **The P1/P3 scenario end to end:** P1 invites P2; P3 invites P2; P2 accepts P1's. P3 opens Manage
  Group, clicks Disband → `group_id`/`invited_by`/`invitation_status` cleared on P3's registration,
  P3's dead invitation to P2 is `declined`. Then P1 invites P3 → **succeeds**, where before it said
  "already in another group". P3 accepts → group of 3. *This is the acceptance test.*
- Manager of a group of 2 → no Disband button; Remove the member first, then it appears.
- Non-manager member → sees Leave Group, never Disband.
- Tournament captain → no Disband button; POST the route by hand → refused.
- `groups_locked` on → button gone, hand-POST refused.
- Cancelled registration → refused.
- Another player's registration id in the URL → refused (ownership check).
- POST without the CSRF token → 403.
- Disband a group of one that has **two** pending invitations → both `declined`, one watchdog notice.
- **Race:** open Manage Group as the solo manager, accept the invitation as the invitee in another
  browser, then click Disband → refused with the refresh message, group intact, invitee still in it.
- Season team assignment (`Registration.team`) unchanged throughout — check before and after.

**Risk:** low and well-contained; it is a new route rather than a change to an existing one, and the
service it calls is already exercised by the admin dissolve. The one thing that would make it
dangerous is skipping the submit-time recount.

### 10.4 Branch and commit plan

~~Branch `fix/roster_sync` off main once the pre-flight in 10.2 is clear.~~
**[Aug 4 evening] `fix/roster_sync` never existed.** The work is on **`fix/CF8_dissolve_group`**,
renamed from `fix/CF1_tournament_cancel_reg` → `fix/CF2_GroupInvitationsForm` as it grew. Other
documents still reference `fix/roster_sync` — they are wrong; this is the branch.

**[Aug 4] Branch note.** Andrew opened **`fix/CF1_tournament_cancel_reg`** off `509a6f2` rather than a
single `fix/roster_sync`. Per-CF branches are the better shape for a nine-commit plan — but commit 2's
CF2 + D2 + CF12 then needs its own, and the ordering constraints below still apply across branches.

> ### ⚠ [Aug 4 evening] Everything from here to the commit table was written BEFORE the work landed.
>
> The two "recommended sequencing changes" and three of the four "hard dependencies" below are
> **resolved or obsolete**. They are struck through rather than deleted so the reasoning survives.
> The live constraints are the short list immediately after them.

**[Aug 4 morning] Two sequencing changes recommended:** — ~~both now done~~

1. ~~**Ship CF12's display half first, ahead of commit 1.**~~ **DONE** — shipped in `e5317f8`.
2. ~~**Re-scope CF4 (commit 5) before committing to it.**~~ **DONE** — re-scoped to 21 sites and
   promoted ahead of CF3 after Andrew reproduced the write-path bug. Shipped in `b46c28b`.

**[Aug 4 morning] Hard dependencies:**

- ~~**CF5 (commit 6) is blocked on CF1 (commit 1)** — `removePlayerFromAllTeamsInTournament()` does not
  exist in the tree.~~ **RESOLVED.** The helper shipped with CF1 in `e5317f8`; CF5 shipped in `7c9a7c5`.
- **CF11 (commit 8) is blocked on CF8's extracted service (commit 3).** **STILL TRUE** — and note the
  service is committed but **untested**, so building CF11 on it now means debugging two callers at once.
- ~~**CF3 (commit 4) collides with commit 2** — land commit 2 first.~~ **RESOLVED** — commit 2 landed.
  But CF3 now collides with **CF8 and CF4** as well; all three edited `GroupInvitationsForm`. Re-locate
  its sites by method name.
- ~~**CF3 must exempt `GroupInvitationsForm:769`**~~ **OBSOLETE.** CF8 moved that query into
  `GroupDissolveService`, where the exemption is documented in the class docblock. CF3 has **nine sites
  to filter and nothing to skip.**

**[Aug 4 evening] The only live constraints now:**

- **Nothing has been executed.** Lint and test before adding to the pile.
- **CF11 needs CF8 tested**, not merely written.
- ~~**CF10/CF11 blocked on decision D-8**~~ **RESOLVED — D-8 decided Aug 5** (§7). **CF10 is reinstated
  and specified in `GROUP_CLEANUP_BRIEF.md` §5, which supersedes its brief below. CF11 is now optional**
  and should not be built until `GROUP_CLEANUP_BRIEF.md` §9.4 query B has run for four weeks.
  Both still sit behind §10.8.
- **CF3, CF10 and CF11 briefs all carry stale line numbers.** Re-locate by method name.

| Commit | Contents | **State (Aug 4 evening)** |
|---|---|---|
| 1 | CF1 — `TournamentCancelRegistrationForm`, `TournamentTeamManager` (hardening + new helper). **[Aug 4] Now five parts, not three:** adds service injection into the form, threads `$allow_captain` through *both* guard methods, and handles the **co-captain** case — without which CF1 is a regression for a cancelling co-captain. Also sets `invitation_status` and clears `is_captain`. | ✅ `e5317f8` + `8156bc1` (CF1b), untested |
| 2 | **CF2 + D2 + CF12** — `GroupInvitationsForm`, plus CF12's one-line write-side change in `OrderCompleteSubscriber::createTournamentRegistration()` (additive; does not collide with CF9's edits at 773) | ✅ `59c12ec`, untested and **never exercised** |
| 3 | CF8 — `CancelRegistrationForm`, extracted dissolve service, `NotificationService::sendGroupDissolved()` | ✅ `b22734f`, untested |
| 4 | CF3 — the ten status filters + shared helper | ⬜ not started · **brief is stale** |
| 5 | CF4 — `pickLiveRegistration()` trait + call sites (scope per audit) | ✅ `b46c28b`, untested · grew to 21 sites / 8 classes |
| 6a | **CF5** — roster builder reconciles against every team *(split from CF9 per **D-14**)* | ✅ `7c9a7c5`, untested · 3 call sites, not 1 |
| 6b | **CF9** — route the inline `Team.players` appends through `TournamentTeamManager`, delete the dead `action = 'join'` branch *(own commit per **D-14**)* | ⬜ **not started** · researched, brief updated |
| 7 | CF7 — team predelete hook | ⬜ not started |
| 8 | **CF10 + B + E** — stranded solo managers: deferred group creation in `invite()` (CF10) plus `clearGroupIfOrphaned()` at thirteen call sites. **CF11's Disband button is no longer part of this** — route 4 clears itself under B | ✅ **[CORRECTED Aug 23] Merged and on PROD.** This cell said "IN PROGRESS… B + E written but uncommitted, unlinted, untested," which was true for about four hours on Aug 5. Service + wiring `dde90b5`; CF10 `995e286`; **B + E `c7251c6`**, plus `7a5f57b` fixing a dead-end admin page LOCAL testing surfaced. Merged via PR #129. Spec: `archive/GROUP_CLEANUP_BRIEF.md` |
| 9 | Docs: update `OUTSTANDING_ISSUES.md`, `SEASON_TOURNAMENT_DRIFT_AUDIT.md` (D6, D7, D13, and **close D19 + the option-list half of D15**, both re-filed July 30 — D19 promoted to 🟠 High, D15 annotated), `ADMIN_TOOLING_REVIEW.md` (T2/T3/T5, R4), `SESSION_HANDOFF.md` | ◐ this plan + `SESSION_HANDOFF.md` updated; the other three **not** |

Commits 1–3 are the ones that stop new drift. If time runs short, land those and stop.

> **[Aug 4 evening] The drift-stoppers are all written.** Commits 1, 2 and 3 exist as code. The
> remaining risk is entirely that **none of it has run.** Commit 9's other three documents
> (`OUTSTANDING_ISSUES.md`, `SEASON_TOURNAMENT_DRIFT_AUDIT.md`, `ADMIN_TOOLING_REVIEW.md`) still carry
> July 28 status blocks that contradict this one — they should not be trusted until commit 9 lands.
>
> **Recommended order from here:** lint and test what exists → commit 9's remaining doc updates →
> **CF9** (highest defensive value of the remainder; it is what stops a duplicate `Team.players`
> append) → CF3 → CF7 → the D-8 package. **[Aug 5] D-8 is now decided**, so that last item is
> CF10 + B + E per `GROUP_CLEANUP_BRIEF.md`, not CF10/CF11.
>
> **Deviation from the plan, recorded:** the Aug 4 evening session shipped CF8, CF4 and CF5 rather
> than commits 3, 4, 5, 6 in order. CF4 was promoted ahead of CF3 because Andrew reproduced a live
> player-facing data-corruption bug at `RegistrationController:134`, one of CF4's write sites. CF3
> was skipped because it does nothing for that bug — it covers season group queries, and the
> reproduction was a tournament player+tournament lookup.

**CF12 is inside commit 2 on purpose, not appended to it.** It is not a `Team.players` drift fix like the
rest of commits 1–3, but it *is* a new-drift source — the admin Save path at `submitForm()` 919, in the
same method CF2 rewrites. Splitting them means touching those lines twice and leaving a live data-corrupting
Save in between. If commit 2 has to be deferred, CF12's display half is independently shippable in an hour
and worth doing alone: it is the half that corrects the SLO Friendly screens with no data repair.

**CF10 and CF11 are independent of commits 1–7** and could be lifted into their own branch if the
roster-sync work stalls — they touch `GroupController::invite()`/`manage()`, the routing file and one
Twig template, none of which CF1/CF2/CF5/CF9 go near. Two ordering constraints only:

- **CF11 needs CF8's extracted dissolve service** (commit 3). Doing CF11 first means either writing
  the service early or writing a fifth copy of the clear-and-decline loop — take the former.
- **CF11's live-member recount wants CF3's shared status helper** (commit 4). Not a hard block; it can
  inline a `getQuery()` with the D-4 exclude list and be switched over when CF3 lands.

Placed at 8 because commits 1–3 are the drift-stoppers and this pair is a UX gap, not a drift source.
It is, however, the only item in this document that is currently generating **admin email tickets**, so
if Andrew wants it sooner it is cheap to promote: CF8's service, then CF10 + CF11.

### 10.5 Deploy

Code-only. No config changes, no update hooks. TEST: `ccsDeploy`. PROD after soak:
`ccsProdDeploy && ccsProdCr`. Run `drush config:status` before any `cex`; expect the three known
media_library display entries as the only drift.

### 10.6 Do not

- Run git write commands in this repo.
- Repair data. No `UPDATE`/`DELETE` against `ccsoccer_registration`, `team`, or `team__players`.
- Dual-write `Team.players` from the season flow.
- Backfill `group_id` for tournament free agents — NULL is intentional and documented.
- Widen `removeMember()`'s captain guard (added deliberately in `9dfe05f`).
- Touch the schedule generators — separate P2 work.

### 10.7 Definition of done

The acceptance test is behavioural, not a row count — no data is being repaired, so the audit numbers
won't drop. What must be true is that **they stop going up**:

1. `php -l` clean on every touched file.
2. Each fix's LOCAL click-through above passes.
3. On LOCAL: run `ROSTER_DATA_AUDIT.sql` and save the summary. Then perform the actions that used to
   create drift — cancel a tournament player on a team, admin-decline a tournament member,
   admin-accept a pending tournament invitation, cancel a season group manager. Re-run the audit.
   **Every class count must be unchanged.** Before the fixes, each of those actions would have
   incremented TA, TB, TD or SB.
4. Deploy TEST, soak, spot-check one real cancellation, then PROD.

---

### 10.8 [Aug 4 evening] Consolidated LOCAL test checklist for what is already written

> ### ✅ PASSED — Andrew, August 5, on LOCAL. This checklist is spent.
>
> All six merged fixes (CF1, CF1b, CF2, CF12, CF4, CF8, CF5) are linted and exercised. Registration
> **5088** was cleaned up as part of it. **This was the hard prerequisite for every other piece of work
> in this document**, and it unblocked the D-8 package the same day.
>
> **[Aug 23] The PROD deploy has since happened**, which lifted operating rule 1. Rule 2
> (`schedule_visible` OFF while iterating schedules) never depended on it and is still in force —
> and now applies to seasons as well as tournaments. See `SESSION_HANDOFF.md`.
>
> **The live checklist is now `GROUP_CLEANUP_BRIEF.md` §9** (29 cases for CF10 + B + E), not this one.
> Keep this section as the regression suite for the PR #124 fixes: re-run item 1 and item 4 before the
> PROD deploy, since those cover the two live bugs (the split-brain reproduction and the admin accept
> fatal) that this cluster exists to close.

**Historical note — nothing below had been run when it was written.** Work top to bottom; the first two
are prerequisites for all the rest.

**0. Prerequisites**

- [ ] `ddev php -l` on the **12** files in the evening commits (`git diff --name-only 810f257 7c9a7c5`)
      plus the files touched by the three morning commits.
- [ ] **`ddev drush cr`** — mandatory, four constructors gained arguments.
- [ ] **Clean up registration 5088** (testuser524, SLO tournament). It is cancelled but still carries
      `team`, `group_id` and `invitation_status = accepted` from the Aug 4 bad accept, and will
      confound every tournament test below.

**1. CF4 — the reproduction that started it. Highest priority.**

- [ ] Register a player for a tournament → captain invites → player accepts → **admin cancels** →
      player **registers again** → captain re-invites → player accepts.
- [ ] Expect: the accept lands on the **live** registration. Captain's Manage Group, Tournament Teams,
      My Registrations and the Roster Builder **all agree**. No "This registration has been cancelled".
- [ ] Repeat via **both** Accept buttons — My Registrations (`GroupController`) and the Register page
      (`RegistrationController`). They route through different controllers and both were broken.
- [ ] Roster builder: drag a player who holds a cancelled + live pair. The assignment must land on the
      live row and the player must leave the workbench.

**2. CF8 — run on LOCAL, not TEST**, so Mailpit shows everything. On TEST only board members receive.

- [ ] Member of a 3-person group cancels → own fields cleared, other two untouched, group shows 2,
      **no notifications**.
- [ ] **Manager of a 3-person group cancels** → all three cleared; **2 notifications, and check the SMS
      actually delivered, not just the email** — that is the Caleb-rewrite regression.
- [ ] Pending **and accepted** invitations for the group become `declined` with `responded_at`.
- [ ] **Nobody's `Registration.team` changed** — check before and after.
- [ ] Re-invite an address whose invitation was just declined → succeeds (E4/Bug-3 guard).
- [ ] Manager of a group of one cancels → clean, no notifications, no errors.
- [ ] A member with preference `none` → no notification, no error, group still cleared.
- [ ] Admin **Dissolve group** button → still works, same result, via the extracted service.

**3. CF5 — needs deliberately dirty data.** A clean drag passes either way and proves nothing.

- [ ] Put a uid in a team's `players` with `reg.team` empty (ghost) or pointing at another team.
- [ ] Drag them → the stale entry must clear.
- [ ] Drag to workbench/pool → cleared off **every** team.
- [ ] Same for Shift+drag merge (`mergeToGroup`) and the manager/member pairing (`createGroup`).

**4. CF1 / CF1b / CF2 / CF12 — the three earlier commits, also never run**

- [ ] Cancel a tournament player who is on a team → removed from `Team.players`, `reg.team` cleared.
- [ ] Cancel a **co-captain** → `Team.co_captain` cleared, not left dangling. This is CF1's part 4 and
      the case the original brief missed.
- [ ] Cancel a **captain** → `Team.captain` cleared, warning shown, team survives.
- [ ] **CF2 has never been exercised at all.** Admin Group Invitations page: accept a pending
      tournament invitation → no fatal, `Team.players` and `Registration.team` both updated.
      Decline → removed from both. This was the live PROD bug D2.
- [ ] **CF12:** open the admin Group Invitations page for a tournament group. Member rows must **not**
      render as "Pending" for paid players on a team, and the stats bar must agree with the rows.
      Press **Save with no edits** → nothing changes (the Save trap).
- [ ] Promote-to-captain works for a post-June-26 registrant.

---

## 11. What this audit does NOT cover

- Orders, payments, refunds, credits — nothing here reads or reconciles money.
- Schedules and games (S2/S8/TS2) — separate P2 work.
- Invitation *identity* checks (E3 — token addressed to someone else) beyond the capacity classes IA/IB/IC.
- `ccsoccer_pool` semantics beyond reporting the flag (T4 is a UI bug, not a data one).
- **`invitation_status` on tournament registrations.** No audit class counts it — TG/TH are `group_id`
  classes and nothing inspects this field on the tournament side. CF12 is a code fix arrived at by reading
  the two screens, not by the audit, and re-running the audit after it will show no change. Add a class if
  a repair is ever revisited; the rule would be update 9068's.
- Whether a season team's *composition* is correct (R3 coed ≥1 woman) — that is validation, not drift.
- **[NEW Aug 4 evening] The phantom captain view.** A tournament registration with no team renders as a
  manager, with an Invite a Player form, because `GroupController:419` falls back to
  `$is_manager = empty(invited_by)` when `$team` is NULL. Submitting then errors "No team found for this
  registration" (`:896`). Found while diagnosing the Aug 4 reproduction; pre-existing, cosmetic, and
  **not covered by any brief here.** It is a display-logic gap rather than data drift, which is why it
  is listed as out of scope rather than promoted to a CF — but it looks alarming to a player and is
  cheap to fix if someone is already in `manage()`.
- **[NEW Aug 4 evening] Destination resolution in the roster-builder group operations.**
  `mergeToGroup()` and `createGroup()` derive the *destination* team from `reg.team` (the first group
  member's, and the member's, respectively). If that player is a ghost the destination resolves to NULL
  and the pair lands nowhere. Same root cause as CF5 but the opposite direction — CF5 fixed the
  *source* side only. `TournamentTeamManager::getPlayerTeam()` already has the roster-scan fallback
  that would fix it. Pre-existing, unchanged by CF5, worth its own small brief.
- Any container other than the two 2026 Early Fall seasons and SLO Friendly 2026.
- **The existing drift.** These fixes are preventive. Ghosts already in the database stay there, and
  the "Team is full" symptom on an affected tournament team persists until either a repair is run or
  capacity is changed to count roster entries backed by a live registration (T2's own alternative).