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

[andrewgazelka]

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.

[github-actions]

Benchmark Results for general

ray_intersection/aabb_size_0.1                     [  18.6 ns ...  18.7 ns ]      +0.29%
ray_intersection/aabb_size_1                       [  18.7 ns ...  18.6 ns ]      -0.30%
ray_intersection/aabb_size_10                      [  18.6 ns ...  18.6 ns ]      +0.13%
ray_intersection/ray_distance_1                    [   1.3 ns ...   1.3 ns ]      +0.14%
ray_intersection/ray_distance_5                    [   1.3 ns ...   1.3 ns ]      +0.08%
ray_intersection/ray_distance_20                   [   1.3 ns ...   1.3 ns ]      -0.25%
overlap/no_overlap                                 [  15.7 ns ...  15.6 ns ]      -0.36%
overlap/partial_overlap                            [  15.6 ns ...  15.6 ns ]      +0.52%*
overlap/full_containment                           [  14.5 ns ...  14.5 ns ]      -0.01%
point_containment/inside                           [   5.4 ns ...   5.4 ns ]      +0.16%
point_containment/outside                          [   5.7 ns ...   5.7 ns ]      -0.06%
point_containment/boundary                         [   5.4 ns ...   5.4 ns ]      -0.23%

Comparing to 54de2ba

[github-actions]

Benchmark Results for general

ray_intersection/aabb_size_0.1                     [  17.3 ns ...  17.3 ns ]      -0.06%
ray_intersection/aabb_size_1                       [  17.2 ns ...  17.3 ns ]      +0.53%
ray_intersection/aabb_size_10                      [  17.3 ns ...  17.2 ns ]      -0.34%
ray_intersection/ray_distance_1                    [   1.5 ns ...   1.5 ns ]      -0.11%
ray_intersection/ray_distance_5                    [   1.5 ns ...   1.5 ns ]      -0.04%
ray_intersection/ray_distance_20                   [   1.5 ns ...   1.5 ns ]      -0.09%
overlap/no_overlap                                 [  15.6 ns ...  15.5 ns ]      -0.08%
overlap/partial_overlap                            [  15.6 ns ...  15.6 ns ]      -0.05%
overlap/full_containment                           [  14.9 ns ...  14.9 ns ]      -0.07%
point_containment/inside                           [   6.1 ns ...   6.1 ns ]      -0.16%
point_containment/outside                          [   6.1 ns ...   6.1 ns ]      -0.06%
point_containment/boundary                         [   6.1 ns ...   6.1 ns ]      +0.06%

Comparing to 035a8ce

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 54.07%. Comparing base (746263a) to head (c9b6903).
⚠️ Report is 20 commits behind head on main.

@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
+ Coverage   53.84%   54.07%   +0.23%     
==========================================
  Files         345      347       +2     
  Lines       32126    32497     +371     
  Branches     1215     1234      +19     
==========================================
+ Hits        17297    17572     +275     
- Misses      14556    14644      +88     
- Partials      273      281       +8     

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

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