Production hardening (0.6.1): SQL buffer + FK race, server_id churn, negative-score flush drop

.github/ensure-prereqs.sh

@@ -6,14 +6,21 @@
 # helpful message pointing at .github/runner-setup.sh and exits non-zero.
 #
 # Tools probed:
-#   bash docker curl unzip tar zip git python3
+#   bash docker curl tar git python3
 #
 # The Python check uses --version because Windows runners with the Microsoft
 # Store stub have python3 on PATH but it isn't a real interpreter.
+#
+# NOTE: unzip/zip are deliberately NOT required. On this Linux runner the
+# SourceMod toolchain is fetched as .tar.gz (tar), and release packaging builds
+# the .zip via python3's stdlib zipfile — so neither the `unzip` nor `zip`
+# binary is ever invoked by ci.yml/release.yml. (`unzip` is used only by
+# tests/run.sh on the Windows dev path.) Requiring them here made CI hard-fail
+# on runners that happen to lack them even though nothing uses them.
 
 set -euo pipefail
 
-REQUIRED_BINS=(docker curl unzip tar zip git)
+REQUIRED_BINS=(docker curl tar git)
 
 missing=()
 for cmd in "${REQUIRED_BINS[@]}"; do

.github/workflows/ci.yml

@@ -26,7 +26,11 @@ jobs:
           for c in python3 python python3.12 python3.11 python3.10; do
             if command -v "$c" >/dev/null 2>&1 \
                && "$c" -c "import sys" >/dev/null 2>&1; then
-              "$c" -m pip install --user --quiet -r schema/scripts/requirements.txt
+              # --break-system-packages: PEP 668 marks the runner's system
+              # Python as externally-managed, blocking a plain --user install.
+              # The flag bypasses that guard; with --user the install stays
+              # confined to the user site (~/.local), not system-wide.
+              "$c" -m pip install --user --break-system-packages --quiet -r schema/scripts/requirements.txt
               break
             fi
           done

.github/workflows/release.yml

@@ -38,7 +38,11 @@ jobs:
           for c in python3 python python3.12 python3.11 python3.10; do
             if command -v "$c" >/dev/null 2>&1 \
                && "$c" -c "import sys" >/dev/null 2>&1; then
-              "$c" -m pip install --user --quiet -r schema/scripts/requirements.txt
+              # --break-system-packages: PEP 668 marks the runner's system
+              # Python as externally-managed, blocking a plain --user install.
+              # The flag bypasses that guard; with --user the install stays
+              # confined to the user site (~/.local), not system-wide.
+              "$c" -m pip install --user --break-system-packages --quiet -r schema/scripts/requirements.txt
               break
             fi
           done
@@ -74,7 +78,10 @@ jobs:
           cp docs/INSTALL.md README.md LICENSE            "$STAGE/" 2>/dev/null || true
 
           tar czf "$STAGE/source.tar.gz" plugin/scripting
-          zip -rq "$STAGE.zip" "$STAGE"
+          # Build $STAGE.zip with python3's stdlib zipfile rather than the `zip`
+          # binary, so the self-hosted runner doesn't need `zip` installed. This
+          # produces the same archive (entries prefixed with "$STAGE/").
+          python3 -c "import shutil,sys; shutil.make_archive(sys.argv[1], 'zip', '.', sys.argv[1])" "$STAGE"
           echo "zip=$STAGE.zip" >> "$GITHUB_OUTPUT"
 
       - name: Generate changelog

CHANGELOG.md

@@ -3,6 +3,47 @@
 All notable changes to bizzymod-stats are documented here. Format follows
 [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); we use SemVer.
 
+## [0.6.1] — UNRELEASED
+
+### Fixed
+
+- **Query buffer truncation in player_stats upsert.** The session-flush
+  transaction's main `INSERT INTO player_stats … ON DUPLICATE KEY UPDATE …`
+  exceeded its 2048-byte `FormatEx` buffer after the schema grew through
+  migration 010, silently chopping the statement mid-keyword. Surfaced as
+  MySQL errors like `Unknown column 'damage_to_ta'`, `'VALUES'` → `'VALUE'`
+  → `'VALU'` → `'V'`, and `syntax error … near ''`. Whole transactions
+  were rejected, so session counters never persisted. Bumped the buffer
+  to 4096. Long-term fix tracked separately: switch the wide upserts to
+  parameterized queries. (#5)
+- **`tank_records.fk_tr_map` foreign-key violation on tank/witch spawn.**
+  `g_CurrentMapId` is resolved through an async `INSERT IGNORE INTO maps
+  → SELECT id` chain that only kicks off in `OnMapStart`. On a plugin
+  hot-reload or DB-late-connect mid-map, `g_CurrentMapId` stayed at 0 and
+  the first boss spawn after produced a foreign-key error against
+  `maps.id`. Three-part fix:
+    - The DB-ready callback (`OnServerLookup`) now triggers
+      `Bizzy_Identity_ResolveMap()` itself, so a late connect catches up.
+    - `Bizzy_Identity_ResolveMap()` refreshes `g_CurrentMap` from the
+      engine on every call (no longer relies on `OnMapStart` having
+      already populated it).
+    - Tank- and witch-spawn inserts now bail with `g_CurrentMapId == 0`
+      instead of attempting the FK-violating insert. (#5)
+- **Server-key persistence / `server_id` churn (#6).** The server key was
+  written via `BuildPath(Path_SM, "cfg/sourcemod/…")`, which resolves to
+  `addons/sourcemod/cfg/sourcemod/` — a directory that does not exist — so the
+  write silently failed and a fresh random key (hence a fresh `servers.id`) was
+  generated on every boot, fragmenting player stats across many `server_id`s.
+  The keyfile now lives in the writable SM `data/` dir, so the key persists and
+  the server keeps one stable `server_id` across restarts.
+- **Negative session score dropped the whole stat flush (#7).** A player who
+  ended a session with negative points (griefing / friendly fire, with
+  `negative_score` on) produced a negative `most_points_in_session`, but that
+  column is `INT UNSIGNED` — MySQL rejected the row with "Out of range value",
+  which failed the career-bests query and rolled back the entire
+  `Bizzy_Session_Flush` transaction, losing that session's stats. Clamp the
+  inserted value with `GREATEST(0, …)`; a negative career "best" is meaningless.
+
 ## [0.6.0] — 2026-05-25
 
 **First public release.** Built from scratch over a 2026-Q2 sprint as a

plugin/scripting/bizzymod_stats/identity.sp

@@ -3,11 +3,14 @@
  * `weapons`. Caches IDs in memory so the hot path never re-queries.
  *
  * Server identity is bootstrapped via a stable key stored in
- * cfg/sourcemod/bizzymod_stats.server_key. If absent we generate one on first
- * run and write it back.
+ * addons/sourcemod/data/bizzymod_stats.server_key. If absent we generate one
+ * on first run and write it back. (data/ is used because BuildPath(Path_SM)
+ * roots at addons/sourcemod/ and data/ always exists + is writable; the old
+ * "cfg/sourcemod/" path resolved to a nonexistent addons/sourcemod/cfg/sourcemod/,
+ * so the write failed and the key + server_id churned every boot. See #6.)
  */
 
-#define BIZZY_KEYFILE "cfg/sourcemod/bizzymod_stats.server_key"
+#define BIZZY_KEYFILE "data/bizzymod_stats.server_key"
 
 void Bizzy_Identity_EnsureServer()
 {
@@ -51,6 +54,14 @@ static void OnServerLookup(Database db, DBResultSet rs, const char[] error, any
     g_ServerId = rs.FetchInt(0);
     LogMessage("[bizzymod-stats] online as server_id=%d (key=%s)", g_ServerId, g_ServerKey);
 
+    // Resolve map_id now in case the DB connected mid-map (server boot
+    // with a map already loaded, or plugin hot-reload). OnMapStart only
+    // fires on actual transitions, so without this hook g_CurrentMapId
+    // would stay 0 until the next map change — and any tank / witch /
+    // crescendo / wave insert in the meantime would FK-violate against
+    // maps.id. See bogware/bizzymod-stats#5.
+    Bizzy_Identity_ResolveMap();
+
     // Now safe to backfill any sessions for players already connected
     // (covers hot-reload).
     for (int i = 1; i <= MaxClients; i++)
@@ -177,7 +188,15 @@ static void OnPlayerLookup(Database db, DBResultSet rs, const char[] error, Data
 
 stock void Bizzy_Identity_ResolveMap()
 {
-    if (g_DB == null || g_CurrentMap[0] == '\0') return;
+    if (g_DB == null) return;
+    // Refresh from the engine every call. OnMapStart sets g_CurrentMap on
+    // every map change, but if this is called from the DB-ready callback
+    // (handling hot reload / late connect) OnMapStart may not have fired
+    // yet on this plugin instance — in which case g_CurrentMap is empty
+    // and we'd no-op. Reading from GetCurrentMap directly is safe and
+    // idempotent. See bogware/bizzymod-stats#5.
+    GetCurrentMap(g_CurrentMap, sizeof g_CurrentMap);
+    if (g_CurrentMap[0] == '\0') return;
     char esc[260];
     Bizzy_DB_Escape(g_CurrentMap, esc, sizeof esc);
     char sql[512];

plugin/scripting/bizzymod_stats/session.sp

@@ -87,7 +87,17 @@ stock void Bizzy_Session_Flush(int client, int duration)
         g_Clients[client].killStreakMax = g_Clients[client].killStreak;
 
     // 1) Upsert rollup row in player_stats.
-    char sql[2048];
+    //
+    // Buffer sized for the worst-case post-substitution length of every
+    // FormatEx call in this function. The biggest one is this player_stats
+    // upsert with ~35 columns + matching ON DUPLICATE KEY UPDATE clause; at
+    // 2048 it silently truncated mid-statement (errors like
+    // `Unknown column 'damage_to_ta'` / `'VALUES'` cut to `'VALUE'`/`'V'`),
+    // producing MySQL syntax errors and dropping the whole transaction.
+    // See bogware/bizzymod-stats#5. Long-term fix is parameterized queries
+    // (no string-build). For now: large fixed buffer + the truncation
+    // guard in Bizzy_DB_AssertQueryLen below.
+    char sql[4096];
     FormatEx(sql, sizeof sql,
         "INSERT INTO player_stats "
         ... "(player_id, gamemode_id, difficulty_id, server_id, "
@@ -170,13 +180,17 @@ stock void Bizzy_Session_Flush(int client, int duration)
         g_Clients[client].distanceUnits);
     txn.AddQuery(sql);
 
-    // 1b) Career bests — single-row GREATEST update per player.
+    // 1b) Career bests — single-row GREATEST update per player. most_points_in_session
+    // is clamped to >=0 in the INSERT: pointsThisSession can go negative (griefing / FF with
+    // negative_score on) but the column is INT UNSIGNED, so a negative is rejected by MySQL
+    // and drops the entire flush transaction. A negative career "best" is meaningless anyway.
+    // See #7.
     FormatEx(sql, sizeof sql,
         "INSERT INTO career_bests "
         ... "(player_id, most_points_in_session, most_kills_in_session, "
         ... " most_headshots_in_session, longest_kill_streak, "
         ... " biggest_tank_punch_damage) "
-        ... "VALUES (%d, %d, %d, %d, %d, %d) "
+        ... "VALUES (%d, GREATEST(0, %d), %d, %d, %d, %d) "
         ... "ON DUPLICATE KEY UPDATE "
         ... " most_points_in_session    = GREATEST(most_points_in_session,    VALUES(most_points_in_session)), "
         ... " most_kills_in_session     = GREATEST(most_kills_in_session,     VALUES(most_kills_in_session)), "

plugin/scripting/bizzymod_stats/tank_witch.sp

@@ -84,7 +84,15 @@ stock void Bizzy_TankWitch_TankSpawn(int tankClient)
     g_Tanks[slot].lastY = RoundToFloor(pos[1]);
     g_Tanks[slot].lastZ = RoundToFloor(pos[2]);
 
-    if (g_DB == null || g_ServerId == 0) return;
+    // Guard against tank_records.fk_tr_map FK violation: the maps row this
+    // map_id references must exist. g_CurrentMapId is resolved async (map
+    // upsert + id lookup) so there's a window — fresh server boot, plugin
+    // hot-reload mid-map, or first tank of a new map before resolution
+    // completes — when g_CurrentMapId is 0 or stale. The identity module
+    // also re-resolves on OnServerLookup (covers DB-late-connect), but this
+    // guard makes the failure mode "lose this one record" instead of
+    // "FK error in errors_*.log". See bogware/bizzymod-stats#5.
+    if (g_DB == null || g_ServerId == 0 || g_CurrentMapId == 0) return;
     char sql[384];
     FormatEx(sql, sizeof sql,
         "INSERT INTO tank_records (server_id, match_round_id, map_id, spawned_at) "
@@ -285,7 +293,8 @@ stock void Bizzy_TankWitch_WitchSpawn(int witchEnt)
     g_Witches[slot].incappedUid = 0;
     g_Witches[slot].state = 0;
 
-    if (g_DB == null || g_ServerId == 0) return;
+    // Same FK guard as tank spawn — witch_records also FKs to maps.id.
+    if (g_DB == null || g_ServerId == 0 || g_CurrentMapId == 0) return;
     char sql[384];
     FormatEx(sql, sizeof sql,
         "INSERT INTO witch_records (server_id, match_round_id, map_id, seen_at, outcome) "