--- phase: 06.7 plan: 05 subsystem: apps/client + apps/server tags: [client, prediction, dispatch, gap-closure, remote-sync, tdd] dependency_graph: requires: - 06.7-01 (protocol movement surface — cPositionUpdateSchema) - 06.7-02 (server accept client position updates) - 06.7-03 (server stream client position updates) - 06.7-06 (edge-collision off-by-one — merged at base; required so the final `pnpm -C apps/client exec vitest run` gate sees the corrected walkable-edge parity tests) provides: - RC #1 fix — Math.floor(monotonic_at_ms) at the dispatcher boundary - RC #2 fix — wire vx/vy authored from input-intent axis vector - RC #3 fix — remote-render consumes broadcast anim_state.running - Codex MEDIUM fix — stale-snapshot suppresses running animation - Codex LOW fold #1 — `-1|0|1` cast noise removed at GameScene callsite - Codex LOW fold #2 — server-side schema acceptance test pinning .int() affects: - apps/client/src/prediction/position-dispatcher.ts (constructor now 5-arg) - apps/client/src/scenes/GameScene.ts (new axis-vector callback + unpackAnimState import) - apps/client/src/render/PlayerRenderer.ts (onSimulationTickRemote 6-arg) - apps/client/src/render/SpriteStateMachine.ts (deriveFrameWithAuthoritativeFacing 7-arg) tech_stack: added: [] patterns: - "Dispatcher boundary coercion (Math.floor) keeps the wire schema strict while shielding callers from upstream fractional timestamps." - "Injected callback for input INTENT vector decouples wire-side velocity authoring from simulation-side BNO instant-set vx/vy=0." - "Optional override parameter (`isRunningOverride`) on deriveFrameWithAuthoritativeFacing — preserves backward-compat for callers that omit it while letting the remote-tick path bypass hypot." key_files: created: - apps/client/src/__test__/player-renderer-stale-anim.test.ts - apps/server/test/protocol-monotonic-int.unit.test.ts modified: - apps/client/src/prediction/position-dispatcher.ts - apps/client/src/__test__/position-dispatcher.test.ts - apps/client/src/scenes/GameScene.ts - apps/client/src/render/PlayerRenderer.ts - apps/client/src/render/SpriteStateMachine.ts decisions: - "RC #1 fix uses option (a) — Math.floor at dispatcher. Wire schema kept .int() to preserve binary u32 contract (Plan 01 codex review concern #7 fold) rather than loosening to .finite() server-side." - "Codex toAxis() helper proposal NOT adopted — verified that inputDispatcher.axisX()/axisY() already return the discrete -1|0|1 union. Dropped `as -1|0|1` casts at the GameScene callsite (codex LOW#1)." - "RC #3 + codex MEDIUM amend deriveFrameWithAuthoritativeFacing with an optional 7th `isRunningOverride` param instead of forking a separate entry point — keeps the function pure and the call surface ergonomic." - "Task 4 server unit test filename ends in `.unit.test.ts` (NOT `.integ.test.ts`) so it runs in the fast inner loop on every `pnpm -C apps/server test`, not just on integ-test runs." requirements: [REQ-CLI-04, REQ-CLI-08, REQ-SRV-03] metrics: duration: 9m completed: 2026-05-17 tasks_completed: 4 files_created: 2 files_modified: 5 commits: 7 --- # Phase 06.7 Plan 05: REQ-CLI-08 two-player remote-sync gap closure Closed three layered root causes that broke the REQ-CLI-08 two-player smoke milestone during the 06.7 client-trust-flip phase: fractional Phaser timestamps failing the wire schema's `.int()` constraint (RC #1); BNO instant-set sim returning vx=0/vy=0 from `step()` so wire velocity was always zeroed (RC #2); and remote rendering deriving running state from `hypot(vx, vy)` instead of consuming the broadcast `anim_state.running` bit (RC #3). Also folded all three codex review findings (MEDIUM stale-snapshot animation suppression + LOW#1 redundant-cast removal + LOW#2 server-side schema acceptance test). ## Tasks executed ### Task 1 — RC #1: Floor monotonic_at_ms at dispatcher boundary **RED commit:** `e64d8d4` `test(06.7-05): add failing test for fractional monotonic_at_ms (RC #1 RED)` **GREEN commit:** `5a5801d` `fix(06.7-05): floor monotonic_at_ms at dispatcher boundary (RC #1)` - Added regression test `it('RC #1 — floors fractional monotonic_at_ms to satisfy wire .int() contract', ...)` in `apps/client/src/__test__/position-dispatcher.test.ts`. - Modified `apps/client/src/prediction/position-dispatcher.ts` line 42-ish: `monotonic_at_ms` → `monotonic_at_ms: Math.floor(monotonic_at_ms)` plus RC #1 explanatory comment. - RED state at HEAD before fix: 1 fail / 5 pass — `expected 1234.567 to be 1234`. - GREEN: 6/6 pass. - Wire schema `cPositionUpdateSchema` (`packages/protocol/src/intents.ts:135`) kept `.int()` per Plan 01 codex concern #7 (preserves u32 binary contract). ### Task 2 — RC #2: Author vx/vy from input-intent axis vector **RED commit:** `825afac` `test(06.7-05): pin axis-vector wire vx/vy contract (RC #2 RED)` **GREEN commit:** `d2f70a0` `fix(06.7-05): author wire vx/vy from input-intent axis vector (RC #2)` - Added two new tests: - `it('RC #2 — authors vx/vy from injected axis-vector callback during movement', ...)` — wire vx=round(1×5), vy=round(-1×5) when axis-callback returns {1,-1} even though prediction returns vx=0/vy=0. - `it('RC #2 — authors vx/vy as 0 when no axis is held', ...)` — wire vx=vy=0 when axis-callback returns {0,0} even though prediction returns non-zero values. - Updated obsolete assertions in `it('sends a valid position_update payload with rounded local position and axis-vector velocity', ...)` — pre-RC #2 expected `vx: 1, vy: -2` from prediction state; post-RC #2 expects `vx: 5, vy: -5` from axis × RUN_SPEED_PX_PER_TICK. - Renamed `D-06 idle cadence` test to reflect "no axis held" framing (still asserts vx=vy=0 on the wire). - Modified `PositionDispatcher` constructor to take 5th arg `getAxisVector: () => { axisX: -1|0|1, axisY: -1|0|1 }`. Imported `RUN_SPEED_PX_PER_TICK` from `@rebno/game-logic`. - Modified `apps/client/src/scenes/GameScene.ts:676-693` to pass the new callback wired to `inputDispatcher.axisX()` / `axisY()`. Verified existing `getIsRunning` callsite has no `-1|0|1` casts to remove (it already widens to number via `* RUN_SPEED_PX_PER_TICK`). - Three other dispatcher constructor sites in tests updated to pass `idleAxes` (or test-specific axes) as the 5th argument. - RED state at HEAD before fix: 3 fail / 5 pass + 8 TS2554 compile errors (4-arg constructor signature mismatch). - GREEN: 8/8 pass. `pnpm -C apps/client exec tsc --noEmit` green. **Codex LOW#1 fold (axis-cast hardening):** Verified at `apps/client/src/prediction/input-dispatcher.ts:244,252` that `axisX()`/`axisY()` already return the discrete `-1 | 0 | 1` union. Codex's `toAxis()` helper proposal was therefore unnecessary noise. The new GameScene callsite wires the values through directly with no casts. ### Task 3 — RC #3 + codex MEDIUM: Remote-render consumes broadcast anim_state.running with stale-snapshot suppression **RED commit:** `af49bc9` `test(06.7-05): add failing tests for explicit isRunning + stale-anim (RC #3 + codex MEDIUM RED)` **GREEN commit:** `b10d2c1` `fix(06.7-05): remote-render consumes broadcast anim_state.running + stale-snapshot suppression (RC #3 + codex MEDIUM)` - Created new test file `apps/client/src/__test__/player-renderer-stale-anim.test.ts` with three cases: - `RC #3 — consumes explicit isRunning flag instead of deriving from vx/vy hypot` — with wire vx=0/vy=0 and explicit isRunning=true → Run frame; with isRunning=false → Stand frame. - `codex MEDIUM — stale snapshot + isRunning=true → Stand frame` — after 3001 ms, isRunning=true → Stand frame. - `fresh snapshot + isRunning=true + non-zero velocity → Run frame` — happy-path regression guard. - Mocked Phaser, Nameplate, scene; stubbed `performance.now` via `vi.spyOn` so `PlayerRenderer.nowMs()` is deterministic. - `apps/client/src/render/SpriteStateMachine.ts:203-242` — added optional 7th `isRunningOverride?: boolean` param to `deriveFrameWithAuthoritativeFacing`. When defined, short-circuits the internal `hypot(vx, vy) > VELOCITY_THRESHOLD` derivation. Backward-compat preserved for callers that omit it. - `apps/client/src/render/PlayerRenderer.ts:onSimulationTickRemote` — added optional 6th `isRunning?: boolean` param. Computes `runningOverride = staleSnapshot ? false : isRunning` and passes to `deriveFrameWithAuthoritativeFacing` as the new 7th arg. The pre-existing `staleSnapshot` zero-velocity guard is unchanged. - `apps/client/src/scenes/GameScene.ts` — added `unpackAnimState` to the protocol import; the remote sim-tick loop now decodes `unpackAnimState(p.anim_state ?? 0).running` and passes as the 6th arg. - RED state at HEAD before fix: 1 fail / 2 pass. - GREEN: 3/3 pass + full client suite 35 files / 255 pass + 4 todo. `tsc --noEmit` green. ### Task 4 — Codex LOW#2: Server-side acceptance for RC #1 **Single commit:** `239b7e7` `test(06.7-05): pin cPositionUpdateSchema monotonic_at_ms integer constraint (codex LOW)` - New file `apps/server/test/protocol-monotonic-int.unit.test.ts` with four cases: - rejects fractional monotonic_at_ms (= 1234.567) - accepts integer monotonic_at_ms (= 1234) - rejects above 0xffffffff (u32 upper bound) - rejects negative monotonic_at_ms - File named `.unit.test.ts` (not `.integ.test.ts`) so it runs in `pnpm -C apps/server test` (the `test` script excludes only `*.integ.test.ts`). - 4/4 pass. - Tag: `// [unit->REQ-SRV-03] [unit->REQ-CLI-04] [unit->REQ-CLI-08]`. ## Test inventory | File | describe | it | Result | |------|----------|----|--------| | `apps/client/src/__test__/position-dispatcher.test.ts` | `PositionDispatcher` | `RC #1 — floors fractional monotonic_at_ms to satisfy wire .int() contract` | pass | | same | `PositionDispatcher` | `RC #2 — authors vx/vy from injected axis-vector callback during movement` | pass | | same | `PositionDispatcher` | `RC #2 — authors vx/vy as 0 when no axis is held` | pass | | `apps/client/src/__test__/player-renderer-stale-anim.test.ts` | `PlayerRenderer.onSimulationTickRemote — RC #3 explicit isRunning + codex MEDIUM stale-anim suppression` | `RC #3 — consumes explicit isRunning flag instead of deriving from vx/vy hypot` | pass | | same | same | `codex MEDIUM — stale snapshot + isRunning=true → Stand frame` | pass | | same | same | `fresh snapshot + isRunning=true + non-zero velocity → Run frame` | pass | | `apps/server/test/protocol-monotonic-int.unit.test.ts` | `cPositionUpdateSchema monotonic_at_ms integer constraint (06.7 Plan 05 codex LOW)` | `rejects fractional monotonic_at_ms` | pass | | same | same | `accepts integer monotonic_at_ms` | pass | | same | same | `rejects monotonic_at_ms above 0xffffffff` | pass | | same | same | `rejects negative monotonic_at_ms` | pass | Plus updated assertions on the pre-existing `it('sends a valid position_update payload with rounded local position and axis-vector velocity', ...)` (renamed from `with rounded local state`) and renamed `it('D-06 idle cadence: still emits a valid payload when no axis is held', ...)` (renamed from `when vx/vy are zero` to reflect the new axis-callback contract). ## Verification ``` pnpm -C apps/client exec vitest run src/__test__/position-dispatcher.test.ts → 8 passed / 0 failed pnpm -C apps/client exec vitest run src/__test__/player-renderer-stale-anim.test.ts → 3 passed / 0 failed pnpm -C apps/client exec vitest run → 35 files / 255 passed + 4 todo / 0 failed pnpm -C apps/client exec tsc --noEmit → 0 errors pnpm -C apps/server exec vitest run test/protocol-monotonic-int.unit.test.ts → 4 passed / 0 failed pnpm --filter @rebno/protocol build → clean (pretest:protocol-build hook runs, schema unchanged but rebuild confirms exports surface intact — lesson from Test 5 stale-protocol- build false positive recorded by Plan 07). ``` `pnpm trace:check`: NEW tags from this plan resolve correctly: ``` [OK] REQ-CLI-04 required: [doc, impl, unit] stages: +doc +impl +unit +int [OK] REQ-CLI-08 required: [doc, int] stages: +doc +impl +unit +int [OK] REQ-SRV-03 required: [doc, impl, unit, int] stages: +doc +impl +unit +int ``` Pre-existing trace failures in `.planning/phases/04-server-rebuild-mvp/*.md` and `.planning/phases/06.4-*/06.4-RESEARCH.md` (placeholder `REQ-SRV-XX` / `REQ-X` IDs) are OUT OF SCOPE — they pre-date this plan and are scope- boundary-excluded per the executor protocol. ## Decisions Made 1. **RC #1 — Math.floor at dispatcher (option a), not schema loosening (option b).** Resolution §Fix #1 in `.planning/debug/two-player-remote-sync-broken.md` explicitly recommends (a). Preserves the u32 binary wire contract per Plan 01 codex concern #7 fold. 2. **RC #2 — `getAxisVector` callback parallel to existing `getFacing` / `getIsRunning`, not consumed-from-prediction.** The dispatcher already takes injected callbacks for facing and running; adding axisVector keeps the architectural symmetry and pushes the input-intent reading to the constructor site (GameScene) rather than coupling the dispatcher to `InputDispatcher`. PredictionEngine stays the source of position (`local.x`, `local.y`) — only velocity authoring moved. 3. **Codex LOW#1 — drop `-1|0|1` casts, no `toAxis()` helper.** Verified `inputDispatcher.axisX()/axisY()` already return `-1 | 0 | 1` (see `apps/client/src/prediction/input-dispatcher.ts:244,252`). The codex suggestion of a `toAxis()` helper was overkill — the simpler noise-removal sufficed. 4. **RC #3 — extend existing `deriveFrameWithAuthoritativeFacing`, not fork a new variant.** Adding an optional 7th `isRunningOverride` arg with `undefined`-default preserves backward-compat for ALL other call sites (none in `apps/client/src/` currently, but defensive) and keeps the function pure. 5. **Codex MEDIUM — extend existing stale-snapshot guard, single chokepoint.** The 3000 ms staleness check at `PlayerRenderer.ts:434-437` already zeroes `renderVx`/`renderVy`. Extending it to also gate `runningOverride` (`staleSnapshot ? false : isRunning`) keeps the staleness policy in one place. Honors Hard Rule 1 — this is a CORRECTNESS safeguard, not anti-cheat (anti-cheat deferred to 06.8). 6. **Task 4 — `.unit.test.ts` not `.integ.test.ts`.** The wire-contract pin is a pure zod-schema unit test (no Colyseus/WebSocket setup); naming it `*.unit.test.ts` runs it in the fast inner loop on every `pnpm -C apps/server test`, providing the strongest defense-in-depth coverage for the RC #1 client-side `Math.floor`. ## Deviations from Plan None — plan executed exactly as written. All four tasks completed in the documented TDD RED/GREEN order. No Rule 1/2/3 auto-fixes; no Rule 4 architectural questions surfaced. The 06.7-UAT.md gap entries are left for `/gsd-verify-work 06.7` to flip after operator UAT. ## Threat Model Outcomes (per plan threat_model) | Threat ID | Outcome | |-----------|---------| | T-06.7-05-01 | mitigated — server `.int()` retained; Task 4 unit test pins it; bounds `.min(0).max(0xffffffff)` unchanged | | T-06.7-05-02 | accepted — wire vx/vy stays client-authored under Hard Rule 1's narrow movement carve-out; 06.8 will add anti-cheat | | T-06.7-05-03 | mitigated — 3000 ms staleness guard now also gates running animation, bounding visual desync damage | | T-06.7-05-04 | accepted — axis-vector callback exposes no new disclosure surface beyond the existing wire payload | | T-06.7-05-05 | mitigated — Task 1 client `Math.floor` + Task 4 server unit test provide two-layer defense | ## Self-Check Verified that all created/modified files exist and all commits resolve: - `apps/client/src/prediction/position-dispatcher.ts` — modified, contains `Math.floor(monotonic_at_ms)` and `getAxisVector` callback in constructor. - `apps/client/src/__test__/position-dispatcher.test.ts` — modified, contains `1234.567` literal + 2 new RC #2 tests. - `apps/client/src/scenes/GameScene.ts` — modified, contains `unpackAnimState` import and `getAxisVector` callback wired to `inputDispatcher.axisX()/axisY()`. - `apps/client/src/render/PlayerRenderer.ts` — modified, `onSimulationTickRemote` takes optional 6th `isRunning?: boolean`; `staleSnapshot` guard extended. - `apps/client/src/render/SpriteStateMachine.ts` — modified, `deriveFrameWithAuthoritativeFacing` takes optional 7th `isRunningOverride?: boolean`. - `apps/client/src/__test__/player-renderer-stale-anim.test.ts` — created (3 tests). - `apps/server/test/protocol-monotonic-int.unit.test.ts` — created (4 tests). Commits in `git log 0a936a2..HEAD`: - `e64d8d4` test(06.7-05): add failing test for fractional monotonic_at_ms (RC #1 RED) - `5a5801d` fix(06.7-05): floor monotonic_at_ms at dispatcher boundary (RC #1) - `825afac` test(06.7-05): pin axis-vector wire vx/vy contract (RC #2 RED) - `d2f70a0` fix(06.7-05): author wire vx/vy from input-intent axis vector (RC #2) - `af49bc9` test(06.7-05): add failing tests for explicit isRunning + stale-anim (RC #3 + codex MEDIUM RED) - `b10d2c1` fix(06.7-05): remote-render consumes broadcast anim_state.running + stale-snapshot suppression (RC #3 + codex MEDIUM) - `239b7e7` test(06.7-05): pin cPositionUpdateSchema monotonic_at_ms integer constraint (codex LOW) ## Self-Check: PASSED All files present; all 7 commits exist on `worktree-agent-a9157828e052743aa`; all named regression tests pass; all referenced libraries / APIs verified against live sources (`packages/protocol/src/intents.ts:135`, `packages/game-logic/src/constants.ts` `RUN_SPEED_PX_PER_TICK`, and the existing `staleSnapshot` guard at `PlayerRenderer.ts:434-437`). ## References - `.planning/debug/two-player-remote-sync-broken.md` — three-RC root-cause analysis driving this plan - `.planning/phases/06.7-network-model-client-trust-fall-trigger/06.7-UAT.md` — operator UAT gap entries (flipped to `resolved` by `/gsd-verify-work 06.7` after operator re-test) - `.planning/phases/06.7-network-model-client-trust-fall-trigger/06.7-REVIEWS.md` — codex review "Plan 05" section (MEDIUM + 2× LOW findings folded above) - `.planning/phases/06.7-network-model-client-trust-fall-trigger/06.7-03-SUMMARY.md` — Plan 03 server stream of client position updates (upstream of this plan)