feat(client+engine): D-254 Atlas companion wave 1 — reader connections + standalone shell + make atlas #183
Assignees
jpmschweitzer (Jeroen Schweitzer)
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: jpmschweitzer/settled-reach#183
Reference in New Issue
Block a user
Summary
Wave 1 of the Atlas companion app (D-254, epic T-1128): the decision record, the server's reader-connection layer, and the standalone desktop shell with
make atlas. After this merges,make atlasgives a read-only second-monitor Atlas against a running server (attach) or a self-spawned one — the legitimate player-facing pattern Jeroen specified, and the lightweight DB-scanning tool for development.T-1129 — D-254 decision record (design pass)
Full architecture record in governance/decisions/architecture.md: connection model (attach SR_PORT/9876 else spawn --port 0), ConnectionRole reader protocol with permitted-message matrix, wire-only data browser (no SQLite GDExtension, D-020; T-949 precedent), TradingReader future seam, loopback-only scope, save/load interaction seam (Phase 5+), and the SS3 amendment: the companion is a generic implant HOST — every registered implant app appears automatically;
available_in_companion: bool = trueon the app manifest is the opt-out for gameplay-dependent apps; the in-game implant ignores the flag entirely.T-1130 — reader connections (server, Oscar)
ConnectionRole(Player | Reader,#[serde(default)]= Player — old clients wire-compatible byte-for-byte; shaped for a futureTradingReader).BridgeResourcerewritten 0-1 Player + 0-N Readers withConnectionId; per-tickaccept_new_connectionsloop replaces the single blocking accept (the old accept made any second connection hang the client and its disconnect kill the server).PlayerInputfrom a reader = drop + strike (disconnect at 3);ObserverSnapshothas no reader-facing send path at all (structural, not filtered).ServerRunning. Reader-only servers (spawn-mode's common case — fixed from the original design during implementation) idle validly; reader disconnects never kill a session.T-1132 — standalone shell + make atlas (client, Stig)
atlas_standalonescene/script: attach-or-spawn boot. One realconnect_to_simattempt — the originally-designed throwaway TCP probe was proven by live run to kill a pre-accept-loop server (probe consumed the blocking accept slot; server's handshake hit broken pipe and exited). Never abandon a connected socket.ServerProcess.start_with_pipe(new method, existing API untouched) +LISTENING:{port}stdout parse, real systems.db, no --test-mode.encode_startup_messageoptional role param (empty omits the key — byte-identical output for every existing caller, matching serde default); client suppressesRequestAllSettingsandAutoPause/AutoResumeauto-sends for readers (each would burn a violation strike — endorsed by Oscar cross-review).available_in_companion; registry default parameter preserves in-game behavior byte-identically. Future implant apps extend the companion automatically.Live verification (real builds, real display)
Spawn-mode boots and renders the live star map (301 systems over the wire); body browse renders full orbital layouts; attach-mode connects first-try against an independent server; two simultaneous readers verified with independent lifecycles; zero orphan processes after every graceful close including
make atlasitself.Out of scope (wave 2, already ticketed)
T-1131 browse data proxy, T-1133 data browser UI, T-1134 seed picker, T-1135 character-spawn split.
Tickets: T-1129, T-1130, T-1132
New atlas_standalone scene/script: attach-or-spawn boot (one REAL connect_to_sim attempt at SR_PORT-or-9876 — a separate throwaway TCP probe was proven by live run to kill a pre-accept-loop server via broken handshake pipe; never abandon a connected socket), else spawn --port 0 via new ServerProcess.start_with_pipe + LISTENING:{port} stdout parse, retry against the resolved port. Reader role wired end to end: protocol.encode_startup_message optional role param (empty omits the wire key — byte-identical for all existing callers), sim_bridge.connection_role suppresses the post-handshake RequestAllSettings auto-send, hud_groups skips AutoPause/AutoResume sends for readers (all three would otherwise burn Reader violation strikes per the T-1130 matrix — endorsed by Oscar). Generic implant host per D-254 SS3: the shell instantiates ALL registered implant apps; implant_app_manifest gains available_in_companion (opt-out, default true) and implant_registry.instantiate_all a standalone filter param (default preserves hud.gd behavior byte-identically). Boot order is instantiate_all THEN open_app (reverse renders a permanently black window — app_changed fires with no listener; matches hud.gd's order). Owned-server lifecycle: _exit_tree stops a spawned child, attached servers survive companion close. Known engine limitation documented: raw SIGTERM bypasses all Godot notifications and orphans a spawned server; WM close paths verified clean. Live-verified: spawn-mode (301 systems rendered from systems.db over the wire), attach-mode, two simultaneous readers, clean shutdown with zero orphan processes. 14 new gdUnit tests (port/LISTENING parsing). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Review — PR #183 (atlas-companion-app) — Tyre (Technical Architect)
Verdict: REQUEST_CHANGES — one reachable, user-facing defect in the client close-interceptor. Server side (T-1130) is clean and merge-ready; the block is a single-line fix in
atlas_standalone.gd.Reviewed against D-254 (which I authored) with extra rigor on the implementation-vs-record fit. The five already-adjudicated deviations are all implemented cleanly and verified. Determinism (D-010), the accept-loop state machine, the BridgeResource shape, protocol evolution, and the generic-host seam all pass.
REQUEST_CHANGES
client/scripts/atlas_standalone.gd:106-108— close-interceptor re-entersopen_appsynchronously insideclose_app's emit; the re-open's state is then clobbered, soft-locking the Atlas to keyboard input.Trace (companion, Atlas fullscreen, user presses M/Escape on the "reach" screen):
atlas_app._handle_key(KEY_M)→HudGroups.close_app()(atlas_app.gd:77/85).close_app()emitsapp_changed("implant/map", GAMEPLAY)athud_groups.gd:131— before it resets_active_app(line 132)._on_hud_app_changed, condition true, which callsHudGroups.open_app(FRONT_APP)re-entrantly. The re-open sets_active_app="implant/map",_active_mode=FULLSCREEN, group z=Z_FULLSCREEN, and emitsapp_changed(..., FULLSCREEN)(churningatlas_app'son_close→on_openin the same frame).close_app, which continues past its emit:_active_app="",_active_mode=GAMEPLAY(lines 132-133), thengameplay_occluded.emit(false).End state: the Atlas node is left visible with group z at fullscreen, but the manager's authoritative state is
_active_app=""— soHudGroups.is_app_active("implant/map")returns false.atlas_app._unhandled_key_inputguards on exactly that (atlas_app.gd:64), so after this single M/Escape press the Atlas is on screen but dead to all keyboard input (M, Escape, arrows, Enter all no-op). The companion is soft-locked with no keyboard recovery.This is the reentrancy hazard the design's §3 option (a) opened without closing: "re-open implant/map" was implemented as a synchronous in-signal call, and
close_appcompleting last wins.test_atlas_standalone.gd:17-19explicitly defers this path to "a live run" rather than a unit test — and a casual live smoke test ("does M leave the Atlas up? yes → no-op as designed") would plausibly not then check that keys still work.Fix (one line): defer the re-open out of the signal frame so
close_appfully settles first —close_appthen runs to completion (_active_app=""), and the deferredopen_appexecutes next from a top-level context, restoring_active_app="implant/map"/is_app_active()==trueand re-arming input. Matches how the script already defers_boot(_boot.call_deferred()at line 92). D-254's §3 record text stays accurate — it describes option (a) at a level of detail that doesn't prescribe the sync-vs-deferred call, so no record amendment is needed, just the code fix.Verified clean (no change requested)
AtlasRequestBuffer/StarMapRequestBuffer/CityNamesRequestBufferand are served (atlas/plugin.rs) fromBodyWorldStateCache(derived),StarMapDataPath(file), andCityContextReaderResource(read-only DB). None mutate ECS sim state.serve_atlas_requeststakesRes<SimRng>but only calls.seed()(reads the immutable seed, never advances the RNG) — noResMut<SimRng>, no perturbation. ReaderVec<PlayerInput>frames are struck+dropped inreceive_bridge_inputs, never reachInputQueue.reader_never_receives_observer_snapshot_while_player_streamsproves the boundary with a direct socket read.PendingConnectionis non-blocking by construction (set non-blocking atnew(),WouldBlock→Waiting, EOF/UnexpectedEof→Failed, malformed startup→Failed). Per-tick work is bounded (onelistener.accept()+ one poll per pending). First blockingaccept_oninmain.rscompletes before the tick loop starts and beforeConnectionListeneris ever queried — no double-accept, no race; thetry_cloneshares the fd correctly. Pending set is technically unbounded, but this is loopback-only per D-254 §3's recorded threat model (non-loopback requires §6 auth first) — acceptable and consistent with the deferred reader-count cap (T-1135).ConnectionId;send_snapshothas no reader-facing counterpart (structural D-010 enforcement);Connection.rolecarried (dead today) precisely so §6'sTradingReaderwidens by match-arm, not struct change. Second-Player rejected post-handshake with a cleandrop(stream)— the right call (you can't know the role until startup arrives, so pre-handshake rejection is impossible; post-handshake clean disconnect is a well-defined, discoverable failure).second_player_attempt_is_cleanly_rejectedcovers it.ServerRunning). Adjudicated deviation #2, clean.SimBridge.server_path=""so SimBridge never spawns — the shell owns_server_process;_exit_tree→stop()plusServerProcessNOTIFICATION_PREDELETEis a correct double safety net (not double-ownership). One real connect attempt, no throwaway probe (deviation #3) — the header doc's live-repro rationale is sound.instantiate_allbeforeopen_appordering documented and matcheshud.gd's contract.available_in_companion: bool = trueopt-out on the manifest +instantiate_all(parent, standalone=false)default is the minimal correct seam; in-game path is byte-identical (default arg), flagged-off apps are never instantiated so never reachable by app-path. A future app-switcher has what it needs (the manifest carriesapp_path, and filtered apps simply aren't in_instances).roleas#[serde(default)]follows D-192; old-client (no key) → Player, new-client Player → omits key → byte-identical (old_format_startup_message_decodes_as_playerhand-encodes the pre-D-254 shape to prove it). Both skew directions are graceful (old server ignores unknownrole; new server defaults absentroleto Player).make gamePlayer path traced end-to-end — unchanged. Reader path early-returns beforeRequestAllSettings/AutoPause(sim_bridge.gd:322,hud_groups.gd:75) to avoid a guaranteed role-violation on every companion boot — a good detail.available_in_companion,instantiate_allstandalone filter, in-game ignores the flag, option-(a) close handling leavingatlas_app.gdunmodified — confirmed unmodified). No record drift.atlastarget:SR_LIVE=1,build-server build-clientdeps, nomake stoporchestration (correct — the shell owns its own lifecycle). Clean.Note: the coastline-invention files (
coast_invention.rs,district_profile.rs,believability.json,aliveness_probe.rs, etc.) and the.pql/changelogdeletions ingit diff main..are merge-base noise (the branch is behind main after PR #182 landed), not part of this PR — no planning data is being clobbered.Review: atlas-companion-app → main (type: code) — round 2 (fix round)
Hoshe (QA): APPROVE
Wire back-compat proven by a hand-encoded pre-D-254 message test (
old_format_startup_message_decodes_as_player, types.rs:1179) and every existingencode_startup_messagecaller keeps byte-identical output. The permitted-message matrix is structurally enforced —send_snapshot/send_bridge_snapshothave no reader-reachable path at all; violation strikes disconnect at 3 per spec;ConnectionIdtagging through atlas/plugin.rs is genuinely 1:1 in-order. Shutdown scope correctly Player-only, with "reader-only server" a documented valid idle. Client Reader suppressions (sim_bridge RequestAllSettings, hud_groups AutoPause/AutoResume) are guard clauses leaving the Player path untouched. Registry filter is a no-op whenstandalone=false, so the in-game call site cannot regress. 13/13 bridge_tcp + 14/14 gdUnit confirmed on targeted reruns; the bridge tests use wall-clock deadlines and pair negative assertions with positive controls.Tyre (Architecture): APPROVE (round 1: REQUEST_CHANGES, 1 finding — fixed and confirmed)
Server side merge-ready on round 1: accept-loop per-tick bounded, determinism holds by construction (readers touch only derived caches / read-only DB, never SimRng or ECS sim state), Player-only snapshot targeting structural, all five adjudicated design deviations implemented cleanly, protocol evolution byte-compatible in both skew directions, D-254 record accurate to what shipped.
Round-1 finding (fixed in
9b5c11396):atlas_standalone.gdclose-interceptor calledopen_appsynchronously from insideclose_app'sapp_changedemit;close_appthen reset_active_app, clobbering the re-open — Atlas left visible butis_app_active()false, killing the input guard: keyboard soft-lock after one M/Escape on the reach screen. Fix:open_app.call_deferredout of the signal frame, clobber mechanism documented at the call site. Two regression tests pin the contract at the HudGroups level — the sync shape provably clobbers (tripwire if emit semantics change), the deferred shape survives with an intermediate not-yet-active assert that makes it a genuine proof rather than a timing-luck pass. Re-review verdict: CONFIRMED, APPROVE.9b5c11396) + 2 regression testsVerdict: APPROVED
Runtime verification: gate ran full cargo suite + full gdUnit (2984 → 2986 with the regression tests) + clippy -D warnings on every push. Author-side live verification (T-1132 report): spawn-mode renders the live star map (301 systems over the wire), attach-mode first-try, two simultaneous readers with independent lifecycles, zero orphan processes on all WM close paths. Merge-path smoke: connection-handshake changes are covered by the layer3 subprocess end-to-end test (real server binary spawn) plus the live attach/spawn runs above; the in-game Player path is byte-identical on the wire (hand-encoded back-compat test) and behaviorally untouched (guard-clause-only client changes).
Pull request closed