[codex] Build the live PvP room/session terminal stack#36
Draft
ThunderConch wants to merge 31 commits into
Draft
[codex] Build the live PvP room/session terminal stack#36ThunderConch wants to merge 31 commits into
ThunderConch wants to merge 31 commits into
Conversation
This commit freezes the current PvP documentation set in an isolated worktree so later refinement can be reviewed as a second pass instead of being mixed with the initial architecture capture. Constraint: Source workspace contains unrelated dirty changes, so docs had to be isolated in a dedicated worktree Constraint: The agreed direction is server-authoritative PvP with local-grown party registration and anti-cheat priorities Rejected: Edit directly on the main workspace branch | unrelated local changes would pollute the documentation diff Confidence: high Scope-risk: narrow Reversibility: clean Directive: Treat this commit as the baseline snapshot before structural refinements; keep later docs-only changes reviewable Tested: Markdown inventory check, relative link check, root README reachability check, npm test Not-tested: Human editorial review for wording/style consistency across every section
…concerns The first pass captured the agreed multiplayer PvP design, but the server section still forced readers to jump through several large mixed-purpose documents. This follow-up separates persistence-facing material from runtime contract material, adds local index pages for both buckets, and rewires the top-level reading path so implementation work can enter the docs tree in smaller, purpose-specific steps. Constraint: The documentation had to stay inside docs/pvp and remain fully reachable from the root README Rejected: Keep every server document flat under docs/pvp/server | navigation cost stayed high as contracts and schema detail grew Confidence: high Scope-risk: narrow Reversibility: clean Directive: Future server docs should be filed under storage or contracts instead of returning to a flat server directory Tested: Markdown relative-link scan across docs/pvp, root README reachability traversal (25/25), npm test (1102 pass / 0 fail) Not-tested: Human editorial review for duplicated phrasing across the new index pages
A final verification pass found that the documentation commits still left trailing whitespace in several Markdown files. This follow-up normalizes the formatting without changing meaning so that repo-level cleanliness checks pass and the documentation branch can be handed off without avoidable noise. Constraint: The follow-up had to be content-preserving and limited to docs/pvp formatting cleanup Rejected: Leave whitespace issues in place | verification would continue to report a dirty diff despite the docs being structurally complete Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep running git diff --check on docs-only branches before calling the documentation pass finished Tested: git diff --check, Markdown relative-link scan across docs/pvp, root README reachability traversal (25/25) Not-tested: npm test rerun after whitespace-only edits (no runtime-affecting changes)
The multiplayer rollout needs one authoritative source for generation rules, restricted species, and effective battle level compression before validation, registration, or room flows can be implemented safely. This commit adds the first server-side rules module, locks the initial policy contract in tests, and links the implementation docs to issue-sized execution slices. Constraint: Early PvP work must preserve story-grown party identity while still enforcing server-authoritative battle rules Constraint: ISSUE-01 is intentionally limited to static policy data and pure logic with no HTTP, DB, or WebSocket coupling Rejected: Start with party registration endpoints first | validation semantics would be unstable without a fixed ruleset contract Rejected: Put PvP policy under src/core immediately | online-only authority boundaries are clearer under src/server Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep future registration and battle flows consuming these exported rules contracts instead of re-encoding policy at the edge Tested: node --import tsx --test test/pvp-ruleset.test.ts Tested: npm run typecheck Tested: npm test Tested: git diff --check Not-tested: Runtime server integration because no PvP HTTP/WS surface exists yet Related: docs/pvp/implementation/issues/ISSUE-01-ruleset-foundation.md
The online party validator now normalizes valid six-member parties, reuses the ISSUE-01 ruleset layer for level compression and restricted-species policy, and rejects client-side tampering signals before HTTP integration is added. Constraint: ISSUE-02 owns only src/server/parties/* plus the targeted validator test Constraint: Effective level behavior must stay aligned with committed ISSUE-01 code/tests even where docs examples differ Rejected: Fold validation into an HTTP handler now | ISSUE-03 owns transport concerns Rejected: Add permissive partial-success output | registration must fail closed on anti-cheat mismatches Confidence: high Scope-risk: narrow Directive: Keep party validation pure and reuse rules/source-of-truth helpers before adding request-layer logic Tested: node --import tsx --test test/pvp-party-validator.test.ts Tested: npm run typecheck Not-tested: End-to-end HTTP registration flow (reserved for ISSUE-03)
Added in-memory snapshot persistence, a registration service, and thin HTTP route surfaces so online PvP can replace one active roster per generation, reuse the validated party snapshot shape, and keep snapshot versioning under server control.\n\nConstraint: Initial multiplayer must treat the server as the authority for party registration and battle inputs\nConstraint: Existing validator and ruleset layers must stay reusable without new dependencies\nRejected: Store party snapshots client-side only | cannot prevent forged registrations or version drift\nRejected: Expose full growth proof in route responses | leaks verification payload unnecessarily\nConfidence: high\nScope-risk: moderate\nDirective: Keep later battle/session layers consuming ActivePartySnapshot rather than raw client payloads\nTested: npm run typecheck\nTested: npm test\nTested: node --import tsx --test test/pvp-party-registration-service.test.ts test/pvp-routes.test.ts\nTested: git diff --check\nNot-tested: Real HTTP server wiring and durable database persistence
This adds the room domain and in-memory persistence needed for friendly private PvP rooms. The server now owns room IDs, room codes, host/guest bindings, and the frozen battle-start snapshot that captures both accepted party snapshots plus the active ruleset. The implementation intentionally stops at the domain/service layer. HTTP surfaces, readiness transitions, and realtime presence remain for the next issue so the room contract can stabilize first. Constraint: ISSUE-04 stops before HTTP, presence, and battle command orchestration Constraint: Existing PvP room state is currently in-memory only Rejected: Client-authored room state | would weaken anti-cheat trust boundaries Rejected: Delaying freeze creation until battle start | would blur room/join contract ownership Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep room authority server-owned; do not move join validation back to clients Tested: node --import tsx --test test/pvp-room-service.test.ts Tested: npm run typecheck Tested: npm test Tested: git diff --check Tested: npx tsc --noEmit --pretty false --project tsconfig.json Not-tested: multi-process/shared-database room code allocation Not-tested: HTTP/readiness/presence integration (reserved for ISSUE-05)
This adds the room HTTP surface for create, join, and get flows along with a viewer-specific projection layer for room snapshots. The server now returns participant-scoped room views that preserve the anti-cheat boundary by hiding opponent party snapshot details while still showing readiness and presence state needed for friendly battle staging. The implementation intentionally stops at HTTP plus projection. WebSocket presence, battle start orchestration, and turn handling stay in later issues so room staging can stabilize as an explicit contract. Constraint: ISSUE-05 is limited to HTTP room staging and viewer-specific projection Constraint: No player profile/display-name service exists yet, so projection uses userId as minimal displayName Rejected: Expose opponent party snapshot metadata in room views | violates hidden-information contract Rejected: Fold projection logic into route handlers | would blur truth vs visibility boundaries Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep room projection participant-scoped; do not expose opponent party data through GET room Tested: node --import tsx --test test/pvp-room-routes.test.ts Tested: npm run typecheck Tested: git diff --check Tested: npm test Not-tested: profile-backed display name mapping Not-tested: websocket presence/starting transition integration (reserved for ISSUE-06/07)
This introduces the authoritative PvP battle domain that sits between room readiness and the future realtime gateway. The new battle module validates seat commands, adapts the existing turn engine into a server-owned session model, emits viewer-scoped events, and records replacement / forfeit transitions without trusting the client. The change also exports the battle surface from src/server/index.ts and locks the contract with session-level tests so ISSUE-07 can focus on WebSocket transport rather than battle semantics. Constraint: Initial multiplayer must keep all battle outcomes server-authoritative to limit result forgery Constraint: Opponent bench details and move lists must remain hidden from the other player Rejected: Let the client resolve turns locally and only sync results | opens result forgery and desync risk Rejected: Bundle transport concerns into the battle domain issue | would blur the transport/domain boundary before reconnect work lands Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep transport-layer code as a thin adapter over BattleSessionService; do not duplicate battle state transitions in ws handlers Tested: node --import tsx --test test/pvp-battle-session.test.ts Tested: npm run typecheck Tested: git diff --check Tested: npm test Not-tested: Real WebSocket fan-out and reconnect behavior (deferred to ISSUE-07/08)
The websocket layer now authenticates room seats, starts the authoritative\nbattle session when both players are online, routes battle.command traffic\nthrough the server, and only pushes seat-scoped battle events back to each\nclient. Heartbeat sweeps and duplicate-seat rejection are included so the\ntransport contract matches the PvP design docs and keeps stale clients from\ncontrolling a room.\n\nConstraint: Initial multiplayer must keep all battle resolution on the server\nConstraint: Clients may only send commands and render server-authored events\nRejected: Let clients simulate turns locally and reconcile later | enables result forgery and state divergence\nRejected: Allow multiple simultaneous seat connections | complicates authority and reconnect semantics too early\nConfidence: high\nScope-risk: moderate\nReversibility: clean\nDirective: Preserve seat-scoped event projection when extending reconnect flows\nTested: node --import tsx --test test/pvp-ws-gateway.test.ts\nTested: npm run typecheck\nTested: npm test\nNot-tested: Real network websocket adapter integration outside the in-memory transport harness
ISSUE-08 adds reconnect resume snapshots, timeout-driven auto commands, and an operator debug view around the authoritative battle session so the websocket server can recover clients without trusting local state. The final green step keeps the test-only PvpGeneration imports pointed at the rules surface instead of widening the battle exports. Constraint: Battle outcomes and growth state must remain server-authoritative Constraint: Minimum delivery was scoped to reconnect/timeout/debug-view coverage Rejected: Client-side timeout reconciliation | would weaken cheat resistance Rejected: Re-export PvpGeneration from every server surface | created avoidable type export collisions Confidence: high Scope-risk: moderate Directive: Keep resume payloads and debug views derived from persisted session state only Tested: node --import tsx --test test/pvp-reconnect.test.ts test/pvp-timeout-policy.test.ts Tested: npm run typecheck Tested: npx tsc --noEmit --pretty false --project /home/minsiwon00/claude/.worktrees/docs-pvp-contracts/tsconfig.json
ISSUE-08 already produced internal battle debug snapshots, but operators still had no stable HTTP-facing contract to inspect a live or failed room. This change adds a minimal ops route that reuses the existing BattleDebugView shape and keeps the auth expansion intentionally tiny by adding a boolean operator flag to the HTTP auth context.\n\nThe route follows the same error-envelope style as existing PvP routes and is injected with a room-to-debug getter so it does not pull in a broader server/auth redesign prematurely. Constraint: Keep the auth change minimal and avoid redesigning the wider HTTP auth stack Rejected: Introduce a full role/permission model now | too broad for the bounded ISSUE-08 debug surface Confidence: high Scope-risk: narrow Reversibility: clean Directive: If ops auth expands beyond a boolean flag, preserve this route's error contract unless all callers are updated together Tested: node --import tsx --test test/pvp-ops-routes.test.ts; node --import tsx --test test/pvp-routes.test.ts test/pvp-room-routes.test.ts; npm run typecheck Not-tested: End-to-end HTTP wiring against a real server transport
The original breakdown stopped at reconnect and ops, which made the next executor slices ambiguous once the server-authoritative path was in place. This extends the issue map with explicit client integration contracts so the next work can stay thin, protocol-bound, and ordered after the server contracts stabilize. Constraint: Client-side slices must layer on top of the already-defined server authoritative contracts Rejected: Fold client integration into ISSUE-08 | would blur server stabilization and client read-model work Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep future client issues thin and protocol-bound; real socket/UI wiring should stay on top of these contracts Tested: Manual review of issue ordering, links, and todo alignment Not-tested: Automated markdown link checker not run
…t UI This adds the first client-side read-model slice on top of the server-owned PvP protocol. The session store projects authoritative battle events into a local state shape, while the protocol adapter handles transport envelopes and keeps battle calculation off the client. Keeping this layer pure makes later websocket and Claude Code integration thinner and safer. Constraint: The initial client slice must not depend on a websocket implementation or perform any battle resolution locally Rejected: Wire socket/UI code directly into the first client slice | would mix protocol, state, and presentation concerns too early Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep server battle calculation authoritative; future client work may send commands but must not compute battle outcomes Tested: node --import tsx --test test/pvp-session-store.test.ts test/pvp-client-protocol.test.ts; npm run typecheck; git diff --check; npm test Not-tested: Live websocket integration against a running PvP server
The docs already pinned the server-authoritative battle contract and the first two client layers (session store and protocol adapter), but the next transport-facing step was still implicit. This commit adds ISSUE-11 for the websocket client connector and wires it into the implementation index and TODO flow so the next executor task is unambiguous. Constraint: PvP client transport must stay environment-independent across Claude Code and TUI Rejected: Implement websocket code immediately | client transport boundary should be specified before runtime-specific edits Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep websocket work layered on top of session-store and client-protocol; do not collapse transport and UI concerns Tested: Relative markdown links for changed docs resolved; git diff --check Not-tested: Runtime websocket behavior (docs-only change)
The PvP reducer layer was already pure and testable, but there was no transport adapter that could actually connect a Claude Code client to the realtime battle server. This adds a socket-agnostic websocket client that owns transport state, serializes outbound battle commands, answers ping/pong frames, and feeds inbound envelopes into the existing protocol/session reducers. The public PvP barrel now exports the transport API, and focused tests lock the connector contract before we wire a live server transport. Constraint: Multiplayer battles must remain server-authoritative and accept only client intents over the wire Constraint: The client transport must stay reusable across Claude Code/TUI runtimes instead of binding to browser globals Rejected: Wait for the full server transport stack first | would leave ISSUE-11 blocked behind later integration work Rejected: Depend directly on global WebSocket | would make the connector harder to test and reuse in non-browser environments Confidence: high Scope-risk: narrow Directive: Keep websocket reconnection/transport concerns separate from the pure protocol and session reducers Tested: npm test; npm run typecheck; git diff --check Not-tested: Live websocket integration against a running PvP server
The websocket transport layer was already in place, but higher-level clients still lacked a deterministic way to distinguish manual exits from accidental disconnects and to schedule reconnect attempts without pulling battle/session responsibilities into the transport module. This adds a thin reconnect controller above the websocket client, exports it through the PvP surface, and documents ISSUE-12 as the next client-stability slice. The controller exposes reconnect state for UI consumers while leaving authoritative recovery to server snapshots. Constraint: Reconnect orchestration must remain environment-agnostic for Claude Code and TUI clients Constraint: Manual disconnects must never schedule implicit reconnect attempts Rejected: Folding backoff state into websocket-client | would blur transport and orchestration responsibilities Rejected: Reconnect directly inside session-store | session-store should stay focused on authoritative battle state application Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep reconnect policy separate from snapshot hydration so future UI wiring can evolve without mutating transport semantics Tested: node --import tsx --test test/pvp-websocket-client.test.ts test/pvp-reconnect-controller.test.ts Tested: npm run typecheck Tested: npm test Tested: git diff --check Not-tested: Real server reconnect UX wiring in Claude Code/TUI entrypoints
Upper-layer consumers should not have to wire websocket, reconnect, and protocol state separately. This change introduces a session-level facade that composes those layers into one subscription surface, keeps request lock state derived in one place, and documents the new implementation slice for the PvP roadmap. Constraint: PvP battle resolution remains server-authoritative and must not move into the client facade Rejected: Wire battle-tui directly to websocket/reconnect primitives | would leak transport concerns into every caller Confidence: high Scope-risk: narrow Directive: Keep session-client as orchestration/facade only; do not move battle rules or rendering policy into it Tested: node --import tsx --test test/pvp-session-client.test.ts (4/4 pass) Tested: npm run typecheck Tested: npm test (1180/1180 pass) Tested: git diff --check Not-tested: battle-tui/CLI integration wiring for action-request UX
The session client already centralized authoritative pending-request state, but higher-level surfaces still had to understand raw request payloads. This slice adds a presentation-only adapter that normalizes move, switch, replacement, and forfeit choices into deterministic menu sections with lock/submission UX state. Constraint: Must preserve server-authoritative command shapes without leaking battle resolution into the client adapter Rejected: Let each UI read raw pendingRequest directly | would duplicate ordering, labels, and lock semantics across clients Rejected: Wire full interactive TUI input loop in this slice | too much coupling before turn-result rendering lands Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep action-request-view presentation-only; transport, reconnect, and battle rules stay below this layer Tested: node --import tsx --test test/pvp-action-request-view.test.ts Tested: npm run typecheck Tested: npm test Tested: git diff --check Not-tested: Manual battle TUI integration with live PvP room input
The session-client consumer should be able to build turn-result UX from one adapter surface, so this adds a payload-first read model plus a session-backed summary fallback. Constraint: Session state does not currently retain raw turn_resolved events for reconnect-time replay Rejected: Expand session-store in this slice | outside assigned file ownership and broader than the presentation-only adapter Confidence: high Scope-risk: narrow Directive: Keep this module presentation-only; do not move battle calculation or transport concerns into it Tested: node --import tsx --test test/pvp-turn-result-view.test.ts; npm run typecheck Not-tested: End-to-end TUI/CLI integration rendering against a live session
This adds a presentation adapter that turns existing session-client and\nsession-store state into a deterministic Korean read model for\nsubmitted-command UX. Consumers can now read acceptance, lock, summary,\nand rejection state without reinterpreting pendingCommand and\nrequest.commandSubmitted on their own.\n\nConstraint: This slice must stay adapter-first and avoid transport or store behavior changes\nRejected: Update session-store/session-client contracts directly | outside the owned scope for ISSUE-16\nConfidence: high\nScope-risk: narrow\nDirective: Extend this read model before changing pendingCommand semantics in consumers\nTested: node --import tsx --test test/pvp-command-status-view.test.ts\nTested: npm run typecheck\nTested: lsp diagnostics on modified TypeScript files
Issue 17 adds a session-level read model that bundles transport status, session metadata, action requests, command submission state, and turn results into one deterministic payload for higher-level consumers. The adapter stays outside the store/session-client contracts, so battle-tui and CLI integration can happen in a later slice without forcing those layers to re-interpret protocol state again. Constraint: Keep session-store, session-client, and existing read model contracts stable while adding the new adapter Rejected: Fold the screen view into session-client state | would blur transport/store concerns and broaden public contract changes Rejected: Build ANSI layout strings now | renderer coupling is intentionally deferred to a later slice Confidence: high Scope-risk: narrow Directive: Keep this module a pure data adapter; add renderer/layout concerns in follow-up slices instead of growing presentation logic here Tested: node --import tsx --test test/pvp-session-screen-view.test.ts Tested: node --import tsx --test test/pvp-action-request-view.test.ts test/pvp-command-status-view.test.ts test/pvp-turn-result-view.test.ts test/pvp-session-screen-view.test.ts Tested: npm run typecheck
ISSUE-17 stopped at a session-level read model, so this change adds the first plain-text consumer layer that higher-level clients can print immediately without waiting for battle-tui or Claude Code loop integration. Constraint: Must stay inside src/pvp and avoid battle-tui, WebSocket, stdin, or other I/O coupling Rejected: Wire renderer directly into battle-tui/game-loop | runtime integration belongs to a later issue and would blur the read-model boundary Confidence: high Scope-risk: narrow Directive: Keep this renderer deterministic and pure; add ANSI or live input wiring only in a dedicated integration issue Tested: node --import tsx --test test/pvp-session-screen-renderer.test.ts Tested: node --import tsx --test test/pvp-action-request-view.test.ts test/pvp-command-status-view.test.ts test/pvp-turn-result-view.test.ts test/pvp-session-screen-view.test.ts test/pvp-session-screen-renderer.test.ts Tested: npm run typecheck Tested: git diff --check Not-tested: battle-tui/game-loop integration, Claude Code command loop integration
This adds a small controller layer between the PvP session client and terminal-facing consumers so higher-level entrypoints can render the current plain-text screen, resolve menu tokens back to authoritative battle commands, and submit those tokens through a deterministic result contract without touching stdin, game-loop, or transport lifecycle code. Constraint: Must preserve existing PvP client/view/store contracts Constraint: Must not couple this step to battle-tui or WebSocket lifecycle changes Rejected: Push token parsing into battle-tui directly | would bypass the shared terminal contract Rejected: Add new transport abstractions here | broader scope than ISSUE-19 Confidence: high Scope-risk: narrow Directive: Keep input-token authority anchored to createPvpActionRequestView rather than duplicating token logic elsewhere Tested: session terminal controller tests; related PvP renderer/request/session-client tests; npm test; npm run typecheck Not-tested: live battle-tui stdin loop integration against a real PvP room
The deterministic terminal controller from ISSUE-19 already knew how to turn session state into a screen and accept tokenized commands, but upper layers still needed to manually juggle subscriptions, submit results, and repaint triggers. This adds a small session-terminal runner that owns that orchestration boundary so future CLI/TUI entry points can focus on transport bootstrap and stdin wiring instead of state fan-out. The runner subscribes to the live session client, keeps one current snapshot/screen/token view, emits revisioned updates to listeners, and re-syncs the submit result against the latest post-submit snapshot. The index surface and implementation docs are updated so later slices can build on one explicit contract. Constraint: Initial PvP client work needs a deterministic non-ANSI orchestration layer before raw stdin or websocket UX wiring Rejected: Attach raw stdin/readline directly in this slice | would couple orchestration tests to process IO too early Rejected: Recompute screen logic inside a second runner-specific renderer | controller already defines the deterministic terminal contract Confidence: high Scope-risk: narrow Directive: Keep this runner transport-agnostic; CLI/TUI layers should own connect/join lifecycle and terminal IO policy Tested: git diff --check Tested: npm run typecheck Tested: npm test (1215 pass) Not-tested: Real src/cli or src/battle-tui integration against live room join flow
ISSUE-21 extracts the start/stop/input/output orchestration layer above the session runner so live terminal flows can grow without burying lifecycle semantics inside raw process adapters. The new CLI boundary accepts injected bootstrap hooks, token input, and screen repaint output. That keeps room bootstrap, websocket connection setup, and raw stdin/stdout effects outside this slice while still letting the runner be exercised through deterministic start, repaint, submit, and cleanup tests. Constraint: Room create/join UX and websocket transport bootstrapping stay outside ISSUE-21 Rejected: Wire process stdin/stdout directly into the runner | would make lifecycle cleanup and deterministic tests harder Confidence: high Scope-risk: narrow Directive: Keep raw terminal adapters and room bootstrap on top of session-terminal-cli unless the public lifecycle contract is revised first Tested: node --import tsx --test test/pvp-session-terminal-cli.test.ts Tested: npm run typecheck Tested: git diff --check Tested: npm test (1219/1219 pass)
ISSUE-22 범위로 방 생성/참가 HTTP 클라이언트와 세션 부트스트랩 계층을 추가했다. 클라이언트는 더 이상 전투 상태를 직접 구성하지 않고, 서버가 발급한 room projection과 auth token을 기반으로 세션 연결만 시작한다. 동시에 구현 이슈 문서를 보강해 다음 slice인 stdio adapter 작업과 계약 경계를 명시했다. Constraint: 초기 멀티플레이어는 서버 권위 전투 로직을 유지해야 한다 Constraint: 브라우저 DOM 타입 없이 Node/CLI 환경에서 동작해야 한다 Rejected: 방 응답을 any로 통과시키기 | 클라이언트-서버 계약 검증이 약해짐 Rejected: 부트스트랩 없이 각 호출 지점에서 세션 생성 | room/auth 처리 중복 증가 Confidence: high Scope-risk: moderate Reversibility: clean Directive: 이후 stdin/stdout adapter는 이 부트스트랩 계층 위에서만 세션 연결을 시작하고 room view를 임의 조작하지 말 것 Tested: node --import tsx --test test/pvp-room-http-client.test.ts test/pvp-session-bootstrap.test.ts Tested: npm run typecheck Tested: npm test Tested: git diff --check Not-tested: 실제 서버와의 end-to-end room create/join 네트워크 연동 Related: ISSUE-22 Related: ISSUE-23
Add a stdio adapter that owns tty/readline input handling and a live PvP CLI entrypoint that boots a room session into the existing terminal runner. The library wrapper keeps battle resolution server-authoritative while the client only submits validated action tokens. Constraint: Initial live PvP UX must reuse the existing terminal runner instead of introducing a second UI stack Constraint: CLI smoke path must work via `node --import tsx src/cli/pvp-live.ts --help` Rejected: Hide the bootstrap behind a second bespoke UI layer | would duplicate session rendering and input semantics Rejected: Merge executable CLI parsing into the library wrapper | blurs testable boundaries between reusable runtime and process entrypoint Confidence: high Scope-risk: moderate Directive: Keep terminal abort handling separate from transport abort reasons; extend the stdio adapter instead of bypassing it Tested: Focused stdio/live CLI tests, CLI help smoke test, typecheck, full npm test Not-tested: Real websocket handshake against a live PvP server
eulneul
reviewed
Apr 12, 2026
Contributor
eulneul
left a comment
eulneul
left a comment
There was a problem hiding this comment.
PR #36 코드 리뷰: Live PvP Room/Session Terminal Stack
개요
룸/파티 관리(HTTP), 배틀 세션 도메인, WebSocket 게이트웨이, 재연결 클라이언트, 터미널 UI 모델, pvp-live CLI 진입점까지 서버 권위 PvP 스택 전체를 연결하는 대규모 그린필드 슬라이스입니다. PR이 draft로 표시된 건 맞는 판단 — 출시 가능한 기능이 아니라 광범위한 아키텍처 기반입니다. 모든 모듈에 소스 파일과 1:1로 테스트가 존재해 커버리지가 매우 인상적입니다.
아키텍처
잘한 점:
타입 → 도메인 서비스 → 전송 어댑터 → 컨트롤러 → CLI레이어링이 깔끔함- 배틀 해석, 턴 진행, 타임아웃 처리 모두 서버에서 이루어지는 서버 권위 구조 일관성 있게 유지됨
structuredClone기반 불변성(immutability) 패턴이 코드베이스 전체에 걸쳐 일관적으로 적용됨- 재연결 컨트롤러의 지수 백오프(
base=1s, max=30s, multiplier=2)가 잘 파라미터화되어 있고 전략 주입도 가능한 구조 PvpSessionBootstrap의TSessionClient제네릭이 테스트용 주입 포인트로 훌륭한 설계
우려되는 점:
PvpWsServer의sessionsByRoomId가 인메모리Map으로만 관리됨. 서버 재시작 시 진행 중인 모든 배틀 세션이 소실됨. draft 범위로 인식된 내용이지만, 실제 배포 전 반드시 해결해야 할 핵심 미결 과제로 추적이 필요함.
구체적인 이슈
1. seenClientCommandIds가 O(n) 탐색을 사용하는 무한 성장 배열 (src/server/battle/battle-command-service.ts)
// 현재 — 커맨드당 O(n), 배틀 내내 계속 증가
if (session.seenClientCommandIds.includes(clientCommandId)) { ... }
session.seenClientCommandIds.push(envelope.payload.clientCommandId);이 배열은 배틀 전체 수명 동안 수락된 커맨드마다 하나씩 쌓이고, .includes()로 중복 체크를 합니다. 타임아웃 자동 커맨드가 많이 쌓이는 긴 배틀에선 무시할 수 없는 성능 문제가 됩니다. Set<string>으로 교체를 권장합니다:
// 탐색
if (session.seenClientCommandIds.has(clientCommandId)) { ... }
session.seenClientCommandIds.add(...)structuredClone은 Set을 직렬화하지 않으므로, 외부엔 string[]로 저장하고 런타임엔 Set으로 변환하거나, 배열 크기가 배틀 턴 수 상한으로 자연스럽게 제한된다는 점을 명시적으로 주석으로 남겨두는 것도 괜찮습니다.
2. sendCurrentSnapshot이 파라미터로 받은 session을 직접 변이(mutate) (src/server/ws/pvp-ws-server.ts)
private sendCurrentSnapshot(session: BattleSessionRecord, ...): void {
session.nextSeq += 1; // ← 파라미터 직접 변이
session.updatedAt = sentAt; // ← 파라미터 직접 변이
session.eventLog.push(...) // ← 파라미터 직접 변이
this.sessionsByRoomId.set(session.roomId, cloneSession(session));
transport.send(snapshot); // ← 상태가 저장된 이후에 전송
}코드베이스 전체에서 유일하게 세션 레코드를 직접 변이하는 지점입니다. transport.send()에서 예외가 발생하면 seq 번호는 이미 증가된 상태로 저장되어 있습니다. 변이 전에 클론하거나, 다음 상태값을 먼저 계산하고 send 성공 후 원자적으로 저장하는 구조로 바꾸는 걸 권장합니다. 이 불일관성은 persistMutationResult를 리팩토링하는 사람에게 함정이 될 수 있습니다.
3. CLI 인자에 auth token 노출 (src/cli/pvp-live.ts)
--auth-token이 shell 히스토리와 /proc/<pid>/cmdline에 남습니다. Claude Code 터미널 툴이라 공개 CLI보다 위험도는 낮지만, 도움말 텍스트에 한 줄 추가할 가치는 있습니다:
--auth-token 인증 토큰 (shell 히스토리 노출 방지를 위해 PVP_AUTH_TOKEN 환경변수 사용 권장)
4. MAX_ROOM_CODE_ATTEMPTS = 32 초과 시 실패가 명시적으로 드러나지 않음 (src/server/rooms/room-service.ts)
32번 모두 룸 코드 충돌이 발생하는 경우(확률은 낮지만), 서비스가 PVP_ROOM_CODE_EXHAUSTED 같은 명시적 에러를 던져야 하며 호출부에서 조용히 넘어가선 안 됩니다.
5. stableStringify 커스텀 구현 (src/server/rooms/room-service.ts)
구현 자체는 올바르지만 이미 해결된 문제입니다. 룸 룰셋 해시 목적으로만 쓰인다면, 표준 JSON 직렬화를 쓰지 않은 이유를 주석으로 남기거나, 안정적인 직렬화가 진짜 필요하다면 커스텀 구현보다 fast-json-stable-stringify 같은 라이브러리를 쓰는 게 낫습니다.
테스트 커버리지
훌륭합니다. 모든 소스 모듈에 대응하는 테스트 파일이 있고, 에러 경로, 재연결 시퀀스, 엣지 케이스(중복 커맨드, 타임아웃 포피트 등)까지 커버하는 것으로 보입니다. node --import tsx --test 패턴도 깔끔합니다.
총평
| 항목 | 상태 |
|---|---|
| 아키텍처 & 레이어링 | 탄탄함 |
| 불변성(immutability) 규율 | 일관적 (예외 1건: 이슈 #2) |
| 커맨드 중복 탐지 | 정확하지만 O(n) — Set 전환 권장 |
| 재연결 / 백오프 | 잘 설계됨 |
| 테스트 커버리지 | 우수 |
| 인증 토큰 처리 | 소폭 보완 필요 |
| 인메모리 전용 상태 | 알려진 미결 과제, 통합 전 필수 해결 |
| 문서 | 23개 이슈 파일 + 컨트랙트 + 스키마, 매우 충실함 |
draft → review 전환 전에 반드시 수정할 사항은 **sendCurrentSnapshot 변이 불일관성(이슈 #2)**과 seenClientCommandIds 배열(이슈 #1) 두 가지입니다. 나머지는 개선 사항이거나 이후 범위로 미뤄도 됩니다.
- sendCurrentSnapshot: clone session before mutation so seq/updatedAt are not modified in-place before the transport send; consistent with the immutable pattern used everywhere else in PvpWsServer - battle-command-service: document why seenClientCommandIds linear scan is acceptable (bounded by turns × seats, cleared on battle end) - pvp-live CLI: fall back to PVP_AUTH_TOKEN / PVP_SERVER_URL env vars so tokens are not required in shell args (and exposed in history) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
)" This reverts commit 25543ac.
Revert "fix: address code review feedback on PvP live session stack"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch turns the agreed PvP design into an executable server-authoritative live battle stack for terminal clients.
Concretely, it:
pvp-liveCLI entrypoint needed to drive battles from Claude CodeWhy
The goal is to make multiplayer PvP feel like in-game trainer battles while still keeping the server authoritative over battle resolution and room/session state. That gives us a foundation for cheat resistance, reconnect recovery, and a Claude Code-driven live terminal UX.
User / developer impact
Validation
node --import tsx --test test/pvp-session-terminal-stdio.test.ts test/pvp-live-cli.test.tsnode --import tsx src/cli/pvp-live.ts --helpnpm run typechecknpm testgit diff --checkNotes
This PR is opened as a draft because the branch is a broad PvP execution slice and the next step should be integration-level validation against a live server environment.