# Doc Cleanup & Consolidation — WIP

**Started:** August 2, 2026 (Caleb + Claude)
**Status:** IN PROGRESS — scratch file. Delete when consolidation is done.
**Not for Andrew yet** — this is the working surface. The output is a rewritten
`OUTSTANDING_ISSUES.md`; this file is the harvest that feeds it.

---

## 1. Why

Open work is currently spread across **seven** documents. Nothing is authoritative,
several are months stale, and status is maintained by hand in places where git already
knows the answer. This caused real damage on Aug 2: `OUTSTANDING_ISSUES.md`'s "in-flight
work to land first" section listed six items as needing deployment when all six were
already on PROD, which produced a wrong sequencing recommendation.

Root cause is the same shape as the bugs in the codebase: **duplicated state that drifts.**

---

## 2. Target structure

One home per kind of thing, no overlap.

| Kind | Home |
|---|---|
| What's deployed | **git** — no doc |
| What's open | **OUTSTANDING_ISSUES.md** (single master list) |
| What happened / how to resume | **SESSION_HANDOFF.md** |
| Why the code is like this | analysis docs (4–5, stable) |
| How to do a thing | reference docs (stable) |

### The rule that makes it stick

> **Analysis docs describe how the code works and what's wrong with it.
> They never say whether it's been fixed. Only the tracker says that.**

Drop the ✅ IMPLEMENTED markers and "STATUS as of <date>" banners from the analysis
docs. That is what makes a two-month-old analysis doc still trustworthy — reasoning
about code doesn't expire the way status does.

Corollary for the handoff: it records **what got done, referencing the tracker item.**
Completed items drop off the tracker; they don't accumulate in it.

Target: ~12 files in repo root, down from 37.

---

## 3. Source status

| Source | Contents | Disposition | Harvested? |
|---|---|---|---|
| `OUTSTANDING_ISSUES.md` | P1–P5, hygiene, parked | **Becomes the master list** | — |
| `ROSTER_RECONCILIATION_PLAN.md` | CF1–CF12, decisions D-1…D-11, commit plan | **Stays separate** (implementation brief, not a list). Tracker points at it. | n/a |
| `ROSTER_DATA_AUDIT.sql` | read-only audit script | Stays, companion to above | n/a |
| `CC_Soccer_Security_Review_2026_07_03.md` | E/I/K open, G/H deferred, prod-verification list | Pull open items → master. **Keep as analysis** (it has genuinely good reasoning, incl. a "Corrected / non-issues" section worth preserving). | ✅ fully ruled |
| `CODE_QUALITY_BACKLOG.md` | B-1…B-19 | **Split** — evidenced items → master list; style items → conventions doc. **Archive file.** | ✅ ruled |
| `SESSION_HANDOFF.md` "Combined Pre-Launch Checklist" | items 1–34, ~10 still live | Pull live items → master. **Drop the section.** | ☐ |
| `PROJECT_STATUS.md` | 3 feature gaps + 4 tech-debt notes | **ARCHIVE — ready** | ✅ fully ruled |
| `TODO.md` | Jan 4 2026, pre-launch | **ARCHIVE — ready** | ✅ fully ruled |

---

## 4. Harvested items

Flat list. **Priority and categorization come after the harvest is complete** — don't sort yet.

### From PROJECT_STATUS.md

| ID | Item | Status |
|---|---|---|
| PS-1 | **Insurance Report** | **KEEP.** Never defined. Player roster for insurance: name, DOB, contact, emergency contact, active seasons. PDF + CSV. Same item as TODO's Priority 2 — dedupe on merge. |
| PS-2 | **Automated registration reminders** | **KEEP** (downstream). 6 / 2 / 1 weeks before season close. To previous-season players not yet registered, excluding already-registered. Needs cron + queue. Saves recurring per-season effort. |
| PS-3 | **Override expiration reminders** | 24h before expiry, to waitlist users with active overrides. → **VERIFY bucket** (an `override_notified` field exists on Registration). |
| PS-4 | **Schedule iCal — revisit** | **KEEP.** Largely working, but needs another pass. Overlaps existing tracker items G4/G5 (public per-season feed emits cancelled games as `STATUS:CONFIRMED`; printable PDF shows cancelled + unpublished as normal; per-user feed is correct) and G6 (legacy Views feed at `schedule-feed/%/ical` is public and emits garbage). **Merge with G4/G5/G6 rather than tracking separately.** |

**Optional Features (TBD) section — 4 items:**

| Item | Ruling |
|---|---|
| Team messaging — captains notify their teams | **KEEP → future / nice-to-have** |
| Auto-subtract jersey cost on late cancellations ($25) | **KEEP** on the larger list; decide placement later. *(Dupe — same item as the tech-debt entry below.)* |
| Bulk cancellation tools — cancel an entire season | **DROP.** Unclear what it would do or when it would be used. |
| Refund reporting — admin tracking | **DROP** for now. |

**NOT DOING section** — explicit no, nothing to carry: automated waitlist progression
(staying manual by requirement), referee management, player statistics, social features.

**Tech-debt section — ruled Aug 2:**

| Item | Ruling |
|---|---|
| Seeded test data has no real Commerce orders | **ARCHIVE.** Worthless now, but the *method* stays relevant — archive material, not a tracker item. |
| Jersey cost auto-deduction ($25 on late cancellations) | **KEEP** on the larger list; placement decided later. Same item as the Optional Features entry above. |
| Some settings hardcoded (credit expiration period, reminder timing) | **KEEP.** Deserves investigation, especially alongside the push to get config into repo files so it isn't overwritten. |
| Error handling (gateway failures, Clickatell network) | **DROP.** Basic handling is in place; add detail where it proves needed (e.g. invites, registration). |

### From TODO.md — ruled Aug 2

**Done, no action:** the 8 COMPLETED items · board pruning decision · migration scripts ·
migration test · deployment prep · Jersey Report · City Payment Report · **mobile testing** ·
**end-to-end testing** · **user acceptance testing** · **Tournament Deposits Management Page**
(grew from a report into a dashboard; in place, may get tweaks later).

| Item | Ruling |
|---|---|
| Schedule Cancellation Overlay (Andrew) — cancellation reason on schedule grid, styled | **DONE** — working for rainouts etc. *(An iCal-subscription piece may have been started separately — see VERIFY.)* |
| Performance testing — roster builder 200+, schedule builder 20+ teams, bulk queue, query optimization | **KEEP → optimization bucket** (downstream) |
| Menu UX polish — board / slofriendly / player / admin, test per role | **KEEP** |
| Insurance Report | **KEEP** — dedupe with PS-1 |
| All Players View Refactor (Andrew) — slow view → MySQL-view pattern, must update all links | **KEEP → optimization bucket** |
| Backup strategy verification (= handoff checklist item 16) | **KEEP** — placement TBD |

**Future / nice-to-have — all dropped:**

- Tournament bracket generation — first-pass schedule generator exists; let it resurface organically
- Free agent system — **shipped** (free agent + CCSoccer pool; players registering now)
- Additional reports (payment tracking, waitlist analytics) — not wanted; revisit if waitlist usage warrants
- Mobile app — far future
- Bulk notification improvements — **largely shipped Aug 1; pick through the remainder together**

### VERIFY bucket — may already be done, cheap to confirm

Items nobody is certain about. Clearing these in one pass should shorten the list before
any real work starts.

- ☐ **Invite email masking** (security review finding H) — believed implemented.
  Note `INVITE_EMAIL_MASKING_PROPOSAL.md` is referenced by the review but absent from repo root.
- ☐ **PS-3 override expiration reminders** — `override_notified` field exists on Registration
- ☐ **Security review production checks** — reCAPTCHA renders on contact/register/login/pass;
  `dev_mode = FALSE`; Devel uninstalled; `error_level: hide`; `hash_salt` and
  `trusted_host_patterns` set
- ☐ **iCal subscription for schedule cancellations** — believed started alongside the
  cancellation overlay; completion unknown
- ☐ **B-10 stub services** — are `SeasonRegistrationService` / `TournamentRegistrationService`
  empty stubs in `services.yml`?

### From CODE_QUALITY_BACKLOG.md — ruled Aug 2

Source: a full-site sweep Andrew ran. Roughly half is evidenced, half is boilerplate any
Drupal codebase would produce. **Split by the rule below** rather than taking all 19.

> **If it has an observed symptom, it's a tracker item.
> If it's a pattern we'd prefer not to repeat, it's a convention — reference, not backlog.**

**→ Master list (evidenced):**

| ID | Item | Notes |
|---|---|---|
| B-14 | Duplicate return in `Registration.php` ~line 269 | **TOP OF LIST** — two-minute fix. *(Not doing it mid-reorganization.)* Second half (Tournament/Season base class) is style → conventions. |
| B-7 | **No automated test coverage** | Strongest item in the doc. Nothing on payment, credits, registration, notifications. This codebase has produced the *same bug shape* 6+ times (missing status filter, bare `reset()`, drifted twin paths) — exactly what tests catch. Start: kernel tests for `OrderCompleteSubscriber` + `CreditManagerService`. |
| B-4 | N+1 in skill calc + team tables | ~300 queries for 20 teams. Corroborated by the "All Players view is slow" complaint (TODO → optimization bucket). Batch-load before loops. |
| B-1 | XSS via `#markup` concatenation | `CompletionPane`, `AgreementsPane` (waiver text rendered raw), `TournamentController`, `AdminController`. Overlaps security review K. |
| B-15 | Magic status strings → StatusConstants | **MERGED with reconciliation-plan D-4** — same finding from two directions. D-4 decided: exclude `cancelled`/`expired` rather than allowlist. StatusConstants encodes it. Do together. |
| B-9 | Missing cascade-delete (Season/Tournament orphans Games, Teams, Registrations) | **MERGED with CF7** — CF7 nulls `Registration.team` on team delete; B-9 is the broader cascade. Same problem, two angles. Some recent work already touches this. |
| B-10 | Stub services `SeasonRegistrationService` / `TournamentRegistrationService` | → **VERIFY bucket** — registered in `services.yml`, may be empty. Cheap to confirm, then complete or remove. |
| B-6 | Aggressive cache disabling (`max-age 0`; `Registration.php` invalidates on every save) | Has a named pattern to follow — the July 5 series-rules cache-tag fix. |
| B-8 | Missing transaction boundaries — credit splitting, bulk roster ops, schedule generation | Money is involved in the first one. |

**→ Needs sizing before it becomes a tracker item:**

| ID | Item | Why |
|---|---|---|
| B-2 | Audit 139+ `accessCheck(FALSE)` | "139 instances" is a grep count, not a finding — no leak demonstrated, most are legitimately admin-gated. **Sample 15 first**, judge the hit rate, then decide. Could otherwise burn a week for nothing. |
| B-5 | Missing DB indexes (`WaitlistManagerService`, `PlayerAdminController`) | Doc says "without confirmed indexes" — i.e. nobody checked. A `SHOW INDEX` settles whether this is real. |
| B-3 | AJAX input validation (team/game IDs, position enums) | Plausible but no exploit shown; endpoints are admin-gated. |

**→ Conventions doc (style, no symptom — guidance when already in a file):**

B-11 generic entity access handlers · B-12 DI inconsistencies (— note: blocks B-7) ·
B-13 fat controllers *(already checklist item 31)* · B-16 inconsistent date formatting ·
B-17 hardcoded values (store ID 1, cents×100, 48\*60\*60) · B-18 JS `fetch()` error handling
(`passkey-autotrigger.js`) · B-19 inline styles in PHP form builders

**Nothing to harvest** from its "Resolved / tracked elsewhere" table — 8 April findings
already correctly mapped.

### From the Aug 2 bulk-notification session — new items

| Item | Notes |
|---|---|
| **Replace `slofriendly` role refs with `tournament_director`** | **Decided Aug 2, not done.** `sendBulk()` queries a role that doesn't exist — silently matches nothing. Grep first; only *role* references change (menu names, CSS classes, URLs, tournament labels legitimately keep the word). Behaviour change: tournament directors start getting immediate `[VERIFY]` copies and are excluded from the queue. |
| **`getRecipientIds()` → exclusion filter** | Uses `status IN ('paid','active')`; D-4 decided against allowlists. Behaviour-neutral today. **Merge with B-15/D-4.** |
| **Write down the allowlist-vs-exclusion convention** | *Fail toward the less harmful error.* Notifications: unknown status should still be notified → exclusion filter. Capacity/rosters: unknown status should NOT count → allowlist. Codebase currently does both by accident. → conventions doc. |
| **Unexplained preference lookups** | Three rounds of four `field_notification_preference` debug lines fire before a send; neither `getRecipientIds()` nor `countSmsOnlyRecipients()` calls `getUserPreference()`. Possibly the Test button. Investigate. |
| **Check Layne Smith's PROD roles** | Stale `board_member` removed on TEST Aug 2 (she's off the board). If PROD still has it she's in the non-production allowlist and receives `[VERIFY]` broadcasts and board alerts. → **VERIFY bucket** |
| **PROD bulk-notification validation** | Three components execute for the first time on PROD. Protocol + seven pass conditions in `SESSION_HANDOFF.md` 🟡 section. Huddle with Andrew before the next send (expected Fri/Sat after teams). |
| **`sendSms()` comment fix** | Done Aug 2, comment only. Needs `ddev php -l` + commit. |

### From CC_Soccer_Security_Review_2026_07_03.md — ruled Aug 2

**Closed:**

| Finding | Ruling |
|---|---|
| **H** — invite autocomplete returns raw emails | **DONE.** Verified by screenshot: autocomplete now shows masked addresses (`es***h@extantlighting.com`). Was the most serious item still open anywhere; it is closed. Note `INVITE_EMAIL_MASKING_PROPOSAL.md` is referenced by the review but absent from repo root — check `archive/`. |
| **E** — jersey pane trusts `variation_id` from request | **DROP.** All jersey variations are the same price, so a swapped variation gains nothing. |
| A, B, C, D, F, J | Already fixed — nothing to carry. |
| **J-optional** — split `view reports` into a separate finance permission | **DROP.** Board members and tournament directors seeing player credit balances is fine — TDs are board. No role needs reports-without-financials, so there is nothing to split. Recorded so a future review does not re-flag it. |

**🔴 I — secrets in git history. ESCALATED Aug 2, action required:**

- The reCAPTCHA **secret key was printed to a terminal during PROD verification** on Aug 2 and is
  now in shell scrollback and in a chat transcript, on top of already being in git history.
  **Rotate it.** New pair at google.com/recaptcha/admin, then enter via the UI at
  `/admin/config/people/captcha/recaptcha` — **not** via config export, since `recaptcha.settings`
  is in `config_ignore` and lives only in each environment's database. Site key is public and
  does not matter; only the secret does.
- Test credentials also in history. Rotating is the fix; deleting the file is not, since history
  keeps them recoverable from any clone.
- **`verify_hostname: false`** on PROD (found while checking the above). With it off, a stolen
  site key produces valid tokens from any domain. Turning it on is standard hardening.
  `use_globally` is also false and the site relies on per-form captcha points, so this is the
  meaningful control. **Test on TEST first** — it can break a site reached via multiple hostnames.

**PROD verification list — CLEARED Aug 2, six of seven clean:**

`error_level: hide` · Devel/webprofiler/kint not installed · `hash_salt` set (110 chars) ·
`trusted_host_patterns` set for apex + www · `site_instance: production` and `dev_mode` unset
(both correct) · captcha points ON for contact ×2, user_login, user_pass, user_register; OFF for
the three node forms, which is right — those are admin-authenticated.
The seventh (reCAPTCHA) is the rotation item above.

**Still needing rulings:** — none. Security review fully harvested Aug 2.

**K — split into three; they are not equivalent:**

| Item | Ruling |
|---|---|
| **K-1** — raw `$e->getMessage()` in JSON responses | **KEEP → fix list.** The only one touching regular users: group-management AJAX is player-facing and exception text leaks file paths, entity IDs, SQL fragments. Any logged-in player can trigger it. Cheap fix. |
| **K-2** — inline `onchange` at `ReportController.php:393` | **PARK.** Admin/board only, value is server-built so not injectable; the only cost is defeating CSP. **Prerequisite for CSP enforcement (checklist item 17), not a standalone fix** — revisit when CSP moves off report-only. |
| **K-3** — admin notes written unescaped (deposit forfeit/refund forms) | **LEAVE DOCUMENTED → nice-to-have.** Admin-only input and **currently rendered nowhere**, so latent. It is a *trap*, not a bug: becomes live XSS only if someone later prints it with `\|raw` or `#markup`. Cheapest defusal is escaping on write plus a comment saying why. |

**G — `invitee_email` fee-waiver spoofing:**

**KEEP on the list, low priority.** Not the general free-registration path — it is specifically the
"join an invited team" tournament branch, which resolves entitlement three ways, one being
`invitee_email == account email`. Drupal never re-verifies email on change, so someone could set
their account email to a known-invited address and, on a `team_paid` team, get their registration
fee zeroed. Live today, but the attacker must know an invited address, gains $35, and the captain
would see a stranger on their roster. **Caleb Aug 2: odds of this happening are nil.** Fix when
convenient: gate on the `invitee` entity-reference match rather than the email string. Original
deferral condition ("before team fees scale") still stands.

**Split out of G — does NOT belong under a security finding:**

- **`Team::getRosterStats()` counts by team id** "because group_id would be 0" — a condition that
  no longer holds for checkout-created teams since `getOrCreateTeamGroupId()` landed. May
  double-count. **→ move to `ROSTER_RECONCILIATION_PLAN.md` as its own CF item** (Caleb's call
  Aug 2). This is roster-counting territory, not fee waivers.
- Two stale docblocks describing the dead `team_<id>_<id>` group_id format:
  `GroupInvitationsForm.php:24` and `Team.php:655-657`. → opportunistic cleanup.

---

## 4b. Priority taxonomy for the final tracker (Caleb, Aug 2)

Once the harvest is complete and everything is in one list, sort into:

1. **Must** — broken, live, or blocking
2. **Recommended** — real problem, no forcing function yet
3. **Can** — worth doing when the file is already open
4. **Nice to have** — traps, hardening, and wants

**Do not sort until the harvest is done.** Two sources remain (pre-launch checklist; then the
final pass over `OUTSTANDING_ISSUES.md` itself).

---

## 5. Loose ends found along the way

- `getPrice` — empty tracked file. `git rm --cached` done Aug 2, deletion staged, not committed.
- `.DS_Store` — untracked, in `.gitignore` line 19. No action.
- **Five `.sql.gz` dumps in repo root**, including `ccsoccer-d11-migrated-200users.sql.gz`
  — pre-launch checklist item 8 says delete it, marked done, still present.
  Check: `git ls-files '*.sql.gz'`
- **`INVITE_EMAIL_MASKING_PROPOSAL.md` referenced by the security review but not in repo root.**
- `CC Soccer - Entity Specification Template.pages` — Mac Pages file, probably superseded by the `.md`.
- Two operating rules live in different docs but apply to the **same page** (admin Group
  Invitations, tournament groups): don't click **Save** (D19/CF12 writes a bogus `pending`),
  and don't use **accept** (D2 fatals + half-writes). Merge into one: *read-only for tournament
  groups until commit 2 ships.*
- Three P1/P3 tracker items have **no home in `fix/roster_sync`**: R3 (coed ≥1 woman is a bonus,
  not a constraint), D3 (tournament completion never re-checks active/visible), D17 (no tournament
  `groups_locked` — **decision wanted before roster work**).

---

## 6. Verified PROD state (Aug 2) — supersedes every doc

| Commit | What | On PROD |
|---|---|---|
| `fa76cfd` | tournament nudge 500 fix | ✅ |
| `d75ce2e` | core 11.4.4 + security batch | ✅ |
| `7fd60bf` | PR #119 `fix/team_display` | ✅ |
| `e4f7300` | PR #121 tournament gender/color | ✅ (HEAD) |
| — | PR #120 `doc/roster_sync` | ✅ |
| `7335f8d` | bulk notification SMS | ❌ **only outstanding deploy** |

Core reports 11.4.4, `updatedb:status` clean. `feature/update_registration_page` merged.

**Only blocking task anywhere:** TEST-tier SMS verification for bulk notifications, then
PROD with the full chain (`ccsProdDeploy && ccsProdUpdb && ccsProdCim && ccsProdCr`).

---

## 7. Next steps

1. ☑ Caleb rules on TODO items — **complete**
2. ☑ Caleb rules on PROJECT_STATUS — **complete**
3. ☐ Archive `TODO.md` and `PROJECT_STATUS.md` to `archive/`
4. ☑ Harvest `CODE_QUALITY_BACKLOG.md` — **complete** (split: 9 to master, 3 to size, 7 to conventions)
5. ☐ Archive `CODE_QUALITY_BACKLOG.md`
6. ☑ Harvest security review — **complete** (H done, E dropped, I escalated, K split 3 ways, G kept low, `getRosterStats` → roster plan, prod-verification list cleared)
7. ☐ Harvest the handoff's pre-launch checklist (items 1–34) — **last harvest**
8. ☐ Draft consolidated `OUTSTANDING_ISSUES.md`; sort into must / recommended / can / nice-to-have
9. ☐ Add `getRosterStats` as a CF item in `ROSTER_RECONCILIATION_PLAN.md`
10. ☐ **Create the conventions doc** for the style items + the allowlist-vs-exclusion rule
11. ☐ Trim `SESSION_HANDOFF.md` — archive old narrative, drop the checklist section, add a
    short "documentation conventions" section so the structure survives
12. ☐ Strip status markers/banners from the analysis docs
13. ☐ Move remaining archive-bound docs into `archive/`
14. ☐ Review the whole thing with Andrew before committing