CityRP Documentation Repository

Clans Overhaul — Plan

Last updated

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.

  1. cityrp.wordfilter gamemode library (gamemode/core/libraries/, not under clans/). 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 in clans/, 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.
  2. 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 normal core.edit_identity gate 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 hold core.edit_identity is bypassed). Logged via cityrp.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).
  3. 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-wrapped init, collision checks on both the module id prefix (bank.withdraw, never a bare can_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 three kinds (BOOLEAN, THRESHOLD, NUMERIC_LIMIT), and the resolution order (stored value → declared default → fail-closed NULL with a cityrp.logging warning if nothing registers the id).
  • Schema for this step only: clan_ranks.rank_order (UNIQUE (clan, rank_order)) alongside the existing rankid, and clan_rank_permissions(clan, rankid, permission_id, value) with value INT NULL — not VARCHAR. This is the direct fix for the QueryParams landmine: DB:QueryParams binds every parameter as a quoted string (db.lua:239), so a VARCHAR value 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.kick needs 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/EXISTS clause (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 SELECT to produce a specific rejection message. The UPDATE/DELETE remains the only thing that grants.
  • Re-validate IsValid(ply) and ply._Initialized inside 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.value must already be INT (from Step 1) before this statement is written, or the ordinal comparison silently inverts per the QueryParams landmine 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.lua has 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 to rank_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 for clan_rank_permissions rejects any attempt to set core.manage_ranks false 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_members re-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.wordfilter with 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 of short (varchar(2000)) — check what live data in clans.short holds 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 to latin1 to match players._SteamID; text columns (name, motto, description, MOTD) are utf8mb4. This is also where defect 8's underlying cross-charset join gets fixed properly, not just the clan_member → clan_members typo — fixing only the typo leaves a latin1/utf8 join behind it.
  • self._Clan vs self.cityrp._Clan: Core's public API/accessors must not expose either raw field. Anything touching the old _Clan reads 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.kick and Step 3's setrank/ 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:ConnectToDatabase runs 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 UPDATE on the tracking row), or take the fleet down for this migration. Applies to every schema change in this phase, not only the clan_members re-key below.
  • clan_members.clan is currently int NULL, and a NULL cannot 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 a NULL clan: a membership record that names no clan is meaningless, not recoverable data. Then set the column NOT NULL, then re-key. Log the deleted rows.
  • Split the clan_members audit 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 configured clan_rank_permissions, dropping that table destroys their configuration. Take a verified backup immediately before, and document the restore procedure rather than implying down() 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.bank widened to BIGINT NOT NULL DEFAULT 0. This is the direct fix for the landmine: the column is currently int NULL, a 32-bit ceiling, and SET bank = bank + ? against a NULL row silently yields NULL — voiding the deposit with no error. Backfill any existing NULL to 0 as part of the same migration, before the NOT NULL constraint 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 when unlock_min_age/unlock_min_members are first satisfied, never re-evaluated to NULL or 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 (both BOOLEAN, not NUMERIC_LIMIT — there is no per-rank withdrawal cap, only a global per_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. No bank.configure_limits permission — 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); the clans.bank cache 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) + 1 computed inside the same INSERT ... SELECT, rejected by UNIQUE (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 EXISTS clause uses LEFT JOIN clan_rank_permissions plus COALESCE(p.value, ?) bound to the registered default — an inner JOIN would wrongly deny a rank relying on the unmaterialised default (BA-3). Deposit's statement has no permission predicate (bank.deposit is universal) but does carry an EXISTS check against clans.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_after computed 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.bank cache update issued after the Step 3/4 ledger callback returns — never retried, never blocking, never a transaction failure. On any disagreement between cache and SUM(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 GiveMoney then SaveData. Offline recipients get a new remote row type (not pmoney, 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. Require IsValid(ply) and ply._Initialized inside the handler. Target server = 'all' and rely on the claim-before-grant UPDATE for exactly-once delivery — this is deliberately the opposite shape from the reloadclan/cache-invalidation use of remote (Phase 1's landmine, "one row per server, never server = '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.noSave servers (build, event), return without done() so a real server picks up the payout instead.
  • 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_members only to decide whether to write clans.bank_unlocked_at for 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 undo guard: 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.