fix(servers): filter empty-name A2S entries so first-player ctxmenu works (#1396)

AGENTS.md

@@ -2793,7 +2793,7 @@ contacting every contributor individually.
 | Add or edit a palette "Navigate" entry (the icon-label-href rows the palette renders alongside player results) | `web/includes/View/PaletteActions.php` (`Sbpp\View\PaletteActions::for($userbank)` — catalog + filter). The catalog's `entries()` method declares each entry as `{icon, label, href, permission, config?}`; `for()` drops entries the user can't reach (admin entries gated via `HasAccess` with `ADMIN_OWNER` OR'd in; public entries optionally gated on a `config.enable*` toggle) and emits the public `{icon, label, href}` triple. The filtered list is JSON-encoded by `web/pages/core/footer.php` (with `JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT` so the content can never escape its `<script>` wrapper) and emitted by `core/footer.tpl` inside `<script type="application/json" id="palette-actions" data-testid="palette-actions">`. `theme.js`'s `loadNavItems()` reads + `JSON.parse`s the blob at boot. Pre-#1304 the entry list was a hardcoded `NAV_ITEMS` array in `theme.js` with no permission check, leaking admin entries to logged-out + partial-permission users; the regression guard is `web/tests/integration/PaletteActionsTest.php` (server-side filter) plus `web/tests/e2e/specs/flows/ui/command-palette-permissions.spec.ts` (end-to-end blob → DOM contract). |
 | Add a "copy this value" affordance to a panel surface (single-source clipboard wiring) | Mark the trigger with `data-copy="<value>"` (`<button type="button">` is the canonical shape; the drawer uses `<button>` inside a `<dd>`, the banlist row uses `<button>` inside `.row-actions`). The document-level COPY BUTTONS delegate in `web/themes/default/js/theme.js` handles every `[data-copy]` site: secure-context callers go through `navigator.clipboard.writeText` with a `.then(success, fallback)` chain, non-secure callers (plain HTTP behind a TLS-terminating proxy) drop to `copyFallback()` — a hidden-textarea + `document.execCommand('copy')` that's the only portable option outside HTTPS (#1308). NEVER add an inline `onclick="event.stopPropagation()"` to a `[data-copy]` button — the bubble-phase stop kills the document delegate (Defect A, #1308). NEVER assume `navigator.clipboard` exists or that `writeText()` resolves — both fall through to the same execCommand fallback (Defect B, #1308). |
 | Add admin-only per-player notes | `web/api/handlers/notes.php` (CRUD) — Notes tab is gated by `bans.detail`'s `notes_visible` flag |
-| Add or extend the server-player right-click context menu on `?p=servers` (View Profile / Copy SteamID / Kick / Ban / Block Comms) | `web/scripts/server-context-menu.js` (event-delegate menu, single `document.addEventListener('contextmenu')` filtered by `closest('[data-context-menu="server-player"]')`) + `web/scripts/server-tile-hydrate.js` (`renderPlayers()` emits the `data-context-menu` / `data-steamid` / `data-name` / `data-server-sid` / `data-can-ban-player` hooks on each `<li>`) + `web/api/handlers/servers.php` (`api_servers_host_players` attaches SteamIDs via `RconStatusCache::fetch($sid)` only when the caller has `WebPermission::Owner \| WebPermission::AddBan` AND per-server RCON access; `can_ban_player` boolean signals whether to render the kick/ban/block items) + `web/includes/Servers/RconStatusCache.php` (`Sbpp\Servers\RconStatusCache::fetch($sid, $ttl=30)` — per-sid on-disk cache under `SB_CACHE/srvstatus/`, mirrors `SourceQueryCache` shape; calls `rcon('status', $sid, true)` with the silent flag so passive probes don't spam the audit log). The admin hint copy in `page_servers.tpl` + the `<script src="./scripts/server-context-menu.js">` include are both gated on `$can_use_context_menu` (= `Perms::for($userbank)['can_add_ban']`) so anonymous viewers don't pay for either. SteamID3 / SteamID2 → SteamID64 conversion happens client-side (`STEAM_X:Y:Z` → `76561197960265728 + Z*2 + Y`; `[U:1:N]` → `76561197960265728 + N`). Ban / Block both route through panel-chromed smart-default URLs (`?p=admin&c=bans&section=add-ban&steam=…&type=0` consumed by `Sbpp\View\AdminBansAddView::prefill_steam` / `?p=admin&c=comms&steam=…&type=0` consumed by `Sbpp\View\AdminCommsAddView::prefill_steam` — #1395 brought Block onto the panel route; pre-fix it pointed at `pages/admin.blockit.php` which is the post-`Actions.CommsAdd` rcon-fan-out iframe target, NOT a stand-alone operator page); Kick stays on the iframe path (`pages/admin.kickit.php`) because it's a one-shot RCON command with no persistent panel surface to anchor on after firing. Both `admin.bans.php` and `admin.comms.php` allowlist the inbound shape via the same regex (`STEAM_X:Y:Z` / `[U:1:N]` / 17-digit SteamID64 / dotted IPv4) so a hostile/malformed referrer can't smuggle arbitrary text into the form's `<input value="…">`; the comms `?type=…` allowlist is `{1,2,3}` (Mute/Gag/Silence) with anything else (including the menu's `?type=0` bridging value, sourced from the bans-menu URL shape where 0=Steam ID) treated as "no pre-selection". The integration test (`web/tests/integration/ServerListHintRegressionTest.php`) is the post-restoration contract — it asserts the hint and the JS include both ship for admins and both stay absent for anonymous viewers (the pre-#1306 contract is superseded). Regression guards: `web/tests/api/ServersTest.php` (handler-shape coverage for the SteamID side-channel + the `can_ban_player` flag), `web/tests/integration/RconStatusCacheTest.php` (cache shape + silent-flag contract), `web/tests/integration/AdminBansAddSmartDefaultTest.php` + `web/tests/integration/AdminCommsAddSmartDefaultTest.php` (server-side prefill allowlist, valid / hostile / bare-section round-trips, type-coercion), `web/tests/e2e/specs/flows/server-player-context-menu.spec.ts` (end-to-end menu open / Ban + Block hrefs both ride the panel route / Kick stays on the iframe route / Escape close / no-steamid no-menu). |
+| Add or extend the server-player right-click context menu on `?p=servers` (View Profile / Copy SteamID / Kick / Ban / Block Comms) | `web/scripts/server-context-menu.js` (event-delegate menu, single `document.addEventListener('contextmenu')` filtered by `closest('[data-context-menu="server-player"]')`) + `web/scripts/server-tile-hydrate.js` (`renderPlayers()` emits the `data-context-menu` / `data-steamid` / `data-name` / `data-server-sid` / `data-can-ban-player` hooks on each `<li>`) + `web/api/handlers/servers.php` (`api_servers_host_players` attaches SteamIDs via `RconStatusCache::fetch($sid)` only when the caller has `WebPermission::Owner \| WebPermission::AddBan` AND per-server RCON access; `can_ban_player` boolean signals whether to render the kick/ban/block items) + `web/includes/Servers/RconStatusCache.php` (`Sbpp\Servers\RconStatusCache::fetch($sid, $ttl=30)` — per-sid on-disk cache under `SB_CACHE/srvstatus/`, mirrors `SourceQueryCache` shape; calls `rcon('status', $sid, true)` with the silent flag so passive probes don't spam the audit log). The admin hint copy in `page_servers.tpl` + the `<script src="./scripts/server-context-menu.js">` include are both gated on `$can_use_context_menu` (= `Perms::for($userbank)['can_add_ban']`) so anonymous viewers don't pay for either. SteamID3 / SteamID2 → SteamID64 conversion happens client-side (`STEAM_X:Y:Z` → `76561197960265728 + Z*2 + Y`; `[U:1:N]` → `76561197960265728 + N`). **The `player_list` field excludes A2S entries with an empty `Name` (#1396)** — some Source-engine variants and SourceMod plugins emit a "host slot" / "console" stub at the start of `GetPlayers` (`Name = ''`, `Frags = 0`, `Time = 0`) that pre-fix rendered as a phantom `<li data-testid="server-player">` above the first visible player with no `data-context-menu` hooks (the empty name fails the SteamID match gate above). The phantom row was a thin border-bottom strip with a misleading "0 · " meta on the right; users perceived the next real player as "the first player of the list" and right-clicks landing in the phantom row's area silently no-op'd. The filter lives at the same `$name === ''` skip the SteamID-by-name lookup uses, so the JS contract stays simple: every row in `player_list` has a displayable name. Bots, real players whose A2S name didn't match the RCON status output, and anonymous callers still render — the filter is strictly "name is empty string". Ban / Block both route through panel-chromed smart-default URLs (`?p=admin&c=bans&section=add-ban&steam=…&type=0` consumed by `Sbpp\View\AdminBansAddView::prefill_steam` / `?p=admin&c=comms&steam=…&type=0` consumed by `Sbpp\View\AdminCommsAddView::prefill_steam` — #1395 brought Block onto the panel route; pre-fix it pointed at `pages/admin.blockit.php` which is the post-`Actions.CommsAdd` rcon-fan-out iframe target, NOT a stand-alone operator page); Kick stays on the iframe path (`pages/admin.kickit.php`) because it's a one-shot RCON command with no persistent panel surface to anchor on after firing. Both `admin.bans.php` and `admin.comms.php` allowlist the inbound shape via the same regex (`STEAM_X:Y:Z` / `[U:1:N]` / 17-digit SteamID64 / dotted IPv4) so a hostile/malformed referrer can't smuggle arbitrary text into the form's `<input value="…">`; the comms `?type=…` allowlist is `{1,2,3}` (Mute/Gag/Silence) with anything else (including the menu's `?type=0` bridging value, sourced from the bans-menu URL shape where 0=Steam ID) treated as "no pre-selection". The integration test (`web/tests/integration/ServerListHintRegressionTest.php`) is the post-restoration contract — it asserts the hint and the JS include both ship for admins and both stay absent for anonymous viewers (the pre-#1306 contract is superseded). Regression guards: `web/tests/api/ServersTest.php` (handler-shape coverage for the SteamID side-channel + the `can_ban_player` flag + the #1396 empty-name filter via `testHostPlayersFiltersEmptyNameEntries` / `testHostPlayersFiltersAllEmptyNameEntries`), `web/tests/integration/RconStatusCacheTest.php` (cache shape + silent-flag contract), `web/tests/integration/AdminBansAddSmartDefaultTest.php` + `web/tests/integration/AdminCommsAddSmartDefaultTest.php` (server-side prefill allowlist, valid / hostile / bare-section round-trips, type-coercion), `web/tests/e2e/specs/flows/server-player-context-menu.spec.ts` (end-to-end menu open / Ban + Block hrefs both ride the panel route / Kick stays on the iframe route / Escape close / no-steamid no-menu / #1396 first-named-player accepts real `mouse.click({button:'right'})`). |
 | Cache an A2S `GetInfo + GetPlayers` round-trip / add another public server-query handler | `web/includes/Servers/SourceQueryCache.php` (`Sbpp\Servers\SourceQueryCache::fetch($ip, $port, $ttl=30)` — per-`(ip, port)` on-disk cache under `SB_CACHE/srvquery/`, atomic tempfile + `rename()` writes mirroring `system.check_version`'s release cache; both success and failure cache so an unreachable server costs ONE A2S probe per ~30s window). The sibling `Sbpp\Servers\RconStatusCache` (`SB_CACHE/srvstatus/`) follows the same shape for RCON `status` round-trips — used by `api_servers_host_players` to surface per-player SteamIDs to admins (see the context-menu row above). Every public handler under `web/api/handlers/servers.php` (`api_servers_host_players` / `host_property` / `host_players_list` / `players`) goes through this — never call `new SourceQuery()` directly from a handler. The cache stamps user-agnostic data only; the handler stamps per-caller fields (`is_owner`, `can_ban`, the per-call `trunchostname`) on top. Per-tile JS debounce on the public servers page lives in `web/themes/default/page_servers.tpl` (`loadTile()` flips `tile.__sbppLoading` + the Re-query button's `disabled` attr while a probe is in flight, releases both in the success / error tails). The matching JS gate on the toggle button has been the precedent since v2.0.0; #1311 brought the refresh button onto the same shape. Tests: `web/tests/integration/SourceQueryCacheTest.php` (cache shape + coalescing + TTL + invalidation, drives `setProbeOverrideForTesting()` so the assertion is deterministic without UDP) + `testHostPlayersCoalescesRapidRepeatCallsViaCache` / `testHostPlayersNegativeCachesUnreachableServers` in `web/tests/api/ServersTest.php` (handler-shape coverage). E2E: `web/tests/e2e/specs/flows/server-refresh-debounce.spec.ts`. |
 | Render admin-authored Markdown to safe HTML | `web/includes/Markup/IntroRenderer.php` (`Sbpp\Markup`) |
 | Build / extend the anonymous opt-out daily telemetry payload (#1126) | `web/includes/Telemetry/Telemetry.php` (`Sbpp\Telemetry\Telemetry` — `tickIfDue`, `collect`, `send`) + `web/includes/Telemetry/Schema1.php` (`Sbpp\Telemetry\Schema1::payloadFieldNames()`, drives the extractor parity test) + `web/includes/Telemetry/schema-1.lock.json` (vendored from [sbpp/cf-analytics](https://github.com/sbpp/cf-analytics) — manual sync via `make sync-telemetry-schema`). Tick is registered at the tail of `init.php` via `register_shutdown_function`; on FPM, `fastcgi_finish_request()` flushes the response BEFORE the cURL POST so telemetry never delays a panel page. Slot reservation is atomic (`UPDATE :prefix_settings WHERE CAST(value AS UNSIGNED) <= :threshold`) at the START of the attempt, so a flapping endpoint costs one ping/day, not one ping/request. Audit-log only enable/disable transitions, never individual pings. The in-panel disclosure surface is the help-icon copy in `page_admin_settings_features.tpl`; the upgrade-time disclosure lives in `docs/src/content/docs/updating/1.8-to-2.0.mdx` (no first-login modal). |

web/api/handlers/servers.php

@@ -298,9 +298,28 @@ function api_servers_host_players(array $params): array
         }
     }
 
+    // Build the public player list. Empty-name A2S entries are
+    // filtered out (#1396): some Source-engine variants and
+    // SourceMod plugins emit a "host slot" / "console" entry at
+    // the start of `GetPlayers` with an empty Name, zero Frags,
+    // and zero Time. Pre-fix the entry flowed through verbatim
+    // and the JS rendered it as a phantom `<li>` (an invisible
+    // thin row with a misleading "0 · " meta on the right and no
+    // `data-context-menu` hooks, because the empty name fails
+    // the SteamID match gate above). Users perceiving the next
+    // real player as "the first player of the list" and
+    // right-clicking near the top would land on the phantom
+    // row instead and the menu silently no-op'd. Filtering the
+    // entry out here matches the same `name === ''` skip the
+    // SteamID-by-name lookup above already applies, so the JS
+    // contract stays simple: every row in `player_list` has a
+    // displayable name.
     $playerList = [];
     foreach ($players as $p) {
         $name = (string) ($p['Name'] ?? '');
+        if ($name === '') {
+            continue;
+        }
         $row  = [
             'id'     => $p['Id']    ?? null,
             'name'   => $name,
@@ -310,7 +329,6 @@ function api_servers_host_players(array $params): array
         ];
         if (
             $canBanPlayer
-            && $name !== ''
             && isset($steamidByName[$name])
             && ($rconNameCount[$name] ?? 0) === 1
             && ($sqNameCount[$name] ?? 0) === 1