Clans

Clans are player-formed groups with a shared identity, a rank ladder, and group-scoped features. This document specifies the modular architecture the system is being rebuilt onto, and indexes the per-module design documents.

Status: In Design — not yet implemented. The system described here replaces the legacy implementation in core/plugins/clans/. See plan.md for the work breakdown and strategy.md for delivery sequencing, gates and rollout.

Design principle: modules, not a monolith

The legacy system put membership, ranks, radio, models and administration into a single 700-line server file with a companion PHP website writing to the same tables. Every new clan feature was added by extending that file, and the two surfaces drifted apart.

The rebuild inverts this. A small core owns clan identity, membership and ranks, and exposes a registration API. Everything else — bank, inventory, vehicles — is a module that registers against the core and owns its own tables, permissions, UI tab and commands.

The organising unit is the activity a clan does together, not the code that happens to implement it.

Why

• Modules ship independently. Core can go live before the bank is designed.
• A module's failure is contained; a broken bank must not prevent clan chat.
• Permissions, UI and persistence follow one declared pattern per module rather than being reinvented each time.
• Horizon features (inventory, vehicles) get a defined seam to land against instead of requiring core surgery.

Module registry

Module	Scope	Status	Design
Core	Identity, membership, ranks, radio, MOTD, administration	In design	core.md
Bank	Shared clan funds — deposits, rank-gated withdrawals, audit ledger	In design	bank.md

The in-game menu, the only management surface, is specified in ui.md. | Inventory | Shared item storage (ammunition, weapons, food) | Deferred | — | | Vehicles | Shared vehicles, callsigns, rank-gated access | Deferred | — | | Models | Clan-restricted player models | Retired — see below | — |

Deferred modules are not designed here. They are listed so the framework can be validated against their known requirements.

Retiring clan models

/clan model is removed rather than ported. Clan activity is currently minimal, the model files remain as game content and can be spawned manually if needed, and the feature carries two of the legacy defects on its own (the shared clanskinchanger timer name and an unguarded nil index).

Retirement is fail-closed. Two independent paths exist today:

• /clan model calls player:SetModel() directly, bypassing validation entirely and relying on its own lookup.
• loadclan calls cityrp.model.register(path, fn) so clan models also appear in the normal model-change flow.

cityrp.model is an allowlist whose access() returns false by default, and the consumer rejects any model absent from the registry (metatables/sv_player.lua:1292). Removing the registration therefore makes clan models unavailable, not unrestricted — there is no risk of leaking clan-exclusive models to all players.

Retiring also moots the "grants access to /clan model" element of /clan shift (#1327), which is separately deferred.

Framework contract

A module declares itself to Core at load time. Core validates the declaration and owns all shared state; modules own only their own tables.

Registration validation

Rule	Why
Permission keys must be prefixed with the module's own id (bank.withdraw, never can_kick)	Prevents a module silently overwriting a Core permission while adding its own
Declared tables must be prefixed (clan_bank_*) and checked for collision at registration	Same, for persistence
A second registration of the same module id is rejected or no-ops	Duplicate registration on hot reload is a documented recurring bug class in this codebase
Each module's init runs inside pcall	On failure the module is marked inert and logged via cityrp.logging; Core and sibling modules continue

The pcall rule is what makes "a broken bank must not prevent clan chat" true rather than aspirational.

State access

Modules read membership, rank and permission state only through Core accessors — never by indexing Core's tables or querying clan_members / clan_ranks directly. Only Core writes those tables.

Accessors must return copies or read-only views, never live references. This is not hypothetical: the legacy code already assigns ranks = cityrp.clans.defaultranks by reference (defect 6 below), so every clan created in a session shares one mutable table. In a modular system that same mistake becomes a cross-module privilege bug — a module mutating what it believes is its own copy would corrupt Core's live permission state for every clan at once.

No caching of permission results. Modules call hasPermission fresh per action. A cached "this player may withdraw" survives a demotion and reopens the ordering attack the atomic-write pattern exists to close. Bank-action volume is low enough that this costs nothing. If a future module genuinely needs caching, Core must expose a synchronous onRankChanged hook and subscription must be mandatory, not optional.

Honest limit

Lua modules cannot be sandboxed from each other at runtime — any module has full VM access and could bypass all of the above. These rules are API discipline backed by review, not enforcement. A mechanical grep for DB_MAIN queries against clan_members / clan_ranks outside Core is a cheap review check, and belongs in CI rather than in a reviewer's head.

Correctness rules for every module

Re-validate players inside async callbacks, never before. Every operation here is an async database round trip followed by a player notification, and the player can disconnect in between. Check IsValid(ply) and ply._Initialized inside the callback. This is the exact misunderstanding behind defect 2, and it is the defect most likely to recur, because it is a language-level trap rather than a clan-specific bug.

Critically: the state mutation, the ledger write and any remote row happen regardless of player validity. Only the notification is conditional. Money must not fail to move because the actor closed the game between the statement and the message.

Explain rejections with a separate diagnostic read. The conditional statement returns only an affected-row count, so "rejected" carries no reason. Do not surface that to players — today they get "You do not have the ability to kick a player with that rank", and losing it is a real supportability regression for them and for staff. On a zero-row result, run a non-authoritative diagnostic SELECT purely to produce the message. The UPDATE remains the only thing that grants; the SELECT only explains, and runs on the failure path only.

There is nothing confidential to protect here — clan members can read the entire rank ladder in the UI.

Hot reload: declarations replace, side effects guard. Re-registering a module must replace its permission declarations, so new code takes effect during development. Anything with a side effect — timers, hooks, net strings, commands — must guard against double registration instead. "Rejected or no-ops" is not a specification; these two halves need opposite behaviour.

Surfaces

The in-game menu is the only surface in scope. The companion web app is out of scope and its repository is disabled — it is neither being rebuilt nor maintained as part of this work, and the ?dev=true escalation it carried is not reachable while it is offline.

The rest of this section records the constraints that apply if a web surface returns. It is deliberately retained rather than deleted, because the conclusions were expensive to reach and the failure modes recur.

Constraint: there is no in-game API to call

A Garry's Mod server can make outbound HTTP requests but cannot host an inbound listener. The web app therefore cannot call into the game server, and clan modification cannot be funnelled through a single Lua implementation. The web app must implement its own write path against the database.

This means the authorisation logic exists twice, in two languages. That is not ideal and cannot be designed away — but it can be managed, and drift between the two copies is precisely what produced the ?dev=true escalation and the dead motd / forum permission columns.

The authorisation boundary is the SQL statement

Rather than a single implementation, the design uses a single statement. Every mutating action is expressed as one parameterised conditional UPDATE whose WHERE clause carries the entire permission check. Both surfaces issue the same statement; neither can grant more than the statement permits, because neither evaluates the permission in its host language at all.

The threshold formula is fully expressible as SQL predicates — the actor's rank, the target's rank and the stored threshold are three joins and two comparisons. See core.md for the formulas and bank.md for the worked withdrawal case.

Consequences:

• The canonical statements are a specification artifact. They belong in this documentation and, ideally, in a shared .sql file both surfaces load rather than two hand-copied strings. Duplicated string literals across a language boundary are a known recurring defect class in this codebase.
• Affected-row count is the only success signal. Zero rows means rejected, for any reason. Neither surface reports which reason.
• Stored procedures are the stronger form — one definition in the database, called by both. Recommended if the team is willing to take on the deployment and migration overhead; the shared-statement approach is the fallback if not.

Rules that still hold on both surfaces

• Identity comes only from what the transport authenticated — the server-side Player object in game, the Steam OpenID session on web. Never a SteamID or rank read from a request body or query string.
• No "already authorised" assertion is ever accepted from a client. This is the general form of the ?dev=true bug and must not recur on either surface.
• No permission bypass flag may exist at all — no debug mode, no sudo parameter, no environment toggle that skips a check. If a developer override is genuinely needed, it belongs behind the existing staff admin system, not a parallel one.
• Rendered UI is not authorisation. Disabling a control in the page is a usability affordance; the statement is the control.

Keeping the two implementations honest

• Permission declarations are data, not code. The registry (id, kind, default) lives in the database so both surfaces read the same definitions rather than each hardcoding a list. Adding a module's permissions must not require a PHP change.
• A shared conformance fixture set — a rank ladder, stored permission values, and a table of (actor rank, action, target rank) → expected allow/deny — that both implementations are tested against. This is the practical anti-drift control; without it, the two copies will diverge again.

In-memory state remains game-authoritative

The game server caches clan state in memory. Any web write must therefore notify the affected servers to reload — see Replacing the remote channel. Today that notification is broken in three separate ways (defects 3, 4 and 5), which is why web edits appear not to take effect until a map change.

Current state

This section records what exists today and why it is being replaced. It is evidence for the rebuild, not a specification.

What is implemented

• core/plugins/clans/sv_init.lua — chat commands /clan create|undo|delete|invite|uninvite|accept|decline|kick|setrank|setleader|leave|info|model and /cradio, plus the !updateclans staff command.
• core/derma/cl_clans.lua — the in-game clan panel. It renders a background image and the text "Coming soon...". There is no in-game clan UI.
• An external PHP site (services/clan) providing Clan Info, Commands, Rank Manager, User Management and Invites. It writes directly to the shared MySQL database and signals game servers by inserting rows into the remote table.

Confirmed defects

These are root causes, each verified against code and, where noted, against an open issue.

#	Defect	Effect
1	clan_members is PRIMARY KEY (steamid); /clan accept does not check existing membership, and the failed INSERT is not error-checked — the code sets _Clan and announces success regardless	A player already in a clan appears to join a new one, then reverts to the old clan on reconnect. Root cause of issues #464, #336, #713
2	cityrp.clans.kick and .setrank return from inside the async DB callback, so they always return nil to their caller	The if(kick) / if(setrank) follow-up blocks never execute; a kicked or promoted player's live state is never updated
3	!updateclans names its callback parameter results but reads result	The command clears cityrp.clans.clans and reloads nothing — the documented recovery path wipes state
4	deleteclan remote handler compares row.value1 (string) against _Clan (number), and returns without calling done() when the actor is offline	Clears no members; the row is retried every 60s indefinitely
5	In-game /clan delete inserts deleteclan rows hardcoded for v2d, v33x, v4b1. Live is v4b1_fl2, a distinct LLServer value, and cityrp.remote.check() matches on exact string equality	Live has never received a deleteclan notification. v4b1 ≠ v4b1_fl2
6	New clans assign ranks = cityrp.clans.defaultranks by reference	All clans created in one server session share one mutable ranks table
7	loadclan selects only rankid, rankname, invite, kick, setrank	The motd and forum permission columns are editable on the web but gate nothing in game
8	setleader for an offline target queries INNER JOIN clan_member (table is clan_members)	The offline path always fails with a SQL error
9	timer.Create("clanskinchanger", ...) uses a fixed global timer name	Two players changing models concurrently collide; the second cancels the first
10	/clan info and /clan model index cityrp.clans.clans[player._Clan] without a nil guard	Runtime error for players not in a clan, or for unloaded clans
11	reloadclan and deleteclan both open if not cityrp.clans then done() end — no return. Execution falls through and immediately indexes cityrp.clans.lastreload	The guard marks the row done and then errors on the next line. In sv_remote.lua, so it survives the rebuild — Phase 0 work, not throwaway
12	cityrp.model.register appends a new condition closure on every loadclan, growing conds unboundedly across reloads; cityrp.model.remove calls table.RemoveByValue on a key-indexed table and is a silent no-op	Model retirement moots the clans half, but the remove bug affects every other consumer, and the append is the same duplicate-registration class the framework guards against

Web app security

The companion site derives its devmode flag from $_GET["dev"], and devmode is the bypass condition on the leader-only rank manager, the kick permission check and the rank-ceiling check. Any authenticated member can obtain full clan leader authority by appending ?dev=true.

This is being tracked and fixed independently of the rebuild — it must not wait on this design. See plan.md.

Dormant schema

Present in the database with no implementation. Listed because the rebuild should either use or drop each one, not inherit them silently.

• clans.bank — adopted by the Bank module.
• clans.motto, clan.description, clan.abb — adopted by Core identity.
• clans._Inventory — reserved for the deferred Inventory module.
• clan_vehicles — full table, reserved for the deferred Vehicles module.
• clan_ranks.forum — web-only permission; the web app is out of scope.
• clan_models and clans.model — become dead once /clan model is retired.
• clan (singular), clan_has_players, clan_has_invite — dead duplicates of clans, clan_members, clan_invites. Candidates for removal.

Player fantasy

[To be designed.]

Ranks and permissions

Specified in core.md.

Membership lifecycle

Specified in core.md.

Security model

Trust boundaries are in Surfaces; module isolation is in Framework contract. Per-module controls are in core.md and bank.md.

Replacing the remote channel

remote is currently an unauthenticated command channel. cityrp.remote.check() polls every 60 seconds and executes whatever row it finds — kick, deleteclan, money_give, and announceupdate (which reaches the server console). Anything with INSERT rights on that table can command every game server it targets, and a game server cannot distinguish a real web-app row from any other writer's.

With the web app out of scope, remote is not needed as a command channel for clans at all. Its only remaining clan job is cache invalidation between game servers, which share one database and each cache clan state in memory: "clan N changed, reload it."

Narrow it accordingly:

• Clan use of remote reduces to a single idempotent, side-effect-free type (reloadclan). Reloading a clan the caller has no authority over leaks nothing and changes nothing, so the unauthenticated-channel risk largely evaporates for this use.
• Do not add clan mutation types (kickmember, setrank, …) to remote. A row that performs a privileged action on behalf of an unauthenticated writer is the exact shape of the existing problem.
• The dangerous existing types (kick, money_give, deleteclan) are outside this rebuild's scope but should be tracked separately — a compromised or over-granted DB account can still drive them.
• Fix the hardcoded server list (defect 5) and the missing done() / string-vs- number comparison in the handler (defect 4), or reload notifications will keep silently failing.

One row per server, from configuration — not server = 'all'. This is counter-intuitive and worth stating precisely, because the obvious fix is wrong.

cityrp.remote.check() selects WHERE done = 0 AND (server = 'all' OR server = <this server>), and checkRow hands the handler a done closure that runs UPDATE remote SET done = 1 WHERE id = <id> — marking the single row done for everyone. A server = 'all' row is therefore consumed by whichever server polls first, and no other server ever sees it. That is correct for a filtered action targeting one server, and wrong for fan-out — and cache invalidation is fan-out.

The legacy /clan delete writing one row per server was structurally right. Its defect is that the list is three literals (v2d, v33x, v4b1) while live runs as v4b1_fl2, matched by exact string equality, so live never matched any of them.

Write one row per server, with the server list derived from configuration. Never from literals in a command handler.

Data model

[To be designed — tables retained, retired, and migrations required.]

Edge cases

[To be designed.]

Dependencies

[To be designed.]

Tuning knobs

[To be designed.]

Acceptance criteria

See acceptance-criteria.md — permission formulas, invariants, membership lifecycle, identity validation, the bank module, and regression criteria for all twelve defects.

Open questions

#	Question	Owner	Notes
1	Are clan models still wanted?	—	Resolved — retire the command. Model files stay as game content and can be spawned manually. Clan activity is minimal, so player impact is low. See Retiring clan models
2	Should the module framework support third-party/addon modules, or core-only?	—	Affects how defensive the registration API must be
3	Player suggestions were referenced in planning but not yet reviewed	—	A set of clan suggestions exists that has not been passed through; they may add requirements
4	Does /clan shift (#1327) belong to Core or its own module?	—	Resolved — out of scope. Comes later as a Core rework. It is essentially an additional gate on command execution, so it retrofits cleanly and does not need a seam reserved for it now
5	Should the one-clan-per-player rule be enforced in logic rather than by primary key?	—	Resolved: enforce in application logic before the write, at both invite-send and invite-accept; keep the primary key as defence in depth. See core.md invariant 3
6	Permission model: hybrid capability registry, or plain booleans plus a seniority ordinal?	—	Resolved: hybrid capability registry, for module extensibility. See core.md
7	How should UI present a tier containing several co-equal ranks?	—	Moot — co-equal ranks were scrapped. Ranks are a strict ladder, so a plain ordered list is correct
8	Concrete values missing for byte ceilings, rate limit and line caps	—	Resolved. Byte ceilings are 4× the character limit; Description and MOTD are byte-counted with no line caps; bank cooldown is 10s per player; MAX_TRANSACTION proposed at $100,000,000
9	Does the ledger write share a transaction with the balance mutation?	—	Moot. The ledger insert is now the authoritative write and the cache is a fire-and-forget derived value, so no transaction is needed. See bank.md

References

• strategy.md — delivery gates, rollout mechanics and the decisions that must be made before code starts
• Issue #1418 — Clan/Business Account (source suggestion for the Bank module)
• Issue #1541 — In-game F1 Clan Menu (player demand for an in-game surface)
• Issue #1624 — Clan Inventory (deferred module requirements)
• Issue #1327 — /clan shift
• Issue #1318 — Team/Clan/Private Radio Overhaul (affects Core radio)
• Issues #464, #336, #713 — membership persistence failures (defect 1 above)
• Issue #1892 — Clan System (tracking issue, currently empty)