console review round 2: six must-fixes from the adversarial pass (one of its fixes corrected)
The 4-dimension review of 4736cba..820caf8 returned 14 surviving findings. All six must-fixes plus the four deferables are in; one of the review's own prescriptions was wrong and is fixed differently (below). THE REAPER, FINAL DOCTRINE (blocker + major). Rev 2 -- "reap anything silent in the staging window" -- was still wrong twice over: a pad sends exactly ONE message in its life (the egg ACK, L4NET.CPP:1259) and is then silent FOREVER, so any manual-launch hold >180s reaped every healthy ACKed pad and force-relaunched their clients; and a REGISTERED game conn is quiet on TCP while loading, so with BT_RELAY_TCP_ONLY=1 (or blocked UDP) the reaper _abort_round()ed the whole night every ~3 minutes blaming a healthy player. The rule is now: an app-level deadline is valid only while a RESPONSE IS OWED. Only egg-sent-never-ACKed pads are reaped, with the debt clock starting at EGG SEND (a pad that sat through a long held-egg wait gets its full window -- an edge neither review round caught); game conns are never reaped at all. Half-open ghosts are TCP keepalive's job (~130s, faster than the deadline anyway). RE-ARM GUARDED ON EVERY CHANNEL (major). The launch_at guard lived only on the LAUNCH-press path; the ctl `rearm` command, stdin, and the always-lit GUI button reached _rearm_for_new_round unconditionally -- one press mid-mission zeroed the counters, permanently killing the mission clock AND making End Mission print "no mission is running": an unstoppable round, the exact class this feature exists to eliminate. Now refused (loudly) while a mission is running or a pair is in flight, and the button greys while launched. THE REVIEW'S OWN FIX WAS WRONG here: it prescribed refusing on `launches_sent >= 2`, but that stays 2 after a FINISHED round -- applying it verbatim resurrected the original dead-LAUNCH wedge, caught immediately by the regression suite. "Running" is `launches_sent >= 2 AND not stop_sent`. RE-ARM RE-KEYS SEATS (major). It restored the template roster but left seat_beacons/seat_prefs keyed by the trimmed round's positional ids, so round 2's trim minted a departed player's tag into the egg and trimmed a present player's out, shifting every callsign/mech a slot. The re-key block is factored out of _maybe_reset_round (_rekey_seats_to_roster) and shared. RESTART SESSION NO LONGER BAKES A WALK-UP'S NAME INTO THE EGG (major). _stop_session and _start_session now restore the stashed configured callsign/mech into the cells BEFORE _collect_egg can snapshot them (_restore_seat_defaults); previously the departed name went into the egg (name= plus rasterized bitmaps) and was then re-captured as the seat's permanent "configured default". LATE REMOTE OPERATOR GETS A ROSTER (major). The only line the GUI can adopt tags from was printed once at relay startup and aged out of the 400-line control history in ~33 minutes of stats chatter -- AUTH now re-issues the live roster line ahead of the replay, so a remote operator connecting at any point gets pilot lights and a working LAUNCH button. DEFERABLES, all four: _drop_game blames the tag stashed AT REGISTRATION (the live-roster resolve named the wrong tag for a trimmed-round conn dying after a re-arm restore -- and the tag-trusting GUI would clear the wrong seat); an operator-BLANKED callsign cell now restores (empty string is a real configured value; the falsy skip left the departed name up and re-captured it); a returning player displaces their own half-open registered ghost (same IP, no mission running) instead of eating ROSTER FULL until keepalive fires; udp_spoofed now rides the [relay-stats] line when non-zero and the warn set is capped at 256. VERIFIED: rearm suite grown to 25 checks (ACKed pad never reaped however silent; never-ACKed pad reaped; game conns never reaped; the egg-send debt clock; re-arm refused mid-mission, allowed after round end) -- plus net 17/17, roster 22/22, checkctx CLEAN. Live rig: mid-mission `rearm` refused with the message and the mission survived; End Mission -> re-arm -> full re-seat -> second mission launched. One bounded artifact observed and documented: each waiting pod bounces once (identity resync) after an explicit re-arm. Docs rewritten to the final doctrine (context/operator-console.md reaper + re-arm + roster-replay + udp_spoofed sections; OPERATOR_GUIDE + tooltip now say re-arm is between-rounds-only and warn about the one-bounce resync). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
820caf8765
commit
c9e561d9ba
+20
-17
@@ -202,7 +202,7 @@ surviving pods' sockets close, so every later `_maybe_reset_round` hits its own
|
||||
three automatic release paths compare against `len(self.roster)`, so with N−1 pods back nothing
|
||||
releases, walk-ups get `ROSTER FULL`, and the relay sits dead.
|
||||
|
||||
`_rearm_for_new_round`'s template restore is therefore **unconditional** — an earlier cut gated it
|
||||
`_rearm_for_new_round` is **guarded on every channel** (ctl command, stdin, the GUI button -- the guard lived only on the LAUNCH path at first, so a ctl `rearm` mid-mission killed the mission clock and made the round unstoppable): it refuses while a mission is RUNNING (`launches_sent >= 2 and not stop_sent` -- NOT `launches_sent` alone, which stays 2 after a finished round and would resurrect the original wedge) and while a launch pair is in flight. It also re-keys `seat_beacons`/`seat_prefs` to the restored roster (`_rekey_seats_to_roster`, shared with the round reset) -- without that, round 2 trimmed the WRONG seats into the egg. Its template restore is **unconditional** — an earlier cut gated it
|
||||
on `eggs_released`, which made Re-arm useless in the one state that most needs it. It also clears
|
||||
`round_hold_until` so an abort's settle window is not inherited.
|
||||
|
||||
@@ -229,19 +229,20 @@ phantom seat in the launch count. Only restarting the relay cleared it.
|
||||
Now: keepalive on every accepted socket (`_enable_keepalive`), a `last_seen` stamp on every read
|
||||
**and on every inbound UDP datagram**, and `_reap_dead_peers()` with `DEAD_PEER_SECONDS = 180`.
|
||||
|
||||
**The reaper is deliberately narrow, and the narrowness is load-bearing.** It runs **only in the
|
||||
active staging window** — egg released, mission not yet running — and skips console pads with no egg
|
||||
sent and game conns that have not HELLO'd. The first cut was gated only on "no mission running",
|
||||
which armed it for the entire **between-rounds** wait, and that is a period when a pod is
|
||||
**legitimately byte-silent**: its seat beacon is write-only for the process lifetime
|
||||
(`L4NET.CPP`: *"the relay ignores its silence"*, and the pod sets its own keepalive on it), and its
|
||||
console pad has no egg yet so it cannot ACK. A real night showed 12-minute and 5-minute gaps
|
||||
between rounds, so a 180 s deadline would have dropped **healthy** players and forced their clients
|
||||
to relaunch — strictly worse than the ghost the reaper exists to remove. Caught in review before it
|
||||
ever ran live. Regression guards: `scratchpad/test_relay_rearm.py` cases 5b/5c.
|
||||
**The reaper's final doctrine (third revision — review 2026-07-26 — KEEP IT):** an app-level
|
||||
deadline is valid only where silence is ABNORMAL, i.e. while a protocol response is OWED. The only
|
||||
thing reaped is a console pad that was sent the egg and never ACKed within `DEAD_PEER_SECONDS`
|
||||
(measured from EGG SEND, not from connect — a pad that sat through a long held-egg wait must get its
|
||||
full window). Everything else is TCP keepalive's job (~130 s detection via `SIO_KEEPALIVE_VALS`).
|
||||
|
||||
Half-open sockets are TCP keepalive's job; this deadline is only the backstop for platforms where
|
||||
the keepalive tunables do not take.
|
||||
Why each earlier revision was wrong, so nobody re-widens it:
|
||||
- Rev 1 armed it whenever no mission ran → it would have reaped healthy pods during 12-minute
|
||||
BETWEEN-ROUNDS waits (a waiting pod is byte-silent by design).
|
||||
- Rev 2 armed it for the whole STAGING window → it reaped every healthy **ACKed** pad 180 s into a
|
||||
manual-launch hold, because a pad sends exactly ONE message in its life (the ACK) and is then
|
||||
silent forever; and it reaped **registered game conns** that are quiet on TCP while loading
|
||||
(fatal with `BT_RELAY_TCP_ONLY=1` or blocked UDP: `_abort_round` bounced everyone, blaming a
|
||||
healthy player, every ~3 minutes). Game conns are now never reaped at all.
|
||||
|
||||
### Console-protocol reassembly [FIXED 2026-07-26]
|
||||
|
||||
@@ -264,8 +265,7 @@ id — and the victim simply stopped receiving on a channel that still looked he
|
||||
now bound to the identity we actually authenticated: the **IP of that host's live TCP game
|
||||
connection**. The **port is deliberately not checked** (it moves on a NAT rebind, which is the whole
|
||||
reason the endpoint map refreshes per datagram), and a genuine IP change cannot happen without the
|
||||
TCP connection breaking and re-registering, so this never rejects a legitimate pod. Rejections are
|
||||
counted (`udp_spoofed`) and logged once per offending `(host, IP)` pair.
|
||||
TCP connection breaking and re-registering, so this never rejects a legitimate pod. Rejections are counted and now SURFACED — `spoofed N` rides the `[relay-stats]` line whenever non-zero — and logged once per offending `(host, IP)` pair (the warn set is capped at 256 under a flood).
|
||||
|
||||
Limitation worth knowing: two pods on the **same machine** share an IP, so this cannot tell them
|
||||
apart — same-machine trust is assumed.
|
||||
@@ -320,8 +320,11 @@ doing nothing:
|
||||
`Launch local instances` was refused by the same guard even though the code below it already
|
||||
built `BT_RELAY` from the remote host. Now: all three enable conditions accept **either** channel
|
||||
(`console_proc` *or* `remote_link`), and the monitor **adopts the roster** from the relay's
|
||||
`roster: N pilot(s) -> hostIDs [...]: [...]` line, which the relay replays to every newly AUTHed
|
||||
operator — so a remote operator gets real pilot lights and a real seat count.
|
||||
`roster: N pilot(s) -> hostIDs [...]: [...]` line, which the relay **re-issues from live state to
|
||||
every newly AUTHed operator ahead of the history replay** — relying on the startup line alone
|
||||
broke after ~33 minutes, when stats chatter aged it out of the 400-line history and a
|
||||
late-connecting remote operator got a blank roster and a dead LAUNCH button. So a remote operator
|
||||
gets real pilot lights and a real seat count at any time of night.
|
||||
- **`Start session` silently overwrites the egg on disk** (including re-rasterized callsigns), with
|
||||
no prompt and no backup.
|
||||
- **`Apply` writes only the six `[mission]` keys** (map/time/weather/scenario/temperature/length).
|
||||
|
||||
Reference in New Issue
Block a user