Clans Overhaul — Execution Strategy
Last updated
May be stale
This page was last updated ago, on 26 September 2026. Check what it says against the code before relying on it.
How the rebuild specified in design.md and sequenced in plan.md actually gets delivered against a live fleet.
Status: Draft for review. This document does not re-specify the system — core.md, bank.md and ui.md own that, and plan.md owns the per-step work breakdown. This document owns sequencing, gates, rollout mechanics and the decisions that must be made before code starts, plus the corrections a codebase audit found in the plan's ordering.
Why this document exists
plan.md phases the work by module: Phase 1 delivers Core, Phase 2 delivers Bank. That is the right decomposition of what to build, and nothing here changes it.
It is the wrong decomposition of when things can land, because the delivery risk is not module-shaped. It runs along three axes that move at completely different speeds:
| Axis | Rollout unit | Speed | Reversible? |
|---|---|---|---|
| Schema | The one shared MySQL database | Instant and fleet-wide — GM:ConnectToDatabase runs pending migrations on every server at boot |
Only by restore. down() is not a safety net once leaders have configured permissions |
| Code | One server at a time | Days — servers restart independently | Yes, by revert and redeploy |
| Policy | Moderation and product | Weeks, and outside engineering's control | n/a |
Phasing by module implicitly assumes these three move together. They do not, and the places where the plan assumes they do are exactly where it breaks:
- Phase 1's schema list mixes additive changes (safe while the legacy plugin still runs) with destructive ones (which break the legacy plugin the moment they land, on every server that has not yet taken the new code).
- Phase 2's
DB:*Checkedaffected-row prerequisite is placed in Phase 2, but Phase 1 Step 2 already depends on it. - The word-filter list curation has a longer lead time than the library that consumes it, and it is not on anyone's critical path today.
So this strategy re-cuts the same work into gates. A gate is a set of tranches that must all be complete before the next gate's work is safe to start. Module work slots into gates; it does not define them.
The gates
Gate A ── Make the ground safe ──────────────┐
(runner, fleet registry, affected rows, CI)│
├──> Gate C ── Core, additive only
Gate B ── Unblock other people ──────────────┘ │
(wordfilter library, staff force-rename) │
runs in parallel with A from day one ▼
Gate D ── Bank
│
▼
Gate E ── Retire legacy
(destructive migrations)
Gate A — Make the ground safe
Nothing in the rebuild lands until the mechanisms the rebuild depends on are themselves sound. Every item here is a pre-existing defect in shared infrastructure, every one is small, and every one silently corrupts a later phase if left. This gate is cheap and it is not optional.
A1 — Migration runner and fleet registry
Blocks every migration in the project, not just clans.
The tracking-row insert is not idempotent, and the failure mode is a halted
boot. Migrations.RunPending records an applied migration with a plain
INSERT INTO schema_migrations, against id INT NOT NULL PRIMARY KEY
(core/libraries/sv_migrations.lua:59-75, :129-190). When two servers boot
together:
- Both read
schema_migrationsand see migration N pending. - Both run
mig.up()— hence the idempotency requirement plan.md already states. - Both insert the tracking row. The loser gets a duplicate-key error,
insertOkis false, the runner logs Critical and returnscb(false, …). init.lua:114then logs "Startup halted: pending migrations failed. DB-dependent plugins will not initialize." — and does not retry or restart.
That server boots with its database-dependent plugins dead, because a migration
succeeded twice. The race window is the entire duration of mig.up(), which
for an ALTER on a live table is seconds to minutes — comfortably wide during a
fleet restart.
plan.md flags "ON DUPLICATE KEY UPDATE on the tracking row" as per-migration
discipline. It cannot be: the tracking insert lives in the runner, not in the
migration files, so no migration author can fix it. It is a one-line change to
RunPending (INSERT IGNORE, or ON DUPLICATE KEY UPDATE name = VALUES(name))
and it belongs before Phase 1's six-or-so migrations, not after.
Related, same file: on a successful apply the server issues changelevel
(init.lua:119). That is correct, but it means a fleet-wide clan migration
changelevels the whole fleet. Schedule accordingly; do not discover it live.
There is no fleet server registry, and two phases require one.
design.md and plan.md both require the
reloadclan fan-out to write "one row per server, with the server list derived
from configuration — never from literals in a command handler." No such
configuration exists. LLServer is assigned per-server in
gamemode/core/server/<name>.lua (22 files: v4b1_fl2, v33x, v2d,
usv2b, staging, beta, …), and the only enumeration of the fleet anywhere
in the gamemode is a partial display-name map in core/scoreboard/scoreboard.lua.
So "derive it from configuration" is itself a small piece of work that must land
before the defect-5 fix can be written correctly. Either a shared list, or an
enumeration of core/server/*.lua minus cityrp.remote.noSave. Pick one, put it
somewhere both sv_remote.lua and the clans module can read, and do it here —
otherwise the Phase 0 hotfix reintroduces hardcoded literals with fresher values,
which is defect 5 again with a longer fuse.
A2 — Phase 0 hotfixes, split by survivability
plan.md already lists these. The strategy note is only that they are two different kinds of work and should not share a review standard:
| Class | Files | Standard |
|---|---|---|
| Survivors — outlive the rebuild | libraries/sv_remote.lua (defects 4, 11), libraries/sv_model.lua (defect 12) |
Full standard. This code is permanent, and sv_model.lua's no-op remove affects every model consumer, not just clans |
| Throwaway — deleted in Gate E | plugins/clans/sv_init.lua (defects 1, 2, 3, 5, 8, 10) |
Stop the bleeding. Timebox it, do not refactor, do not add tests to code with a scheduled deletion date |
The one exception inside the throwaway set is defect 1 (/clan accept not
rejecting an existing member and not checking the INSERT result). It is the
root cause of #464, #336 and #713 — three separate player-facing issues — and its
fix is the application pre-check that Gate C's clan_members re-key later relies
on as its primary control. Get that one right; it is the only Phase 0 fix with a
successor that depends on it.
Depends on A1's fleet registry for defect 5.
A3 — DB:RunChecked / DB:QueryParamsChecked affected rows
Moved forward from plan.md's Phase 2 Step 0. Phase 1 Step 2 already needs it.
plan.md places this in Phase 2 as Bank's prerequisite. But Phase 1 Step 2 states
that for core.kick, "affected-row count is the only success signal; 0 rows
means denied for any reason" — and today there is no path to that count:
-- db.lua:187-190
if query:find("INSERT INTO") then
cb(true, r.lastid)
else
cb(true, r.data) -- a DELETE/UPDATE yields no rows: no count, no signal
end
core.kick's statement is a conditional DELETE, so it lands in the else
branch and gets nothing back. Worse for Bank, whose atomic append is an
INSERT ... SELECT: it matches INSERT INTO, so it returns r.lastid — a
plausible-looking non-error value that is indistinguishable between one row
appended and zero rows appended. A silent-wrong-answer is a worse starting
point than an empty one.
Every atomic statement in both phases branches on this count. It is a Gate A item.
This also answers plan.md's open question 3 (the exact signature). The count
is not something that needs computing — tmysql4 already returns affected in
the same per-statement result table db.lua reads status, error, data and
lastid from; the wrapper simply never reads it. Confirm the field name against
the module, then:
- Recommended: add it as a fourth callback argument —
cb(ok, result, err, affected). Existing callers destructure positionally, so appending is the only non-breaking shape. - Rejected: changing what
resultmeans for non-INSERTqueries. Silent breakage of every existingRunCheckedcaller, for no gain.
Still needs the lead-programmer sign-off plan.md asks for — but the conversation is now about one appended argument, not about a design.
A4 — Make the acceptance criteria enforceable
acceptance-criteria.md and plan.md directly contradict each other, and the contradiction has to be resolved before Step 2 rather than discovered at sign-off.
- acceptance-criteria.md marks pure-fn and db-script evidence BLOCKING — roughly 90 criteria.
- plan.md says twice that "automated coverage is out of scope for this plan (GLuaTest is unconfigured in this repo)".
GLuaTest is not unconfigured. ci.test.yml
runs on every pull request, and its Unit Tests and Integration Tests jobs
are required checks on master. The integration run boots each leg against a
throwaway MariaDB database. lua/tests/cityrp2/libraries/ holds working test
files, including the callback-queuing mock that acceptance-criteria.md itself
recommends reusing.
So there is no framework or trigger to add; the criteria need cases:
| Evidence class | What it needs | Recommendation |
|---|---|---|
| pure-fn (~70 criteria: threshold formula, validators, distribution maths) | Unit cases under lua/tests/cityrp2/ |
Do it. The unit job already gates every pull request, so each case turns a BLOCKING criterion from aspiration into a gate |
| db-script (PERM-SQL1, BA-1…BA-7, BD-11, LIFE-9, REG-4/5/8) | Integration cases, which run against a real database | Do it. These are the criteria that catch the QueryParams lexicographic inversion and the UNIQUE (clan, seq) race — nothing else can catch them, by their own definition |
| integration | A running server | Manual, as today |
A BLOCKING criterion that ships without a case should be downgraded to manual in acceptance-criteria.md. Shipping a specification whose BLOCKING gate nothing enforces is worse than shipping an honest advisory one.
Gate B — Unblock other people
Phase 1a exactly as plan.md scopes it: the cityrp.wordfilter gamemode library
and the staff force-rename/force-retag ! command. No clan-specific work lands
here. Runs in parallel with Gate A from day one.
Two strategy notes:
Start the moderation ask on day one of Gate A, not when the library is done. The engineering is bounded and well-specified. The list contents are a moderation decision with no engineering lever, and plan.md's own risk register names it: "Word list curation is a content decision. Engineering cannot pick the list." It is the longest-lead-time item in the whole programme and it is currently on nobody's critical path. It needs a named owner and a date before Gate A closes.
Ship with a seeded reserved list so Step 4 is not hostage to the other two.
Splitting reserved / slur / profanity into independently-curated categories
was the right call, and it has a scheduling payoff worth taking deliberately:
reserved (admin, mod, staff, owner, limelight) is a factual list of
things that impersonate this server, not a moderation judgement — engineering can
seed it. That covers the abbreviation case, which
core.md identifies as the load-bearing one now that
the limit is 7 characters and ADMIN/STAFF/OWNER all fit. Gate C Step 4 can
then proceed with slur and profanity still empty, and those lists drop in
later with no code change.
Gate C — Core, additive only
Steps 1–6 run as plan.md sequences them, and
the vertical-slice gate at Step 2 is honoured as written: if core.kick
requires reshaping the registration API, the accessor contract or the statement
pattern, that reshaping happens once, at Step 2, before Step 3 builds nine more
actions against the same shape.
What changes is the schema sequencing, and it changes because of the axis mismatch at the top of this document.
The rule for this gate
Schema is fleet-wide and instant; code is per-server and gradual. Therefore every migration in Gate C must be tolerable by the legacy
sv_init.luastill running on servers that have not yet taken the new code.
plan.md's "Schema work landing in Phase 1" is a single list that mixes changes of three very different risk classes. Split it:
| Class | Contents | May land |
|---|---|---|
| C1 — Additive | clans.abbreviation, clans.description, clans.created_at, clans.state + deleted_at, clans.motd, clans.join_rankid, clans.next_rankid, clan_ranks.rank_order (UNIQUE (clan, rank_order)), clan_rank_permissions (value INT NULL), widening clans.name/motto, charset normalisation of join keys to latin1 |
Immediately. Legacy code neither reads nor writes any of these. All of Gate C builds against them |
| C2 — Constraint-tightening | clan_members re-key to (clan, steamid); clan set NOT NULL after deleting NULL-clan rows |
Only once Step 3's invite/accept pre-check is live on every server. The pre-check is the primary control; the constraint is defence in depth and lands second |
| C3 — Destructive | Drop clan_ranks.invite/kick/setrank/motd/forum; drop clan, clan_has_players, clan_has_invite; drop clans.short, clans.model, clan_models |
Gate E only — see below |
C2 has a trap that must not be split across migrations. plan.md correctly
sequences the re-key last, and core.md
correctly says clan_members should be keyed (clan, steamid) "with a separate
unique constraint on steamid if one-clan-per-player is retained". Those two
statements have to be executed as one migration. Dropping
PRIMARY KEY (steamid) without simultaneously adding UNIQUE (steamid) does not
merely remove defence in depth — it converts defect 1 from "the INSERT fails and
the player reverts on reconnect" into "the INSERT succeeds and the player is
genuinely in two clans", on every server still running the legacy plugin. That
is strictly worse than the bug being fixed, and it is a data-corruption outcome
rather than a display one.
C3 is why Gate E exists. core.md says
to drop clan_ranks.invite/kick/setrank/motd "once clan_rank_permissions
replaces them", and plan.md folds that into Phase 1. But legacy loadclan
SELECTs rankid, rankname, invite, kick, setrank and legacy clan creation
INSERTs all of them (clans/sv_init.lua:339, :605). Dropping those columns
while any server runs legacy code breaks clan loading and clan creation on that
server outright. The drops are correct; they are simply the last tranche, not
part of Phase 1.
Schema the design docs do not currently account for
A codebase audit against the live baseline found four columns the design assumes
exist and which do not. clans is untouched since the baseline migration and
holds exactly id, name, short, bank, model, _Inventory, motto (migrations run
to 0009; clan work starts at 0010). All four belong in C1.
| Missing | Why it is needed | Notes |
|---|---|---|
clans.created_at |
unlock_min_age (3 days) is the primary control closing the deposit-then-undo scam, and it is unimplementable without a clan age. There is no creation timestamp anywhere — the dead clan singular table has foundingDate; the live clans table has nothing |
Backfill from MIN(clan_members.time) per clan; log any clan with no member rows. Must land in Gate C, not Gate D — Bank cannot add it retroactively with a meaningful backfill |
clans.state + deleted_at |
Invariant 7 ("COUNT(members) ≥ 1 OR state ≠ active", "soft-delete zombie clans"), the lifecycle table's "clan active", accept-time re-validation, and bank.md's "the ledger outlives the clan" (BD-5) all presume a soft-delete state. Today /clan delete hard-DELETEs the clans row, so a dissolved clan's ledger rows would reference an id that no longer resolves to anything |
Needs an explicit decision — see Decision 9 |
clans.motd |
Core lists MOTD in scope and specifies it at 0–2000 bytes with a core.motd_edit permission, but no MOTD text is stored anywhere today. clan_ranks.motd is a permission column, not content |
MOTD is a new feature, not a port. Size Step 4 accordingly |
clans.description |
Specified at 0–500 bytes. description exists only on the dead clan singular table; the live clans table has none |
Same as above — new, not ported |
Two smaller ones in the same class: clans.motto is varchar(70) against a
spec of 80 characters / 320 bytes, and clans.name is varchar(40) against 32
characters / 128 bytes utf8mb4. Both need resizing in C1;
core.md says so, plan.md's schema list
omits them.
Separately, and cheaply fixed once created_at exists: /clan undo's 300-second
entitlement is held in a Lua table (cityrp.clans.undo, sv_init.lua:8), so a
server restart inside the window silently destroys it. Criteria BU-1 and BU-2
assume it survives. With created_at stored, the window becomes derivable rather
than remembered.
Where the gate ends
Gate C closes when Steps 1–6 are complete and deployed to every server in the
fleet, not when they merge. Gate D's withdrawal statement joins clan_members
and clan_rank_permissions directly; a server mid-rollout is a server joining
against a table shape its code does not expect. This is plan.md's open question
4, and the answer is yes, it is a hard gate.
Gate D — Bank
plan.md Phase 2 Steps 1–7 unchanged, with Step 0 already delivered in Gate A.
The only strategy note: Step 3's atomic ledger append is the highest-risk
single artifact in the programme. It is one statement carrying a balance check,
a permission check, a sequence allocation and an append; its correctness depends
on value being INT (C1), on LEFT JOIN + COALESCE resolving unmaterialised
defaults (BA-3), on UNIQUE (clan, seq) serialising cross-process writers (BA-4),
and on the affected-row count from A3 to branch at all. Four separate Gate A/C
prerequisites converge on one query.
Build the deposit statement first, as plan.md says — it is the same shape
with one EXISTS clause instead of two, and it validates the whole mechanism at
lower stakes. Do not start withdrawal until deposit's db-script criteria pass.
Gate E — Retire the legacy plugin
The tranche plan.md does not currently have, and the one that lets C3 land.
- Every server is on the new module (verified, not assumed).
- Remove
core/plugins/clans/sv_init.lua's legacy paths and thecityrp.modelregistration for clan models (plan.md Step 6's fail-closed retirement). - Leave
cityrp.command.redirectToAcommandshims where a/command moved, per AGENTS.md footgun #14. - Then run the C3 destructive migrations, with a verified backup taken immediately beforehand.
plan.md is right that down() is not a safety net once leaders have configured
clan_rank_permissions. State the restore procedure in the migration's own
comment, not only in the plan.
Rollout mechanics: how the cutover actually happens
The design docs specify the destination and the work, but never name a mechanism for running new and legacy clan code side by side — and something has to, because a half-migrated clan system on live is the worst available state.
This repo already has the right primitive, and AGENTS.md footgun #19 names it:
.meta.lua availability conditions, not a file-scope LLServer gate.
- The rebuilt module carries
.meta.luawithservers = {"staging", "beta"}. Flip to fleet-wide when Gate C's criteria pass. - The legacy plugin carries the mirror
not_serversfor the same set. - The loader excludes the excluded plugin before any of its realm files,
sub-resources, entities or derma register — which a
if LLServer == … then return endat the top of a file does not do, on either realm.
Two constraints this creates, both worth stating explicitly in the tranche that introduces it:
Mutual exclusion is mandatory. Both plugins define cityrp.clans and the
same /clan command names. The two .meta.lua condition sets must be exact
complements — a server matching both, or neither, is a broken server. This is
also duplicate-registration territory (footgun #7): the new module's registration
API must already distinguish "declarations replace" from "side effects guard"
before this canary runs, or a hot reload during testing produces exactly the
duplicate-command class of bug the framework contract exists to prevent.
The clans plugin gains a client realm. It is server-only today — a single
sv_init.lua, no .meta.lua at all — and footgun #18 makes that load-bearing
for anything declaring a dependency on it. Step 5's menu adds cl_init.lua and
derma, changing its realm profile. Decide at Step 5 whether the panel moves from
core/derma/cl_clans.lua into the plugin's own derma/ sub-resource
(recommended — the module should own its surface) or stays in core. Either way
the F1 shell integration is confirmed to be exactly what
ui.md claims: cityrp_Clan is registered but absent from
f1_categoriesToBeMade (core/derma/cl_main.lua:188), so adding the string is
the whole shell-level cost.
Decisions needed before code starts
Ordered by how early they bite. Each has a recommendation; none is resolved by being listed.
| # | Decision | Bites at | Recommendation |
|---|---|---|---|
| 1 | Is pure-fn evidence actually BLOCKING? | Gate A4, before Step 2 | Yes. Add pull_request triggers to ci.test.yml. |
| 2 | db-script evidence: MySQL service container in CI, or downgrade to manual? | Gate A4 | Container. PERM-SQL1, BA-1…7, BD-11, LIFE-9 and REG-4/5/8 are unverifiable any other way, by their own definition. If no, amend acceptance-criteria.md honestly |
| 3 | DB:*Checked signature (plan.md OQ3) |
Gate A3 | Append affected as a fourth callback argument. tmysql4 already returns it; nothing is computed |
| 4 | Menu ships in Gate C with a hidden Bank tab, or waits for Gate D? (plan.md OQ1) | Step 5 / Step 7 | Ship in Gate C. #1541 has been open since ~2017 and the menu is the only management surface; holding it for Bank's separate scope is the wrong trade |
| 5 | Is the core.kick slice backend-only? (plan.md OQ2) |
Step 2 | Yes. The slice exists to de-risk the framework shape; adding UI to it widens the thing being validated |
| 6 | Is Gate D hard-gated on Gate C being fleet-complete? (plan.md OQ4) | Gate C → D | Yes. Bank's statements join Core's tables directly |
| 7 | Who owns word-list curation, and by when? | Gate B, blocks Step 4 | Needs a named owner and a date, not an assignment to "moderation". Longest lead time in the programme |
| 8 | Clan deletion: soft-delete with clans.state, or keep the hard DELETE? |
Gate C, C1 | Soft-delete. Invariant 7 and bank.md's "the ledger outlives the clan" both assume it; the current hard delete leaves ledger rows pointing at nothing |
| 9 | Ship NUMERIC_LIMIT with no consumer? |
End of Gate C | core.md already flags this — bank.withdraw became boolean, so nothing claims the kind. Decide at Gate C close: ship it as a declared kind with no consumer, or drop it until a module needs it |
| 10 | Does the web app's feature set exceed the planned menu? | Before Gate C closes | plan.md's own risk: with the web app gone, anything the menu cannot do, nobody can do. Audit rank creation and permission editing specifically |
Critical path
A1 runner + fleet registry
└─> A2 hotfixes (survivors first)
└─> A3 affected rows ─────────────┐
└─> A4 CI trigger ────────────────┤
├─> C1 additive schema
B wordfilter + force-rename ────────┤ └─> Step 1 skeleton
(start curation ask on day 1) │ └─> Step 2 core.kick ◄── framework validation gate
│ └─> Step 3 lifecycle
│ └─> C2 re-key (+ UNIQUE, same migration)
└────────────────────────────────> Step 4 identity (needs B)
└─> Step 5 menu
└─> Step 6 retire models
└─> Gate D Bank
└─> Gate E + C3 drops
The two real critical-path risks are not code. They are word-list curation (Gate B, no engineering lever, blocks Step 4) and the CI decision (Gate A4, which determines whether ~90 BLOCKING criteria mean anything). Both should be resolved in week one, while Gate A's engineering is in flight.
The one gate that must not be rushed is Step 2. Nine further actions in
Step 3 and every module permission thereafter are built against whatever shape
core.kick proves out.
Tracking
Issue #1892 "Clan System" is the Scheme of Work, currently carrying one sub-issue (#1624, the deferred Inventory module). The tranches above map onto it as sub-issues, one per tranche, so the gate structure is visible on the board rather than living only in this document.
Per .claude/skills/github-issues/SKILL.md:
Priority and Effort are org issue fields set on the issue, not project
fields and not labels — gh project item-edit is the wrong tool for everything
except Status.
Suggested breakdown, with the issues that already exist folded in:
| Tranche | Existing issues it closes or advances |
|---|---|
| A1 runner + fleet registry | — (new) |
| A2 hotfixes | #464, #336, #713 (defect 1) |
| A3 affected rows | — (new; unblocks Bank) |
| A4 CI trigger | — (new) |
| B wordfilter + force-rename | — (new; prerequisite for Step 4) |
| C Core Steps 1–6 | #1541 (in-game menu), #1318 (radio overhaul, affects Core radio) |
| D Bank | #1418, #1600 |
| E retire legacy + C3 | — (new) |
| Deferred | #1624 (inventory), #1327 (/clan shift), clan vehicles |
References
- design.md — module framework, surfaces, defect register
- plan.md — phase and step work breakdown
- core.md — Module 1 specification
- bank.md — Module 2 specification
- ui.md — in-game menu route
- acceptance-criteria.md — testable conditions
- plugins.md —
.meta.luaavailability conditions and sub-resources - coding-standards.md — the diff-scoped lint gates every tranche runs