From 4c7d33ddee0a0df5b3660c7cf46d2a3da36ad8da Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Mon, 25 May 2026 17:02:14 -0400
Subject: [PATCH] fix(1441): kick the player from their server when the ban is
 added
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Right-clicking a player → Ban → submitting the form would land
the ban row in the database (and SourceMod's on-connect check
would correctly reject subsequent reconnects), but the player's
existing session stayed alive until they voluntarily
disconnected — often 20-40 minutes later on a low-turnover
server. The user-visible symptom from #1441: "ban is added to
database and marked as active but player is not kicked from
server".

Root cause was a v1.x → v2.0 cutover oversight. The
post-`Actions.BansAdd` `.then()` handler in
`page_admin_bans_add.tpl` carried:

  if (r.data && r.data.kickit && typeof window.ShowKickBox === 'function') {
      window.ShowKickBox(r.data.kickit.check, r.data.kickit.type, ...);
  }

`window.ShowKickBox` was defined in `web/scripts/sourcebans.js`,
deleted with the rest of the v1.x bulk JS at #1123 D1 (v2.0.0).
The `typeof ... === 'function'` guard silently resolves to
`false`, the body of the `if` never runs, AND no console error
fires — so the surrounding success branch falls through to a
generic "Ban added" toast while the load-bearing rcon
side-effect (kick the player off every server they're currently
on) silently doesn't happen. This is the worst class of UI
regression: chrome reads as success while the action never
fires.

`api_bans_add` has been returning the kickit envelope shape
(`{kickit: {check, type}, reload: true}`) the whole time — only
the client-side branch that consumes it was dead. SourceMod's
on-connect hook DOES catch the next time the player tries to
reconnect, which masked the bug to anyone testing the flow
quickly (the "ban then click Kick" two-step the reporter
describes is what muscle-memory has been compensating with).

Fix
---

Mirror the existing comms.add → blockit.php iframe pattern that
has been working continuously since v2.0. In the `.then()`
handler:

  if (r && r.ok && r.data && r.data.kickit) {
      var iframe = document.createElement('iframe');
      iframe.id = 'srvkicker';
      iframe.style.display = 'none';
      iframe.src = 'pages/admin.kickit.php?check=' +
          encodeURIComponent(r.data.kickit.check) +
          '&type=' + encodeURIComponent(r.data.kickit.type);
      document.body.appendChild(iframe);
      ...
  }

The iframe's `page_kickit.tpl` script (untouched server-side)
auto-fires `Actions.KickitLoadServers` on the iframe's load
event, enumerates enabled servers, and fans out per-server
`Actions.KickitKickPlayer` rcon calls. The operator never sees
the iframe (`display: none`) — they just see the "Ban Added"
toast followed by a 2s reload of the parent page (matching
comms.add's shape exactly).

Also standardised the fallback branch's toast title from
'Ban added' (sentence case) to 'Ban Added' (Title Case) so
both branches are visually symmetric AND match comms.add's
'Block Added'. The mixed casing was pre-existing but invisible
because only the fallback branch ever ran on the live panel;
with both branches now running on different installs
(kickit-enabled vs kickit-disabled), the inconsistency would
have become operator-visible.

Coverage
--------

- `web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts` (NEW)
  pins both branches end-to-end. Stubs `Actions.BansAdd` AND
  `pages/admin.kickit.php` at the network layer so the spec
  stays robust under multi-worker DB pressure against the
  shared `sourcebans_e2e` DB; asserts the iframe is spawned
  EXACTLY ONCE (`toBe(1)` per reviewer Finding #1 — a future
  double-mount regression from a `.then()` chain that
  resolves twice or a `dispatchEvent` re-firing the submit
  handler must surface here, `toBeGreaterThanOrEqual(1)`
  would silently pass). Sister test asserts the
  kickit-DISABLED response (no `r.data.kickit`) does NOT
  spawn the iframe.

- `web/tests/e2e/specs/flows/admin-ban-lifecycle.spec.ts`
  + `web/tests/e2e/pages/SubmitBanFlow.ts` widen the post-add
  toast matcher to `/ban added/i` so both casings round-trip;
  comments document the new contract.

- `web/themes/default/page_kickit.tpl` docblock updated to
  describe the new caller chain (the kickit iframe is now
  reached from `page_admin_bans_add.tpl`'s success branch as
  well as the existing context-menu top-level navigation
  added at #1433).

AGENTS.md
---------

- New anti-pattern entry banning
  `typeof window.<LegacyHelper> === 'function'` guards against
  deleted v1.x helpers (`ShowKickBox`, `TabToReload`) with
  canonical file paths, the JS field-name asymmetry callout
  (`r.data.kickit.check` vs `r.data.block.steam` — the URL
  parameter `check=` is symmetric, the envelope field names
  are not, because `api_bans_add` and `api_comms_add` chose
  different envelope shapes), and the regression-guard
  pointer. Search anchor for future sweeps:
  `rg "typeof window\.\w+ === 'function'" web/themes/`.

- New "Where to find what" row documenting the
  bans.add → kickit.php iframe fan-out contract alongside the
  comms.add → blockit.php sibling, the field-name asymmetry
  reminder, and the regression-guard pointers
  (`ban-kickit-iframe.spec.ts` + `kickit-iframe.spec.ts` +
  `KickitTest.php`).

Adversarial-reviewer follow-up
------------------------------

A spawned reviewer subagent enumerated 11 findings; #1-#5
(this PR's correctness + docs polish) are addressed:

  1. Iframe-request count assertion tightened to `toBe(1)`.
  2. AGENTS.md anti-pattern reworded — pre-fix it claimed
     the new branch was "byte-for-byte symmetric to comms.add";
     it isn't (different envelope field names). Now spells
     out the JS field-name asymmetry as documented structural
     API drift.
  3. Fallback toast title standardised to Title Case.
  4. New "Where to find what" row added (this commit).
  5. Anti-pattern entry now cites canonical file paths
     (`page_admin_bans_add.tpl`, `page_admin_comms_add.tpl`,
     `ban-kickit-iframe.spec.ts`, `kickit-iframe.spec.ts`,
     `KickitTest.php`).

#6-#9 (reload-on-success asymmetry, iframe-load-error UX,
parent-reload vs iframe-rcon-fan-out race, iframe-never-removed)
are pre-existing comms.add behaviour the PR inherits unchanged.
Reviewer's recommendation is to file them as follow-up issues;
they're architectural observations, not blockers for #1441's
scope. #10 (duplicate `srvkicker` id) and #11 (theoretical
XSS via `k.check`) were analysed and confirmed safe by design
— three defence-in-depth gates: server-side
`SteamID::HANDLER_STRICT_REGEX`, client-side
`encodeURIComponent`, Smarty auto-escape.

Fixes: #1441

CLA: I have read the CLA Document and I hereby sign the CLA

Co-authored-by: Fishy <fishyfishy118@gmail.com>
---
 AGENTS.md                                     |  55 +++
 web/tests/e2e/pages/SubmitBanFlow.ts          |  21 +-
 .../specs/flows/admin-ban-lifecycle.spec.ts   |  24 +-
 .../e2e/specs/flows/ban-kickit-iframe.spec.ts | 401 ++++++++++++++++++
 web/themes/default/page_admin_bans_add.tpl    |  60 ++-
 web/themes/default/page_kickit.tpl            |  26 +-
 6 files changed, 553 insertions(+), 34 deletions(-)
 create mode 100644 web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts

diff --git a/AGENTS.md b/AGENTS.md
index b1adc21d8..32d6a659b 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -2512,6 +2512,60 @@ contacting every contributor individually.
  via `data-action` + a page-tail vanilla-JS dispatcher per the
  canonical confirm-modal shape under "Add a confirm + reason
  modal" in "Where to find what".
+- `typeof window.<LegacyHelper> === 'function'` guards on a v1.x
+ helper inside a v2.0 page-tail handler (e.g.
+ `typeof window.ShowKickBox === 'function'` for the kick-on-ban
+ surface; `typeof window.TabToReload === 'function'` for the
+ post-action page-reload trigger) → both were defined in
+ `web/scripts/sourcebans.js`, deleted at #1123 D1 (v2.0.0). The
+ `typeof` test silently resolves to `false`, the body of the
+ `if` never runs, AND no console error fires — so the surrounding
+ success branch falls through to a generic toast while the
+ actual side-effect (kick a player, reload the tab) silently
+ doesn't happen. This is the worst class of UI regression: the
+ chrome reads as success while the load-bearing action never
+ fired. The kick-on-ban path was the canonical case (#1441 —
+ operators reported "banning doesn't kick" on the v2.0 panel; the
+ DB row landed and SourceMod's on-connect check did reject
+ subsequent connects, but the player's existing session stayed
+ alive until they voluntarily disconnected, often 20-40min later
+ on a low-turnover server). The fix shape is the existing
+ comms.add → blockit.php iframe pattern (canonical:
+ `web/themes/default/page_admin_comms_add.tpl`'s
+ `Actions.CommsAdd .then()` handler; mirrored at
+ `web/themes/default/page_admin_bans_add.tpl`'s
+ `Actions.BansAdd .then()` handler post-#1441): create a hidden
+ `<iframe id="srvkicker" style="display: none">` pointing at
+ `pages/admin.kickit.php?check=…&type=…` and let the iframe's
+ server-side template enumerate enabled servers and fire the
+ rcon kick command via `Actions.KickitLoadServers` +
+ `Actions.KickitKickPlayer`. Mirror the comms.add structure
+ (success branch keeps button busy through a 2s page reload, no
+ `setBusy(btn, false)` in that branch). **The URL parameter
+ `check=` is symmetric across both surfaces; the JS envelope
+ field names are NOT — `r.data.block.steam` on comms.add,
+ `r.data.kickit.check` on bans.add — because `api_bans_add` and
+ `api_comms_add` chose different envelope shapes. Read the
+ server-side response shape FIRST when porting between the two
+ branches; the field-name asymmetry is structural API drift, not
+ a contract violation.** Regression guard:
+ `web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts` (pins the
+ iframe-spawn contract end-to-end — stubs both
+ `Actions.BansAdd` AND `pages/admin.kickit.php` at the network
+ layer so the spec stays robust under multi-worker DB pressure
+ against the shared `sourcebans_e2e` DB, with `toBe(1)` on the
+ iframe-request count so a future double-mount regression
+ surfaces immediately). The deeper rcon round-trip is exercised
+ by `web/tests/e2e/specs/flows/kickit-iframe.spec.ts` (#1433 —
+ the topbar-context-menu path through the same
+ `pages/admin.kickit.php` document) plus
+ `web/tests/api/KickitTest.php` (PHPUnit — handler permission +
+ malformed-input envelopes). Search anchor for any future sweep:
+ `rg "typeof window\.\w+ === 'function'" web/themes/` —
+ every such guard is a probable dead-helper trap unless the
+ helper is in fact still defined somewhere (e.g.
+ `window.SBPP.showToast` from `theme.js`, which IS the live
+ contract).
 - MooTools `$('id').value` / `$('id').checked` / `$('id').setStyle(…)`
  idioms in inline page-tail scripts → MooTools is gone (deleted
  with sourcebans.js at #1123 D1). Use vanilla DOM:
@@ -4173,6 +4227,7 @@ contacting every contributor individually.
 | Add a loading indicator to the player drawer or one of its lazy panes (so the chrome doesn't read as blank while the JSON action is in flight) | `renderDrawerLoading()` (header skeleton for the in-flight `bans.detail`) and `renderPaneSkeleton()` (placeholder for History / Comms / Notes activation) in `web/themes/default/js/theme.js`. Both lean on the `.skel` CSS rule in `theme.css` (linear-gradient + `shimmer` keyframe + dark-mode override + the `@media (prefers-reduced-motion: reduce)` per-rule override that keeps the shimmer sliding even under reduced motion, #1362). The header skeleton carries `[data-testid="drawer-loading"]` + `aria-busy="true"` + per-block `[data-skeleton]` (terminal markers under `#drawer-root[data-loading="true"]`); the lazy-pane skeleton carries `[data-pane-empty]` + `aria-busy="true"` and deliberately omits `[data-skeleton]` because the panel parent's `hidden` attribute doesn't compose into `[data-skeleton]:not([hidden])` and a nested marker would stall every page-load waiter that runs after the drawer opens. Class name is `.skel` (singular) — NOT `.skeleton`; the pre-fix `class="skeleton"` typo had no matching rule and the shimmer rows rendered as transparent zero-background divs (the user-visible "drawer is blank" regression). Regression guards: `web/tests/e2e/specs/flows/drawer-loading-indicator.spec.ts` (stalls `bans.detail` then `bans.player_history` via `page.route`, asserts the skeleton header is visible + the `.skel` block paints a `linear-gradient` background via `getComputedStyle(el).backgroundImage`, releases the routes, and confirms the drawer flips to `renderDrawerBody` / the pane fills with content) **plus** `web/tests/e2e/specs/flows/loading-animations.spec.ts` (#1362 — samples `getComputedStyle(.skel).backgroundPositionX` at multiple frame boundaries under both `reducedMotion: 'reduce'` AND `'no-preference'`, asserts the values change across samples; catches the v2.0 RC1 regression where the global reset froze the shimmer alongside the spinner). |
 | Surface unban-reason / removed-by inline on a public-list row (admin-lifted bans / comms — banlist-ureason or commslist-ureason inline) | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#1315). Reason cell on the desktop table emits a `<div class="text-xs text-faint mt-1" data-testid="ban-unban-meta">` (or `comm-unban-meta` for comms) with "Unbanned by `<admin>`: `<reason>`" when `$ban.state == 'unbanned'` (or `$comm.state == 'unmuted'`); mobile cards mirror with the `-mobile` testid suffix. Always gated on `!$hideadminname` so anonymous viewers under a hidden-admins config don't get the admin name leaked. The `ureason` / `removedby` row fields come from the page handler's existing data path (`page.banlist.php` lines 635-643, `page.commslist.php` lines 626-635) — read-only render, no write-side overlap with #1301 / #1323's unban-reason flow. The commslist surface is higher-priority than the banlist (no drawer fallback on `<tr data-testid="comm-row">`); banlist users have the drawer as the canonical detail view. |
 | Re-apply (Reban) affordance on the public banlist for expired / unbanned rows | `web/themes/default/page_bans.tpl`. The desktop row-actions cell emits `<a class="btn btn--secondary btn--sm" data-testid="row-action-reapply" href="index.php?p=admin&c=bans&section=add-ban&rebanid={$ban.bid}&key={$admin_postkey}">…<i data-lucide="rotate-ccw">…Re-apply</a>` when `$can_add_ban && ($ban.state == 'expired' \|\| $ban.state == 'unbanned')`. The smart-default block on `admin.bans.php`'s `add-ban` section detects `?rebanid=…` and pre-populates the form via `BansPrepareReban`. Mobile parity (this PR) — the mobile card was restructured from a single wrapping `<a>` to the `page_comms.tpl` shape (`<div data-testid="ban-card">` wrapping `<a class="ban-card__summary" data-testid="drawer-trigger">` + a sibling `<div class="row-actions ban-card__actions">`), so the same Re-apply button surfaces under `data-testid="row-action-reapply-mobile"`. The drawer trigger stays the inner anchor's `data-drawer-href` + `data-testid="drawer-trigger"` so the existing responsive spec selector keeps working. |
+| Fan rcon kick / block commands out to every enabled server after a successful `Actions.BansAdd` / `Actions.CommsAdd` (the hidden `<iframe id="srvkicker">` pattern) | `web/themes/default/page_admin_bans_add.tpl` (#1441 — `Actions.BansAdd .then(r => ...)` handler reads `r.data.kickit` and spawns the iframe pointing at `pages/admin.kickit.php?check=<check>&type=<type>`) + `web/themes/default/page_admin_comms_add.tpl` (the older sibling — `Actions.CommsAdd .then(r => ...)` reads `r.data.block` and spawns the iframe pointing at `pages/admin.blockit.php?check=<steam>&type=<type>&length=<length>`). The URL parameter `check=` is symmetric across both surfaces; the JS envelope field names are NOT — bans.add returns `r.data.kickit.check` while comms.add returns `r.data.block.steam`, because `api_bans_add` and `api_comms_add` chose different envelope shapes when the kickit/blockit envelopes were originally added. Always read the server-side envelope shape (`web/api/handlers/bans.php::api_bans_add` / `web/api/handlers/comms.php::api_comms_add`) FIRST when porting between the two branches. The iframe template (`web/themes/default/page_kickit.tpl` / `page_blockit.tpl`) auto-fires `Actions.KickitLoadServers` (or `Actions.BlockitLoadServers`) on `window.addEventListener('load', ...)` which enumerates enabled servers and fans out per-server `Actions.KickitKickPlayer` / `Actions.BlockitBlockPlayer` rcon calls. The iframe is `display: none` — the operator never sees it, but the panel can spawn it from any surface that needs server-side rcon side-effects (the right-click context menu's "Kick player" item navigates to the same `pages/admin.kickit.php` URL as a top-level document instead — see "Add or extend the server-player right-click context menu" row). Don't reach for `setBusy(submitBtn, false)` after spawning the iframe — the success branch keeps the button busy through the 2s `setTimeout` that reloads the parent page (matching `page_admin_comms_add.tpl`'s shape), preventing operators from queuing a second submit while rcon fans out. Regression guards: `web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts` (#1441 — stubs both `Actions.BansAdd` and `pages/admin.kickit.php` at the network layer; asserts the iframe is spawned exactly once with `toBe(1)` on the request count so a future double-mount regression — e.g. a `.then()` chain that resolves twice, a `dispatchEvent` re-firing the submit handler, a `setBusy` gap — surfaces immediately) + `web/tests/e2e/specs/flows/kickit-iframe.spec.ts` (#1433 — exercises the deeper rcon round-trip via the context-menu path) + `web/tests/api/KickitTest.php` (PHPUnit — handler permission gate + malformed-input envelopes). Pre-#1441 the bans-add side carried a `typeof window.ShowKickBox === 'function'` guard pointing at a v1.x helper that was deleted at #1123 D1; the `if` body silently never ran and operators reported "banning doesn't kick" (the canonical "chrome reads as success while the load-bearing action never fired" UI regression — see the matching anti-pattern entry under Anti-patterns). |
 | Wrap a public-list legacy advanced-search form (banlist / commslist) in a default-collapsed `<details class="filters-details">` disclosure | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#1315). The same `.filters-details` rules in `web/themes/default/css/theme.css` cover the chrome (summary chevron, `[open]` state, hover bg, focus ring, count badge); the public-list usage adds a `.filters-details__body > form.card` rule that suppresses the inner card framing because the disclosure body wraps a `{load_template file="admin.bans.search"}` (or `…comms.search`) — i.e. a sibling page-render that emits its own `<form class="card">`. The View DTO (`Sbpp\View\BanListView` / `Sbpp\View\CommsListView`) carries a `bool $is_advanced_search_open` set by the page handler from `isset($_GET['advSearch']) && (string) $_GET['advSearch'] !== ''` so the legacy `?advSearch=…&advType=…` URL shim auto-opens the disclosure. Bare `?p=banlist` / `?p=commslist` and simple-bar filters (`?searchText=` / `?server=` / `?time=`) leave it closed so the unfiltered list reaches above the fold. Selectors anchor on `[data-testid="banlist-advsearch-disclosure"]` / `[data-testid="commslist-advsearch-disclosure"]` (the `<details>`) + the `…-toggle` (the `<summary>`) + the `…-active` count badge. The same `.filters-details` chrome was introduced for admin-admins by #1303 — both surfaces share the CSS so a future tweak is single-source. |
 | Surface admin-authored per-ban / per-comm comments inline on the public banlist + commslist (`<details data-testid="ban-comments-inline">` / `comm-comments-inline`) | `web/themes/default/page_bans.tpl` + `web/themes/default/page_comms.tpl` (#BANLIST-COMMENTS, this PR). Both desktop tables render a native `<details class="ban-comments-inline">` next to the player-name cell whose `<summary>` doubles as the count chip (icon + tabular-nums count + "comment(s)" label) and whose body lists each comment with `<strong>{$com.comname}</strong>` + timestamp + `<div data-testid="ban-comment-text">{$com.commenttxt nofilter}</div>`. The `nofilter` is load-bearing because `$com.commenttxt` is server-built HTML produced by `encodePreservingBr` (per-segment `htmlspecialchars`, only `<br/>` survives) plus the URL-wrap regex that wraps already-escaped `https?://...` strings in `<a>` tags — same trust contract as the existing comment-edit-mode "Other comments" foreach at the top of `page_bans.tpl`. The mobile cards emit a non-interactive `<div data-testid="ban-comments-count-mobile">` count indicator instead — the card wraps every cell in a single `<a data-drawer-href>`, so a nested `<details>` would be invalid HTML (interactive content inside interactive content); mobile users tap through to the drawer (`renderOverviewPane` paints the same comments under `[data-testid="drawer-comments"]`). Both surfaces gate on `$view_comments` (mirrors the page handler's `Config::getBool('config.enablepubliccomments') \|\| $userbank->is_admin()` check at `page.banlist.php` line 766 / `page.commslist.php` line 734). The drawer surface (`api_bans_detail.comments_visible` + `api_bans_detail.comments`) is unchanged — both surfaces share the same data path and same gate so there's no leak risk. The commslist regression was worse than the banlist's because there's no drawer fallback on `<tr data-testid="comm-row">`; the inline disclosure is the ONLY on-page way for admins to see comm-block comment text. Regression guards: `web/tests/integration/BanlistCommentsVisibilityTest.php` (admin sees disclosure regardless of flag, anonymous gated correctly, drawer mirrors disclosure via `bans.detail`) + `web/tests/e2e/specs/flows/banlist-comments-visibility.spec.ts` (desktop disclosure renders + opens, drawer paints same data, mobile count indicator is a `<div>` not a `<details>`). The fix restores the v1.x `mooaccordion` discoverability that was lost when the v2.0 rewrite of `page_bans.tpl` collapsed the inline panel down to a silent `<span>[N]</span>` count badge with no affordance. |
 | Filter the public banlist by ban state (`?p=banlist&state=permanent\|active\|expired\|unbanned`) — server-side rowset narrowing for a chip strip | `web/pages/page.banlist.php` (#1352). The `?state=` value is allowlisted (`['permanent','active','expired','unbanned']`, anything else falls through to "All"), composed into the existing `$publicFilterAnd` / `$publicFilterWheren` SQL fragments via the per-state predicates documented inline. **Symmetric defensive shape across the four arms**: `permanent` / `active` BOTH require `RemovedOn IS NULL` (otherwise a pre-2.0 admin-lifted row with `ends > now` would match BOTH `?state=active` AND `?state=unbanned`); `expired` carries THREE arms (`RemoveType = 'E'` post-migration shape + `RemoveType IS NULL AND length > 0 AND ends < now AND RemovedOn IS NULL` for pre-475 installs the prune writer never touched + `RemoveType IS NULL AND RemovedOn IS NOT NULL AND length > 0 AND (RemovedBy IS NULL OR RemovedBy = 0)` for the fork-divergence shape `810.php` pass 2 backfills); `unbanned` carries TWO arms (`RemoveType IN ('D', 'U')` post-migration shape + `RemovedOn IS NOT NULL AND RemoveType IS NULL AND RemovedBy IS NOT NULL AND RemovedBy > 0` for pre-2.0 admin-lifts that `810.php` pass 1 backfills). The `RemovedBy > 0` vs `RemovedBy IS NULL OR = 0` split between unbanned / expired is the **load-bearing distinction** — natural expiry sets `RemovedBy = 0` (PruneBans) or NULL (pre-475); admin lifts set `RemovedBy = <aid>`. The View DTO (`Sbpp\View\BanListView`) carries `string $active_state` + `string $chip_base_link`; the chip strip in `web/themes/default/page_bans.tpl` renders as real `<a href="index.php?p=banlist{$chip_base_link}&state=…">` anchors with `aria-current="true"` (NOT `aria-pressed` — only valid on role=button; axe rejects it on `<a>` as `aria-allowed-attr`) and `data-active="true"` server-rendered on the matching chip — never `<button onclick=…>` (the legacy `web/scripts/banlist.js applyStateFilter` row-hide layer was a client-side filter on top of server-side pagination, so on installs with thousands of bans the unbanned chip silently rendered an empty page 1). The "Hide inactive" toggle is suppressed in the template when `$active_state !== ''` so the two predicates (`hideinactive` is `RemoveType IS NULL`; `?state=expired` / `unbanned` ask for the OPPOSITE) don't visually fight. Pagination URLs preserve `&state=` via `$stateFilterLink`; the per-row state classifier in the same file mirrors the SQL filter (a row pulled in by `?state=unbanned` renders the Unbanned pill, never the legacy mis-classified "Active" pill). The JSON API parity surface is `api_bans_detail` + `api_bans_player_history` in `web/api/handlers/bans.php` — same `isPre2AdminLift` defensive branch so the drawer's detail view + the history pane don't visibly contradict the SQL filter. Regression guards: `web/tests/integration/BanListStateFilterTest.php` (per-state row inclusion / exclusion + chip render contract + suppressed Hide-inactive toggle + symmetric `expired` arm 3 contract), `web/tests/integration/UpdaterBackfillRemoveTypeTest.php` (the migration's idempotency + two-pass cross-contamination contract), `web/tests/api/BansTest.php::testDetailReportsUnbannedForPre2AdminLiftWithRemoveTypeNull` (API parity), `web/tests/e2e/specs/responsive/banlist.spec.ts` + `web/tests/e2e/specs/responsive/filters.spec.ts` (mobile chip click navigates + asserts `aria-current` rather than `history.replaceState` + `aria-pressed`'ing). |
diff --git a/web/tests/e2e/pages/SubmitBanFlow.ts b/web/tests/e2e/pages/SubmitBanFlow.ts
index 3c4add1db..26201a0eb 100644
--- a/web/tests/e2e/pages/SubmitBanFlow.ts
+++ b/web/tests/e2e/pages/SubmitBanFlow.ts
@@ -137,14 +137,17 @@ export function adminSubmissionRow(page: Page, steam: string): ReturnType<Page['
  *      the queue).
  *   6. Wait for the success toast: theme.js's showToast emits a
  *      `.toast[data-kind="success"]` element synchronously inside
- *      the BansAdd `.then()` handler. The visible title is "Ban added"
- *      (the kickit-disabled fallback string from
- *      page_admin_bans_add.tpl — server-side `'message' => null` when
- *      `config.enablekickit=1` and ShowKickBox is undefined in
- *      sbpp2026, so the inline script falls through to the literal
- *      'Ban added' default). We disambiguate via `hasText` while
- *      keeping `[data-kind="success"]` as the primary attribute
- *      selector.
+ *      the BansAdd `.then()` handler. The visible title is "Ban Added"
+ *      (post-#1441 the kickit branch of page_admin_bans_add.tpl
+ *      surfaces the toast directly — kickit is enabled by default,
+ *      so the api response carries `'kickit' => [...]` + `'message'
+ *      => null`, and the inline script paints the toast literal
+ *      rather than relying on a server-supplied message envelope).
+ *      We disambiguate via case-insensitive `hasText` while keeping
+ *      `[data-kind="success"]` as the primary attribute selector.
+ *      The hidden `srvkicker` iframe spawned in the same branch is
+ *      harmless here — the e2e DB has no enabled servers and the
+ *      iframe's per-row kick loop never runs.
  *
  * `reasonOption` defaults to "Inappropriate Language" because it's a
  * legacy-locked option in the static `<optgroup label="Behavior">`
@@ -183,7 +186,7 @@ export async function adminApprove(
 
     await page.locator('[data-testid="addban-submit"]').click();
 
-    const toast = page.locator('.toast[data-kind="success"]').filter({ hasText: 'Ban added' });
+    const toast = page.locator('.toast[data-kind="success"]').filter({ hasText: /ban added/i });
     await expect(toast).toBeVisible();
 }
 
diff --git a/web/tests/e2e/specs/flows/admin-ban-lifecycle.spec.ts b/web/tests/e2e/specs/flows/admin-ban-lifecycle.spec.ts
index a553edefb..1746e8cda 100644
--- a/web/tests/e2e/specs/flows/admin-ban-lifecycle.spec.ts
+++ b/web/tests/e2e/specs/flows/admin-ban-lifecycle.spec.ts
@@ -107,19 +107,35 @@ test.describe('flow: admin ban lifecycle', () => {
         // The inline handler fires sb.api.call(Actions.BansAdd) and
         // shows a `success` toast on the resolution path. The toast
         // matrix is:
-        //   - api success + kickit enabled (default) + ShowKickBox
-        //     undefined (sourcebans.js gone) → falls through to the
-        //     'Ban added' toast (no message envelope).
+        //   - api success + kickit enabled (default) → kickit branch
+        //     in page_admin_bans_add.tpl creates the hidden
+        //     `srvkicker` iframe AND surfaces the 'Ban Added' toast
+        //     (post-#1441 the iframe replaces the dead `ShowKickBox`
+        //     legacy helper; the kick-on-ban contract is now the
+        //     symmetric mirror of comms.add → blockit.php).
         //   - api success + kickit disabled → message envelope
         //     surfaces the same 'Ban Added' title.
         // We anchor on `data-kind="success"` (the deterministic
         // attribute set by theme.js's showToast) and disambiguate
-        // via the case-insensitive title the matrix produces.
+        // via the case-insensitive title the matrix produces. The
+        // kickit-iframe spawn path is exercised end-to-end by
+        // `ban-kickit-iframe.spec.ts`; this lifecycle spec stays
+        // anchored on the toast so the 2s reload below doesn't race
+        // with the rest of the lifecycle assertions.
         const successToast = page
             .locator('.toast[data-kind="success"]')
             .filter({ hasText: /ban added/i });
         await expect(successToast).toBeVisible();
 
+        // The kickit branch schedules a 2s `window.location.href`
+        // reset. We navigate to the ban list explicitly below so the
+        // reload never fires inside this spec (Playwright's goto
+        // cancels the pending timer). The hidden `srvkicker` iframe
+        // is harmless in either case: the e2e DB has no enabled
+        // servers, so `Actions.KickitLoadServers` returns an empty
+        // list and the iframe's per-row `processRow` loop never
+        // runs.
+
         // ---- 2. List: find the new ban row -------------------------------
         // The ban list lives at ?p=banlist (the marquee page rendered
         // by page_bans.tpl in the v2.0.0 default theme). Admin row
diff --git a/web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts b/web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts
new file mode 100644
index 000000000..d3cd57b44
--- /dev/null
+++ b/web/tests/e2e/specs/flows/ban-kickit-iframe.spec.ts
@@ -0,0 +1,401 @@
+/**
+ * Flow: clicking "Add ban" on the admin form spawns a hidden
+ * `#srvkicker` iframe pointing at `pages/admin.kickit.php` so the
+ * banned player is actually KICKED from every connected server,
+ * not just inserted as a DB row (#1441).
+ *
+ * Background
+ * ----------
+ *
+ * `api_bans_add` returns a `kickit` envelope (`{check, type}`)
+ * whenever `config.enablekickit` is on (the install default per
+ * `web/install/includes/sql/data.sql` + `web/updater/data/130.php`).
+ * The handler's contract is "the frontend MUST surface the kick
+ * action somehow"; pre-#1441 the inline IIFE in
+ * `web/themes/default/page_admin_bans_add.tpl` checked
+ * `typeof window.ShowKickBox === 'function'` — a v1.x helper that
+ * lived in `web/scripts/sourcebans.js`, deleted with the bulk file
+ * at #1123 D1 (v2.0.0). The `typeof` test silently resolved to
+ * `false` and the success branch fell through to a "Ban added"
+ * toast while no live server ever kicked the banned player.
+ *
+ * Operators reported the symptom as "banning a player from the
+ * panel doesn't kick them" — the DB carries the ban, every
+ * subsequent connect attempt by the player IS rejected by the
+ * SourceMod plugin's on-connect check, but the player's existing
+ * session stays alive until their next disconnect. On a server
+ * with a low player turnover (Counter-Strike rounds, TF2 12v12
+ * pubs), this can mean the offending player griefs for another
+ * 20-40 minutes before voluntarily disconnecting.
+ *
+ * Fix (#1441)
+ * -----------
+ *
+ * Replace the dead `ShowKickBox` branch with the existing
+ * comms.add -> blockit.php iframe pattern: synthesise a hidden
+ * `#srvkicker` iframe pointing at `pages/admin.kickit.php?check=...
+ * &type=...`. The iframe page enumerates enabled servers via
+ * `Actions.KickitLoadServers` and fires `sm_kick` via rcon for
+ * each via `Actions.KickitKickPlayer`. This mirrors the
+ * blockit/gag flow one branch over (`page_admin_comms_add.tpl`
+ * lines ~370-394) so the kick-on-ban and block-on-gag contracts
+ * stay structurally symmetric.
+ *
+ * What this spec asserts
+ * ----------------------
+ *
+ *  - kickit-enabled path: `Actions.BansAdd` returning a `kickit`
+ *    envelope causes the page to create a hidden `<iframe
+ *    id="srvkicker" style="display: none">` whose `src` is
+ *    `pages/admin.kickit.php?check=<encoded-steam>&type=0`. The
+ *    iframe document request fires at the URL we expect, AND the
+ *    "Ban Added" success toast paints alongside it.
+ *  - kickit-disabled path: `Actions.BansAdd` returning a
+ *    `kickit: null` envelope (the `config.enablekickit = '0'`
+ *    shape) does NOT create the iframe. The fallthrough toast
+ *    still fires.
+ *
+ * Why this spec stays client-side-only
+ * ------------------------------------
+ *
+ * The bug being fixed is purely in the parent template's
+ * IIFE — "the JS that fires when bans.add returns a kickit
+ * envelope". The iframe's INTERNAL behavior (its bootstrap
+ * `Actions.KickitLoadServers` + `Actions.KickitKickPlayer`
+ * round-trip) is exercised by `kickit-iframe.spec.ts` (the
+ * topbar-context-menu surface that reaches the same
+ * `pages/admin.kickit.php` document) and `KickitTest.php`
+ * (PHPUnit), so this spec deliberately stubs
+ * `pages/admin.kickit.php` at the network layer instead of
+ * letting the iframe hit the live PHP / DB stack.
+ *
+ * The reason that matters: the live stack runs against the
+ * shared `sourcebans_e2e` DB, and a sibling worker's
+ * `truncateAndReseed()` (correctly serialized via the
+ * `GET_LOCK`-backed named lock in `Sbpp\Tests\Fixture`) can
+ * still wipe `:prefix_admins` between this test's parent-page
+ * auth check and the iframe's auth check, leaving the iframe's
+ * `UserManager::HasAccess()` call with `$this->admins[1]
+ * === null` and the iframe document body reading "No Access"
+ * (matching AGENTS.md "Playwright E2E specifics" → "missing
+ * admin row during a reseed window -> `forbidden / No access`").
+ * Stubbing the iframe URL decouples this spec from that race
+ * surface AND keeps the iframe contract under our control —
+ * the only thing we're asserting is "the parent JS spawned
+ * an iframe with the right URL", which is the entire bug
+ * surface.
+ *
+ *  - The route matcher uses `**\/api.php` (NOT
+ *    `/pages/api.php`) so a regression that re-introduces the
+ *    pre-#1433 endpoint resolution bug would surface here too —
+ *    a broken endpoint resolver would target `/pages/api.php`,
+ *    miss our route, and the parent form's `bans.add` POST
+ *    would time out.
+ */
+
+import { expect, test } from '../../fixtures/auth.ts';
+
+const ADD_BAN_ROUTE = '/index.php?p=admin&c=bans&section=add-ban';
+
+const FIXTURE = {
+    steam:    'STEAM_0:1:1441000',
+    nickname: 'kickit-iframe-fixture',
+    reason:   'Test reason for #1441 regression coverage',
+} as const;
+
+test.describe('flow: ban-add surfaces the kickit iframe (#1441)', () => {
+    // Skip on mobile-chromium for the same reason as
+    // `kickit-iframe.spec.ts`: the contract is browser-shape-agnostic
+    // (lives in page_admin_bans_add.tpl's IIFE which runs identically
+    // on both form factors).
+    test.beforeEach(({}, testInfo) => {
+        test.skip(
+            testInfo.project.name !== 'chromium',
+            'Browser-shape-agnostic; skip the second project to avoid doubling Apache contention.',
+        );
+    });
+
+    test('kickit-enabled BansAdd response spawns hidden #srvkicker iframe with correct URL', async ({ page }) => {
+        // Stub `pages/admin.kickit.php` at the network layer with
+        // a tiny inert HTML body. Two contracts this gives us:
+        //
+        //  1. The iframe's HTTP request still fires AND its URL +
+        //     query string are observable via `page.on('request')`
+        //     and `page.on('response')`, so we can pin the exact
+        //     URL the parent JS chose.
+        //  2. The iframe never reaches `web/pages/admin.kickit.php`'s
+        //     auth gate, so the test is completely insulated from
+        //     the shared-DB truncate-and-reseed race that
+        //     `Sbpp\Tests\Fixture` can't prevent across workers
+        //     (the AGENTS.md `workers: 1` rule is the canonical
+        //     answer, but we don't rely on it here — this spec is
+        //     safe at any worker count).
+        const iframeDocRequests: string[] = [];
+        let kickitDocRequestUrl: string | null = null;
+        await page.route('**/pages/admin.kickit.php**', async (route) => {
+            const url = route.request().url();
+            iframeDocRequests.push(url);
+            if (kickitDocRequestUrl === null) {
+                kickitDocRequestUrl = url;
+            }
+            await route.fulfill({
+                status:      200,
+                contentType: 'text/html; charset=UTF-8',
+                // Tiny inert body — the parent JS never reads it,
+                // the test never reads it, but a non-empty body
+                // keeps the iframe in a "loaded" state so any
+                // subsequent assertion that probes the iframe's
+                // ready state behaves predictably.
+                body:        '<!doctype html><html><head><title>kickit stub</title></head><body></body></html>',
+            });
+        });
+
+        // Stub the parent form's `Actions.BansAdd`. Any other
+        // action falls through to the real handler — but with
+        // the iframe URL routed above, the iframe won't fire
+        // any API calls at all (its inert body has no JS).
+        let bansAddSeen = false;
+        await page.route('**/api.php', async (route) => {
+            const req = route.request();
+            if (req.method() !== 'POST') {
+                await route.continue();
+                return;
+            }
+            let body: { action?: string; params?: Record<string, unknown> } = {};
+            try {
+                body = JSON.parse(req.postData() ?? '{}');
+            } catch {
+                await route.continue();
+                return;
+            }
+
+            if (body.action === 'bans.add') {
+                bansAddSeen = true;
+                await route.fulfill({
+                    status:      200,
+                    contentType: 'application/json',
+                    body:        JSON.stringify({
+                        ok:   true,
+                        data: {
+                            bid: 9999,
+                            // `reload: false` keeps the parent
+                            // page put for the duration of the
+                            // test — the production reload-after-
+                            // 2s contract is irrelevant to what
+                            // we're asserting here AND would
+                            // tear down the iframe before we
+                            // could observe it. The reload
+                            // semantics are pinned separately
+                            // by `admin-ban-lifecycle.spec.ts`.
+                            reload: false,
+                            // `kickit` is the load-bearing envelope:
+                            // its presence is what triggers the
+                            // iframe spawn. Both `check` (the
+                            // steam ID for Steam-type bans) and
+                            // `type` (0 = Steam, 1 = IP) MUST be
+                            // forwarded to the iframe URL via
+                            // encodeURIComponent so a hostile
+                            // value can't break out of the
+                            // query string.
+                            kickit: {
+                                check: FIXTURE.steam,
+                                type:  0,
+                            },
+                            message: null,
+                        },
+                    }),
+                });
+                return;
+            }
+
+            await route.continue();
+        });
+
+        await page.goto(ADD_BAN_ROUTE);
+        const form = page.locator('[data-testid="addban-form"]');
+        await expect(form).toBeVisible();
+
+        await form.locator('[data-testid="addban-nickname"]').fill(FIXTURE.nickname);
+        await form.locator('[data-testid="addban-steam"]').fill(FIXTURE.steam);
+        // Default reason is the first dropdown option; we pick the
+        // "other" path so the per-field reason custom input is the
+        // value that lands on the server. This matches the existing
+        // admin-ban-lifecycle.spec.ts shape.
+        await form.locator('[data-testid="addban-reason"]').selectOption({ value: 'other' });
+        await form.locator('[data-testid="addban-reason-custom"]').fill(FIXTURE.reason);
+
+        await form.locator('[data-testid="addban-submit"]').click();
+
+        // The kickit branch fires AFTER the stub responds. The
+        // iframe creation is synchronous inside the `.then()`
+        // callback, so the iframe element should exist as soon
+        // as the api round-trip resolves.
+        const iframe = page.locator('iframe#srvkicker');
+        await expect(iframe).toBeAttached();
+
+        // The iframe MUST be display: none — operators don't see
+        // the kick-status table on the add-ban form (it's a
+        // background side-effect). The check is via the inline
+        // style attribute the JS sets, not the computed style
+        // (which would also match a CSS-class-driven approach we
+        // don't ship). Mirroring `page_admin_comms_add.tpl`'s
+        // shape: `iframe.style.display = 'none'`.
+        await expect(iframe).toHaveAttribute('style', /display:\s*none/);
+
+        // The iframe URL must carry the encoded SteamID + type. We
+        // anchor on the exact pattern so a regression that drops
+        // either parameter (or swaps to a different query-string
+        // shape that the kickit page doesn't consume) fails the
+        // gate. `encodeURIComponent('STEAM_0:1:1441000')` ->
+        // `STEAM_0%3A1%3A1441000`.
+        const expectedSrcSuffix =
+            `pages/admin.kickit.php?check=${encodeURIComponent(FIXTURE.steam)}&type=0`;
+        await expect(iframe).toHaveAttribute(
+            'src',
+            new RegExp(`${expectedSrcSuffix.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`),
+        );
+
+        // The iframe document request MUST have actually fired with
+        // the URL we expect. This is the proof that the iframe was
+        // mounted (not just attached as a DOM node with a src
+        // attribute that the browser silently failed to fetch).
+        await expect
+            .poll(() => kickitDocRequestUrl, {
+                message:
+                    'The browser should have fired a GET against ' +
+                    'pages/admin.kickit.php?check=...&type=0 to load the iframe document. ' +
+                    'If this is null, the iframe was attached to the DOM but the browser ' +
+                    'never tried to load its src — typically because the src URL is malformed.',
+                timeout: 5_000,
+            })
+            .not.toBeNull();
+        expect(kickitDocRequestUrl).toMatch(
+            new RegExp(`${expectedSrcSuffix.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`),
+        );
+
+        // The "Ban Added" toast paints inside the same kickit
+        // branch (post-#1441 the literal title is 'Ban Added',
+        // case-sensitive — matching comms.add's 'Block Added' AND
+        // the kickit-disabled fallback's 'Ban Added', which was
+        // standardised in the same PR per reviewer Finding #3).
+        // The case-insensitive `/ban added/i` matcher is defensive
+        // — any future renames (e.g., "Ban Created" / "Banned") will
+        // surface here first; the iframe assertion above is the
+        // strict pin for the kickit-enabled branch.
+        const successToast = page
+            .locator('.toast[data-kind="success"]')
+            .filter({ hasText: /ban added/i });
+        await expect(successToast).toBeVisible();
+
+        expect(bansAddSeen, 'Actions.BansAdd should have been called').toBe(true);
+        // Strict-equality on the request count is the contract:
+        // `setBusy(submitBtn, true)` is supposed to keep the
+        // button busy through the kickit branch (matching the
+        // comms.add shape — the operator can't queue a second
+        // submit while rcon fans out at every server). A
+        // regression that double-mounts the iframe (e.g., a
+        // `.then()` chain that resolves twice, a `dispatchEvent`
+        // re-firing the submit handler, a setBusy gap) MUST
+        // surface here. `toBeGreaterThanOrEqual(1)` would have
+        // silently passed.
+        expect(
+            iframeDocRequests.length,
+            `pages/admin.kickit.php should be requested exactly once; saw ${iframeDocRequests.length}: ${JSON.stringify(iframeDocRequests)}`,
+        ).toBe(1);
+    });
+
+    test('kickit-disabled BansAdd response does NOT spawn the iframe', async ({ page }) => {
+        // Same iframe-route stub as the kickit-enabled test —
+        // belt-and-suspenders so that if the iframe IS spawned in
+        // error, the request is observable here too AND the
+        // iframe still doesn't hit the live PHP stack.
+        const iframeDocRequests: string[] = [];
+        await page.route('**/pages/admin.kickit.php**', async (route) => {
+            iframeDocRequests.push(route.request().url());
+            await route.fulfill({
+                status:      200,
+                contentType: 'text/html; charset=UTF-8',
+                body:        '<!doctype html><html><head><title>kickit stub</title></head><body></body></html>',
+            });
+        });
+
+        let bansAddSeen = false;
+        await page.route('**/api.php', async (route) => {
+            const req = route.request();
+            if (req.method() !== 'POST') {
+                await route.continue();
+                return;
+            }
+            let body: { action?: string; params?: Record<string, unknown> } = {};
+            try {
+                body = JSON.parse(req.postData() ?? '{}');
+            } catch {
+                await route.continue();
+                return;
+            }
+            if (body.action === 'bans.add') {
+                bansAddSeen = true;
+                // Mirror the kickit-disabled return shape from
+                // `api_bans_add` — `kickit: null` + a message
+                // envelope. The frontend's kickit branch MUST
+                // skip on `kickit === null` and fall through to
+                // the toast path.
+                await route.fulfill({
+                    status:      200,
+                    contentType: 'application/json',
+                    body:        JSON.stringify({
+                        ok:   true,
+                        data: {
+                            bid:     9998,
+                            reload:  false,
+                            kickit:  null,
+                            message: {
+                                title: 'Ban Added',
+                                body:  'The ban has been successfully added',
+                                kind:  'green',
+                                redir: 'index.php?p=admin&c=bans',
+                            },
+                        },
+                    }),
+                });
+                return;
+            }
+            await route.continue();
+        });
+
+        await page.goto(ADD_BAN_ROUTE);
+        const form = page.locator('[data-testid="addban-form"]');
+        await expect(form).toBeVisible();
+
+        await form.locator('[data-testid="addban-nickname"]').fill(FIXTURE.nickname);
+        await form.locator('[data-testid="addban-steam"]').fill(FIXTURE.steam);
+        await form.locator('[data-testid="addban-reason"]').selectOption({ value: 'other' });
+        await form.locator('[data-testid="addban-reason-custom"]').fill(FIXTURE.reason);
+        await form.locator('[data-testid="addban-submit"]').click();
+
+        // The success toast must paint — proves the fallthrough
+        // path actually ran instead of crashing on the kickit
+        // branch's missing payload.
+        const successToast = page
+            .locator('.toast[data-kind="success"]')
+            .filter({ hasText: /ban added/i });
+        await expect(successToast).toBeVisible();
+
+        // The iframe MUST NOT be attached and the iframe
+        // document URL MUST NOT be requested. Both halves are
+        // pinned — `toHaveCount(0)` is the DOM-shape guard,
+        // `iframeDocRequests.length === 0` is the network-shape
+        // guard. A regression that mounts the iframe AFTER the
+        // toast paints would fail the DOM guard; a regression
+        // that fires the request without attaching the iframe
+        // would fail the network guard.
+        const iframe = page.locator('iframe#srvkicker');
+        await expect(iframe).toHaveCount(0);
+        expect(
+            iframeDocRequests.length,
+            `pages/admin.kickit.php must NOT be requested on the kickit-disabled path; saw ${iframeDocRequests.length}: ${JSON.stringify(iframeDocRequests)}`,
+        ).toBe(0);
+
+        expect(bansAddSeen).toBe(true);
+    });
+});
diff --git a/web/themes/default/page_admin_bans_add.tpl b/web/themes/default/page_admin_bans_add.tpl
index 3bd854fa3..b8a30e564 100644
--- a/web/themes/default/page_admin_bans_add.tpl
+++ b/web/themes/default/page_admin_bans_add.tpl
@@ -17,11 +17,19 @@
     legacy ProcessBan validation, and dispatches Actions.BansAdd
     directly through sb.api.call. Toasts go through window.SBPP.showToast
     when present (theme.js, sbpp2026) with sb.message as a fallback.
-    The kickit branch defers to ShowKickBox/TabToReload only when
-    sourcebans.js is also loaded — sbpp2026 has no native UI for the
-    kickit popup yet so the call would otherwise no-op silently. The
-    default theme keeps its own page_admin_bans_add.tpl which still
-    uses onclick="ProcessBan();" via sourcebans.js.
+
+    Kickit (kick-on-ban) — when `config.enablekickit` is on (the
+    install default), api_bans_add returns a `kickit` envelope
+    (`{check, type}`). The success branch spawns a hidden iframe
+    pointing at `pages/admin.kickit.php?check=...&type=...` — the
+    iframe enumerates enabled servers and fires `sm_kick` via rcon
+    on each. This mirrors the comms.add → blockit.php pattern one
+    flow over and replaces the v1.x `ShowKickBox()` / `TabToReload()`
+    helpers that lived in `web/scripts/sourcebans.js` (deleted at
+    #1123 D1). Pre-#1441 the branch checked `typeof window.ShowKickBox
+    === 'function'` which silently resolved to `false`, falling
+    through to a "Ban added" toast while the ban DB row landed but
+    no live server ever kicked the player.
 
     Permission gate: $permission_addban is precomputed in admin.bans.php
     from ADMIN_OWNER | ADMIN_ADD_BAN.
@@ -373,18 +381,48 @@
             reason: reason,
             fromsub: Number(($id('fromsub') && $id('fromsub').value) || 0)
         }).then(function (r) {
+            if (r && r.ok && r.data && r.data.kickit) {
+                // Success - keep the button busy (matches the comms.add
+                // shape) so the operator can't queue a second submit
+                // while the iframe fires rcon at every server.
+                var k = r.data.kickit;
+                toast('success', 'Ban Added',
+                    'The ban has been successfully added.');
+                // The iframe is load-bearing - pages/admin.kickit.php
+                // loops the enabled servers and fires `sm_kick` via
+                // rcon for each one. Without it the DB row exists but
+                // no live server kicks the banned player. Mirror of
+                // the comms.add → blockit.php pattern one branch over
+                // (#1441 - replaces the v1.x ShowKickBox helper that
+                // was deleted with sourcebans.js at #1123 D1).
+                var iframe = document.createElement('iframe');
+                iframe.id = 'srvkicker';
+                iframe.style.display = 'none';
+                iframe.src = 'pages/admin.kickit.php?check='
+                    + encodeURIComponent(k.check)
+                    + '&type=' + encodeURIComponent(k.type);
+                document.body.appendChild(iframe);
+                if (r.data.reload) {
+                    setTimeout(function () {
+                        window.location.href = window.location.href.replace(/#\^.*$/, '');
+                    }, 2000);
+                }
+                return;
+            }
             setBusy(btn, false);
             if (!r || r.ok === false) {
                 toast('error', 'Add ban failed', (r && r.error && r.error.message) || 'Unknown error');
                 return;
             }
-            if (r.data && r.data.kickit && typeof window.ShowKickBox === 'function') {
-                window.ShowKickBox(r.data.kickit.check, r.data.kickit.type);
-                if (r.data.reload && typeof window.TabToReload === 'function') window.TabToReload();
-                return;
-            }
             var msg = (r.data && r.data.message) || null;
-            toast('success', (msg && msg.title) || 'Ban added', (msg && msg.body) || 'The ban has been successfully added');
+            // 'Ban Added' (Title Case) matches the kickit branch above
+            // AND comms.add's 'Block Added'. Pre-#1441 the kickit branch
+            // never fired, so only this fallback's lowercase 'Ban added'
+            // was visible to operators; the casing inconsistency was
+            // invisible. With #1441 both branches now run on different
+            // installs (kickit enabled vs disabled) and operators would
+            // see two casings; standardise on Title Case for symmetry.
+            toast('success', (msg && msg.title) || 'Ban Added', (msg && msg.body) || 'The ban has been successfully added');
             reset();
         });
     });
diff --git a/web/themes/default/page_kickit.tpl b/web/themes/default/page_kickit.tpl
index ba1055e57..60d6faf9e 100644
--- a/web/themes/default/page_kickit.tpl
+++ b/web/themes/default/page_kickit.tpl
@@ -21,16 +21,22 @@
     working.
 
     Anti-FOUC bootloader (#1438): the `pages/admin.kickit.php` URL is
-    reachable two ways in v2.0 chrome: as an iframe inside the
-    post-Ban "Ban Added" `sb.message.show` dialog (legacy default-theme
-    surface — `#dialog-placement` only exists on the default theme),
-    AND as a top-level navigation from the public Servers page's
-    right-click context menu's "Kick player" item (`web/scripts/server-context-menu.js`
-    builds the href directly to `pages/admin.kickit.php?check=…`). The
-    latter is the user-reported #1438 path: an operator in dark mode
-    right-clicks a player, picks Kick, and lands on this chromeless
-    iframe template rendered as a full-page document — and the page
-    paints stark white because `<html>` never gets the `dark` class.
+    reachable two ways in v2.0 chrome: as a hidden `#srvkicker` iframe
+    spawned by the post-Ban "Ban Added" success branch in
+    `page_admin_bans_add.tpl` (the modern theme's mirror of the
+    comms.add → blockit.php flow; post-#1441 this replaces the
+    pre-v2.0 `sb.message.show` dialog surface — `#dialog-placement`
+    only existed on the v1.x default theme and the legacy
+    `parent.document.getElementById('dialog-control')` lookups in
+    the JS below gracefully no-op when run inside the new hidden
+    iframe), AND as a top-level navigation from the public Servers
+    page's right-click context menu's "Kick player" item
+    (`web/scripts/server-context-menu.js` builds the href directly
+    to `pages/admin.kickit.php?check=…`). The latter is the
+    user-reported #1438 path: an operator in dark mode right-clicks
+    a player, picks Kick, and lands on this chromeless iframe
+    template rendered as a full-page document — and the page paints
+    stark white because `<html>` never gets the `dark` class.
     The inline bootloader below mirrors `core/header.tpl`'s shape
     (same THEME_KEY 'sbpp-theme', same default 'system', same dark-
     resolution predicate; only ADDS the class, never removes — :root