fix(client): T-1146 — gate gameplay input while a fullscreen implant occludes play #190

2026-07-21T19:52:01+02:00

jpmschweitzer commented

2026-07-21 19:52:01 +02:00

Summary

T-1146 — gameplay input no longer leaks to the server while a fullscreen implant occludes gameplay ("walking blind with the map open"). Found by Stig during T-1145; pre-existing since the implant system landed. Client-only.

The gate

Poll-site, in input_mapper.gd itself (both the held-movement _process poll and the discrete-action _unhandled_input handler) — the file's existing dialogue_active/free_camera_mode early-return idiom. Nothing is enqueued while occluded, so no backlog can flush when the implant closes.

The subtle catch that made this more than a one-liner

The natural gate signal — is_implant_active() — is also true in INSERT mode (the economics monitor side panel), where gameplay stays visible and playable by design. Gating on it would have silently broken WASD every time that panel is open. The gate reads HudGroups.gameplay_occluded instead (true only for FULLSCREEN — the actual "player can't see their character" condition), with regression tests pinning the INSERT distinction.

Held-state semantics (pinned by tests)

W held while opening the implant: movement stops on the next poll.
W still held while closing: movement does not resume — a release-then-repress is required (_suppress_move_until_release, armed on the close transition, cleared only when every movement key is observed released; releasing one of two held keys does not clear it). No lurch when the map closes on a forgotten finger.
Facing is exempt from the latch (turning to look on close is a re-sync, not a lurch).

Audited action set

Gated: movement (8 dirs), facing send, INTERACT, perception mode, pause/unpause, stance toggles, hub teleport, quicksave/quickload. Exempt with reasons: OPEN_MENU (Esc is the only keyboard path to close a fullscreen implant), BUG_REPORT + OPEN_JOURNAL (client-only, never reach send_input). Dialog-driven direct SimBridge.send_input calls (settings dialog, dialogue pause, quit-to-menu save) are deliberate UI actions outside InputMapper's queue — out of scope by design. The pre-existing server-side AutoPause/AutoResume defense observed firing correctly alongside the new client gate in the live logs.

Drive-by

A genuine pre-existing gdlint class-definitions-order violation in input_mapper.gd (public var after private vars) fixed — the file lints clean for the first time.

Verification

16 unit/state tests driving the real autoloads (real open_app/close_app transitions, engine-global Input.action_press state): gating, exemptions, INSERT distinction, both held-state transitions, the all-keys-released latch rule.
2 LIVE tests against a real spawned server through the real client pipeline (InputMapper → flush → SimBridge, not raw protocol): baseline position, open implant, 20 ticks of held-W → position unchanged; close without release, 10 more ticks → still unchanged; re-press → movement resumes. Reuses test_input_roundtrip's port-retry/wall-clock/COOLDOWN_TICKS conventions.
12-suite regression sweep of every InputMapper consumer: all green. Cold-parse with menu launch clean. Full suites via the pre-push gate.

Fix round: full-suite isolation (second commit)

The new suites passed in isolation but hit four distinct cross-suite leaks in gate order, each diagnosed to root cause rather than papered over:

Facing skew (unit suite): D-054 movement is facing-relative and _update_facing_from_mouse() reads engine/session globals every _process — "press W" resolved to MOVE_NORTHWEST after earlier suites moved those globals. Fixed by pinning facing North via the purpose-built T-1088 facing_angle_provider seam.
Snapshot starvation (live suite): poll_snapshot() is consume-and-clear on a shared autoload; a node leaked into the gdUnit tree by an earlier suite steals every snapshot during awaits. Fixed with a snapshot_received signal capture (fires on every consume, ours or the thief's) alongside the direct poll + an explicit pump.
Tick poisoning (live suite): with SimBridge left test_mode=true + CONNECTED by ambient suites, the leaked poller applies TestHarness mock snapshots (~tick 15+) into GameState.current_tick during the server-spawn awaits; the fresh server's own ticks (1..5) then stamp backward and its InputQueue panics (tick ordering violated, last=16 new=3). Fixed by neutralizing SimBridge before the first await, zeroing current_tick, and clearing _last_snapshot/_outbound_buffer in both hooks.
Crash-on-assert-failure: gdUnit asserts don't halt; player-entity dereferences are now guarded so a missing Player reports cleanly instead of a Godot runtime error.

Full suite after the fix round: 3368/3368. The leaked-poller tree contamination itself is a pre-existing test-infra defect affecting any live-pipeline test — ticketed separately for a hunt under the maintenance initiative.

Tickets: T-1146

## Summary T-1146 — gameplay input no longer leaks to the server while a fullscreen implant occludes gameplay ("walking blind with the map open"). Found by Stig during T-1145; pre-existing since the implant system landed. Client-only. ### The gate Poll-site, in `input_mapper.gd` itself (both the held-movement `_process` poll and the discrete-action `_unhandled_input` handler) — the file's existing `dialogue_active`/`free_camera_mode` early-return idiom. Nothing is *enqueued* while occluded, so no backlog can flush when the implant closes. ### The subtle catch that made this more than a one-liner The natural gate signal — `is_implant_active()` — is also true in **INSERT mode** (the economics monitor side panel), where gameplay stays visible and playable *by design*. Gating on it would have silently broken WASD every time that panel is open. The gate reads `HudGroups.gameplay_occluded` instead (true only for FULLSCREEN — the actual "player can't see their character" condition), with regression tests pinning the INSERT distinction. ### Held-state semantics (pinned by tests) - W held while **opening** the implant: movement stops on the next poll. - W still held while **closing**: movement does **not** resume — a release-then-repress is required (`_suppress_move_until_release`, armed on the close transition, cleared only when *every* movement key is observed released; releasing one of two held keys does not clear it). No lurch when the map closes on a forgotten finger. - Facing is exempt from the latch (turning to look on close is a re-sync, not a lurch). ### Audited action set Gated: movement (8 dirs), facing send, INTERACT, perception mode, pause/unpause, stance toggles, hub teleport, quicksave/quickload. Exempt with reasons: OPEN_MENU (Esc is the only keyboard path to *close* a fullscreen implant), BUG_REPORT + OPEN_JOURNAL (client-only, never reach `send_input`). Dialog-driven direct `SimBridge.send_input` calls (settings dialog, dialogue pause, quit-to-menu save) are deliberate UI actions outside InputMapper's queue — out of scope by design. The pre-existing server-side AutoPause/AutoResume defense observed firing correctly alongside the new client gate in the live logs. ### Drive-by A genuine pre-existing gdlint `class-definitions-order` violation in `input_mapper.gd` (public var after private vars) fixed — the file lints clean for the first time. ### Verification - 16 unit/state tests driving the real autoloads (real `open_app`/`close_app` transitions, engine-global `Input.action_press` state): gating, exemptions, INSERT distinction, both held-state transitions, the all-keys-released latch rule. - 2 LIVE tests against a real spawned server through the **real client pipeline** (InputMapper → flush → SimBridge, not raw protocol): baseline position, open implant, 20 ticks of held-W → position unchanged; close without release, 10 more ticks → still unchanged; re-press → movement resumes. Reuses `test_input_roundtrip`'s port-retry/wall-clock/COOLDOWN_TICKS conventions. - 12-suite regression sweep of every InputMapper consumer: all green. Cold-parse with menu launch clean. Full suites via the pre-push gate. ### Fix round: full-suite isolation (second commit) The new suites passed in isolation but hit **four** distinct cross-suite leaks in gate order, each diagnosed to root cause rather than papered over: 1. **Facing skew (unit suite):** D-054 movement is facing-relative and `_update_facing_from_mouse()` reads engine/session globals every `_process` — "press W" resolved to `MOVE_NORTHWEST` after earlier suites moved those globals. Fixed by pinning facing North via the purpose-built T-1088 `facing_angle_provider` seam. 2. **Snapshot starvation (live suite):** `poll_snapshot()` is consume-and-clear on a shared autoload; a node leaked into the gdUnit tree by an earlier suite steals every snapshot during awaits. Fixed with a `snapshot_received` signal capture (fires on every consume, ours or the thief's) alongside the direct poll + an explicit pump. 3. **Tick poisoning (live suite):** with SimBridge left `test_mode=true` + CONNECTED by ambient suites, the leaked poller applies TestHarness mock snapshots (~tick 15+) into `GameState.current_tick` during the server-spawn awaits; the fresh server's own ticks (1..5) then stamp backward and its InputQueue **panics** (`tick ordering violated, last=16 new=3`). Fixed by neutralizing SimBridge before the first await, zeroing `current_tick`, and clearing `_last_snapshot`/`_outbound_buffer` in both hooks. 4. **Crash-on-assert-failure:** gdUnit asserts don't halt; player-entity dereferences are now guarded so a missing Player reports cleanly instead of a Godot runtime error. Full suite after the fix round: **3368/3368**. The leaked-poller tree contamination itself is a pre-existing test-infra defect affecting any live-pipeline test — ticketed separately for a hunt under the maintenance initiative. Tickets: T-1146

jpmschweitzer added 3 commits 2026-07-21 19:52:01 +02:00

chore(meta): activate T-1146 — implant-open input leak fix cdc5396c21

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

fix(client): T-1146 gameplay input gated while an implant screen occludes gameplay cd52fe8843

InputMapper polled the D-054 move_* actions (and queued discrete
gameplay actions) unconditionally — WASD with a fullscreen implant
open walked the character blind. Poll-site gate (both _process and
_unhandled_input, the file's existing dialogue/free-camera early-return
idiom): nothing is enqueued while occluded, so no backlog can flush on
close.

Gate signal is HudGroups.gameplay_occluded, deliberately NOT
is_implant_active(): that flag is also true for INSERT mode (economics
monitor panel), where gameplay stays visible and playable by design —
the naive gate would have broken WASD there. Local _gameplay_occluded
mirror via the existing signal; regression tests pin the INSERT
distinction.

Held-state semantics: open-while-held stops on the next poll;
close-while-held requires release-then-repress (a
_suppress_move_until_release latch armed on the close transition,
cleared only when EVERY movement key is released — one-of-two released
does not clear, tested). Facing exempt from the latch (re-sync, not
lurch).

Audited action set: movement/facing-send/INTERACT/perception/pause/
stance/teleport/quicksave-load gated; OPEN_MENU exempt (Esc must close
the implant), BUG_REPORT/OPEN_JOURNAL exempt (client-only, never reach
send_input); dialog-driven direct SimBridge sends (settings, dialogue
pause, quit-to-menu save) out of scope by design. The pre-existing
server-side AutoPause defense observed firing correctly alongside.

Drive-by: pre-existing gdlint class-definitions-order violation in
input_mapper.gd fixed (public/private var ordering) — file lint-clean
for the first time.

Tests: 16 unit/state (real autoloads, real open_app/close_app
transitions, Input.action_press engine state) + 2 LIVE against a real
spawned server through the REAL client pipeline (position frozen for
20 held-W ticks while occluded; still frozen 10 ticks after close
without re-press; resumes on re-press) — port-retry/wall-clock/
COOLDOWN_TICKS conventions reused from test_input_roundtrip. 12-suite
regression sweep of every InputMapper consumer green.

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

test(client): T-1146 — full-suite isolation for the occlusion-gate suites 4af3dd2607

Unit suite: pin facing to North via the T-1088 facing_angle_provider seam.
D-054 makes movement facing-relative, and _update_facing_from_mouse() derives
facing from engine/session globals (viewport mouse position vs the
GameState.player_position anchor) — in full-suite order 'press W' resolved to
MOVE_NORTHWEST (the gate failure at line 187).

Live suite, three independent leaks from the shared autoloads, each of which
passed in isolation and failed in gate order:
- snapshot starvation: poll_snapshot() is consume-and-clear; a node leaked
  into the gdUnit tree by an earlier suite steals every snapshot during our
  awaits. Capture snapshot_received (fires on every consume, ours or theirs)
  alongside a direct poll + explicit SimBridge._process pump.
- tick poisoning: SimBridge left test_mode=true + CONNECTED lets the leaked
  poller apply TestHarness mock snapshots (~tick 15+) into
  GameState.current_tick during our spawn awaits; the fresh server's own
  ticks (1..5) then stamp backward and its InputQueue panics ('tick ordering
  violated', last=16 new=3). Neutralize SimBridge before the first await,
  zero current_tick, clear _last_snapshot/_outbound_buffer both directions.
- crash-on-assert-failure: gdUnit asserts don't halt; guard the player-entity
  dereferences so a missing Player reports cleanly instead of a runtime error.

Full suite: 3368/3368.

jpmschweitzer referenced this issue from a commit

2026-07-21 20:02:09 +02:00

fix(client): T-1146 review round — gate the T-1088 seams, kill the settle fallback tautology, neutralize AutoPause leak

jpmschweitzer added 1 commit 2026-07-21 20:02:09 +02:00

fix(client): T-1146 review round — gate the T-1088 seams, kill the settle fallback tautology, neutralize AutoPause leak 32a7b1e425

PR #190 review (Hoshe + Tyre), all five findings addressed:
- Tyre: queue_move_step/queue_stance_toggle (the T-1088 click-to-move seams)
  now share the full suppression predicate — all three input_queue producers
  in the module are gated, making the header's 'every action this file can
  produce' claim true (previously the seams relied on the sandbox caller's
  own freeze). +3 regression tests.
- Hoshe: the live suite's settle=baseline timeout fallback compared baseline
  to itself, vacuously passing the no-movement claim — a timeout now fails
  loudly (both tests).
- Hoshe: the unit suite's real open_app/close_app transitions fire the
  global auto-pause handler; with ambient SimBridge.state left CONNECTED by
  earlier suites (test_hub_teleport has no after_test) each transition
  silently appended AutoPause/AutoResume to the shared _outbound_buffer.
  before_test now forces DISCONNECTED (restored in after_test) and clears
  the buffer.
- Hoshe: documented why the live suite's reset_test_state() call is inert
  but still correct.

Full suite: 3374/3374.

jpmschweitzer commented

2026-07-21 20:02:25 +02:00

Review: implant-input-gate → main (type: code)

Reviewers: Hoshe (code quality / QA) + Tyre (architecture). Both reviewed from the branch worktree at commit 4af3dd260; fix round 32a7b1e42 re-confirmed by both.

Hoshe (QA): APPROVE (after fix round)

Initial verdict REQUEST_CHANGES, three findings — all fixed in 32a7b1e42, none retracted:

#	File	Issue	Resolution
1	test_input_gate_live.gd	`settle = baseline` timeout fallback compared baseline to itself — the no-movement/no-resume claims could vacuously pass exactly when the pipeline was starved	Timeout now fails loudly (assert + return) in both tests
2	test_input_mapper_occlusion_gate.gd	Real `open_app()`/`close_app()` transitions fire the global auto-pause handler; with ambient `SimBridge.state` left CONNECTED by earlier suites (test_hub_teleport.gd has no after_test), each transition silently appended AutoPause/AutoResume to the shared `_outbound_buffer`	before_test saves state, forces DISCONNECTED, clears the buffer; after_test restores
3	test_input_gate_live.gd	`reset_test_state()` call undocumented in a file that documents every other shared-state touch	One-line comment: mock-side-only, inert here, restores ambient mock for later suites

Re-confirmation: "No new problems introduced by the fix round. PR #190 is ready to merge from a QA standpoint."

Tyre (Architecture): APPROVE (after fix round)

Initial verdict REQUEST_CHANGES, one finding + corollary — fixed in 32a7b1e42:

#	File	Issue	Resolution
1	input_mapper.gd (queue_move_step / queue_stance_toggle)	The T-1088 click-to-move seams feed the same `input_queue` but lacked the `_gameplay_occluded` guard — the occlusion invariant was delegated to the sandbox caller's freeze (latent hole once Phase 5/T-962 lifts click-to-move into the shared session driver)	Both seams now share the full suppression predicate; all three `input_queue` producers gated; +3 regression tests pinning both directions by value
2	input_mapper.gd (header)	Header claim "every gameplay-sim-bound action this file can produce is gated" was false while the seams were ungated	With the guard added the claim is literally true — header stands

Architecture assessment (positive): poll-site placement is the right layer (nothing enqueued → no backlog flush); the _gameplay_occluded signal-mirror is correct and deliberately NOT the INSERT-inclusive is_implant_active(); the release latch is a minimal re-entrant-safe state machine; server-side AutoPause (→ TickRate::Paused) confirmed as the authoritative defense — the client gate is defense-in-depth, not a moved security boundary.

Re-confirmation: "The occlusion invariant is self-contained in the module now... No new issues. Ship it."

Verdict: APPROVED

Runtime verification: 16 unit tests + 2 live spawned-server tests + 3 seam regression tests; cold-parse clean; full gdUnit suite 3374/3374 through the pre-push gate. The cross-suite leaks uncovered during the gate runs (leaked scene-tree poller consuming/applying snapshots run-wide) are ticketed as T-1148 under the maintenance initiative.

# Review: implant-input-gate → main (type: code) Reviewers: Hoshe (code quality / QA) + Tyre (architecture). Both reviewed from the branch worktree at commit `4af3dd260`; fix round `32a7b1e42` re-confirmed by both. ## Hoshe (QA): APPROVE (after fix round) Initial verdict REQUEST_CHANGES, three findings — all fixed in `32a7b1e42`, none retracted: | # | File | Issue | Resolution | |---|------|-------|------------| | 1 | test_input_gate_live.gd | `settle = baseline` timeout fallback compared baseline to itself — the no-movement/no-resume claims could vacuously pass exactly when the pipeline was starved | Timeout now fails loudly (assert + return) in both tests | | 2 | test_input_mapper_occlusion_gate.gd | Real `open_app()`/`close_app()` transitions fire the global auto-pause handler; with ambient `SimBridge.state` left CONNECTED by earlier suites (test_hub_teleport.gd has no after_test), each transition silently appended AutoPause/AutoResume to the shared `_outbound_buffer` | before_test saves state, forces DISCONNECTED, clears the buffer; after_test restores | | 3 | test_input_gate_live.gd | `reset_test_state()` call undocumented in a file that documents every other shared-state touch | One-line comment: mock-side-only, inert here, restores ambient mock for later suites | Re-confirmation: "No new problems introduced by the fix round. PR #190 is ready to merge from a QA standpoint." ## Tyre (Architecture): APPROVE (after fix round) Initial verdict REQUEST_CHANGES, one finding + corollary — fixed in `32a7b1e42`: | # | File | Issue | Resolution | |---|------|-------|------------| | 1 | input_mapper.gd (queue_move_step / queue_stance_toggle) | The T-1088 click-to-move seams feed the same `input_queue` but lacked the `_gameplay_occluded` guard — the occlusion invariant was delegated to the sandbox caller's freeze (latent hole once Phase 5/T-962 lifts click-to-move into the shared session driver) | Both seams now share the full suppression predicate; all three `input_queue` producers gated; +3 regression tests pinning both directions by value | | 2 | input_mapper.gd (header) | Header claim "every gameplay-sim-bound action this file can produce is gated" was false while the seams were ungated | With the guard added the claim is literally true — header stands | Architecture assessment (positive): poll-site placement is the right layer (nothing enqueued → no backlog flush); the `_gameplay_occluded` signal-mirror is correct and deliberately NOT the INSERT-inclusive `is_implant_active()`; the release latch is a minimal re-entrant-safe state machine; server-side AutoPause (→ TickRate::Paused) confirmed as the authoritative defense — the client gate is defense-in-depth, not a moved security boundary. Re-confirmation: "The occlusion invariant is self-contained in the module now... No new issues. Ship it." ## Verdict: APPROVED Runtime verification: 16 unit tests + 2 live spawned-server tests + 3 seam regression tests; cold-parse clean; full gdUnit suite 3374/3374 through the pre-push gate. The cross-suite leaks uncovered during the gate runs (leaked scene-tree poller consuming/applying snapshots run-wide) are ticketed as T-1148 under the maintenance initiative.

jpmschweitzer closed this pull request

2026-07-21 20:03:05 +02:00

jpmschweitzer referenced this issue from a commit

2026-07-21 20:05:00 +02:00

chore(meta): T-1146 to review — PR #190 up (implant input gate)

jpmschweitzer referenced this issue from a commit

2026-07-21 20:05:00 +02:00

chore(meta): T-1146 done — PR #190 merged (implant input gate, double review + fix round)