Clans Overhaul — Plan
Last updated
May be stale
This page was last updated ago, on 30 August 2026. Check what it says against the code before relying on it.
Delivery plan for the modular clans rebuild specified in design.md.
Phase numbers match module numbers: Phase 1 delivers Module 1 (Core), Phase 2 delivers Module 2 (Bank).
Status: Draft. Phase 0 is independent of the rebuild and should not wait on it.
This document owns the work breakdown — what gets built, in what order, within each phase. strategy.md owns delivery sequencing — the gates between phases, how schema ordering is constrained by code rollout, the cutover mechanism, and the decisions that block the start of each gate. Where the two differ on ordering, strategy.md records why and takes precedence.
Scope boundary
The companion web app is out of scope — its repository is disabled. Nothing in this plan depends on it, and no phase includes work on it. See Deferred for what that leaves unresolved.
Phase 0 — Hotfixes (do not wait on this design)
Live defects, all game-side. Listed here for traceability but they should ship as ordinary fixes as soon as they are reviewed.
| Priority | Fix | Where |
|---|---|---|
| High | /clan accept does not reject players already in a clan, and does not check the INSERT result before declaring success. The insert fails on the clan_members primary key, but in-memory state is set and success is announced anyway — so the player reverts to their old clan on reconnect |
clans/sv_init.lua |
| High | !updateclans reads result from a callback parameter named results, wiping clan state and reloading nothing — the documented recovery path is broken |
clans/sv_init.lua |
| High | cityrp.clans.kick and .setrank return from inside the async DB callback, so they always return nil; the permission-dependent follow-up never runs and live state is not updated |
clans/sv_init.lua |
| Medium | deleteclan rows are hardcoded to v2d/v33x/v4b1. Live is v4b1_fl2, matched by exact string equality, so live never receives them |
clans/sv_init.lua |
| Medium | deleteclan handler compares row.value1 (string) to _Clan (number) and can return without calling done(), causing infinite 60s retries |
libraries/sv_remote.lua |
| Medium | reloadclan and deleteclan both open if not cityrp.clans then done() end with no return — the guard marks the row done, then errors on the next line |
libraries/sv_remote.lua |
| Low | cityrp.model.remove calls table.RemoveByValue on a key-indexed table and silently does nothing; affects every model consumer, not just clans |
libraries/sv_model.lua |
The two sv_remote.lua fixes and the sv_model.lua fix are not throwaway —
those files survive the rebuild. The clans/sv_init.lua fixes are throwaway,
since that file is being replaced.
| Medium | setleader offline path joins clan_member instead of clan_members | clans/sv_init.lua |
| Low | /clan info indexes cityrp.clans.clans[player._Clan] with no nil guard | clans/sv_init.lua |
The clanskinchanger shared-timer collision and the /clan model nil index are
deliberately not listed — that command is being retired in Phase 1, so fixing
it first would be wasted work.
Phase 0 does not attempt to fix the architecture. It stops the bleeding.
Phase 1a — Word filter and staff force-rename tool
Pulled out of Phase 1 and run first (or in parallel with Phase 1 Steps 1–3,
below). Two architecture decisions shape this plan and are not re-litigated
here: Step 2 of Phase 1 builds core.kick as a vertical slice before the
rest of Core, and this word-filter/force-rename work is split into its own
Phase 1a because it sits on the moderation-policy dependency that blocks
other people's work — pulling it forward unblocks their critical path, and
because it does not depend on any clans-specific schema or framework code,
there is no reason to make it wait on Phase 1's sequencing.
cityrp.wordfiltergamemode library (gamemode/core/libraries/, not underclans/). Multiple named categories (reserved,slur,profanity), each independently curated and independently enabled/disabled per call site. Normalisation pipeline: casefold, strip separators/punctuation, fold leetspeak digits (4→a,3→e,0→o,1→i/l), fold common homoglyphs. Two match modes: whole-field (for short fields like the abbreviation) and substring (for longer free-text). This is the first consumer-agnostic profanity/slur filter in the gamemode — no equivalent exists today anywhere, which is exactly why it is being built as a shared library instead of inline inclans/, matching the existing allowlist-drift lesson from image hosts (AGENTS.md footgun #9 — this is the same failure shape: one allowlist copied into several call sites and drifting apart). List contents are owned by moderation; the category-to-field policy matrix is a product decision made per-consumer (clans' matrix is in core.md) — the library itself ships with empty or seed lists and does not hardcode either.- Staff force-rename / force-retag tool. Registered via
cityrp.acommands.AddCommand— this is a staff tool and belongs on!, never on/with a restricted access flag (AGENTS.md footgun #14). Needs a category, a syntax string, and a tip per the acommands contract. Scope: rename and re-tag (abbreviation) a clan without deleting or refunding it, bypassing the normalcore.edit_identitygate and the collision/word-filter screens that would otherwise apply to the offending value (the new value staff choose still goes through validation and the word filter — only the requirement that the acting player holdcore.edit_identityis bypassed). Logged viacityrp.logs.add()as an admin action, independent of the audit trail conventions Bank uses for money (footgun #8 — don't conflate the two logging APIs). - No clan-specific work lands here. This phase produces a library and a staff tool usable by any future feature, not clan schema or clan commands — Phase 1 Step 4 (below) is Core's consumer of this library, wired in once both are ready.
Dependency: Phase 1 Step 4 (identity field validation) cannot ship until this phase's library exists — that is the one point where Phase 1 waits on Phase 1a. Everything else in Phase 1 is independent of it.
Phase 1 — Core (Module 1)
Core is the framework. The module registration API and the base clan
feature set ship together — building the framework as a separate deliverable
with no modules attached would validate nothing. Per the architecture review,
the framework is proven against the smallest real permission check
(core.kick) before the rest of Core is built against imagination.
Step 1 — Minimal framework skeleton
Just enough to register a module and declare a permission — not the full membership/rank feature set.
- Module registration API:
id,pcall-wrappedinit, collision checks on both the module id prefix (bank.withdraw, never a barecan_kick) and any declared table prefix (clan_bank_*). Second registration of the same module id is rejected or no-ops — hot-reload duplicate registration is a recurring bug class here (AGENTS.md footgun #7), and the framework doc is explicit that "declarations replace, side effects guard" are opposite behaviours, not one rule. - Permission registry: declaration format (
id,module,label,kind,default,unit), the threekinds (BOOLEAN,THRESHOLD,NUMERIC_LIMIT), and the resolution order (stored value → declared default → fail-closedNULLwith acityrp.loggingwarning if nothing registers the id). - Schema for this step only:
clan_ranks.rank_order(UNIQUE (clan, rank_order)) alongside the existingrankid, andclan_rank_permissions(clan, rankid, permission_id, value)withvalue INT NULL— notVARCHAR. This is the direct fix for theQueryParamslandmine:DB:QueryParamsbinds every parameter as a quoted string (db.lua:239), so aVARCHARvalue column would compare lexicographically and invert threshold checks above rank 9 — live clans already have up to 28 ranks, so this is reachable today, not a hypothetical. - Core accessors (read-only copies/views, never live table references) for
rank ordinals — the only thing
core.kickneeds to read. - Nothing consumes any of this yet.
Step 2 — Vertical slice: core.kick
Build one permission end to end before building the rest of Core. core.kick
is chosen because it is the smallest complete exercise of the full stack: a
THRESHOLD permission registry read, ordinal resolution
(ord_target/ord_thresh/ord_actor), one atomic SQL statement, and the
Core accessor layer — with no dependency on identity fields, radio, MOTD, or
the UI.
- Register
core.kick(THRESHOLD) against the Step-1 registry. - Implement the threshold formula server-side as a pure function:
allowed = defined ∧ (ord_target ≥ ord_thresh) ∧ (ord_target > ord_actor). The formula's boundary cases are enumerated in acceptance-criteria.md as a manual reference for reviewing the implementation — automated coverage is out of scope for this plan (GLuaTest is unconfigured in this repo; fixing that is tracked separately and not scheduled here). - Implement the kick action as a single parameterised statement carrying the
permission check in its
WHERE/EXISTSclause (per design.md) — not a Lua read-then-decide-then-write. Affected-row count is the only success signal; 0 rows means denied for any reason. - On a 0-row result, run the separate non-authoritative diagnostic
SELECTto produce a specific rejection message. TheUPDATE/DELETEremains the only thing that grants. - Re-validate
IsValid(ply)andply._Initializedinside the async DB callback, not before it (AGENTS.md footgun #2) — this is also the direct regression fix for defect 2, which returned from inside the callback and silently dropped the live-state update. - The membership row deletion happens regardless of whether the kicked
player's notification can be delivered; only the notification is
conditional on
IsValid. - Landmine check specific to this slice:
clan_rank_permissions.valuemust already beINT(from Step 1) before this statement is written, or the ordinal comparison silently inverts per theQueryParamslandmine above — this is the concrete case that motivated the schema decision. Verify by hand against a clan with 10+ ranks before sign-off (mirrors acceptance criterion PERM-SQL1) — the bug is in how the ordinal reaches MySQL, not in the formula itself, so eyeballing the formula alone will not catch it. - No UI in this step. Exercised via a direct call/console command for
validation; the in-game menu's Roster
[Kick]button wires into the same function in Step 5.
This step is the framework validation gate: if core.kick requires
reshaping the registration API, the accessor contract, or the statement
pattern, that reshaping happens here, once, before Step 3 builds nine more
actions against the same shape.
Step 3 — Rest of membership lifecycle and rank management
Built against the now-validated Step 2 shape. Covers: invite, cancel invite,
accept (re-validated at accept time), leave (including the sole-leader
rejection), leadership transfer, setrank, rank CRUD (create, rename,
reorder, delete with optional migrate_to), and clan deletion (core.delete_clan,
cascading members and invites — Bank's distribution step is Phase 2, but the
cascade and the $25,000 refund path belong here).
- Leadership transfer must be a single atomic statement, not demote-then-
promote —
db.luahas no transaction API, so the only way to avoid a zero-or-two-leader race window is one statement that swaps both rows'rank_order(or reassigns membership torank_order = 1) in one query. Two separate statements reopen exactly the window this rule exists to close. - Rank deletion with
migrate_to: reassign holders and remove the rank in one statement for the same reason — no transaction API to fall back on if a partial failure needs rolling back. INV-4a/rank-1-inalienable-permission enforcement: the write path forclan_rank_permissionsrejects any attempt to setcore.manage_ranksfalse on rank 1, checked in the same statement or as a hard precondition before it — never a check-then-write gap.- Rank-order renumbering on delete/reorder, and the "no threshold may resolve to a rank at or above its owner" sweep (invariant 4) run inline on every structural rank change, not only in a periodic job.
- Seed data for a new clan's ranks is a deep copy of the template (invariant 9) — this is the direct fix for defect 6 (ranks assigned by reference). Verify by creating two clans and confirming a mutation to one's ranks does not appear on the other before sign-off.
clan_membersre-key to(clan, steamid)(see Migration hazards below) must be live before this step's invite/accept pre-check ships, since the pre-check and the constraint are meant to work together as primary and defence-in-depth.
Step 4 — Identity, radio, MOTD, administration
Name, abbreviation, motto, description, MOTD — full validation per core.md (character vs. byte counting, charset, trim/collapse, reserved-word and prohibited-content screening), clan radio, MOTD administration.
- Depends on Phase 1a. The reserved-word and prohibited-content screens
call
cityrp.wordfilterwith the field-specific category matrix from core.md — this step cannot start until Phase 1a's library exists. - Validation must be identical at creation and at every edit path (ID-10) — implement as one shared validator function called from both, not two hand-copied checks.
abbreviation varchar(7)is a new column, not a repurposing ofshort(varchar(2000)) — check what live data inclans.shortholds before deciding whether to preserve or drop it (this is a data-preservation check at migration time, not a design constraint — see core.md).- Charset: join-key columns (
clan_members.steamid,clan_invites.steamid) migrate tolatin1to matchplayers._SteamID; text columns (name, motto, description, MOTD) areutf8mb4. This is also where defect 8's underlying cross-charset join gets fixed properly, not just theclan_member→clan_memberstypo — fixing only the typo leaves alatin1/utf8join behind it. self._Clanvsself.cityrp._Clan: Core's public API/accessors must not expose either raw field. Anything touching the old_Clanreads during this port needs to be read in context and routed through a named accessor.
Step 5 — In-game menu
Replaces the "Coming soon..." stub (cityrp_Clan), per ui.md.
Sections: Roster / Ranks / Invites / Identity, landing on Roster. A Bank
section placeholder ships in this step's tab bar but stays hidden or disabled
until Phase 2 — see open question
below on menu sequencing. Everything scales through cityrp.ui
(Scale, CreateScaledFont, OnScaleChanged) — do not follow
player_settings/cl_panel.lua's opt-out (AGENTS.md footgun #15).
- Roster wires directly into Step 2's
core.kickand Step 3'ssetrank/ transfer-leadership functions.[Kick]/[Rank ▾]render only on rows the viewer's threshold reaches (never shown disabled). - Ranks tab: master–detail ladder, permission editor built by iterating the
permission registry and dispatching on
kind— no hardcoded form, so a module registering a new permission (Bank, in Phase 2) needs no UI change. This is the concrete reason Step 1's registry-driven declaration format matters beyond Core itself. - No-clan landing state, invites list, deposit-adjacent copy is not built here (Bank UI is Phase 2) but the Identity tab and creation flow ($50,000 cost, confirmation) are.
- Progressive disclosure per ui.md table.
Step 6 — Retire /clan model
Remove the command and the cityrp.model.register call in loadclan.
Fail-closed by construction: cityrp.model's access() defaults to false
and the consumer at metatables/sv_player.lua:1292 rejects any unregistered
model, so removing the registration makes clan models unavailable, not
unrestricted. Also apply the sv_model.lua table.RemoveByValue-on-a-
key-indexed-table fix from Phase 0 here if it has not already landed, since
this is the step that would otherwise next touch that code path. Announce to
players before it lands (open risk, tracked below).
Schema work landing in Phase 1
clans.abbreviation, clans.join_rankid, clans.next_rankid,
clan_ranks.rank_order (UNIQUE (clan, rank_order)), clan_rank_permissions
(value column INT NULL), and charset normalisation of the clan tables to
match players. clans.bank's widening to BIGINT NOT NULL DEFAULT 0
moves to Phase 2 — it is Bank's landmine to fix immediately before Bank
ships writing to it, not a Core concern, and there is no reason to widen a
column a full phase before anything writes meaningful amounts through it.
Migration hazards specific to this fleet:
GM:ConnectToDatabaseruns pending migrations on every server against one shared database, with nothing serialising them. Two servers restarting together both see a migration pending and both execute it. Make every clan migration idempotent and guarded (WHERE rank_order IS NULL,ON DUPLICATE KEY UPDATEon the tracking row), or take the fleet down for this migration. Applies to every schema change in this phase, not only theclan_membersre-key below.clan_members.clanis currentlyint NULL, and aNULLcannot participate in a primary key — the re-key to(clan, steamid)fails outright on any such row, mid-migration, halting boot fleet-wide. Delete any row with aNULLclan: a membership record that names no clan is meaningless, not recoverable data. Then set the columnNOT NULL, then re-key. Log the deleted rows.- Split the
clan_membersaudit and the re-key into two migrations, and sequence the re-key last in the phase, after Step 3's invite/accept pre-check is live and proven — the application pre-check is what actually rejects duplicate membership; the constraint is defence in depth and should land second. - Forward-only after first write.
down()exists per migration, but once leaders have configuredclan_rank_permissions, dropping that table destroys their configuration. Take a verified backup immediately before, and document the restore procedure rather than implyingdown()is a safety net.
Phase 2 — Bank (Module 2)
Per bank.md. Depends on Phase 1's permission registry and Core
accessors (Step 1), and specifically on Phase 1 Step 3's membership/rank
tables being fully migrated (the withdrawal statement joins clan_members
and clan_rank_permissions directly). Also depends on the DB:RunChecked /
DB:QueryParamsChecked affected-row change — see Step 0 below, which is
called out as its own step rather than folded into Step 3 because bank.md
flags it as needing explicit lead-programmer sign-off before implementation,
not just before this module ships.
Step 0 — DB:RunChecked / DB:QueryParamsChecked affected-row count
Both currently discard the affected-row count for non-INSERT queries
(db.lua:164, db.lua:257). The atomic ledger-append pattern in Step 3 is
built entirely around branching on that count (1 row = accepted, 0 rows =
rejected, duplicate-key = retry), so this is a hard prerequisite, not
parallel work. Flagged, not resolved, here — see Open questions below;
this needs explicit lead-programmer agreement on the exact signature change
before Step 3 starts, per bank.md's own note.
Step 1 — Schema
clans.bankwidened toBIGINT NOT NULL DEFAULT 0. This is the direct fix for the landmine: the column is currentlyint NULL, a 32-bit ceiling, andSET bank = bank + ?against aNULLrow silently yieldsNULL— voiding the deposit with no error. Backfill any existingNULLto0as part of the same migration, before theNOT NULLconstraint is applied, or the migration fails on those rows.- New table
clan_bank_ledger(event stream):clan,seq,steamid,action(deposit/withdraw/reclaim/dissolution_payout/correction),delta,balance_after,created_at, plus audit fields (actor name, request id, originating surface) per bank.md.UNIQUE (clan, seq)is the concurrency control for this entire module — see Step 3. clans.bank_unlocked_at— nullable timestamp, the one-way latch. Written once whenunlock_min_age/unlock_min_membersare first satisfied, never re-evaluated toNULLor recomputed on every action.- Per-
(steamid, clan)consent-tracking columns/table for the deposit consent prompt (last-interaction timestamp). - Idempotency and one-row-per-server rules from Phase 1's migration hazards apply identically here — this table is new, so there is no backfill complexity, but the shared-DB/no-serialisation hazard still applies to the tracking row for this migration itself.
Step 2 — Permission registration and pure functions
- Register
bank.deposit,bank.withdraw(bothBOOLEAN, notNUMERIC_LIMIT— there is no per-rank withdrawal cap, only a globalper_withdrawal_cap, so keeping the SQL predicate a boolean is what keeps the per-transaction cap out of SQL entirely) against Phase 1's registry. Nobank.configure_limitspermission — the tuning knobs are server operator config with no in-game editing surface, so there is nothing for a permission to gate (see bank.md). validateBankAmount(amount)— one pure function, called by deposit, withdrawal and reclaim. Rejects non-numeric, NaN, non-finite, non-integer,≤ 0, and> MAX_TRANSACTION($100,000,000). Write and review this before Step 3, since both statements depend on it — the boundary cases are listed in acceptance-criteria.md as a manual reference; automated coverage is out of scope for this plan (see Phase 1 Step 2 note on GLuaTest).- Rolling-cap and reclaim-window arithmetic (the pure calculation only, not its atomic enforcement, which is Step 3/4).
Step 3 — Atomic ledger append: deposit and withdrawal
The core of the module. One statement per action type; balance check,
permission check and ledger append happen in the same INSERT ... SELECT,
never a Lua read-then-decide-then-write — see the worked withdrawal statement
in bank.md.
- No transaction is used, and none is needed — this is the direct answer
to the
db.lua-has-no-transaction-API landmine for this operation. The ledger insert is the sole authoritative write (event sourcing per bank.md); theclans.bankcache update that follows is fire-and-forget and explicitly allowed to fail without rolling anything back. Consistency comes from doing the whole balance-check-permission-check-append as one MySQL statement (single-statement atomicity), not from a multi-statement transaction the codebase has no primitive for. seq = COALESCE(MAX(seq), 0) + 1computed inside the sameINSERT ... SELECT, rejected byUNIQUE (clan, seq)on collision. On a duplicate-key error, retry — recompute against the now-current stream. This is what replaces the old cache-row lock as the serialisation point across processes; an in-process Lua lock protects nothing since multiple game servers and the (now-disabled) web app share one MySQL instance.- Withdrawal's
EXISTSclause usesLEFT JOIN clan_rank_permissionsplusCOALESCE(p.value, ?)bound to the registered default — an innerJOINwould wrongly deny a rank relying on the unmaterialised default (BA-3). Deposit's statement has no permission predicate (bank.depositis universal) but does carry anEXISTScheck againstclans.bank_unlocked_at IS NOT NULL— deposits are blocked until the bank unlocks, same as withdrawals (see bank.md). Still the simpler of the two statements — build it first as a warm-up before the withdrawal statement. balance_aftercomputed and stored in the same statement — makes the current balance a single-row read and cache rebuilds cheap.- Rate limiting: 10s cooldown per player, shared across deposit/withdraw/ reclaim entry routes, enforced server-side at the entry point — not inside the retry path (BC-7/BC-8).
- Requires Step 0's affected-row fix to branch on 1-row/0-row/duplicate-key.
Step 4 — Self-reclaim
Same atomic-UPDATE shape as withdrawal, distinct action = 'reclaim', no
bank.withdraw predicate — eligibility is "this is your own deposit, within
self_reclaim_window," not a rank permission. Explicitly not counted
against rolling_withdrawal_cap (BR-5) — this needs its own predicate path
rather than reusing withdrawal's cap-check clause, since exempting it is
deliberate (closing the reclaim-starvation griefing vector), not an oversight
to fix later.
Step 5 — Cache, reconciliation and dissolution distribution
- Fire-and-forget
clans.bankcache update issued after the Step 3/4 ledger callback returns — never retried, never blocking, never a transaction failure. On any disagreement between cache andSUM(delta), the ledger wins; rebuild the cache and log the mismatch at Error (observability signal, not a fault) on clan load and periodically. - Dissolution distribution formula
(bank.md) reads the ledger only, never the
cache, and includes every SteamID that ever appears in the ledger — not
only current members. Runs synchronously as part of
/clan delete, no hold or countdown (see bank.md's arbitrage analysis for why a hold was rejected, not merely omitted — do not reintroduce one without re-solving that). - Payout delivery: online recipients get
GiveMoneythenSaveData. Offline recipients get a newremoterow type (notpmoney, which is moneypack-specific) — claim the row before granting (UPDATE remote SET done = 1 WHERE id = ? AND done = 0, proceed only if one row affected), never grant-then-mark. RequireIsValid(ply)andply._Initializedinside the handler. Targetserver = 'all'and rely on the claim-before-grantUPDATEfor exactly-once delivery — this is deliberately the opposite shape from thereloadclan/cache-invalidation use ofremote(Phase 1's landmine, "one row per server, neverserver = 'all'"): that case is fan-out (every server must see it), this case is single delivery (exactly one server must pay out), so the fix for one is not the fix for the other. Worth an explicit code comment at both call sites given how easy the two shapes are to conflate. - On
cityrp.remote.noSaveservers (build,event), return withoutdone()so a real server picks up the payout instead.
Step 6 — Rolling cap, unlock gate, consent prompt
- Clan-scoped rolling 24h withdrawal cap ($75,000 default), evaluated against the ledger (sum of withdrawals in the trailing 24h for the clan, not the actor) — closes promote→withdraw→demote cycling by construction, since the clock is per-clan, not per-person.
- Unlock latch: evaluate
age >= unlock_min_age AND members >= unlock_min_membersonly to decide whether to writeclans.bank_unlocked_atfor the first time. Every subsequent gate check reads the stored flag, never re-evaluates the condition — this is the direct fix for the "obvious implementation relocks it" trap called out in bank.md. - Deposit consent prompt: shown before deposit completes on first-ever
deposit to this clan, or after
consent_reprompt_days(30) since the player's last deposit or withdrawal or reclaim with this clan — passive viewing does not reset the clock. Stored per(steamid, clan). /clan undoguard: reject with a balance-nonzero check regardless of whether the 3-day unlock gate structurally prevents the scenario today — defensive guard in case the gate is ever loosened (BU-2).
Step 7 — Bank UI tab
Lights up the Step 5 placeholder from Phase 1. Balance, deposit (with consent
prompt), own reclaimable deposits — visible to everyone (bank.deposit is
universal). Full ledger — visible only to bank.withdraw holders. Deletion
confirmation gains the balance/distribution copy from
ui.md. All amounts/labels through cityrp.ui per
Phase 1's scaling requirement, same as every other tab.
Data migration
| Item | Action |
|---|---|
Orphaned clan_members rows |
Players may be silently in clans they believe they left, from failed joins. Audit and clean |
Non-contiguous rank IDs (live data has 8, 170) |
Backfill rank_order per clan by sorting on legacy rankid, preserving ties. Leave rankid untouched to preserve foreign keys |
clans.next_rankid |
Seed to MAX(rankid) + 1 per clan |
clans.join_rankid |
Backfill to any rank at MAX(rank_order); log the choice |
clans.short (varchar(2000)) |
Establish what it holds before the abbreviation migration — it may be worth keeping or safe to drop |
| Clans with zero members or no leader | Flag for manual review; do not auto-resolve silently |
Dead tables (clan, clan_has_players, clan_has_invite) |
Confirm unused, then drop |
clan_models, clans.model |
Dead once /clan model retires. Keep the rows until the model files themselves are decommissioned — they record which model belonged to which clan, which is the only remaining reference if a clan later asks |
| Rank/member caps (50 / 100) | Live maxima are 28 and 18, so nothing is currently over cap. Grandfather any that appear |
Deferred
Not in this plan. Recorded so the framework can be sanity-checked against them.
| Item | Notes |
|---|---|
| Web app | Repository disabled. If it returns, it must share the authorisation boundary rather than reimplementing it — see design.md |
/clan shift (#1327) |
A later Core rework. Essentially an additional gate on command execution, so it retrofits cleanly; no seam needs reserving now |
| Clan inventory (#1624) | Future module |
| Clan vehicles | Future module; clan_vehicles table already exists |
Clan models are retired, not deferred — see design.md.
Open risks
| Risk | Impact | Mitigation |
|---|---|---|
| In-game becomes the only management surface | With the web app gone, anything the menu cannot do, nobody can do | Audit the web app's feature set against the planned menu before Phase 1 closes — rank creation and permission editing especially |
/clan model retirement is player-facing |
Any clan still using it loses the feature | Low impact — clan activity is minimal — but announce before it lands rather than after |
| Rank IDs are non-contiguous in live data | Rebuilt logic may assume contiguity | rank_order is the only column compared; verify the backfill on a copy first |
| Orphaned membership rows | Players silently in old clans | Audit during migration |
| Word list curation is a content decision | Engineering cannot pick the list | Needs whoever owns moderation policy, before Phase 1 ships |
Open questions flagged during planning
Raised while adding step-level detail to Phases 1–2. Deliberately not resolved here — each needs a decision from whoever owns that call before the relevant step starts, not a default baked silently into the plan.
| # | Question | Where it bites | Notes |
|---|---|---|---|
| 1 | Does the in-game menu ship in Phase 1 Step 5 with a hidden/disabled Bank tab placeholder, or does the whole menu wait for Phase 2 so Bank lands complete? | Phase 1 Step 5, Phase 2 Step 7 | This plan assumes the former (ship Roster/Ranks/Invites/Identity in Phase 1, light up Bank in Phase 2) because the menu is the only clan management surface and #1541 has been open since ~2017 — holding it hostage to Bank's separate scope seemed like the wrong trade-off to make silently. Flagging in case the product/design side disagrees. |
| 2 | Is core.kick in Phase 1 Step 2 backend-only (console command / direct call for validation), with the Roster [Kick] button wired up later in Step 5? |
Phase 1 Step 2 vs Step 5 | Assumed yes, to keep the vertical slice minimal and UI-independent per the architecture review's stated purpose ("de-risk the framework shape"). If the intent was to validate the UI wiring too as part of the slice, Step 2's scope needs to grow. |
| 3 | Exact signature for the DB:RunChecked / DB:QueryParamsChecked affected-row-count change |
Phase 2 Step 0 | bank.md explicitly calls this out as needing lead-programmer agreement before implementation, not just before Bank ships. This plan places it as a blocking Step 0, but the actual sign-off conversation still needs to happen — it is not resolved by being scheduled. |
| 4 | Whether Phase 2's schema/data work can start on a server before every server in the fleet has completed Phase 1's clan_members re-key migration |
Phase 1 → Phase 2 boundary | The Bank withdrawal statement joins clan_members directly; if any server is mid-fleet-rollout on Phase 1's re-key, Phase 2's migrations landing on that same shared database could interact with rows in an inconsistent shape. This plan assumes Phase 1 is fully rolled out fleet-wide before Phase 2's Step 1 migration runs, but that is an assumption about rollout sequencing, not something derivable from the design docs alone. |