From 7d6ef0b1d3d798a9511fd445e459cb9316d2d6f4 Mon Sep 17 00:00:00 2001 From: Bentley Davis <10065854+NonPolynomialTim@users.noreply.github.com> Date: Sun, 6 Sep 2026 22:14:02 -0400 Subject: [PATCH 1/4] fix(coop): finish starting the next stage on the multi-stage host Follow-up to #184. Both machines now reach stage 2 of a co-op multi-stage mission, but the host could not see any aliens there while the client could - "the client was seeing aliens where I saw nothing". The two machines agreed on everything a census can measure: identical unit ids, identical positions, identical map fingerprint. The split was visibility. BattlescapeGenerator::nextStage() ends in resetTurnCounter(), which parks the rebuilt save at turn 0 with _beforeGame = true. Only startFirstTurn() / resetUnitTiles() clears that flag, and vanilla reaches startFirstTurn() through BriefingState::btnOkClick -> InventoryState -> OK. Co-op shows neither screen on this path, so the host was left mid-initialisation: * _beforeGame stayed true, and TileEngine::calculateLineVoxel sets excludeAllUnits while it is ("don't start unit spotting before pre-game inventory stuff"), so no line-of-sight ray on the host could ever hit a unit and the host spotted nothing at all; * resetUnitTiles() never ran, so tiles were not matched up with units and the player's own units kept the setVisible(false) nextStage() gave them; * the turn counter stayed at 0, which also silences the host's own click_close and next_turn packets (NextTurnState::close gates both on turn >= 1). The client never hit any of it because it enters the stage through SavedBattleGame::load(), whose tail is resetUnitTiles() + recalculateFOV(). (A fresh co-op mission start is unaffected for the same reason: there the host re-loads its own "battlehost" blob via CoopState::loadWorld, so it goes through the loader too. The next-stage path is the only one that wires a BattlescapeState straight onto the in-memory save.) Fix: call startFirstTurn() + recalculateFOV() right after bgen.nextStage() and BEFORE setupCoop()/sendMissionFile(), so the blob shipped to the client is the state the host actually plays. Measured on the player's own mid-battle save (scrubbed fixture from #184): before host beforeGame=true turn=0 spotted=[] discoveredFloor=242 client beforeGame=false turn=0 spotted=[1000052] discoveredFloor=988 after both beforeGame=false turn=1 spotted=[1000052] discoveredFloor=988 Repro/regression: tools/coop_test/test_coop_nextstage_census.py asserts the stage-2 unit census, map fingerprint, fog and spotted-alien sets match on both machines, and with NEXTSTAGE_FINISH=1 also clears stage 2 and asserts both machines reach an identical debriefing and return to the geoscape. Skips cleanly where TFTD data is not staged. battle_state gains a "beforeGame" readout - the desync is invisible to a unit census and only readable there. --- src/Battlescape/BattlescapeState.cpp | 22 ++ src/CoopMod/TestServer.cpp | 6 + tools/coop_test/test_coop_nextstage_census.py | 297 ++++++++++++++++++ 3 files changed, 325 insertions(+) create mode 100644 tools/coop_test/test_coop_nextstage_census.py diff --git a/src/Battlescape/BattlescapeState.cpp b/src/Battlescape/BattlescapeState.cpp index 4be859871..e9a65adc0 100644 --- a/src/Battlescape/BattlescapeState.cpp +++ b/src/Battlescape/BattlescapeState.cpp @@ -6390,6 +6390,28 @@ void BattlescapeState::finishBattle(bool abort, int inExitArea) BattlescapeGenerator bgen = BattlescapeGenerator(_game); bgen.nextStage(); + // FINISH STARTING the rebuilt stage before it is shipped, so the blob the + // client loads is the state the host itself plays. + // + // nextStage() ends in resetTurnCounter(), which parks the save at turn 0 with + // _beforeGame = true. Vanilla clears that in SavedBattleGame::startFirstTurn(), + // reached through BriefingState::btnOkClick -> InventoryState -> OK. Co-op shows + // neither screen on this path, so the host used to be left mid-initialisation: + // * _beforeGame stayed true, and TileEngine::calculateLineVoxel excludes EVERY + // unit from line-of-sight while it is ("don't start unit spotting before + // pre-game inventory stuff") - so the host could never spot a single alien; + // * resetUnitTiles() never ran, so tiles were not matched up with units and the + // player's own units kept the setVisible(false) nextStage() gave them; + // * the turn counter stayed at 0, which silences the host's own click_close and + // next_turn packets (NextTurnState::close gates both on turn >= 1). + // The client never hit any of it because it enters through SavedBattleGame::load(), + // whose tail is resetUnitTiles() + recalculateFOV() - hence the player report that + // the client saw aliens where the host saw nothing at all. recalculateFOV() here + // mirrors that same load tail so both machines compute visibility from the same + // positions (nextStage() already did the ambient lighting pass). + _save->startFirstTurn(); + _save->getTileEngine()->recalculateFOV(); + // tag coop units + ship the rebuilt stage to the client. As at every // other coop mission-start site (ConfirmLandingState, GeoscapeState, // NewBattleState, ...), the briefing is only a vehicle for setupCoop() diff --git a/src/CoopMod/TestServer.cpp b/src/CoopMod/TestServer.cpp index 55ed605a9..6fbefada7 100644 --- a/src/CoopMod/TestServer.cpp +++ b/src/CoopMod/TestServer.cpp @@ -6166,6 +6166,12 @@ std::string TestServer::execute(const std::string& line) // invisible until its collapse ends). Empty except during a client ghost. resp["hiddenItemIds"] = bg->getBattleGame() ? bg->getBattleGame()->coopHiddenItemIdsJson() : Json::Value(Json::arrayValue); resp["isPreview"] = bg->isPreview(); + // SavedBattleGame::_beforeGame - the pre-inventory flag nextStage()/ + // resetTurnCounter() raises and startFirstTurn()/resetUnitTiles() clears. + // While it is true TileEngine::calculateLineVoxel excludes every unit from + // line-of-sight, so this machine can spot nothing at all - a "sees no + // aliens" desync is invisible in a unit census and only readable here. + resp["beforeGame"] = bg->isBeforeGame(); resp["clientPanicHandle"] = _game->getCoopMod()->_clientPanicHandle; resp["serverOwner"] = connectionTCP::getServerOwner(); resp["saveOwnerId"] = connectionTCP::coop_save_owner_player_id; diff --git a/tools/coop_test/test_coop_nextstage_census.py b/tools/coop_test/test_coop_nextstage_census.py new file mode 100644 index 000000000..2decc48d6 --- /dev/null +++ b/tools/coop_test/test_coop_nextstage_census.py @@ -0,0 +1,297 @@ +"""Coop multi-stage (next-stage) transition: the two machines must land in the +SAME stage-2 battle - same map, same units, same positions. + +Follow-up to test_coop_nextstage_crash.py (#184, which only proved both +machines reach STR_ALIEN_COLONY_P2 without crashing). Player report on the +2.0.34 nightly: + + "we were able to get to the next stage in the base attack, but it was not + working correctly. The client was seeing aliens where I (the host) saw + nothing. In fact, I (the host) wasn't seeing any aliens at all." + +So the discriminator here is a CENSUS COMPARE after the transition, not merely +"both are in stage 2": + + * live hostile count on host == on client + * every unit id/faction/position identical on both + * map fingerprint identical on both + +Run: python tools/coop_test/test_coop_nextstage_census.py +Exit 0 = pass; 2 = failure (census mismatch / crash / did not advance). + +Env: + NEXTSTAGE_DUMP= write both machines' raw stage-2 censuses as JSON + NEXTSTAGE_FINISH=1 after the census compare, also clear stage 2 and + assert both machines reach the debriefing/geoscape +""" + +import json +import os +import sys +import time + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from harness import GameClient +from tftd_common import make_tftd_user_dir +import session +import harness + +FIX = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures") +SAVE_SRC = os.environ.get("TFTD_SAVE") or os.path.join(FIX, "tftd_base_assault.sav") +SAVE = "tftd_base_assault.sav" +PORT = "47956" +DUMP = os.environ.get("NEXTSTAGE_DUMP") +FINISH = os.environ.get("NEXTSTAGE_FINISH") == "1" + + +def tftd_data_present(): + exe_dir = os.path.dirname(harness.EXE) + return os.path.isdir(os.path.join(exe_dir, "TFTD", "GEODATA")) + + +def states(gc): + return gc.cmd({"cmd": "get_state"})["states"] + + +def battle(gc): + return gc.cmd({"cmd": "battle_state"}) + + +def proc_dead(gc): + return gc.proc is not None and gc.proc.poll() is not None + + +def census(b): + """(id, faction, coop, out, x, y, z) per unit, sorted by id.""" + rows = [] + for u in b.get("units", []): + rows.append((u["id"], u["faction"], u.get("coop"), bool(u.get("isOut")), + u["x"], u["y"], u["z"])) + return sorted(rows) + + +def hostiles(b): + return sorted(u["id"] for u in b.get("units", []) + if u.get("faction") == 1 and not u.get("isOut")) + + +def fmt(rows, limit=200): + return "\n".join(" id=%-4d f=%d coop=%s out=%s pos=(%d,%d,%d)" % r + for r in rows[:limit]) + + +def dump(tag, b): + if not DUMP: + return + os.makedirs(DUMP, exist_ok=True) + with open(os.path.join(DUMP, "%s.json" % tag), "w", encoding="utf-8") as f: + json.dump(b, f, indent=1, sort_keys=True) + + +def main(): + if not tftd_data_present(): + print("SKIP: TFTD (xcom2) game data not staged next to the exe " + "(no TFTD/GEODATA) - this repro is TFTD-only.") + sys.exit(0) + if not os.path.exists(SAVE_SRC): + print(f"SKIP: fixture {SAVE_SRC} not found.") + sys.exit(0) + + host_dir = make_tftd_user_dir("nsx_host", saves=[SAVE_SRC]) + client_dir = make_tftd_user_dir("nsx_client") + host = GameClient("host", 47961, host_dir) + client = GameClient("client", 47962, client_dir) + fail = None + try: + host.spawn(); client.spawn() + host.connect(timeout=120); client.connect(timeout=120) + session.resume_campaign_battle(host, client, SAVE, port=PORT, timeout=180) + + hb, cb = battle(host), battle(client) + assert hb.get("missionType") == "STR_ALIEN_BASE_ASSAULT", \ + f"unexpected stage-1 mission {hb.get('missionType')}" + print(f"stage 1: host mission={hb.get('missionType')} turn={hb.get('turn')} " + f"units={len(hb.get('units', []))} | client units={len(cb.get('units', []))}") + dump("stage1_host", hb); dump("stage1_client", cb) + + print("stage-1 live aliens: host=%s client=%s" % (hostiles(hb), hostiles(cb))) + + # --- drive the transition (same as the #184 regression test) --------- + print("killing all remaining aliens on the HOST (faction=1)...") + print("kill_unit_real ->", + host.cmd({"cmd": "battle_action", "action": "kill_unit_real", "faction": 1})) + host.wait_for("all aliens dead on host", + lambda: (not hostiles(battle(host))) or None, + timeout=30, interval=1.0) + print("battle_autoend ->", host.cmd({"cmd": "battle_autoend"})) + print("close_nextturn ->", host.cmd({"cmd": "close_nextturn"})) + + print("waiting for BOTH machines to enter stage 2 (STR_ALIEN_COLONY_P2)...") + deadline = time.time() + 120 + while time.time() < deadline: + time.sleep(1.5) + for gc, tag in ((host, "host"), (client, "client")): + if proc_dead(gc): + raise AssertionError( + f"{tag.upper()} CRASHED on next-stage transition: rc=" + f"{gc.proc.returncode} (0x{gc.proc.returncode & 0xffffffff:08x})") + hs = states(host) + if not any("BattlescapeState" in s for s in hs): + continue + hb, cb = battle(host), battle(client) + if (hb.get("missionType") == "STR_ALIEN_COLONY_P2" and hb.get("inBattle") + and cb.get("missionType") == "STR_ALIEN_COLONY_P2" and cb.get("inBattle")): + break + else: + raise AssertionError( + "machines did not both reach stage 2 within 120s: " + f"host={battle(host).get('missionType')} " + f"client={battle(client).get('missionType')}") + + # let the coop turn handshake settle before sampling + time.sleep(6) + hb, cb = battle(host), battle(client) + dump("stage2_host", hb); dump("stage2_client", cb) + print("beforeGame: host=%s client=%s" % (hb.get("beforeGame"), cb.get("beforeGame"))) + print(f"host : mission={hb.get('missionType')} turn={hb.get('turn')} " + f"battleInit={hb.get('battleInit')} coopTurn={hb.get('coopTurn')} " + f"units={len(hb.get('units', []))} mapFp={hb.get('mapFingerprint')} " + f"mapXYZ={hb.get('mapSizeXYZ')}") + print(f"client: mission={cb.get('missionType')} turn={cb.get('turn')} " + f"battleInit={cb.get('battleInit')} coopTurn={cb.get('coopTurn')} " + f"units={len(cb.get('units', []))} mapFp={cb.get('mapFingerprint')} " + f"mapXYZ={cb.get('mapSizeXYZ')}") + + # --- THE DISCRIMINATOR ----------------------------------------------- + h_hostiles, c_hostiles = hostiles(hb), hostiles(cb) + print(f"stage-2 live aliens: host={h_hostiles}") + print(f"stage-2 live aliens: client={c_hostiles}") + + problems = [] + if not h_hostiles: + problems.append( + "HOST HAS NO LIVE ALIENS in stage 2 (client has %d) - the player's " + "'I wasn't seeing any aliens at all'" % len(c_hostiles)) + if h_hostiles != c_hostiles: + problems.append("live-alien id sets differ: host=%s client=%s" + % (h_hostiles, c_hostiles)) + if hb.get("mapFingerprint") != cb.get("mapFingerprint"): + problems.append("map fingerprints differ (different stage-2 maps): " + "host=%s client=%s" + % (hb.get("mapFingerprint"), cb.get("mapFingerprint"))) + hc, cc = census(hb), census(cb) + if hc != cc: + honly = [r for r in hc if r not in cc] + conly = [r for r in cc if r not in hc] + problems.append( + "unit census differs (%d host-only, %d client-only rows)\n" + " host-only:\n%s\n client-only:\n%s" + % (len(honly), len(conly), fmt(honly), fmt(conly))) + if problems: + raise AssertionError("STAGE-2 DESYNC:\n - " + "\n - ".join(problems)) + + print("PASS: stage-2 census matches on both machines " + f"({len(h_hostiles)} live aliens, {len(hc)} units, same map)") + + # --- THE REAL SYMPTOM: fog of war / spotted aliens -------------------- + # Identical units at identical positions still leaves "the client sees + # aliens where the host sees nothing" possible: what a player SEES is + # BattleUnit::_visibleUnits + Tile discovery, neither of which travels in + # the unit census. Drive past the "Turn 1" screen so the first real + # player turn is live on both, then compare. + print("\n== closing the stage-2 turn screen on both machines") + for gc, tag in ((host, "host"), (client, "client")): + st = states(gc) + print(f" {tag} stack: {[s.split('::')[-1] for s in st[-4:]]}") + if any("NextTurnState" in s for s in st): + print(f" {tag} dismiss_popup ->", gc.cmd({"cmd": "dismiss_popup"})) + time.sleep(6) + hb, cb = battle(host), battle(client) + dump("stage2_turn1_host", hb); dump("stage2_turn1_client", cb) + for tag, b in (("host", hb), ("client", cb)): + print(f" {tag}: turn={b.get('turn')} side={b.get('side')} " + f"coopTurn={b.get('coopTurn')} discoveredFloor={b.get('mapDiscoveredFloor')} " + f"spotted={b.get('spotted')} beforeGame={b.get('beforeGame')}") + vis = [] + if hb.get("turn") != cb.get("turn"): + vis.append("turn counters differ: host=%s client=%s" + % (hb.get("turn"), cb.get("turn"))) + if not hb.get("spotted") and cb.get("spotted"): + vis.append("HOST SPOTS NO ALIENS while the client spots %s - the " + "player's exact report" % cb.get("spotted")) + if sorted(hb.get("spotted", [])) != sorted(cb.get("spotted", [])): + vis.append("spotted-alien sets differ: host=%s client=%s" + % (hb.get("spotted"), cb.get("spotted"))) + if hb.get("mapDiscoveredFloor") != cb.get("mapDiscoveredFloor"): + vis.append("discovered-floor counts differ (fog of war desync): " + "host=%s client=%s" + % (hb.get("mapDiscoveredFloor"), cb.get("mapDiscoveredFloor"))) + if census(hb) != census(cb): + vis.append("unit census drifted apart once the turn started") + # The mechanism, asserted directly: SavedBattleGame::_beforeGame. nextStage() + # -> resetTurnCounter() raises it; only startFirstTurn()/resetUnitTiles() + # clears it. While it is up TileEngine::calculateLineVoxel excludes every unit + # from LOS, so the machine cannot spot anything. The client clears it for free + # via SavedBattleGame::load(); the host has to be given the same finish. + for tag, b in (("host", hb), ("client", cb)): + if b.get("beforeGame"): + vis.append("%s is still BEFORE GAME in stage 2 (startFirstTurn never " + "ran): no unit can be spotted there" % tag) + if b.get("turn", 0) < 1: + vis.append("%s stage-2 turn counter is %s, never started (turn >= 1 also " + "gates the host's click_close/next_turn coop packets)" + % (tag, b.get("turn"))) + if vis: + raise AssertionError("STAGE-2 VISIBILITY DESYNC:\n - " + "\n - ".join(vis)) + print("PASS: stage-2 visibility matches on both machines " + f"(spotted={hb.get('spotted')}, discoveredFloor={hb.get('mapDiscoveredFloor')})") + + if FINISH: + finish_stage2(host, client) + except Exception as e: + fail = e + print(f"[FAIL] {e}") + for tag, gc in (("host", host), ("client", client)): + try: + print(f" {tag} states: {states(gc)[-4:]}") + except Exception as ee: + print(f" {tag}: unreachable ({ee})") + finally: + host.shutdown(); client.shutdown() + sys.exit(2 if fail else 0) + + +def finish_stage2(host, client): + """Clear stage 2 and prove both machines get back out to the geoscape.""" + print("\n== finishing stage 2 (kill all aliens -> debrief -> geoscape)") + print("kill_unit_real ->", + host.cmd({"cmd": "battle_action", "action": "kill_unit_real", "faction": 1})) + host.wait_for("stage-2 aliens dead on host", + lambda: (not hostiles(battle(host))) or None, + timeout=45, interval=1.0) + print("battle_autoend ->", host.cmd({"cmd": "battle_autoend"})) + print("close_nextturn ->", host.cmd({"cmd": "close_nextturn"})) + + for gc, tag in ((host, "host"), (client, "client")): + gc.wait_for(f"{tag} at debriefing", + lambda gc=gc: any("DebriefingState" in s for s in states(gc)) or None, + timeout=120, interval=2.0) + print("both machines reached the debriefing") + hd = host.cmd({"cmd": "debrief_state"}) + cd = client.cmd({"cmd": "debrief_state"}) + print("host debrief :", {k: v for k, v in hd.items() if k != "ok"}) + print("client debrief:", {k: v for k, v in cd.items() if k != "ok"}) + + # DebriefingState::btnOkClick (the real OK button) on both machines. + for gc, tag in ((host, "host"), (client, "client")): + print(f"{tag} dismiss_popup ->", gc.cmd({"cmd": "dismiss_popup"})) + for gc, tag in ((host, "host"), (client, "client")): + gc.wait_for(f"{tag} back on the geoscape", + lambda gc=gc: (any("GeoscapeState" in s for s in states(gc)) + and not battle(gc).get("inBattle")) or None, + timeout=120, interval=2.0) + print("PASS: both machines returned to the geoscape after stage 2") + + +if __name__ == "__main__": + main() From 7a8b9e04f42ce5ac7be4cb2d6da8ac54df7f4472 Mon Sep 17 00:00:00 2001 From: Bentley Davis <10065854+NonPolynomialTim@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:10:50 -0400 Subject: [PATCH 2/4] fix(coop): skip the battle checksum across a multi-stage transition Owner playtest of the previous commit hit a CO-OP DESYNC DETECTED dialog on BOTH machines the moment the mission rolled into stage 2. The state was correct; the comparison was not. A multi-stage transition hands the peer a whole new battle: the host rebuilds the stage, ships the blob (setupCoop -> sendMissionFile) and ENTERS it immediately, while the peer still has to request, download and load that blob. A next_turn closed inside that window carries the host's stage-2 terms and is verified against the peer's stage-1 state, so chkBattleItemId, chkBattleCensus and chkBattleUnits all differ - because they describe two different battles, not two disagreeing copies of one. Both bundles from the playtest say so directly: the client reported turn 13 / 44 units / itemId 351 (stage 1) while the host reported turn 1 / 69 units / itemId 463 (stage 2), and 351 is exactly the stage-1 value a harness probe measures immediately before the transition. Both machines converge on identical terms the moment the load completes. The window was previously unreachable: before the preceding commit the host sat at turn 0 after a transition and NextTurnState::close gates click_close/next_turn on turn >= 1, so the packet was never sent. Restoring startFirstTurn() is what exposed it - a latent protocol race, not new drift. Fix: attachBattleChecksum stamps chkBattleStage (getMissionType), and verifyBattleChecksum returns early when the peer stamped a stage that differs from this machine's. Same "not comparable in this window" discipline as the neighbouring corpseReplayPendingAny() and rxPassDeferred() guards, and for the same reason: the next stamp compares the two machines once the peer is on the same stage. Additive - a peer that stamps no stage reads back as the empty string and is compared exactly as before. PROTOCOL.md updated in the docs repo. test_coop_nextstage_census.py now RACES the transition instead of waiting for both machines (which is why the automated run never saw what a human hit): it closes the host's stage-2 turn screen the instant it exists, then asserts zero desync bundles on either machine. Measured on the identical forced window - without the guard: 2 bundles, 0 skips; with it: 0 bundles, 1 skip log line. ci: point build-linux's apt at archive.debian.org and drop bullseye-security Unrelated to the co-op work, but it is what is currently failing this PR. Debian 11 is end of life: bullseye-security stopped being published on 2026-08-31 (bookworm- and trixie-security are still published daily) and its Release file expired at 2026-09-07 21:13 UTC, so apt-get update now fails outright in the container. Before that it had begun 404ing pool files its own index still advertised, as mirrors drifted with nothing republishing them. The existing retry loop cannot help with either: a 404 and an expired Release are not mirror flakes. Plain bullseye is frozen at its final point release (2025-08-09) and carries NO Valid-Until, so it never expires, and it is already mirrored on archive.debian.org where retired releases stay indefinitely. Every package the job installs exists there (only 6 ever resolved to a newer security build), this container is a throwaway build box, and none of these libraries ship inside the AppImage - they are headers and build tooling. Keeps the glibc 2.31 floor the container exists for. debian:12 would fix apt too but raises the floor to 2.36 and drops Ubuntu 22.04 LTS (2.35), Mint 21 and RHEL/Rocky 9 (2.34) - a player support decision, not an apt workaround. --- .github/workflows/ci-main.yml | 27 ++++++- src/CoopMod/SharedEcon.cpp | 36 ++++++++++ tools/coop_test/test_coop_nextstage_census.py | 72 ++++++++++++++++++- 3 files changed, 130 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index 597935959..dbdcdd4ff 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -706,13 +706,36 @@ jobs: shell: bash steps: - name: Install deps - # Same mirror-flake retry as build-winxp - see the comment there. No sudo - # (root in the container); git must land BEFORE checkout, which the + # No sudo (root in the container); git must land BEFORE checkout, which the # `submodules: recursive` step needs and the bare container lacks. + # + # Debian 11 is END OF LIFE, so this step no longer uses deb.debian.org. + # bullseye-security stopped being published on 2026-08-31 (bookworm- and + # trixie-security are still published daily) and its Release file expired at + # 2026-09-07 21:13 UTC, which makes `apt-get update` fail outright here. + # Before that it had already begun 404ing pool files its own index still + # advertised, as the mirrors drifted with nothing republishing them. The retry + # loop below cannot help with either: a 404 and an expired Release are not the + # mirror flakes it was written for. + # + # Plain `bullseye` is frozen at its final point release (2025-08-09) and + # carries NO Valid-Until, so it never expires - and it is already mirrored on + # archive.debian.org, where retired releases stay indefinitely. Point there and + # drop the dead security suite: every package installed below exists in plain + # bullseye (only 6 ever resolved to a newer security build), this container is + # a throwaway build box, and none of these libraries ship inside the AppImage - + # they are headers and build tooling. + # + # This deliberately KEEPS the glibc 2.31 floor the container exists for (see + # the job comment). debian:12 would fix apt too, but raises the floor to 2.36 + # and drops Ubuntu 22.04 LTS (2.35), Mint 21 and RHEL/Rocky 9 (2.34) - a player + # support decision, not an apt workaround. env: DEBIAN_FRONTEND: noninteractive run: | set -eu + echo 'deb http://archive.debian.org/debian bullseye main' > /etc/apt/sources.list + rm -f /etc/apt/sources.list.d/*.list /etc/apt/sources.list.d/*.sources || true apt_get() { apt-get -o Acquire::Retries=5 -o Acquire::http::Timeout=30 "$@"; } ok=0 for n in 1 2 3 4; do diff --git a/src/CoopMod/SharedEcon.cpp b/src/CoopMod/SharedEcon.cpp index 6d578cd71..2438bac51 100644 --- a/src/CoopMod/SharedEcon.cpp +++ b/src/CoopMod/SharedEcon.cpp @@ -3743,6 +3743,16 @@ void attachBattleChecksum(Game* game, Json::Value& msg) msg["chkBattleItemId"] = Json::Value::Int64(itemIdCounter); msg["chkBattleCensus"] = Json::Value::Int64(census); msg["chkBattleUnits"] = Json::Value::Int64(units); + // coop (#188): WHICH battle these terms describe. In a multi-stage mission the + // host rebuilds the next stage, ships it, and enters it - so between the ship and + // the peer finishing its load the two machines legitimately hold DIFFERENT + // battles, and every term above differs for that reason alone. Stamping the + // mission type lets the receiver tell "we disagree about this battle" (a real + // desync) from "we are not talking about the same battle yet" (a transition). + if (const SavedBattleGame* battle = game->getSavedGame()->getSavedBattle()) + { + msg["chkBattleStage"] = battle->getMissionType(); + } } void verifyBattleChecksum(Game* game, const Json::Value& msg, const std::string& context) @@ -3760,6 +3770,32 @@ void verifyBattleChecksum(Game* game, const Json::Value& msg, const std::string& const int64_t peerUnits = msg.get("chkBattleUnits", -1).asInt64(); if (peerItemId < 0 && peerCensus < 0 && peerUnits < 0) return; // old peer / no battle + // coop (#188): NOT COMPARABLE ACROSS A STAGE TRANSITION. A multi-stage mission + // hands the peer a whole new battle: the host rebuilds the stage, ships the blob, + // and enters it immediately, while the peer still has to request, download and + // load that blob. A next_turn that lands inside that window carries the HOST's + // stage-2 terms and is checked against the peer's stage-1 state - the item ids, + // the census and the unit set all differ, because they describe two different + // battles rather than two disagreeing copies of one. That fired the desync + // dialog on both machines at every transition (owner report, stage 1 of an alien + // colony: peer itemId 351/turn 13/44 units vs host 463/turn 1/69 units), even + // though both converge on identical terms the moment the load completes. + // + // Skipping is right rather than merely quiet, exactly as for the death-replay + // window below: the next stamp compares the two machines once the peer is on the + // same stage. Additive - a peer that stamps no stage reads back as the empty + // string and is compared exactly as before. + const std::string peerStage = msg.get("chkBattleStage", "").asString(); + const SavedBattleGame* const stageBattle = game->getSavedGame()->getSavedBattle(); + if (!peerStage.empty() && stageBattle && peerStage != stageBattle->getMissionType()) + { + Log(LOG_INFO) << "[COOP] battle checksum on " << context + << " skipped - the peer is on stage '" << peerStage + << "', this machine is on '" << stageBattle->getMissionType() + << "' (multi-stage transition in flight)"; + return; + } + int64_t myItemId, myCensus, myUnits; if (!battleChecksumTerms(game, myItemId, myCensus, myUnits)) return; // no battle here diff --git a/tools/coop_test/test_coop_nextstage_census.py b/tools/coop_test/test_coop_nextstage_census.py index 2decc48d6..92406d7e3 100644 --- a/tools/coop_test/test_coop_nextstage_census.py +++ b/tools/coop_test/test_coop_nextstage_census.py @@ -25,8 +25,10 @@ assert both machines reach the debriefing/geoscape """ +import glob import json import os +import shutil import sys import time @@ -61,6 +63,33 @@ def proc_dead(gc): return gc.proc is not None and gc.proc.poll() is not None +def desync_reports(user_dir): + """Bundles the in-game drift tripwire wrote. Each one is a player-visible + CO-OP DESYNC DETECTED dialog, so a clean transition must produce none.""" + return sorted(glob.glob(os.path.join(user_dir, "desync-reports", "*.zip"))) + + +def check_no_desync(host_dir, client_dir, when): + """The drift tripwire must stay silent across a stage transition. + + A transition hands the client a whole new battle: the host rebuilds the stage, + ships the blob, enters it and closes its "Turn 1" screen - which sends next_turn + WITH the battle checksum - while the client may still be downloading and loading + that blob. Checking the host's stage-2 terms against the client's stage-1 state + reports a divergence that does not exist: they describe two different battles, + not two disagreeing copies of one (owner report: peer itemId 351 / turn 13 / + 44 units against host 463 / turn 1 / 69 units). SharedEcon's chkBattleStage guard + skips the compare while the machines are on different stages. Without it BOTH + machines write a bundle and show the dialog at EVERY transition. + """ + hr, cr = desync_reports(host_dir), desync_reports(client_dir) + if hr or cr: + raise AssertionError( + "DESYNC TRIPWIRE FIRED %s - host=%d client=%d bundle(s):\n %s" + % (when, len(hr), len(cr), "\n ".join(hr + cr))) + print(f" no desync bundles {when} (tripwire silent on both machines)") + + def census(b): """(id, faction, coop, out, x, y, z) per unit, sorted by id.""" rows = [] @@ -99,6 +128,8 @@ def main(): host_dir = make_tftd_user_dir("nsx_host", saves=[SAVE_SRC]) client_dir = make_tftd_user_dir("nsx_client") + for d in (host_dir, client_dir): + shutil.rmtree(os.path.join(d, "desync-reports"), ignore_errors=True) host = GameClient("host", 47961, host_dir) client = GameClient("client", 47962, client_dir) fail = None @@ -126,6 +157,40 @@ def main(): print("battle_autoend ->", host.cmd({"cmd": "battle_autoend"})) print("close_nextturn ->", host.cmd({"cmd": "close_nextturn"})) + # RACE THE TRANSITION, deliberately. The host rebuilds stage 2, ships the + # blob and enters it; the client still has to request, download and load + # that blob. Closing the host's "Turn 1" screen the instant it exists sends + # next_turn (and the battle checksum) INTO that window - which is what a + # human hits and what an unraced test never reaches. Without the + # chkBattleStage guard both machines fire the desync dialog here. + print("racing: closing the HOST's stage-2 turn screen before the client " + "finishes loading...") + deadline = time.time() + 120 + raced = None + while time.time() < deadline: + time.sleep(0.15) + if proc_dead(host): + raise AssertionError( + f"HOST CRASHED on next-stage transition: rc={host.proc.returncode}") + st = states(host) + if not (any("NextTurnState" in x for x in st) + and any("BattlescapeState" in x for x in st)): + continue + hb = battle(host) + if hb.get("missionType") != "STR_ALIEN_COLONY_P2": + continue + cb = battle(client) + raced = (cb.get("missionType"), cb.get("turn"), len(cb.get("units", []))) + print(f" host at stage 2 (turn {hb.get('turn')}, " + f"{len(hb.get('units', []))} units); client still {raced}") + print(" host dismiss_popup ->", host.cmd({"cmd": "dismiss_popup"})) + break + if raced is None: + print(" (host's stage-2 turn screen never observed; continuing)") + elif raced[0] == "STR_ALIEN_COLONY_P2": + print(" NOTE: the client had already adopted stage 2 - the compare " + "window was not entered this run (timing, not a failure)") + print("waiting for BOTH machines to enter stage 2 (STR_ALIEN_COLONY_P2)...") deadline = time.time() + 120 while time.time() < deadline: @@ -199,13 +264,14 @@ def main(): # BattleUnit::_visibleUnits + Tile discovery, neither of which travels in # the unit census. Drive past the "Turn 1" screen so the first real # player turn is live on both, then compare. - print("\n== closing the stage-2 turn screen on both machines") + print("\n== settling the stage-2 turn screens") for gc, tag in ((host, "host"), (client, "client")): st = states(gc) - print(f" {tag} stack: {[s.split('::')[-1] for s in st[-4:]]}") - if any("NextTurnState" in s for s in st): + print(f" {tag} stack: {[x.split('::')[-1] for x in st[-4:]]}") + if any("NextTurnState" in x for x in st): print(f" {tag} dismiss_popup ->", gc.cmd({"cmd": "dismiss_popup"})) time.sleep(6) + check_no_desync(host_dir, client_dir, "across the stage-2 transition") hb, cb = battle(host), battle(client) dump("stage2_turn1_host", hb); dump("stage2_turn1_client", cb) for tag, b in (("host", hb), ("client", cb)): From d55559a76129dcd641266d0dd5626444cae81769 Mon Sep 17 00:00:00 2001 From: Bentley Davis <10065854+NonPolynomialTim@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:15:51 -0400 Subject: [PATCH 3/4] ci: apply the archive.debian.org apt fix to ci-validate's build-linux too The PR gate runs .github/workflows/ci-validate.yml (pull_request_target), not ci-main.yml, and it carries its own copy of build-linux on the same debian:11 container. The previous commit only fixed ci-main's, so the gate kept failing with the same expired-Release error. NOTE for whoever lands this: pull_request_target evaluates the workflow from the BASE branch, so this file's fix cannot take effect on THIS pull request's own checks - it applies to runs after it is on main. build-linux is a required check, so #188 needs the CI fix on main first (or an admin merge). --- .github/workflows/ci-validate.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-validate.yml b/.github/workflows/ci-validate.yml index 10400928b..870120007 100644 --- a/.github/workflows/ci-validate.yml +++ b/.github/workflows/ci-validate.yml @@ -385,13 +385,23 @@ jobs: shell: bash steps: - name: Install deps - # Same mirror-flake retry as build-winxp - see the comment there. No sudo - # (root in the container); git must land BEFORE checkout, which the + # No sudo (root in the container); git must land BEFORE checkout, which the # `submodules: recursive` step needs and the bare container lacks. + # + # Debian 11 is END OF LIFE - same fix and same reasoning as ci-main.yml's + # build-linux, which see. bullseye-security stopped being published on + # 2026-08-31 and its Release file expired at 2026-09-07 21:13 UTC, so + # `apt-get update` fails outright here; the retry loop cannot help, because + # an expired Release and a 404 are not the mirror flakes it was written for. + # Plain bullseye is frozen, carries no Valid-Until and is already mirrored on + # archive.debian.org, so point there and drop the dead security suite. Keeps + # the glibc 2.31 floor (see ci-main). env: DEBIAN_FRONTEND: noninteractive run: | set -eu + echo 'deb http://archive.debian.org/debian bullseye main' > /etc/apt/sources.list + rm -f /etc/apt/sources.list.d/*.list /etc/apt/sources.list.d/*.sources || true apt_get() { apt-get -o Acquire::Retries=5 -o Acquire::http::Timeout=30 "$@"; } ok=0 for n in 1 2 3 4; do From 85580261ee29f153eee36c7eb323c4b34305d9d8 Mon Sep 17 00:00:00 2001 From: Bentley Davis <10065854+NonPolynomialTim@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:27:44 -0400 Subject: [PATCH 4/4] fix(coop): scope chkBattleStage to multi-stage missions; unquarantine-proof the gate Two follow-ups from validating the previous commit against CI. 1. chkBattleStage is now stamped ONLY for multi-stage missions. A single-stage battle - every UFO/terror/base mission, and every parallel-turn fixture - now puts no extra field on the wire at all, so its next_turn is byte-identical to before the guard existed. The guard only ever mattered for a stage transition. The predicate is "is this ANY stage of a multi-stage chain": the deployment has a nextStage, OR some deployment's nextStage names it. Both halves are needed, and the second is the non-obvious one. The LAST stage of a chain carries no nextStage of its own (STR_ALIEN_COLONY_P2, STR_TLETH_P3, STR_MARS_THE_FINAL_ASSAULT) - and the host is exactly the machine sitting on that last stage while the peer is still loading the previous one. A plain "has a nextStage" test would leave the HOST unstamped precisely when the peer needs the marker, reinstating the false alarm. attachBattleChecksum and verifyBattleChecksum have one caller each (NextTurnState and the next_turn handler), so this runs once per turn boundary and the deployment sweep needs no cache. Regression unchanged and still green: test_coop_nextstage_census races the host's stage-2 turn screen against the client's load and still logs the skip ("the peer is on stage 'STR_ALIEN_COLONY_P2', this machine is on 'STR_ALIEN_BASE_ASSAULT'") with zero desync bundles on either machine. 2. test_parallel_heavy_death_repro no longer gates the suite. It is a REPRO TOOL, not a guard - its own docstring says so - and its exit codes are the reverse of what a gate assumes: exit 0 = REPRO FIRED (the heavy-alien-death desync reproduced) exit 3 = NO REPRO (every alien side stayed in census - i.e. clean) exit 2 = harness/setup error The runner scores exit 0 as PASS, so gating on it counts the bug FIRING as success and a clean run as failure. Every green run of it on main (30 Aug run 33331963166, 4 Sep run 33857882577, and #184's over-budget entry, which the runner only reports when rc==0) was green because the drift fired - exactly the signal a green pipeline hides. It has been this way since it landed in #166 on 2026-08-30; the exit codes have never been modified. It still reproduces intermittently because the #166 fixes sit behind a lever that defaults off (g_wireOrderState) pending the battlescape rewrite, so this is the known state and not a regression. Quarantined: it runs and prints its verdict but does not gate. Its rc=0 deserves an alert of its own - it is currently the only automated thing that notices the drift firing - but that is a separate change. Also verified along the way: coop-tests and build-linux are BOTH red on unmodified main (control dispatch of ci-validate on f8b083421), so the gate was not failing because of this branch. --- src/CoopMod/SharedEcon.cpp | 39 ++++++++++++++++++++++++++++++++++++- tools/ci/run_coop_suite.ps1 | 17 +++++++++++++--- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/CoopMod/SharedEcon.cpp b/src/CoopMod/SharedEcon.cpp index 2438bac51..10b19c80a 100644 --- a/src/CoopMod/SharedEcon.cpp +++ b/src/CoopMod/SharedEcon.cpp @@ -3728,6 +3728,40 @@ static bool coopWireOn(); static bool coopBoundaryPersistShouldAlarm(const char* bucket, const std::string& kind, std::uint32_t bseq); static void coopBoundaryPersistHeal(const char* bucket, const std::string& kind, std::uint32_t bseq); +/** + * Is `type` any stage of a MULTI-STAGE mission? + * + * BOTH halves are needed, and the second is the non-obvious one. The LAST stage of a + * chain carries no nextStage of its own (STR_ALIEN_COLONY_P2, STR_TLETH_P3, + * STR_MARS_THE_FINAL_ASSAULT) - and the host is exactly the machine sitting on that + * last stage while the peer is still loading the previous one. A plain "has a + * nextStage" test would therefore leave the HOST unstamped precisely when the peer + * needs the marker, reinstating the false alarm chkBattleStage exists to prevent. + * + * Scoped this way so a SINGLE-stage battle - every UFO/terror/base mission, and every + * parallel-turn fixture - puts no extra field on the wire at all: its next_turn is + * byte-identical to before the guard existed. attachBattleChecksum/verifyBattleChecksum + * have exactly one caller each (NextTurnState / the next_turn handler), so this runs + * once per turn boundary and the deployment sweep needs no cache. + */ +static bool coopMissionIsMultiStage(const Game* game, const std::string& type) +{ + if (type.empty() || !game) return false; + const Mod* mod = game->getMod(); + if (!mod) return false; + + if (const AlienDeployment* dep = mod->getDeployment(type)) + { + if (!dep->getNextStage().empty()) return true; // an earlier stage + } + for (const std::string& name : mod->getDeploymentsList()) + { + const AlienDeployment* d = mod->getDeployment(name); + if (d && d->getNextStage() == type) return true; // a later stage + } + return false; +} + void attachBattleChecksum(Game* game, Json::Value& msg) { // coop (#151): PvP (gamemodes 2/3) runs a ROLE-AWARE sim where the two machines @@ -3751,7 +3785,10 @@ void attachBattleChecksum(Game* game, Json::Value& msg) // desync) from "we are not talking about the same battle yet" (a transition). if (const SavedBattleGame* battle = game->getSavedGame()->getSavedBattle()) { - msg["chkBattleStage"] = battle->getMissionType(); + if (coopMissionIsMultiStage(game, battle->getMissionType())) + { + msg["chkBattleStage"] = battle->getMissionType(); + } } } diff --git a/tools/ci/run_coop_suite.ps1 b/tools/ci/run_coop_suite.ps1 index 310c7de86..af819987e 100644 --- a/tools/ci/run_coop_suite.ps1 +++ b/tools/ci/run_coop_suite.ps1 @@ -60,12 +60,23 @@ if ($PlanFile) { if ($ListOnly) { $tests; exit 0 } # to stdout, so callers can diff the shard split -# Known-broken on main (real failures, not flakes) - run but do not gate. +# Known-broken on main, or not gate-shaped - run but do not gate. # Add entries here if a test regresses; remove them as they are fixed so they gate # again. Empty = the whole suite gates (all green as of 2026-07-15). $quarantine = @( - "test_pvp_campaign_month", # issue #171: month-roll geoscape assert can't drain MissionDetectedState/SaveGameState - "test_crash_reporter" # issue #172: marker-bundle 60s timeout, intermittent + "test_pvp_campaign_month", # issue #171: month-roll geoscape assert can't drain MissionDetectedState/SaveGameState + "test_crash_reporter", # issue #172: marker-bundle 60s timeout, intermittent + # A REPRO TOOL, not a guard - its own docstring says so, and its exit codes are + # the reverse of what a gate assumes: exit 0 = "the heavy-alien-death desync + # REPRODUCED", exit 3 = "every alien side stayed in census" (i.e. clean). Gating on + # it therefore scores the bug FIRING as success and a clean run as failure - every + # green run of it on main (30 Aug, 4 Sep) was green because the drift fired, which + # is precisely the signal a green pipeline hides. The fixes from #166 still sit + # behind a lever that defaults off (g_wireOrderState) pending the battlescape + # rewrite, so it keeps reproducing intermittently (~1 in 3-4 per its docstring). + # Run it and print the verdict; do not gate on it. Its rc=0 is worth alerting on + # separately - it is currently the only automated thing that notices the drift. + "test_parallel_heavy_death_repro" ) # --- Per-test time budgets ------------------------------------------------------