fix(simulation): PR #176 review round — all 12 findings + 2 recommendations addressed

H1 demux: ShapeProbe defensive multi-shape rejection (union frames now Err, not first-match; +2 tests) and doc claim made honest. H2/T1 SystemIndex.reset_test_state() folded into SimBridge.reset_test_state() (load() inline per autoload rule) + has_pending_request() accessor. H3 no-op tests now assert the replay flag both directions. H4 retry test actually ingests a failure and asserts the retry semantic. H5 error fixture uses the normalized status string. H6 bridge_tcp e2e sends all five frame shapes over real TCP (star-map + city-names buffers asserted). H7 positive replay-on-CONNECTED test via the test_local_bridge test-mode-flip precedent (stub bridge captures + decodes the request bytes). H8/T2 stale PLACEHOLDER doc replaced with the confirmed contract. H9 is_capital doc matches the COALESCE reality. T-r1 demux ceiling written down (next shape = tagged envelope). T-r2 AtlasLayerResponse governance ceiling comment. Lead item: the four cargo-fmt-formatted files from the gate round are now committed (layer_proxy/plugin/bridge-mod/main). H10 note for the record: the 13 snapshot_*.msgpack fixtures in commit 845737617 were regenerated because they were stale against their own generator (pre-existing version-key removal) — verified harmless, no client reads that key.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

client/scripts/autoloads/sim_bridge.gd

@@ -103,6 +103,12 @@ func reset_test_state() -> void:
 		harness.reset()
 	# T-949: don't leak a star-map request across tests (autoload state).
 	_star_map_wanted = false
+	# SystemIndex's static cache is the same cross-suite leak class (PR #176
+	# review H2/T1) — reset it here so the dozens of suites already calling
+	# SimBridge.reset_test_state() cover both. load() inline per the autoload
+	# parse-order rule (CLAUDE.md).
+	var SI := load("res://ui/implant/widgets/system_index.gd")
+	SI.reset_test_state()
 
 
 func _test_snapshot() -> Dictionary:

client/scripts/protocol/protocol.gd

@@ -774,9 +774,11 @@ static func decode_atlas_layer_response(bytes: PackedByteArray) -> Variant:
 ## road_graph/settlements (T-960): passthrough fields for the L2 road/rail
 ## graph and L3 settlement placements, mirroring the district_grid precedent
 ## (T-1046) — raw decoded maps/arrays, no further client-side reshaping.
-## PLACEHOLDER key names ("road_graph"/"settlements") pending confirmation
+## Key names "road_graph"/"settlements" are the CONFIRMED wire contract —
-## against dudley-atlas-server's RoadGraphLayer/SettlementLayer field names —
+## identical to server/src/atlas/layer_proxy.rs AtlasLayerResponse's field
-## this is the one spot to rename if the server picks different keys.
+## names (pinned 2026-07-14; round-tripped by test_atlas_overlays.gd and the
+## server's msgpack round-trip tests). This remains the one client-side spot
+## to touch if the contract ever changes.
 static func atlas_response_from_raw(raw: Variant) -> Variant:
 	if not raw is Dictionary or not raw.has("status"):
 		return null

client/tests/test_atlas_city_names.gd

@@ -149,8 +149,11 @@ func test_city_names_received_error_status_leaves_markers_empty() -> void:
 	var v := _make_viewer()
 	v.show_body({"body_id": NON_SOL_BODY_ID}, {"system_id": NON_SOL_SYSTEM_ID})
 
+	# Normalized string form — production handlers only ever see the output of
+	# Protocol.city_names_response_from_raw, which reduces {"Error": msg} to
+	# "Error" (review H5: the raw wire shape only passed by str() coincidence).
 	v._on_city_names_received(
-		{"body_id": NON_SOL_BODY_ID, "status": {"Error": "db unavailable"}, "cities": []}
+		{"body_id": NON_SOL_BODY_ID, "status": "Error", "cities": []}
 	)
 
 	assert_that(v.get_markers()).override_failure_message(

client/tests/test_atlas_data_delivery.gd

@@ -228,9 +228,60 @@ func test_sim_bridge_still_routes_snapshots_after_starmap_additions() -> void:
 
 func test_request_star_map_is_noop_in_test_mode() -> void:
 	# SimBridge defaults to test_mode = true (SR_LIVE unset) — request_star_map
-	# must not error even with no live bridge/server.
+	# must not error even with no live bridge/server, but it MUST still record
+	# the wish so replay-on-connect can fire later (review H3: this flag's
+	# unreset leak was the cross-suite crash fixed in this same PR).
+	SimBridge.reset_test_state()
 	SimBridge.request_star_map()
+	assert_bool(SimBridge._star_map_wanted).override_failure_message(
+		"test-mode request_star_map must still set the replay flag"
+	).is_true()
+	SimBridge.reset_test_state()
 
 
 func test_request_city_names_is_noop_in_test_mode() -> void:
+	# Must not error — and must NOT touch the star-map replay flag (guards a
+	# future copy-paste wiring city-names into the wrong flag).
+	SimBridge.reset_test_state()
 	SimBridge.request_city_names("GJ903b")
+	assert_bool(SimBridge._star_map_wanted).is_false()
+
+
+class _CaptureBridge:
+	var sent: Array = []
+
+	func send_message(bytes: PackedByteArray) -> int:
+		sent.append(bytes)
+		return OK
+
+
+func test_replay_on_connect_sends_star_map_request_live() -> void:
+	# Review H7: the replay-on-CONNECTED path (_set_state →
+	# _send_star_map_request) is the exact mechanism behind the cross-suite
+	# crash fixed in this PR, and every other test runs test_mode=true where
+	# the guard short-circuits before touching _bridge — proving only
+	# "doesn't crash", never "actually replays". Exercise the positive case
+	# per the test_local_bridge.gd test-mode-flip precedent.
+	var original_test_mode: bool = SimBridge.test_mode
+	var original_state: SimBridge.ConnectionState = SimBridge.state
+	var original_bridge = SimBridge._bridge
+	SimBridge.reset_test_state()
+
+	var stub := _CaptureBridge.new()
+	SimBridge.test_mode = false
+	SimBridge._bridge = stub
+	SimBridge.state = SimBridge.ConnectionState.CONNECTING
+	SimBridge._star_map_wanted = true
+	SimBridge._set_state(SimBridge.ConnectionState.CONNECTED)
+
+	assert_int(stub.sent.size()).override_failure_message(
+		"reaching CONNECTED with _star_map_wanted set must replay the request"
+	).is_equal(1)
+	var decoded = Messagepack.decode(stub.sent[0])
+	assert_that(decoded.value).is_equal({"star_map": true})
+
+	# Restore autoload state for later suites.
+	SimBridge.test_mode = original_test_mode
+	SimBridge._bridge = original_bridge
+	SimBridge.state = original_state
+	SimBridge.reset_test_state()

client/tests/test_system_index.gd

@@ -99,14 +99,23 @@ func test_ingest_ignores_non_ready_status() -> void:
 
 
 func test_request_refresh_retries_after_a_failed_ingest() -> void:
-	# request_refresh() is a no-op in test mode either way (SimBridge has no
+	# The retry semantic itself (review H4): a failed ingest must clear the
-	# live connection), so this only proves the _requested/_loaded bookkeeping
+	# in-flight flag so a later request_refresh() actually asks again — not
-	# doesn't get stuck: a failed ingest must clear _requested so a later
+	# silently no-op forever because a prior request was "in flight".
-	# request_refresh() is willing to ask again (not just silently no-op
+	SystemIndex.reset_test_state()
-	# forever because a prior request was already "in flight").
+	SystemIndex.request_refresh()
-	SystemIndex.ingest({"status": "Ready", "nodes": [{"system_id": "GJ 1"}]})
+	assert_bool(SystemIndex.has_pending_request()).is_true()
-	assert_bool(SystemIndex.is_loaded()).is_true()
+	assert_bool(SystemIndex.is_loaded()).is_false()
-	SystemIndex.request_refresh()  # no-op — already loaded
+	# A failed fetch arrives: stays unloaded, and the in-flight flag clears...
+	SystemIndex.ingest({"status": "Error", "error": "db unavailable", "nodes": []})
+	assert_bool(SystemIndex.has_pending_request()).override_failure_message(
+		"a failed ingest must clear _requested so a later refresh can retry"
+	).is_false()
+	assert_bool(SystemIndex.is_loaded()).is_false()
+	# ...so a retry genuinely re-requests instead of no-opping.
+	SystemIndex.request_refresh()
+	assert_bool(SystemIndex.has_pending_request()).is_true()
+	SystemIndex.reset_test_state()
 
 
 func test_request_refresh_does_not_crash_when_already_loaded() -> void:

client/ui/implant/widgets/system_index.gd

@@ -32,6 +32,23 @@ static func is_loaded() -> bool:
 	return _loaded
 
 
+## True while a StarMapRequest is considered in flight — the test-observable
+## counterpart of the retry bookkeeping (see ingest()'s Error path).
+static func has_pending_request() -> bool:
+	return _requested
+
+
+## Test-only: clear the process-global static cache. Statics leak across
+## gdUnit suites in one process — the same class as the
+## SimBridge._star_map_wanted leak fixed in this PR (review H2/T1).
+## SimBridge.reset_test_state() calls this, so every suite already using it
+## gets both resets.
+static func reset_test_state() -> void:
+	_cache = []
+	_loaded = false
+	_requested = false
+
+
 ## Ask the bridge for the star map if it hasn't been fetched yet. Safe to call
 ## from every screen's _ready()/on_install() — idempotent once loaded or a
 ## request is already in flight.

server/src/atlas/attractor_matching.rs

@@ -40,8 +40,9 @@ pub struct CityRecord {
     /// `atlas_city_names.kind == 'capital'` (authored, not derived from
     /// population). Threaded onto [`CityPlacement`] for the Atlas
     /// [`SettlementLayer`](crate::atlas::layer_proxy::SettlementLayer) (T-960 §2).
-    /// Defaults to `false` when the source doesn't track `kind` (e.g. the
+    /// Every current reader (`city_context_reader` and the believability
-    /// believability harness's own settlement read).
+    /// harness's own settlement read) selects `COALESCE(kind,'city')`, so an
+    /// un-authored `kind` yields `false` — there is no kind-less source left.
     pub is_capital: bool,
 }
 

server/src/atlas/layer_proxy.rs

@@ -72,6 +72,11 @@ pub struct DistrictGridLayer {
 /// A layer response: the computed `Layer1Output` + the coarse district grid
 /// (D-225, T-1046) + the road-graph and settlement overlays (T-960 §1/§2), or
 /// a non-ready status.
+///
+/// Growth ceiling (governance-bounded): the one-`Option`-field-per-layer
+/// pattern tops out around six fields — D-226's 2026-07-13 amendment (d)
+/// rules out any L5/tile Atlas layer ever, leaving T-1112 (quarter
+/// footprints) and T-1113 (region climate) as the only remaining candidates.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct AtlasLayerResponse {
     pub body_id: String,
@@ -726,8 +731,13 @@ mod tests {
         assert_eq!(decoded.body_id, "GJ1c");
         let rg = decoded.road_graph.expect("road_graph survives round trip");
         assert_eq!(rg.nodes[0].position, (12, 58));
-        assert_eq!(rg.edges[0].maintenance, MaintenanceAuthority::Administrative);
+        assert_eq!(
-        let settlements = decoded.settlements.expect("settlements survives round trip");
+            rg.edges[0].maintenance,
+            MaintenanceAuthority::Administrative
+        );
+        let settlements = decoded
+            .settlements
+            .expect("settlements survives round trip");
         assert_eq!(settlements.settlements[0].name, "Port Aldren");
         assert_eq!(
             settlements.settlements[0].size_class,

server/src/atlas/plugin.rs

@@ -863,10 +863,8 @@ mod tests {
         use std::sync::atomic::{AtomicU32, Ordering};
         static SEQ: AtomicU32 = AtomicU32::new(0);
         let n = SEQ.fetch_add(1, Ordering::Relaxed);
-        let path = std::env::temp_dir().join(format!(
+        let path =
-            "sr_plugin_starmap_{}_{n}.json",
+            std::env::temp_dir().join(format!("sr_plugin_starmap_{}_{n}.json", std::process::id()));
-            std::process::id()
-        ));
         std::fs::write(&path, r#"{"_meta": {}, "nodes": [], "edges": []}"#).unwrap();
 
         let mut world = World::new();

server/src/bridge/mod.rs

@@ -57,8 +57,14 @@ pub enum BridgeError {
 /// fields), the two *new* map shapes each carry a mandatory boolean
 /// discriminator field the others don't have at all (`star_map` /
 /// `city_names`): a missing required field is a hard deserialize failure, not
-/// a silent ignore, so every shape's required-field set is mutually
+/// a silent ignore, so no *minimal well-formed* instance of one shape
-/// exclusive. `AtlasLayerRequest` itself is untouched byte-for-byte.
+/// satisfies another — and [`decode_inbound`] additionally REJECTS union
+/// frames that carry more than one shape's discriminators outright (PR #176
+/// review H1). `AtlasLayerRequest` itself is untouched byte-for-byte.
+///
+/// **Ceiling (D-225 trajectory):** four shapes is the practical limit of this
+/// hand-rolled sniffing. The next new inbound shape must migrate the channel
+/// to the tagged-envelope framing D-225 deferred — do not add a fifth probe.
 #[derive(Debug)]
 pub enum Inbound {
     /// A batch of player inputs (the gameplay path).
@@ -71,17 +77,61 @@ pub enum Inbound {
     CityNamesRequest(CityNamesRequest),
 }
 
+/// Key-presence probe for the defensive multi-shape check in
+/// [`decode_inbound`]: `Option<IgnoredAny>` records whether a key exists
+/// without caring about its value or type, so a union frame is detected even
+/// when the individual values wouldn't parse as their target types.
+#[derive(serde::Deserialize)]
+struct ShapeProbe {
+    body_id: Option<serde::de::IgnoredAny>,
+    up_to: Option<serde::de::IgnoredAny>,
+    star_map: Option<serde::de::IgnoredAny>,
+    city_names: Option<serde::de::IgnoredAny>,
+}
+
 /// Demux a received frame payload into an [`Inbound`] (D-225, T-949). Tries,
 /// in order: `Vec<PlayerInput>` (array) → `AtlasLayerRequest` (map,
 /// `body_id`+`up_to`) → `StarMapRequest` (map, `star_map` discriminator) →
-/// `CityNamesRequest` (map, `city_names` discriminator + `body_id`). Every map
+/// `CityNamesRequest` (map, `city_names` discriminator + `body_id`).
-/// shape's required fields are mutually exclusive (see the [`Inbound`] doc),
+///
-/// so this order is for stability, not correctness — a frame that satisfies
+/// Mutual exclusivity is enforced, not assumed: no minimal well-formed
-/// none of the four shapes is a genuinely malformed input frame.
+/// instance of one shape satisfies another (see the [`Inbound`] doc), and a
+/// defensive pre-check rejects any map frame carrying the discriminators of
+/// more than one shape — e.g. a buggy encoder emitting
+/// `{"star_map": true, "city_names": true, ...}` — instead of silently
+/// routing it to whichever shape is tried first (PR #176 review H1). A frame
+/// satisfying none of the four shapes is a genuinely malformed input frame.
 pub fn decode_inbound(payload: &[u8]) -> Result<Inbound, BridgeError> {
     if let Ok(inputs) = rmp_serde::from_slice::<Vec<PlayerInput>>(payload) {
         return Ok(Inbound::Inputs(inputs));
     }
+    // Defensive multi-shape rejection: serde ignores unknown fields, so a
+    // union frame would otherwise route silently by try-order. Unreachable
+    // from the shipped client encoders (each sends one minimal shape) — this
+    // guards buggy or adversarial frames.
+    if let Ok(probe) = rmp_serde::from_slice::<ShapeProbe>(payload) {
+        let atlas = probe.body_id.is_some() && probe.up_to.is_some();
+        let star_map = probe.star_map.is_some();
+        let city_names = probe.city_names.is_some();
+        let shapes = usize::from(atlas) + usize::from(star_map) + usize::from(city_names);
+        if shapes > 1 {
+            let dump_len = payload.len().min(256);
+            tracing::error!(
+                "inbound frame matches {} request shapes at once (atlas={}, star_map={}, city_names={}) — rejecting ambiguous frame. Raw ({} of {} bytes): {:02x?}",
+                shapes,
+                atlas,
+                star_map,
+                city_names,
+                dump_len,
+                payload.len(),
+                &payload[..dump_len]
+            );
+            return Err(BridgeError::DeserializationWithDump(format!(
+                "ambiguous inbound frame matches {shapes} request shapes (payload {} bytes)",
+                payload.len()
+            )));
+        }
+    }
     if let Ok(req) = rmp_serde::from_slice::<AtlasLayerRequest>(payload) {
         return Ok(Inbound::AtlasRequest(req));
     }
@@ -442,7 +492,10 @@ impl Plugin for BridgePlugin {
             .add_systems(Update, receive_bridge_inputs.in_set(TickPhase::PreInput))
             .add_systems(Update, send_bridge_snapshot.in_set(TickPhase::PostSnapshot))
             .add_systems(Update, send_atlas_responses.in_set(TickPhase::PostSnapshot))
-            .add_systems(Update, send_star_map_responses.in_set(TickPhase::PostSnapshot))
+            .add_systems(
+                Update,
+                send_star_map_responses.in_set(TickPhase::PostSnapshot),
+            )
             .add_systems(
                 Update,
                 send_city_names_responses.in_set(TickPhase::PostSnapshot),
@@ -605,4 +658,44 @@ mod inbound_tests {
         // it's missing the required `up_to` field.
         assert!(rmp_serde::from_slice::<AtlasLayerRequest>(&city_names_frame).is_err());
     }
+
+    /// PR #176 review H1: a union frame carrying more than one shape's
+    /// discriminators must be REJECTED, not silently routed to whichever
+    /// shape `decode_inbound` happens to try first.
+    #[test]
+    fn ambiguous_union_frame_is_rejected() {
+        #[derive(serde::Serialize)]
+        struct StarAndCity {
+            star_map: bool,
+            city_names: bool,
+            body_id: String,
+        }
+        let frame = rmp_serde::to_vec_named(&StarAndCity {
+            star_map: true,
+            city_names: true,
+            body_id: "GJ1c".into(),
+        })
+        .unwrap();
+        assert!(
+            decode_inbound(&frame).is_err(),
+            "star_map+city_names union frame must be rejected"
+        );
+
+        #[derive(serde::Serialize)]
+        struct AtlasAndStar {
+            body_id: String,
+            up_to: CascadeLayer,
+            star_map: bool,
+        }
+        let frame = rmp_serde::to_vec_named(&AtlasAndStar {
+            body_id: "GJ1c".into(),
+            up_to: CascadeLayer::Topography,
+            star_map: true,
+        })
+        .unwrap();
+        assert!(
+            decode_inbound(&frame).is_err(),
+            "atlas+star_map union frame must be rejected"
+        );
+    }
 }

server/src/main.rs

@@ -211,9 +211,9 @@ fn main() {
             star_map_data_path
         );
     }
-    app.insert_resource(settled_reach_server::atlas::atlas_data_proxy::StarMapDataPath(
+    app.insert_resource(
-        star_map_data_path,
+        settled_reach_server::atlas::atlas_data_proxy::StarMapDataPath(star_map_data_path),
-    ));
+    );
 
     // Settlement reader for Layer-3 placement (#955): reads a body's settlements
     // from systems.db on a cache miss so the cascade work item stays DB-free.

server/tests/bridge_tcp.rs

@@ -346,10 +346,14 @@ fn single_tick_drains_all_ready_inbound_frames() {
     let listener = TcpListener::bind("127.0.0.1:0").expect("failed to bind");
     let server_addr = listener.local_addr().expect("failed to get local address");
 
-    // Client: three frames back-to-back in one tick window — two input
+    // Client: five frames back-to-back in one tick window — two input
-    // batches plus one atlas request (the D-225 demux path). Returns the
+    // batches plus one of EACH request shape (atlas, star-map, city-names:
-    // stream so it stays open until assertions complete (no EOF race).
+    // the full D-225/T-949 demux surface over the real framing/poll path —
+    // PR #176 review H6). Returns the stream so it stays open until
+    // assertions complete (no EOF race).
     let client_handle = thread::spawn(move || {
+        use settled_reach_server::atlas::atlas_data_proxy::{CityNamesRequest, StarMapRequest};
+
         let mut stream = TcpStream::connect(server_addr).expect("failed to connect");
         for tick in [20u64, 21] {
             let inputs = vec![PlayerInput {
@@ -365,6 +369,15 @@ fn single_tick_drains_all_ready_inbound_frames() {
         };
         let payload = rmp_serde::to_vec_named(&req).expect("failed to serialize");
         write_framed(&mut stream, &payload).expect("write atlas frame");
+        let sm = StarMapRequest { star_map: true };
+        let payload = rmp_serde::to_vec_named(&sm).expect("failed to serialize star map");
+        write_framed(&mut stream, &payload).expect("write star map frame");
+        let cn = CityNamesRequest {
+            city_names: true,
+            body_id: "GJ1c".into(),
+        };
+        let payload = rmp_serde::to_vec_named(&cn).expect("failed to serialize city names");
+        write_framed(&mut stream, &payload).expect("write city names frame");
         stream
     });
 
@@ -397,6 +410,18 @@ fn single_tick_drains_all_ready_inbound_frames() {
         1,
         "the atlas request must drain in the same tick"
     );
+    assert_eq!(
+        world.resource::<StarMapRequestBuffer>().0.len(),
+        1,
+        "the star-map request must drain in the same tick (H6: real wire path)"
+    );
+    let city_names = &world.resource::<CityNamesRequestBuffer>().0;
+    assert_eq!(
+        city_names.len(),
+        1,
+        "the city-names request must drain in the same tick (H6: real wire path)"
+    );
+    assert_eq!(city_names[0].body_id, "GJ1c");
     assert!(
         world.resource::<ServerRunning>().0,
         "draining must not shut the server down"