fix(comms): route Block Comms context-menu item through panel chrome (#1395)

AGENTS.md

@@ -2183,9 +2183,20 @@ contacting every contributor individually.
   RCON access via `_api_servers_admin_can_rcon` — the action stays
   publicly registered (anonymous viewers still see hostname / map /
   online count); the SteamID surfacing is what's permission-gated.
-  The anti-pattern that stays anti is the MooTools-era plumbing —
-  `sb.contextMenu`, `AddContextMenu`, the global helpers, the
-  separate `contextMenoo.js` file. Reach for the documented
+  The Ban / Block menu items route through the panel-chromed
+  smart-default URLs
+  (`?p=admin&c=bans&section=add-ban&steam=…&type=0` /
+  `?p=admin&c=comms&steam=…&type=0`) so the operator lands on the
+  real form with the SteamID pre-populated; Kick is the one
+  remaining iframe-routed item (`pages/admin.kickit.php`) because
+  it's a one-shot RCON command with no persistent panel surface
+  to anchor on (#1395 unified Block onto the panel route — pre-fix
+  it went to the same iframe surface as Kick which was actually the
+  post-`Actions.CommsAdd` rcon fan-out target, NOT a stand-alone
+  operator page; hitting it directly rendered chromeless and POSTed
+  to a 404). The anti-pattern that stays anti is the MooTools-era
+  plumbing — `sb.contextMenu`, `AddContextMenu`, the global helpers,
+  the separate `contextMenoo.js` file. Reach for the documented
   data-attribute hooks instead. Don't reintroduce the help text
   without the wiring (`page_servers.tpl`'s hint copy is now gated
   on `$can_use_context_menu` so anonymous viewers don't see it).
@@ -2768,7 +2779,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`). 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/e2e/specs/flows/server-player-context-menu.spec.ts` (end-to-end menu open / item visibility / 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`). 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). |
 | 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/includes/View/AdminCommsAddView.php

@@ -14,13 +14,28 @@
  * View instance satisfies both themes during the v2.0.0 rollout window
  * (#1123): SmartyTemplateRule scans whichever theme is active and the
  * property name has to line up on both legs of the matrix.
+ *
+ * `prefill_steam` / `prefill_type` carry the smart-default shape used
+ * by the `?p=admin&c=comms&steam=<STEAMID>` deep link (used by the
+ * public servers list's right-click context menu — see
+ * `web/scripts/server-context-menu.js`). Mirrors the
+ * `Sbpp\View\AdminBansAddView` pair for the sibling "Ban player"
+ * affordance. The property NAMES match; the `prefill_type` SEMANTICS
+ * diverge by surface — for comms the valid values are 1=Mute,
+ * 2=Gag, 3=Silence (the `:prefix_comms.type` column), with 0
+ * meaning "no pre-selection". Bans uses 0=Steam ID, 1=IP for the
+ * sibling field. The form's `<select id="type">` lands on the
+ * native first-option default (Mute) when `prefill_type === 0` —
+ * no `selected` attribute fires anywhere on the option list. (#1395)
  */
 final class AdminCommsAddView extends View
 {
     public const TEMPLATE = 'page_admin_comms_add.tpl';
 
     public function __construct(
         public readonly bool $permission_addban,
+        public readonly string $prefill_steam = '',
+        public readonly int $prefill_type = 0,
     ) {
     }
 }

web/pages/admin.comms.php

@@ -46,6 +46,48 @@
     echo "<script type=\"text/javascript\">sb.ready(function(){sb.message.show('Loading..','<b>Loading...</b><br><i>Please Wait!</i>','blue','',true);sb.hide('dialog-control');sb.api.call(Actions.CommsPaste,{sid:" . (int) $_GET['sid'] . ",name:" . $pNameJs . ",type:0}).then(function(r){if(r&&r.ok&&r.data){if(typeof window.__sbppApplyBlockFields==='function')window.__sbppApplyBlockFields(r.data);sb.show('dialog-control');sb.hide('dialog-placement');}else if(r&&r.ok===false&&r.error){sb.message.error('Error',r.error.message);sb.show('dialog-control');}});});</script>";
 }
 
+/*
+ * Smart-default pre-fill for SteamID via `?steam=…&type=…`
+ * (mirrors `admin.bans.php`'s sibling block for the "Ban player"
+ * menu item — see #PLAYER_CTX_MENU / #1395). The public servers
+ * list's right-click context menu's "Block comms" item lands
+ * admins on `?p=admin&c=comms&steam=STEAM_…&type=0` to pre-populate
+ * the form without firing a JSON action — the form has to be
+ * usable on the no-JS path (every other approach taken by the
+ * legacy `pages/admin.blockit.php?check=…&type=0` URL routed to a
+ * chromeless iframe page whose relative POST hit `/pages/api.php`
+ * → 404). The pre-fill happens server-side via the View DTO
+ * rather than through `__sbppApplyBlockFields` so the surface
+ * works pre-JS-boot.
+ *
+ * Allowed shapes (mirrors admin.bans.php's allowlist verbatim so
+ * the menu's URL contract is symmetric across the two
+ * affordances): STEAM_X:Y:Z / [U:1:N] / 17-digit SteamID64 /
+ * dotted IPv4. Comms doesn't actually ban by IP, but keeping the
+ * regex symmetric with bans means a future menu / deep-link
+ * change only has to touch one allowlist. An IPv4 value lands in
+ * the steam input and will fail server-side validation on submit
+ * via `Actions.CommsAdd`; that's the right behaviour (loud
+ * failure) vs. silently dropping a value the user can see.
+ *
+ * Allowed `type` values are 1 (Mute), 2 (Gag), 3 (Silence) — the
+ * `:prefix_comms.type tinyint` column's domain. Anything else
+ * (including the menu's `?type=0` bridging value, which is
+ * sourced from the bans-menu URL shape where 0=Steam ID) falls
+ * back to 0 (no pre-selection) and the form's `<select id="type">`
+ * lands on the native first-option default (Mute).
+ */
+$prefillSteamRaw = isset($_GET['steam']) ? trim((string) $_GET['steam']) : '';
+$prefillTypeRaw  = isset($_GET['type']) ? (int) $_GET['type'] : 0;
+$prefillSteam    = '';
+$prefillType     = 0;
+if ($prefillSteamRaw !== '') {
+    if (preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}|\d{1,3}(?:\.\d{1,3}){3})$/', $prefillSteamRaw) === 1) {
+        $prefillSteam = $prefillSteamRaw;
+        $prefillType  = in_array($prefillTypeRaw, [1, 2, 3], true) ? $prefillTypeRaw : 0;
+    }
+}
+
 // SourceComms reuses the bans permission set: there is no
 // ADMIN_ADD_COMM flag, so the gate uses ADMIN_OWNER|ADMIN_ADD_BAN.
 // Splatting Perms::for(...) into the View pulls `can_add_ban` (and
@@ -56,6 +98,8 @@
 $perms = \Sbpp\View\Perms::for($userbank);
 \Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminCommsAddView(
     permission_addban: $perms['can_add_ban'],
+    prefill_steam: $prefillSteam,
+    prefill_type: $prefillType,
 ));
 ?>
 <script type="text/javascript">