fix(smash): the gate declares the lobby it needs, and checks it got it by andrewgazelka · Pull Request #1027 · hyperion-mc/hyperion

andrewgazelka · 2026-07-28T11:05:07Z

Effect

nix run .#smash-e2e passes again, all 15 kits. It has been red since #1019.

What broke

#1019 moved LobbyConfig::default() from 4/8 to 2/4, which the operator asked for. tools/smash-match.py had those numbers written into it a second time -- as max(1, self.args.clients - 1), as "min_players is four" in its docstring, and as "one short of the lobby's minimum" at the call site -- so the sweep's three clients went from below the threshold to above it. The countdown started, the match committed, and every later kit change was refused with You cannot change kit once the game has started.

Reproduced on e4bbcc2 before changing anything:

     0.60s       PROVED lobby started a match: server announced the countdown with 3 players connected
     0.68s       PROVED four in play: 3 clients hold a Login packet: P1=67110240, P2=1377, P3=1378
    30.79s [P3 ] <- chat: §cYou cannot change kit once the game has started.
    45.78s [P3 ] <- chat: §cYou cannot change kit once the game has started.
    60.83s [P1 ] <- chat: §cYou cannot change kit once the game has started.

It was not the minimum that fired, and that decides the fix

The countdown started 0.6 seconds in, not at min_players. countdown_for checked the three-quarters band before the minimum, so at 2/4 three players satisfy 3 * 4 >= 4 * 3 and take the 30-second band.

So min_players - 1 was never a sound expression of "a roster that cannot start a countdown". The real condition depends on full_players too. At 4/8 the subtraction worked by luck.

Why not read the threshold off the wire

#1018 puts Waiting for players {n}/{min_players} on a boss bar, and a harness that reads its own limit off that packet cannot disagree with the server. I rejected it as the fix, because it cannot be one. The sweep needs two clients to prove hurts_target and to call wound_attacker for heals_caster; at min_players: 2 the number it would read off the wire is one, and a one-client sweep goes green while proving less. Observation cannot manufacture headroom, only control can. The bar also advertises only min_players, which per the section above is not the number that decides this.

So the gate declares the lobby, and the harness checks it got it

LobbyConfig::from_env() reads SMASH_MIN_PLAYERS and SMASH_FULL_PLAYERS, in the SMASH_-prefixed namespace hyperion_event_runner::run already reads the address and certificates from. Not added to the shared Args, which bedwars would then carry and never read.
nix/e2e.nix's mkCheck gained serverEnv. smashGateLobby in flake.nix declares 4/8 and a sweep roster of 3 in one place and feeds both the app and the check.
smash-match.py takes --sweep-clients and subtracts nothing. The docstring no longer says four.

4/8 and not just a higher minimum, because full_players is what gates the three-quarters band. These are the pre-#1019 numbers, under which three players start nothing even by the old band order, so the gate does not depend on the countdown_for change below.

The guard is the repro

hub_only fails the run if the lobby leaves the hub while the sweep needs it, checked before the sweep and again before each kit. It reads the phase off the server's own broadcasts rather than recomputing a threshold, so it stays right whatever the numbers become and is not a second copy of them.

Broken deliberately, by pointing smashGateLobby.env at production's 2/4:

     0.72s       SWEEP ABANDONED the lobby left the hub before the sweep began. 3 clients is enough for this server to start a countdown, so kits can no longer be changed and nothing further would be proved. Either --sweep-clients is too many for this lobby, or the server needs a higher SMASH_MIN_PLAYERS/SMASH_FULL_PLAYERS.
     0.72s       RESULT: 10 of 12 steps did not happen
rc=1

571 seconds and a hang become 0.72 seconds and a cause. Restored to 4/8 afterwards.

Breaking it is also what found a real bug in it. The first version stopped the run but only recorded its reason in sweep_failures, which sweep prints at the end of a sweep it no longer reached -- so the run failed with the reason nowhere in the transcript, and then went on to run a mostly-passing match underneath the failure. It now logs directly and returns straight to the report.

countdown_for checks the minimum first

Fixed here rather than filed, because it is a reordering in a file this PR already touches and its failure mode is silent. min_players was a partial lie: at min_players: 4, full_players: 4, three players started a countdown under a stated minimum of four.

Provably inert for both configurations anyone runs. the_live_configurations_are_unaffected_by_the_minimum_check keeps the old three-branch order in the test and asserts the new one agrees with it at every count from 0 to full_players * 3, for 2/4 and 4/8. a_lobby_never_starts_below_its_own_minimum is the case that would have caught it, over every config from 1/1 to 12/12.

Checks

cargo nextest run -p smash: 257 passed. cargo clippy -p smash --all-targets: clean. cargo fmt --check: clean.

nix run .#smash-e2e, all 15 kits and all 12 claims:

 254.43s       === what the match proved ===
 254.43s         proved      the server published its ability registry
 254.43s                     51 abilities across 15 kits (Blaze, Chicken, Cow, Creeper, Enderman, Guardian, Iron Golem, Skeleton, Sky Squid, Slime, Snowman, Spider, Wither Skeleton, Wolf, Zombie), every one of them declaring what a client would see
 254.43s         proved      every declared ability did what it declared
 254.43s                     51 abilities fired by a real client, each one checked against the effects its own registry entry names
 254.43s         proved      every declared ability was heard
 254.43s                     51 abilities fired by a real client, each one answered by a ClientboundSoundPacket carrying the vanilla sound event its own registry entry names
 254.43s         proved      cooldowns refused a second use
 254.43s                     47 abilities answered a second right-click inside their cooldown with 'That ability is recharging.' on the action bar
 254.43s         proved      four in play
 254.43s         proved      lobby started a match
 254.43s                     server announced the countdown with 4 players connected
 254.43s         proved      kits equipped
 254.43s         proved      arena is a committed map
 254.43s         proved      knockback from an ability
 254.43s         proved      a hit was heard where it landed
 254.43s         proved      life lost and respawned
 254.43s         proved      match ended, back in the hub
 254.43s       RESULT: a whole match ran at protocol 776
rc=0

The sweep is 73 seconds for all 51 abilities, not the 571 seconds the broken run took: that figure was time spent on refused kit changes, not real work. The check's 480s timeout has room.

Not done, and one thing found

I did not run the smash-e2e check derivation, only the app. CI covers it and the timing above says it fits.

ENG-10889: tools/smash-selector.py has this same defect twice, FULL_PLAYERS = 8 with a comment citing LobbyConfig::default and "Three, which is one short of min_players" above its own connect(3). smash-selector-e2e is likely broken by #1019 in exactly the way this gate was. Not run and not fixed here. hotbar-check.py:22 and smash-map-check.py:1141 have stale prose about the same numbers but still behave.

## Effect `nix run .#smash-e2e` passes again. It has been red since #1019. ## What broke #1019 moved `LobbyConfig::default()` from 4/8 to 2/4 at the operator's request. `tools/smash-match.py` had those numbers written into it a second time -- as `max(1, self.args.clients - 1)`, as "`min_players` is four" in its docstring, and as "one short of the lobby's minimum" at the call site -- so the sweep's three clients went from below the threshold to above it. The countdown started, the match committed, and every kit change after that was refused with `You cannot change kit once the game has started.` The gate got through 12 of 15 kits and then looped forever. ## It was not the minimum that fired Worth stating, because it decides the fix. `countdown_for` checked the three-quarters band before the minimum, so at 2/4 three players satisfy `3 * 4 >= 4 * 3` and take the 30-second band. Measured: 0.60s PROVED lobby started a match: server announced the countdown with 3 players connected So `min_players - 1` was never a sound expression of "a roster that cannot start a countdown" -- the real condition depends on `full_players` too. At 4/8 it worked by luck. ## Control, not observation The obvious fix is to read the threshold off the boss bar #1018 added, so the harness cannot disagree with the server. It cannot work. The sweep needs two clients to prove `hurts_target` and `heals_caster`, and at `min_players: 2` the number it would read off the wire is one. A one-client sweep goes green while proving less. Observation cannot manufacture headroom. So the gate states the lobby it needs and the harness verifies it got it: - `LobbyConfig::from_env()` reads `SMASH_MIN_PLAYERS` and `SMASH_FULL_PLAYERS`, in the `SMASH_`-prefixed namespace the binary already reads its address from. Not on the shared `Args`, which bedwars would then carry and never read. - `nix/e2e.nix`'s `mkCheck` takes `serverEnv`, and `smashGateLobby` in `flake.nix` declares 4/8 and a sweep roster of 3 in one place, feeding both the app and the check. - `smash-match.py` takes `--sweep-clients` and subtracts nothing. The docstring no longer says four. ## The guard is the repro `hub_only` fails the run if the lobby leaves the hub while the sweep needs it. It reads the phase off the server's own broadcasts rather than recomputing a threshold, so it stays correct whatever the numbers become and is not a second copy of them. Broken deliberately, by pointing `smashGateLobby` at production's 2/4: 0.72s SWEEP ABANDONED the lobby left the hub before the sweep began. 3 clients is enough for this server to start a countdown, so kits can no longer be changed and nothing further would be proved. Either --sweep-clients is too many for this lobby, or the server needs a higher SMASH_MIN_PLAYERS/SMASH_FULL_PLAYERS. 0.72s RESULT: 10 of 12 steps did not happen 571 seconds and a hang become 0.72 seconds and a cause. The first version of the guard stopped the run but only recorded its reason in `sweep_failures`, which `sweep` prints at the end of a sweep it no longer reached, so the run failed with the reason nowhere in the transcript. Caught by breaking it; that is why it logs directly. ## countdown_for now checks the minimum first Separately found and fixed here rather than filed, because it is a reordering in a file this PR already touches and its failure mode is silent. `min_players` was a partial lie: at `min_players: 4, full_players: 4`, three players started a countdown under a stated minimum of four. Provably inert for both live configurations. `the_live_configurations_are_unaffected_by_the_minimum_check` keeps the old three-branch order and asserts the new one agrees with it at every count from 0 to `full_players * 3`, for 2/4 and 4/8. `a_lobby_never_starts_below_its_own_minimum` is the case that would have caught it, over every config from 1/1 to 12/12.

github-actions · 2026-07-28T11:10:09Z

Benchmark Results for general

ray_intersection/aabb_size_0.1                     [  18.7 ns ...  18.7 ns ]      -0.19%
ray_intersection/aabb_size_1                       [  18.7 ns ...  18.7 ns ]      +0.04%
ray_intersection/aabb_size_10                      [  18.6 ns ...  18.7 ns ]      +0.26%
ray_intersection/ray_distance_1                    [   1.3 ns ...   1.3 ns ]      +0.05%
ray_intersection/ray_distance_5                    [   1.3 ns ...   1.3 ns ]      +0.27%
ray_intersection/ray_distance_20                   [   1.3 ns ...   1.3 ns ]      -0.31%
overlap/no_overlap                                 [  15.8 ns ...  15.6 ns ]      -1.45%*
overlap/partial_overlap                            [  15.7 ns ...  15.8 ns ]      +1.00%*
overlap/full_containment                           [  15.1 ns ...  14.8 ns ]      -1.36%*
point_containment/inside                           [   5.4 ns ...   5.4 ns ]      -0.11%
point_containment/outside                          [   5.6 ns ...   5.7 ns ]      +0.23%
point_containment/boundary                         [   5.4 ns ...   5.4 ns ]      +0.10%

Comparing to 8318617

codecov · 2026-07-28T11:14:38Z

Codecov Report

❌ Patch coverage is 87.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.48%. Comparing base (a25c3a9) to head (47e3c15).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
events/smash/src/module/lobby.rs	87.50%	3 Missing ⚠️

@@            Coverage Diff             @@
##             main    #1027      +/-   ##
==========================================
+ Coverage   51.14%   51.48%   +0.34%     
==========================================
  Files         335      337       +2     
  Lines       30052    30574     +522     
  Branches     1137     1167      +30     
==========================================
+ Hits        15369    15742     +373     
- Misses      14438    14580     +142     
- Partials      245      252       +7

Files with missing lines	Coverage Δ
events/smash/src/module/lobby.rs	`98.01% <87.50%> (-1.13%)`	⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

## Effect `smash-selector-e2e` gets the same treatment `smash-e2e` got in #1027. It had the identical defect, twice, and #1019 broke it the same way. ## The two copies `FULL_PLAYERS = 8`, under a comment naming `LobbyConfig::default` as its source, which is what made it a copy. #1019 moved that default to 4 and the number here stayed at 8, still claiming to be it. It is now `--full-clients`, declared by the gate off `SMASH_FULL_PLAYERS` so the two cannot disagree. And "Three, which is one short of `min_players`" above its own `connect(3)`. Three is now one *over* the minimum, so a countdown runs underneath the hub checks instead of after them. Three stays, because the checks need three named roles -- a holder, a watcher, and somebody to take a freed mob -- but it is no longer described as arithmetic on a threshold, because it never was. ## The guard, not the number `hub_only` fails the run if the lobby leaves `Waiting` while the hub checks still need it, called once after the clients join and again before the first claim. It reads the phase off the server's own broadcasts, so it is right whatever the thresholds become. `smash-selector-e2e` now also gets `serverEnv = smashGateLobby.env`, so it runs against the same declared 4/8 lobby as `smash-e2e`. ## One shared helper, and what it deliberately does not do The message both harnesses print is now `hub_lost` in `smash-match.py`, which `smash-selector.py`, `hotbar-check.py`, `hud-check.py` and `identity-check.py` all already load. One failure, one wording. It shares the consequence, not the number, and it cannot share the number. There is no single safe roster: the sweep wants 3, the selector wants 3 for unrelated reasons, and filling a lobby wants 8. The thing that was duplicated is `LobbyConfig::default`, and the right number of copies of that in Python is zero rather than one. A helper returning "the largest roster that will not start" would be the fourth copy wearing a hat, so `hub_lost` says in its own docstring that it will not answer that. ## Stale prose, three files `hotbar-check.py` claimed the lobby needs four players; it needs two, and that gate uses one client so only the reason was wrong. `smash-map-check.py` and `hud-check.py` both called 8 "`full_players`" rather than "at or above it". A comment asserting a false property is worse than no comment.

## Effect `nix run .#smash-selector-e2e` gets the treatment `smash-e2e` got in #1027. It had the identical defect, twice, and #1019 broke it the same way. Filed as ENG-10889 while fixing #1027; fixing rather than leaving it filed. ## The two copies **`FULL_PLAYERS = 8`**, under a comment naming `LobbyConfig::default` as its source, which is exactly what made it a copy. #1019 moved that default to 4 and the number here stayed at 8, still claiming to be it. It is now `--full-clients`, which the gate derives from its own `SMASH_FULL_PLAYERS` so the two cannot disagree. **"Three, which is one short of `min_players`"** above its own `connect(3)`. Three is now one *over* the minimum, so a countdown ran underneath the hub checks instead of after them -- the comment's own stated failure mode, arrived at by the comment going stale. Three stays. The checks need three named roles (a holder, a watcher, and somebody to take a freed mob), which is a fact about this gate and not arithmetic on a threshold. What changed is that it is no longer described as arithmetic on a threshold, because it never was. ## The guard, not the number `hub_only` fails the run if the lobby leaves `Waiting` while the hub checks still need it: once after the clients join, and again before the first claim, since the reads in between cost real seconds. It reads the phase off the server's own broadcasts, so it stays right whatever the thresholds become. `smash-selector-e2e` also now gets `serverEnv = smashGateLobby.env`, so it runs against the same declared 4/8 lobby as `smash-e2e` rather than inheriting the product's. ## One shared helper, and the thing it refuses to do The message both harnesses print is now `hub_lost` in `smash-match.py`, which `smash-selector.py`, `hotbar-check.py`, `hud-check.py` and `identity-check.py` all already load. One failure, one wording. **It shares the consequence, not the number, and it could not share the number.** There is no single safe roster to share: the ability sweep wants 3, the selector wants 3 for entirely unrelated reasons, and filling a lobby wants 8. The thing that was duplicated is `LobbyConfig::default`, and the right number of copies of that in Python is zero rather than one. A helper returning "the largest roster that will not start" would be the fourth copy wearing a hat, so `hub_lost` says in its own docstring that it will not answer that. ## No mechanical gate, and what a reviewer now has to catch I considered a lint over `tools/*.py` -- forbid an integer literal on a line mentioning `min_players`/`full_players`, or a module constant named `*_PLAYERS`. I did not add one. It would have caught `FULL_PLAYERS = 8` and would not have caught `connect(3)` under a false comment, which is the other half of this same bug. A regex over the spelling that happened to occur last time is the failure mode that lets the fourth instance through while reading as protection. The real protection is the runtime guard, which does not care how the number was arrived at. It is opt-in, so three things still need a human: 1. **A new hub-dependent harness that never calls `hub_only`.** Nothing forces adoption. 2. **A roster flag without the matching `SMASH_*` export.** `--full-clients` and `--sweep-clients` only mean anything because their gate also sets the threshold; the flag and the env have to move together. Only `smash-e2e` and `smash-selector-e2e` are wired this way. 3. **`hud-check.py` and `smash-map-check.py` still pass 8 as "enough to fill" against a server they do not configure.** They pass today only because 8 is above a `full_players` of 4. If it ever goes above 8 they will quietly stop testing the full band instead of failing. I did not close this: those gates belong to other agents, and closing it means giving them `serverEnv` too. Named here rather than left implicit. ## Stale prose, three files `hotbar-check.py` claimed the lobby needs four players. It needs two, and that gate uses one client, so only the reason was wrong. `smash-map-check.py` called 8 "`full_players`" rather than "at or above it". `hud-check.py` had the same in its docstring; its `--clients` help was already corrected on main by another agent and I took theirs over mine in the rebase. ## Checks `nix run .#smash-selector-e2e`, all 15 claims: ``` PASS the hub has a podium per mob, with that mob standing on it PASS every mob wears its kit's name, always visible 15 of 15 PASS a click plays the mob's own declared sound 15 of 15 kits answered PASS and only the player who clicked hears it S2 heard none of 15 selections PASS all fifteen mobs answer in different voices 15 distinct sounds over 15 kits PASS the other player sees the new skin on the wire PASS and it did not cost the wearer its world PASS a right-click on a podium picks that mob PASS a taken mob is red without anybody reading anything PASS a mob somebody else has is refused, by name PASS the refusal is about that mob and not about clicking PASS a holder who disconnects frees their mob at once PASS a full lobby holds one mob each S2=Iron Golem; S3=Skeleton; ... S9=Wolf PASS a selection made in the hub survives the countdown 8 clients started the match on the mob they clicked for PASS a click after the match commits is refused RESULT: the kit selector works, on the wire rc=0 ``` Guard broken deliberately, by pointing `smashGateLobby.env` at production's 2/4: ``` 0.72s FAILED the lobby left the hub before the hub checks began. 3 clients is enough for this server to start a countdown, so the hub is gone and nothing after this would be proved. Either the gate's roster is too many for this lobby, or the server needs a higher SMASH_MIN_PLAYERS/SMASH_FULL_PLAYERS. RESULT: 1 checks failed rc=1 ``` That run is also the confirmation ENG-10889 was missing: at 2/4 three clients start a countdown 0.72 seconds in, so `smash-selector-e2e` really was broken by #1019 and not merely suspicious. Restored to 4/8 afterwards. ## `smash-e2e` may be red on main, but I could not confirm it here, and this PR does not touch it Reported honestly rather than overstated: `nix run .#smash-e2e` failed on a Sky Squid `shields_caster` probe in every run of mine that reached it (5a486fd twice, 54de2ba once), filed as ENG-10906. But I cannot confirm a real bug from this machine, and the ticket now says so. The box has sat at load average ~20 all night (22 users, seven agents). The shield probe hits the caster immediately after the press expecting the immunity tag already applied, which makes it the tightest-timing probe in the sweep and the first to flake under load. When I stacked a `cargo` compile on top, the sweep took 377s instead of 121s and *five* unrelated timing probes failed (both shields, Storm Squid's launches, `buffs_melee` on two kits) -- a load signature, not five bugs. The load-immune instrument disagrees with the wire: `events/smash/tests/abilities.rs::every_declared_effect_actually_happens` passes on 54de2ba, and it is not a blind test -- I made `arm_shield` a no-op and watched it fail in 1.2s with verbatim the same sentence. So the shield works in-process, and the leading explanation for the wire failure is the probe racing the shield's application, not a broken shield. #1032 already retired the old "hit 50ms after the press" probe for exactly this reason. The wire verdict needs CI's `Flake` job or an idle host; ENG-10906 is rescoped to that rather than closed or asserted. None of this is in scope for this PR, which touches only the harness wiring and prose. Its own gate, `smash-selector-e2e`, is green. Python: `python3 -m py_compile` on all five changed harnesses, and `ruff check` clean on `smash-selector.py` before and after.

andrewgazelka merged commit 3be8426 into main Jul 28, 2026
9 of 11 checks passed

andrewgazelka deleted the fix/smash-e2e-threshold branch July 28, 2026 11:08

github-actions Bot added the fix label Jul 28, 2026

andrewgazelka mentioned this pull request Jul 28, 2026

fix(smash): the selector gate stops guessing the lobby too #1033

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(smash): the gate declares the lobby it needs, and checks it got it - #1027

fix(smash): the gate declares the lobby it needs, and checks it got it#1027
andrewgazelka merged 1 commit into
mainfrom
fix/smash-e2e-threshold

andrewgazelka commented Jul 28, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

codecov Bot commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

andrewgazelka commented Jul 28, 2026

Effect

What broke

It was not the minimum that fired, and that decides the fix

Why not read the threshold off the wire

So the gate declares the lobby, and the harness checks it got it

The guard is the repro

countdown_for checks the minimum first

Checks

Not done, and one thing found

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026

Benchmark Results for general

Uh oh!

codecov Bot commented Jul 28, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant