From 11f7927f24160994dfcd20cd0a4c5dd51cec4548 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 22 Jul 2026 12:07:33 +0200 Subject: [PATCH] =?UTF-8?q?fix(client):=20T-1153=20=E2=80=94=20v2=20granul?= =?UTF-8?q?arity=20is=20authoritative=20in=20the=20staleness=20guard;=20le?= =?UTF-8?q?gacy=20compared=20only=20when=20v2=20absent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second live-round blocker: the T-1150 legacy granularity comparison stayed armed alongside the v2 check, and the server ALWAYS sends the u32::MAX Region sentinel in the legacy slot — which can never equal the client's pinned legacy value, so every Region response was stale-dropped after the v2 check passed. _echoed_granularity_matches() now branches on PRESENCE of granularity_v2: present -> v2 is the only comparison; absent (old server) -> legacy fallback. Mock-fleet audit while fixing: three tests had responses diverging from the real wire — the oversized-orbital round-trip omitted the legacy sentinel (passed for the wrong reason), the Region-accept test used legacy=1, and the mismatch-drop test left v2 at a masking default that would have inverted under the new rule. All wire-accurate now with a named SERVER_LEGACY_GRANULARITY_REGION_SENTINEL const; +2 tests (legacy- only old-server acceptance; v2-wins-regardless-of-legacy precedence proof). All three verified to fail against the reverted fix. Full suite 3524/3524. --- client/tests/test_atlas_window_request.gd | 111 +++++++++++++++++- client/tests/test_atlas_zoom_ladder.gd | 60 +++++++--- .../apps/atlas/atlas_window_request.gd | 62 +++++++--- 3 files changed, 198 insertions(+), 35 deletions(-) diff --git a/client/tests/test_atlas_window_request.gd b/client/tests/test_atlas_window_request.gd index 2e5c6f261..f40f335c1 100644 --- a/client/tests/test_atlas_window_request.gd +++ b/client/tests/test_atlas_window_request.gd @@ -14,6 +14,17 @@ extends GdUnitTestSuite # duplicated-load). const AtlasWindowRequest := preload("res://ui/implant/apps/atlas/atlas_window_request.gd") +## Dudley's WINDOW_GRANULARITY_REGION_KEY (server/src/atlas/layer_proxy.rs) — +## `u32::MAX`, the RESERVED KEY-SPACE TAG a real server ALWAYS puts in the +## legacy `granularity` slot for every Region response (never a real +## multiplier — District=1/Quarter=4 are the only legal wire multipliers). +## Do NOT "fix" this to 1 — using a convenient value here is EXACTLY the gap +## the live round caught (a mock that diverges from the wire in the one +## field that matters silently un-repros the bug). See +## AtlasWindowRequest's `_echoed_granularity_matches()` doc for the full +## rationale. +const SERVER_LEGACY_GRANULARITY_REGION_SENTINEL: int = 4294967295 + ## Build a hand-authored DistrictWindowLayer dict, granularity-aware ## (T-1150, extended T-1152/T-1153 for granularity_v2) — mirrors @@ -63,12 +74,24 @@ func _make_request() -> Variant: ## center/n must be dropped as stale, not accepted — a different rung's ## derive answering a request for a different rung is exactly as stale as a ## mismatched center (T-1150 extends §2's guard to this axis). +## +## **Live-round correction:** the mock MUST carry a mismatched +## `granularity_v2` too (explicit `"Quarter"`, not `_mock_window()`'s +## `"District"` default) — a real Quarter response ALWAYS carries +## `granularity_v2: "Quarter"` on the wire, never the District default this +## test's fixture used to leave implicit. Under the v2-authoritative-when- +## present precedence rule (see on_response()'s own doc), a v2-MATCHING +## response is accepted regardless of what the legacy int says — leaving +## granularity_v2 at its District default here would have made this test +## pass for the wrong reason (an accidentally-matching v2 field masking a +## genuinely mismatched legacy int), exactly the class of gap the live round +## caught in the oversized-orbital round-trip test. func test_on_response_with_mismatched_granularity_is_dropped_as_stale() -> void: var req = _make_request() req.request_now("GJ380c", Vector2i(2, 2), 2) assert_bool(req.is_pending()).is_true() - var quarter_window: Dictionary = _mock_window(Vector2i(2, 2), 2, 4, 0) + var quarter_window: Dictionary = _mock_window(Vector2i(2, 2), 2, 4, 0, "Quarter") req.on_response(_mock_response("GJ380c", quarter_window)) assert_bool(req.is_pending()).override_failure_message( @@ -106,14 +129,30 @@ func test_on_response_with_mismatched_granularity_v2_is_dropped_as_stale() -> vo ## echo at the SAME (center, n) — must be ACCEPTED and cached under the ## Region key, retrievable on a follow-up request without a new network round ## trip. +## +## **Live-round correction:** the mock's legacy `granularity` field is now +## Dudley's ACTUAL wire sentinel (`WINDOW_GRANULARITY_REGION_KEY` = +## `u32::MAX` = 4294967295), not a convenient `1` — the original version of +## this test used `1`, which coincidentally matched the request's own +## pinned `_granularity` and therefore never exercised the real mismatch a +## live server actually produces. See _echoed_granularity_matches()'s own +## doc (atlas_window_request.gd) for why this is load-bearing: without the +## v2-authoritative-when-present fix, THIS test would have failed with the +## real sentinel — it only passed before because the mock was wrong. func test_on_response_matching_granularity_v2_region_is_accepted_and_cached() -> void: var req = _make_request() req.request_now("GJ380c", Vector2i(0, 0), 6400, AtlasWindowRequest.GRANULARITY_V2_REGION) assert_bool(req.is_pending()).is_true() - var region_window: Dictionary = _mock_window(Vector2i(0, 0), 6400, 1, 0, "Region") + var region_window: Dictionary = _mock_window( + Vector2i(0, 0), 6400, SERVER_LEGACY_GRANULARITY_REGION_SENTINEL, 0, "Region" + ) req.on_response(_mock_response("GJ380c", region_window)) - assert_bool(req.is_pending()).is_false() + assert_bool(req.is_pending()).override_failure_message( + "a response carrying the REAL legacy sentinel (u32::MAX) in the old" + + " granularity slot must still be accepted — v2 is authoritative" + + " whenever present, the legacy field must not be compared at all" + ).is_false() var received: Array = [] req.window_ready.connect(func(w: Dictionary) -> void: received.append(w)) @@ -124,6 +163,30 @@ func test_on_response_matching_granularity_v2_region_is_accepted_and_cached() -> assert_bool(req.is_pending()).is_false() +## **The direct precedence-rule proof (live-round finding #2, the sharpest +## case):** a response whose `granularity_v2` MATCHES the request but whose +## LEGACY `granularity` field could never possibly match (the Region +## sentinel) must still be ACCEPTED — proving the legacy comparison is +## SKIPPED entirely when v2 is present, not merely "also checked and +## happens to pass." This is the literal shape of the live bug: real server +## responses ALWAYS carry the Region sentinel in the legacy slot, so any +## code path that still consults the legacy field when v2 is already +## authoritative would drop every single one of these, forever. +func test_on_response_v2_match_is_accepted_regardless_of_legacy_field_value() -> void: + var req = _make_request() + req.request_now("GJ380c", Vector2i(0, 0), 6400, AtlasWindowRequest.GRANULARITY_V2_REGION) + + var region_window: Dictionary = _mock_window( + Vector2i(0, 0), 6400, SERVER_LEGACY_GRANULARITY_REGION_SENTINEL, 0, "Region" + ) + req.on_response(_mock_response("GJ380c", region_window)) + + assert_bool(req.is_pending()).override_failure_message( + "v2 match must be sufficient on its own — the legacy sentinel value must" + + " never be consulted once granularity_v2 is present on the response" + ).is_false() + + # ============================================================================= # (b) old-server-shape response (no granularity/min_wl_m keys) -> defaults # ============================================================================= @@ -170,6 +233,48 @@ func test_on_response_missing_granularity_and_min_wl_defaults_and_is_accepted() assert_bool(req.is_pending()).is_false() +## **Live-round sibling test (instruction #2's "old-server path stays +## covered"):** a response that carries the LEGACY `granularity` key WITH AN +## EXPLICIT VALUE (1, i.e. genuinely present, not merely defaulted via +## absence — the case test_on_response_missing_granularity_and_min_wl_defaults_and_is_accepted +## above doesn't exercise, since it omits the key entirely) but has NO +## `granularity_v2` key at all — the true "hypothetically old, pre-T-1152 +## server" shape — must still be accepted for a plain District request via +## the legacy-comparison FALLBACK branch in `_echoed_granularity_matches()`. +## This is the other half of the v2-authoritative-when-present precedence +## rule: v2 present -> v2 alone decides; v2 ABSENT -> legacy alone decides +## (never both, never neither). +func test_on_response_legacy_only_no_v2_key_still_accepted_for_district() -> void: + var req = _make_request() + req.request_now("GJ380c", Vector2i(4, 4), 2) # defaults to District granularity + assert_bool(req.is_pending()).is_true() + + # Legacy-only shape: "granularity" IS present (district=1), "granularity_v2" + # key is absent entirely — not present-with-a-District-value, ABSENT. + var legacy_only_window := { + "center": [4, 4], + "n": 2, + "granularity": AtlasWindowRequest.DEFAULT_GRANULARITY, + "min_wl_m": 0, + "morphology": PackedByteArray([8, 14, 0, 1]), + "elev_q": PackedByteArray([40, 90, 5, 60]), + "temp_dc": [120, 95, -32768, 60], + "moisture_q": PackedByteArray([50, 30, 90, 20]), + "vegetation": PackedByteArray([2, 1, 6, 3]), + "glaciation": PackedByteArray([0, 0, 1, 2]), + } + assert_bool(legacy_only_window.has("granularity_v2")).override_failure_message( + "sanity: this fixture must NOT carry granularity_v2 at all — that's the point" + ).is_false() + + req.on_response(_mock_response("GJ380c", legacy_only_window)) + + assert_bool(req.is_pending()).override_failure_message( + "a legacy-only response (granularity=1 present, granularity_v2 absent) must" + + " still be accepted for a District request via the legacy-fallback branch" + ).is_false() + + # ============================================================================= # (c) n-clamp mirror (Tyre C1) — quarter n=32 stores clamped n=16 # ============================================================================= diff --git a/client/tests/test_atlas_zoom_ladder.gd b/client/tests/test_atlas_zoom_ladder.gd index 544fc27be..979f75007 100644 --- a/client/tests/test_atlas_zoom_ladder.gd +++ b/client/tests/test_atlas_zoom_ladder.gd @@ -14,6 +14,20 @@ extends GdUnitTestSuite const AtlasWindowRequest := preload("res://ui/implant/apps/atlas/atlas_window_request.gd") const AtlasDescendGeometry := preload("res://ui/implant/apps/atlas/atlas_descend_geometry.gd") +## Dudley's WINDOW_GRANULARITY_REGION_KEY (server/src/atlas/layer_proxy.rs) — +## `u32::MAX`, a RESERVED KEY-SPACE TAG the real server ALWAYS puts in the +## legacy `granularity` slot for every Region response (never a real +## multiplier — District=1/Quarter=4 are the only legal wire multipliers). +## Do NOT "fix" this to 1 — that would silently un-repro the live-round bug +## this constant exists to guard against (a real server's actual wire byte, +## not a convenient test value). See _echoed_granularity_matches()'s own doc +## (atlas_window_request.gd) for why this value can NEVER equal a client's +## stored `_granularity` (which stays pinned at DISTRICT_GRANULARITY=1 for +## every rung a T-1152-aware client requests) — that mismatch is exactly +## what silently dropped every Region response before the v2-authoritative +## fix. +const SERVER_LEGACY_GRANULARITY_REGION_SENTINEL: int = 4294967295 + ## Build a hand-authored DistrictWindowLayer dict (n=2 by default) — mirrors ## test_atlas_window_viewer.gd's own _mock_window(). @@ -92,15 +106,26 @@ func test_enter_orbital_n_is_the_clamped_value_not_raw_circumference() -> void: ).is_greater(expected_clamped) -## **The live-round regression, end to end:** enter_orbital() on a -## real-sized body (GJ380c/Lendel, radius 6238.4 km, raw cols far past the -## Region clamp ceiling) followed by a server response echoing the CLAMPED -## n + "Region" granularity must be ACCEPTED and become the held window — not -## silently dropped as stale forever (the exact live bug: `wv._held_n = -## 19139` vs. echoed `6400`, blank ladder on every real-sized body). This is -## the round-trip the existing suite never exercised — every prior -## enter_orbital() test asserted on request-side state only, never delivered -## a response. +## **The live-round regression, end to end (fix #1: the n-clamp mirror one +## layer up):** enter_orbital() on a real-sized body (GJ380c/Lendel, radius +## 6238.4 km, raw cols far past the Region clamp ceiling) followed by a +## server response echoing the CLAMPED n + "Region" granularity must be +## ACCEPTED and become the held window — not silently dropped as stale +## forever (the exact live bug: `wv._held_n = 19139` vs. echoed `6400`, +## blank ladder on every real-sized body). This is the round-trip the +## existing suite never exercised — every prior enter_orbital() test +## asserted on request-side state only, never delivered a response. +## +## **WIRE-ACCURATE response shape (fix #2, second live-round finding):** the +## response dict below carries `"granularity": +## SERVER_LEGACY_GRANULARITY_REGION_SENTINEL` explicitly — the ACTUAL byte a +## real server sends, not the field's absence. The first version of this +## test omitted the legacy key entirely, which let `w.get("granularity", +## DEFAULT)` silently default to `1` (matching `_granularity`'s own pinned +## value) — an ACCIDENTAL pass that never exercised the real sentinel +## mismatch, exactly the class of gap the live round exists to catch. This +## version fails without the v2-authoritative-when-present fix in +## `_echoed_granularity_matches()`. func test_enter_orbital_oversized_body_accepts_the_clamped_region_response() -> void: var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) add_child(v) @@ -116,12 +141,14 @@ func test_enter_orbital_oversized_body_accepts_the_clamped_region_response() -> "no response delivered yet — must still be null" ).is_null() - # The server's real response: echoes the CLAMPED n, "Region" granularity, - # center (0,0) — exactly what handle_atlas_request/clamp_window_n_v2 - # actually produces for an oversized orbital request. + # The server's REAL response: echoes the CLAMPED n, "Region" granularity_v2 + # (String), center (0,0), AND the legacy sentinel in "granularity" — exactly + # what handle_atlas_request/clamp_window_n_v2 actually produces on the wire + # for an oversized orbital request (confirmed against Dudley's contract). var region_window: Dictionary = { "center": [0, 0], "n": clamped_n, + "granularity": SERVER_LEGACY_GRANULARITY_REGION_SENTINEL, "granularity_v2": "Region", "morphology": PackedByteArray([8, 14, 0, 1]), "elev_q": PackedByteArray([40, 90, 5, 60]), @@ -133,10 +160,11 @@ func test_enter_orbital_oversized_body_accepts_the_clamped_region_response() -> SimBridge.atlas_layers_received.emit(_mock_response("GJ380c", region_window)) var failure_msg: String = ( - "a response echoing the server's own clamped n + Region granularity must be" - + " ACCEPTED and become the held window — the live bug left this permanently" - + " null (w_n=%d never matched a stale unclamped _held_n=%d) on every" - + " real-sized body" + "a response echoing the server's own clamped n + Region granularity_v2 (with" + + " the legacy sentinel u32::MAX in the old granularity slot) must be ACCEPTED" + + " and become the held window — the live bug left this permanently null" + + " (w_n=%d never matched a stale unclamped _held_n=%d, THEN the legacy" + + " sentinel never matched the stored _granularity=1) on every real-sized body" ) % [clamped_n, raw_cols] assert_that(v.get_district_window()).override_failure_message(failure_msg).is_equal( region_window diff --git a/client/ui/implant/apps/atlas/atlas_window_request.gd b/client/ui/implant/apps/atlas/atlas_window_request.gd index ca5999f90..12ae7fc76 100644 --- a/client/ui/implant/apps/atlas/atlas_window_request.gd +++ b/client/ui/implant/apps/atlas/atlas_window_request.gd @@ -259,12 +259,35 @@ func _on_debounce_timeout() -> void: ## Handle an AtlasLayerResponse (routed by the owning viewer from its own ## SimBridge.atlas_layers_received subscription — this object has no signal ## connection of its own, matching atlas_generation_proxy.gd's on_response() -## shape). Ignores responses for a stale body/center/n/granularity/min_wl_m/ -## granularity_v2 (the player panned, zoomed across a rung boundary, or -## navigated away while a request was in flight, or a different rung's derive -## answers a request for a different rung, T-1150/T-1152) — the echoed fields -## ARE the staleness guard (§2, extended T-1150/T-1152), compared here -## against what THIS object most recently asked for. +## shape). Ignores responses for a stale body/center/n/min_wl_m/granularity +## (legacy OR v2, see below) — the player panned, zoomed across a rung +## boundary, or navigated away while a request was in flight, or a different +## rung's derive answers a request for a different rung, T-1150/T-1152 — the +## echoed fields ARE the staleness guard (§2, extended T-1150/T-1152), +## compared here against what THIS object most recently asked for. +## +## **Live-round finding (the second C1-shaped bug): v2 is AUTHORITATIVE over +## the legacy field whenever v2 is present — the legacy comparison is +## SKIPPED entirely, not run alongside it.** A T-1152-aware server (this +## codebase's) ALWAYS populates `granularity_v2` on the wire (Dudley's +## contract, `DistrictWindowLayer.granularity_v2`'s own doc: "Always +## populated (never `None`)"), and for `Region` responses specifically the +## LEGACY `granularity` slot carries `WINDOW_GRANULARITY_REGION_KEY` +## (`u32::MAX` = 4294967295) — a reserved KEY-SPACE TAG, not a real +## multiplier, that can never equal this object's own stored `_granularity` +## (which stays pinned at `DEFAULT_GRANULARITY`=1 for every rung this object +## requests, per that field's own doc — the legacy slot has no concept of +## Region at all). Comparing the legacy field UNCONDITIONALLY alongside v2 +## therefore drops EVERY Region response as stale forever, even though the +## v2 comparison alone would have correctly accepted it — exactly the live +## bug (`_held_n` fixed; this is the same "old comparison still active +## alongside the new one" class of bug, one layer up in the staleness +## checks). Fix: branch on whether `granularity_v2` is actually PRESENT in +## the response dict (`w.has(...)`, not `w.get(..., default)` — the +## presence/absence distinction is the whole point here) — present (every +## real server, always) -> v2 is the ONLY granularity comparison; absent (a +## hypothetically old, pre-T-1152 server) -> fall back to the legacy +## comparison alone, matching this object's own pre-T-1152 behavior exactly. func on_response(response: Dictionary) -> void: if str(response.get("body_id", "")) != _body_id: return @@ -289,21 +312,13 @@ func on_response(response: Dictionary) -> void: var w: Dictionary = window var echoed_center := _vec_from_center(w.get("center", [0, 0])) var echoed_n := int(w.get("n", 0)) - var echoed_granularity := int(w.get("granularity", AtlasWindowCache.DISTRICT_GRANULARITY)) var echoed_min_wl_m := int(w.get("min_wl_m", 0)) - # T-1152: granularity_v2 is ALWAYS populated on a real server response - # (resolve_window_granularity_v2() always resolves to a concrete rung — - # see DistrictWindowLayer.granularity_v2's own doc), but the mock/old-shape - # response fixtures this suite's own tests build predate the field — - # default to "District" so an old-shape mock keeps matching a - # district-granularity request exactly as it did before this field existed. - var echoed_granularity_v2 := str(w.get("granularity_v2", AtlasWindowCache.DEFAULT_GRANULARITY_V2)) + var granularity_matches: bool = _echoed_granularity_matches(w) if ( echoed_center != _center or echoed_n != _n - or echoed_granularity != _granularity or echoed_min_wl_m != _min_wl_m - or echoed_granularity_v2 != _granularity_v2 + or not granularity_matches ): return # stale — answers a window we've since panned/zoomed away from, or a different rung @@ -313,6 +328,21 @@ func on_response(response: Dictionary) -> void: window_ready.emit(w) +## The granularity half of on_response()'s staleness check, split out for the +## v2-authoritative-when-present precedence rule (see on_response()'s own +## doc for the full live-round rationale). Presence, not value, is the +## branch: `w.has("granularity_v2")` — a real server ALWAYS sets this key +## (even if its value happened to coincidentally equal a default), so +## checking presence rather than "is it the default value" is the only +## correct way to distinguish "an old server that never heard of this field" +## from "a new server whose value happens to match." +func _echoed_granularity_matches(w: Dictionary) -> bool: + if w.has("granularity_v2"): + return str(w.get("granularity_v2")) == _granularity_v2 + var echoed_granularity := int(w.get("granularity", AtlasWindowCache.DISTRICT_GRANULARITY)) + return echoed_granularity == _granularity + + func _schedule_retry() -> void: var timer := get_tree().create_timer(RETRY_DELAY) timer.timeout.connect(