Add /tkm:friendly-battle leave + peer disconnect eventing (PR46)#46
Merged
Conversation
This imports the new friendly-battle planning/docs tree into the master-based implementation branch and links it from the docs home so future work can execute against one canonical direction. The legacy PvP docs are carried alongside it as a reference track because the new docs link back to them and maintainers still need the older contracts for historical context. Constraint: The serverless-friendly-battle line must proceed from master without depending on the legacy PvP PR stack Constraint: Docs home must distinguish current source-of-truth guidance from legacy reference material Rejected: Import only docs/friendly-battle | would break cross-links to the legacy PvP reference set Rejected: Rewrite or delete legacy PvP docs in PR1 | too broad for the direction-lock slice Confidence: high Scope-risk: narrow Directive: Treat docs/friendly-battle as the canonical direction for this implementation line and docs/pvp as legacy/reference only Tested: npm run build && npm test; relative markdown link resolution check Not-tested: automated markdown lint or external docs-site rendering
PR1 review showed two ambiguity sources remained after the initial docs axis landed: the legacy PvP index still read like the active implementation contract, and the transport gate lacked a canonical verdict artifact plus measurable pass/fail criteria. This follow-up tightens the handoff. The legacy PvP index now explicitly warns readers that friendly-battle docs are the current source of truth, and the transport gate now defines mandatory artifacts, command-count expectations, failure-UX requirements, and a canonical report path for PR2 decisions. Constraint: PR2 needs an unambiguous kill-or-commit contract before transport spike work continues Rejected: Leave reviewer guidance in comments only | the next executor could still branch from the wrong docs axis Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep docs/pvp positioned as historical reference unless the product explicitly returns to the server-authoritative track Tested: npm test; relative-link check across updated friendly-battle/pvp docs Not-tested: Human review of wording outside the reviewed source-of-truth handoff
The transport feasibility gate required a report file, but the report template could not capture two of the failure-UX proofs called out by review: the most likely bad input and an actionable retry command. The gate doc also did not say clearly enough that a placeholder or TBD-filled report still fails the gate. This tightens the docs contract so PR2 cannot satisfy the gate with an empty shell and so the canonical report structure matches the validation criteria. Constraint: PR2 feasibility must be reviewable from repo artifacts alone Rejected: Rely on reviewer memory for missing failure UX fields | too easy to miss during later PRs Confidence: high Scope-risk: narrow Directive: Keep gate requirements and report template fields in lockstep whenever validation criteria change Tested: npm test Tested: git diff --check Tested: targeted docs-only review against working tree diff Not-tested: none
…ly battle stack This spike keeps the product surface provisional while validating that same-machine host/join/ready/start/action exchange works inside the Claude Code plugin environment. It also hardens the failure UX enough to decide whether the serverless-first direction remains viable before session and battle contracts are formalized. Constraint: PR2 is a transport spike, not the final friendly battle CLI Rejected: Build session and battle abstractions first | transport feasibility had to be de-risked earlier Confidence: high Scope-risk: narrow Directive: Keep the spike transport isolated from product UX and absorb only its proven contracts into later PRs Tested: transport spike tests, CLI spike tests, typecheck, build, git diff --check, subagent code review Not-tested: cross-machine LAN, NAT/firewall behavior
PR3 fixes the friendly-battle contract surface before the battle adapter lands. It establishes explicit session/progression/snapshot/battle shapes, removes accidental coupling to the current core turn-action types, and reserves a dedicated storage namespace so later PRs can wire battle runtime and CLI flows onto the same vocabulary. Constraint: Progression, snapshot, session, and battle state must remain separate source-of-truth layers Constraint: Friendly battle storage must not collide with legacy singleton battle/session files Rejected: Reuse core TurnAction directly | would couple the session contract to current battle-engine internals Rejected: Keep snapshot metadata on participant records | duplicates source of truth and muddies session boundaries Confidence: high Scope-risk: narrow Directive: Keep downstream PR4+ adapters translating between friendly-battle contracts and core battle types instead of importing core runtime types into the contract layer Tested: node --import tsx --test test/friendly-battle-contracts.test.ts; node --import tsx --test test/friendly-battle-paths.test.ts; npm run typecheck; npm run build; git diff --check; subagent review APPROVED Not-tested: Live transport/session orchestration and progression-to-snapshot conversion flows
This wires the new friendly-battle session layer into the existing turn battle engine with a minimal adapter, while keeping the battle engine as the source of truth for switch resolution and turn settlement. The adapter now waits for both sides, validates normal-turn submissions, emits session-facing battle events, and preserves the legitimate Struggle fallback instead of masking invalid client input. A small session-generation invariant is also locked in so later snapshot work cannot silently bridge mismatched generations. Constraint: PR4 must stay transport-agnostic and prove session-facing battle flow in isolation Rejected: Re-implement switch logic inside friendly-battle adapter | would fork core battle semantics too early Rejected: Accept invalid move selections and rely on downstream fallback | would hide protocol/client bugs Confidence: high Scope-risk: moderate Directive: Keep friendly-battle choice validation aligned with core engine behavior before adding transport or CLI layers Tested: node --import tsx --test test/friendly-battle-contracts.test.ts test/friendly-battle-battle-adapter.test.ts test/turn-battle.test.ts; npm run typecheck; npm run build; git diff --check Not-tested: End-to-end host/join transport integration
친선전 시작 시 progression을 직접 battle layer에 넘기지 않도록 snapshot 계약과 검증 로직을 추가했다. 파티 순서, 레벨, 닉네임, 기술 구성을 battle-ready 형태로 동결하고, generation hook을 통해 후속 PR에서 세대별 규칙을 확장할 수 있게 했다. Constraint: 친선전은 로컬 progression 감성을 유지해야 한다 Constraint: PR6/PR7에서 재사용 가능한 순수 snapshot 경계가 필요하다 Rejected: session state에 full progression 참조를 그대로 보관 | battle/session 경계가 흐려짐 Rejected: learned/fallback move만 재계산 | 로컬 성장 상태와 현재 세팅 보존 요구와 충돌 Confidence: high Scope-risk: moderate Directive: snapshot은 battle runtime과 메모리 참조를 공유하지 않도록 clone 경계를 유지할 것 Tested: node --import tsx --test test/friendly-battle-snapshot.test.ts Tested: node --import tsx --test test/friendly-battle-*.test.ts Tested: npm run typecheck Tested: npm run build Tested: git diff --check Tested: LSP diagnostics on changed files (0 errors) Tested: subagent code review 승인(Schrodinger) Not-tested: 실제 host/join 세션에 snapshot을 연결하는 통합 경로는 PR6에서 검증 예정
PR6 local harness now reuses the canonical state hydration path for external profiles, reads battle teams back from persisted snapshot files, and hardens the host/join CLI boundary with explicit numeric validation. This keeps same-machine friendly battles aligned with the snapshot artifacts we actually persist instead of drifting with later in-memory mutations. The follow-up test pass also locks the cleanup policy on both success and failed host handshakes so the harness stays predictable while the serverless-friendly architecture evolves. Constraint: PR6 must stay serverless-friendly and rely only on local artifact exchange plus the existing CLI surface Rejected: Rebuild battle teams from in-memory profile objects after artifact creation | allows stale mutations to bypass persisted snapshot authority Rejected: Duplicate partial state-default merging inside the local harness | risks drift from the canonical loader and migration path Confidence: high Scope-risk: moderate Directive: Any future external profile loader should call hydrateState rather than re-implementing default merges or migrations Tested: npm run typecheck Tested: node --import tsx --test test/state.test.ts test/friendly-battle-local-harness.test.ts Tested: node --import tsx --test test/friendly-battle-battle-adapter.test.ts test/friendly-battle-contracts.test.ts test/friendly-battle-local-harness.test.ts test/friendly-battle-paths.test.ts test/friendly-battle-snapshot.test.ts test/friendly-battle-spike-cli.test.ts test/friendly-battle-transport-spike.test.ts Not-tested: Cross-machine/manual operator run outside the same workstation shell
The local harness already supported same-machine battles, but users still had to invoke an internal script directly. This change adds a product-facing tokenmon friendly-battle surface, keeps the existing local harness reusable, and makes the host-generated JOIN_COMMAND re-enter through the public CLI. Constraint: PR7 stays serverless/local-only and must avoid repo-wide recovery scans after the compact loop Constraint: The friendly-battle wrapper must stay a thin facade over the local harness Rejected: Rewriting the local harness into a broader transport/session layer in this slice | too broad for the current PR boundary Confidence: high Scope-risk: moderate Directive: Keep src/cli/friendly-battle.ts facade-only; push future session/runtime complexity down into transport/runtime layers Tested: npm test; npm run build; node --import tsx --test test/friendly-battle-cli.test.ts test/friendly-battle-local-harness.test.ts; git diff --check; LSP diagnostics on affected files; local help/alias-conflict quick checks Not-tested: Manual multi-terminal run from two live Claude profiles after this wrapper change
The repository had no GitHub Actions workflow, which left PRs without check-runs or commit statuses even when local verification passed. This adds a minimal Node 22 CI pipeline that typechecks, runs tests, and builds so future PRs surface machine-verifiable evidence directly in GitHub. Constraint: Repository currently targets Node >=22.0.0 and uses npm lockfile installs Constraint: Keep CI minimal and aligned with existing package scripts only Rejected: Add lint or matrix jobs now | repo does not define a lint script and extra jobs would add noise before baseline CI exists Rejected: Add only test step | would miss type/build regressions already part of local verification expectations Confidence: high Scope-risk: narrow Directive: Keep CI script list in sync with package.json; do not add non-existent repo scripts just for convention Tested: npm run typecheck; npm test; npm run build; git diff --check; subagent code review Not-tested: Remote GitHub Actions execution after push
The local friendly-battle tests were assuming zsh and prebuilt dist hook artifacts, while audio playback could still surface async ENOENT failures on CI hosts that do not ship desktop media players. This change switches shell-based smoke tests to the platform default shell, runs the session-start hook from source during tests, hardens detached audio spawns, and gives the same-machine battle smoke a little more room under full-suite load. Constraint: GitHub Actions runners may not have zsh or desktop audio players Constraint: npm test runs before npm run build in CI Rejected: Install zsh and audio players in CI | would hide portability bugs in the repo Rejected: Keep the 4s local battle timeout | remained flaky under full-suite load Confidence: high Scope-risk: narrow Directive: Keep integration tests shell-agnostic and avoid dist dependencies in prebuild test paths Tested: node --import tsx --test test/friendly-battle-local-harness.test.ts Tested: node --import tsx --test test/friendly-battle-spike-cli.test.ts Tested: node --import tsx --test test/session-start.test.ts Tested: node --import tsx --test test/sfx.test.ts Tested: npm test Tested: npm run typecheck Tested: npm run build Not-tested: GitHub Actions rerun on the pushed commit
Friendly battle local v1 started as a same-machine spike, but the current product goal is same-network instant battles between two Claude profiles. This change separates the host bind address from the guest-facing join address, threads that contract through the CLI, and reshapes the smoke tests so they keep exercising the printed command flow without relying on two heavyweight shell children racing each other. The result is that hosts can listen on wildcard interfaces while still printing a concrete join command for the guest, and the surrounding tests better reflect the supported same-network flow. Constraint: This feature must stay inside the existing Claude Code plugin repo without adding a separate always-on server component Constraint: Friendly battle v1 still optimizes for ad-hoc matches, not cheat resistance, matchmaking, or reconnect recovery Rejected: Keep a single --host flag for both bind and guest join address | breaks same-network use when the host listens on 0.0.0.0 Rejected: Keep two spawned shell children in spike smoke tests | too slow and flaky on headless CI runners Confidence: high Scope-risk: moderate Directive: Do not collapse listen host and guest-facing join host back together unless same-network host UX is re-specified end-to-end Tested: npm test Tested: npm run typecheck Tested: npm run build Tested: Subagent code review + focused friendly-battle regression review Not-tested: Real two-machine manual LAN run outside the automated harness
The single-owner loop was spending too much time compacting context instead of advancing the remaining two-machine product flow. This checkpoint captures the passing transport, snapshot, and CLI groundwork so OMX team workers can branch from a clean base and attack the next bounded gaps in parallel. Constraint: omx team requires a clean leader workspace before it can create worker worktrees Rejected: stash the worktree | would hide the current passing base from team workers Confidence: high Scope-risk: narrow Directive: Treat this commit as the team launch pad; follow-up work should target authoritative repeated-turn flow and Claude Code-friendly UX only Tested: node --import tsx --test test/friendly-battle-transport-spike.test.ts test/friendly-battle-local-harness.test.ts test/friendly-battle-spike-cli.test.ts test/friendly-battle-cli.test.ts test/friendly-battle-snapshot.test.ts test/friendly-battle-battle-adapter.test.ts; npm run typecheck Not-tested: Cross-machine manual run across two physical hosts
…endly-battle-remote-snapshot-handshake
…endly-battle-remote-snapshot-handshake
This switches the spike/local friendly-battle smoke flow from a one-off action exchange to the authoritative battle-event loop we actually need for remote play, and it tightens the guest join path so user-provided timeout-ms is respected instead of leaking through socket shutdown. Constraint: Friendly battles must stay in-repo and Claude Code friendly without introducing a separate authoritative server stack yet Constraint: Remote same-network play still needs deterministic CLI/test coverage before higher-level product UX work Rejected: Leave the first-action smoke in place | no longer matched the authoritative event contract used by the adapter Rejected: Depend on socket end/close after timeout | leaked well past timeout-ms in join failure scenarios Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep transport/API tests aligned with the authoritative battle_event and submit_choice flow; do not reintroduce action-exchange shims Tested: npm run typecheck; node --import tsx --test test/friendly-battle-battle-adapter.test.ts test/friendly-battle-cli.test.ts test/friendly-battle-local-harness.test.ts test/friendly-battle-spike-cli.test.ts test/friendly-battle-transport-spike.test.ts Not-tested: Real two-machine same-network manual smoke outside the local test harness
…rdening This records what the current remote snapshot handshake branch already ships versus what is still missing before we can call the same-network friendly battle flow a complete product experience. The new roadmap note keeps the host/join surface, session foundation, and transport hardening separate from the still-missing turn choice UX so the next PR can stay focused. Constraint: Keep this PR docs-only and describe the remaining gap in markdown Rejected: Keep extending code in this branch | would blur the transport-hardening checkpoint Confidence: high Scope-risk: narrow Directive: Update the gap note whenever product-facing friendly-battle UX lags behind transport/session groundwork Tested: git diff --check; node --import tsx --test test/friendly-battle-cli.test.ts Not-tested: cross-machine manual LAN smoke for this docs-only commit
Avoids touching tcp-direct.ts by routing voluntary leave through the
existing EOF-driven shutdown: the leaving daemon pushes a synthetic
battle_finished{reason:'cancelled'} locally, closes the transport,
acks the IPC leave request, and shuts down. The peer's daemon catches
the transport EOF in its turn loop and pushes a synthetic
battle_finished{reason:'disconnect'} into its own local queue so the
peer skill's next wait_next_event returns a clean envelope instead of
timing out.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add { op: 'leave' } to DaemonRequest union and extend the protocol
round-trip test to cover the new variant.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…venting
- daemon.ts: add 'leave' IPC handler — marks session aborted, closes TCP
transport (so peer gets EOF), returns ack, then calls shutdown(0,'finished')
via setImmediate so the response flushes first
- daemon.ts: add transportClose ref so ipcHandler can tear down TCP on leave
- daemon.ts: both host+guest catch blocks now push a synthetic
battle_finished{winner:null,reason:'disconnect'} event before shutdown so
any pending wait_next_event on the leaving side resolves cleanly
- friendly-battle-turn.ts: add --leave subcommand (runLeave) — reads session
record, skips to frozen envelope if daemon already gone, otherwise sends
{op:'leave'} via IPC and prints the ack envelope
- SKILL.md: add Step 9 (leave flow), update Step 0 dispatch, Step 2 turn loop
aborted-phase handling, Step 8 help text, and usage table
- test/friendly-battle-daemon-leave.test.ts: new end-to-end integration test
(1 test case, 1198 total)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Q4 fix (major): the daemon's battle_finished envelope now emits a distinct questionContext for voluntary leave (cancelled), peer disconnect (disconnect), and normal win/loss, and eventStatus maps cancelled and disconnect to 'aborted' instead of victory/defeat. The skill's Step 2 aborted branch can now key on questionContext to distinguish "You left the battle." from "Opponent left the battle." without sniffing stderr or the reason field. Q5 fix (minor): the leave test's afterEach now does SIGTERM → 500ms poll → SIGKILL → 200ms wait instead of naked SIGTERM, mirroring PR44's killAllDaemons pattern. Closes the tmp-dir / socket teardown race that the naked SIGTERM left open on slow CI runners. Full suite: 1198/1198 pass after the fixes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Friendly step-by-step walkthrough so anyone can reproduce the PR44–PR46 visual QA on a single machine without needing two physical hosts. Covers: - What each PR in the stack (#40 → #46) adds and what to eyeball - How to sync the tkm plugin install to feat/friendly-battle-pvp-leave via a plain git checkout when the plugin clone is ThunderConch/tkm - How to use an isolated CLAUDE_CONFIG_DIR (~/.claude-fb-guest) for the second Claude Code session so the two terminals don't race over the same tokenmon state - Per-PR screenshot checklist (basic turn loop, forced switch, surrender confirm, leave / opponent-left) - Rollback instructions (git checkout master + ~/.claude-fb-guest cleanup) - Troubleshooting the 6 most common stuck spots Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The PR44 reviewer fixup added a 5-second idle timeout on the daemon's UNIX socket to guard against DoS-by-stalled-clients. The guard was applied to the whole socket lifetime, which also covered the handler's execution window — so wait_next_event long polls (default 60_000ms) were being killed at the 5s mark, dropping the response and breaking the real /tkm:friendly-battle open flow as soon as the host started polling for a guest. Fix: keep the 5s idle timeout for the pre-request phase (still catches a client that connects and never writes), but clear it via socket.setTimeout(0) as soon as the request line is parsed. Long handlers can now legitimately block for the full timeoutMs the client requested without being reaped. Two regression tests added: - handler that blocks 6s still delivers its response (would have caught this bug before merging the reviewer fixup) - client that connects without writing is still destroyed in ~5s (proves the DoS guard still protects the pre-request phase) Full suite: 1200/1200 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two bugs visual QA surfaced on the PR46 stack:
1. Daemon emitted moveOptions / partyOptions with 0-based indexes from
Array#map, while SKILL.md and the CLI action parser expect 1-based
indexes (gym contract). The skill would render "0. 파동탄" and dispatch
--action move:0, which runAction's /^move:([1-4])$/ regex rejected
as "unknown action token", leaving the daemon stuck mid-turn.
Fix: daemon now emits index + 1 for both moveOptions and partyOptions.
The CLI still subtracts 1 before forwarding to the battle adapter, so
the wire format (0-based) is unchanged.
2. questionContext only had the turn headline, so the AskUserQuestion
prompt showed no battle state — users couldn't see opponent HP or
their own current HP without squinting at the status line.
Fix: new buildBattleContext helper renders a gym-style line beneath
the headline:
⚔️ 상대 <name> Lv.L HP:cur/max | 내 <name> Lv.L HP:cur/max
On the host side the battle-adapter runtime has both teams so both
sides render. On the guest side there is no runtime yet so we only
render the local pokemon from ownSnapshot with a "(상대 HP는 다음
턴 결과에서 확인)" hint.
Full suite: 1200/1200 pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Visual QA on the live PR46 stack hit a daemon death with no observable stack trace. The host CLI destroys child.stdout/stderr right after DAEMON_READY (so the parent can exit), which means any subsequent process.stderr.write or uncaughtException in the daemon was silently discarded — making post-mortem impossible. Fix: open a per-session append-mode log file at $CLAUDE_CONFIG_DIR/tokenmon/<gen>/friendly-battle/sessions/<id>.crash.log during runDaemon's startup. Mirror process.stderr.write into the file, and install uncaughtException + unhandledRejection handlers that flush the stack to disk before exit. Logging is best-effort and never crashes the daemon itself (every fs call is wrapped). Next time a daemon dies during /tkm:friendly-battle the operator can just `cat` the crash log to see the actual error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
30-minute turn-loop timeout instead of the init handshake timeout Crash logs from the live PR46 visual QA showed two real bugs that the 612d31c crash-log feature actually surfaced: 1. The stderr.write wrapper still forwarded to the original stream. The parent CLI destroys child.stderr right after DAEMON_READY, so any subsequent process.stderr.write triggered an async 'error' event on the dead pipe → uncaughtException → daemon dead. This made the crash-log feature actively worse: every legitimate "daemon host error: …" stderr write killed the daemon mid-catch before its shutdown path could update the session record. Fix: the wrapper now ONLY writes to the crash log file and never touches the original stream. 2. The host/guest turn loops used the daemon's options.timeoutMs (the init handshake budget, ~30s in real use) for both localActionQueue shifts and TCP waitForGuestChoice / waitForBattleEvent calls. So a user thinking about a move for >30s had their daemon time out and die mid-battle. Fix: introduce TURN_LOOP_TIMEOUT_MS = 30 minutes, used for all four in-loop wait sites. Tests can override it via the TKM_FB_TURN_TIMEOUT_MS env var so leave/disconnect specs still resolve in seconds. Init transport waits (waitForGuestJoin, waitUntilCanStart, eager event drain right after waitForStarted) keep using the CLI's --timeout-ms so the handshake still aborts quickly when nobody joins. Full suite: 1200/1200 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
showing the forced-switch menu to spectator daemons Visual QA on a real two-terminal battle hit three real bugs that shared a root cause: the guest daemon was making display decisions locally instead of trusting the host's battle-adapter runtime. 1. Guest's HP showed "100/100" forever because buildBattleContext was reading the species base stat from the frozen snapshot instead of the level-scaled current/max HP. 2. Guest's move PP never decreased after using a move because nothing was decrementing the snapshot. 3. When one Dialga fainted, BOTH terminals popped the forced-switch menu because daemon eventStatus mapped phase=awaiting_fainted_switch to status=fainted_switch without checking whether the local role was actually in waitingFor. Architecture fix: host is now the single source of truth. - contracts.ts: FriendlyBattleChoicesRequestedEvent gains an optional liveState field carrying both teams' active pokemon (name, level, hp, maxHp, fainted, moves with currentPp/maxPp/disabled) plus the full party (name, level, hp, maxHp, fainted). - battle-adapter.ts: new buildFriendlyBattleLiveBattleState helper + populates liveState in all three choices_requested emit sites (initial battle setup, normal turn-end, fainted_switch). - daemon.ts: new buildEnvelopeFieldsFromLiveState helper renders envelope display fields straight from the host-authored liveState. eventToEnvelopeFields prefers liveState when present and falls back to the legacy runtime/snapshot path only for older synthetic test events. eventStatus now keys fainted_switch / select_action on whether the local role is in event.waitingFor — spectator side gets 'ongoing' so the skill loops back to wait_next_event instead of prompting for a duplicate switch. Test updates: - friendly-battle-battle-adapter.test.ts: stripLiveState helper so the structural deepEqual assertions stay focused on the wire-shape contract; liveState payload is exercised separately via daemon integration tests. - friendly-battle-daemon-fainted-switch.test.ts: drainUntilFaintedSwitch now resolves to false on inner timeout (the spectator side never reaches fainted_switch under the new role-aware behavior). - friendly-battle-daemon-ipc.test.ts: idle-guard regression test's lower-bound elapsed window relaxed from 4500ms to 4000ms so node:test scheduler jitter under parallel CPU load doesn't flake. Full suite: 1200/1200 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codex adversarial review flagged that the host turn loop was pushing resolved events to localEventQueue BEFORE calling host_transport.sendBattleEvents(). The window meant the host skill's wait_next_event could surface the next choices_requested and prompt the user for a fresh action while the guest still hadn't received the previous turn over TCP. The guest's stale in-flight choice would then hit the battle-adapter's "not waiting for X" reject path. Swapping the order makes the guest no later than the host on event arrival — TCP send commits to the kernel buffer before the local push, so loopback delivery is effectively atomic with the local push. Q2 (spectator wait_next_event blocking) verdict was PASS — spectator unblocks via the next host-pushed event, peer-disconnect synthetic event, or the 30-min queue timeout. Q3 (liveState host/guest mapping) verdict was PASS — internally consistent player→host / opponent→guest across battle-adapter, daemon, contracts. Full suite: 1200/1200 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two real bugs found during live visual QA:
1. Guest daemon's outer loop awaited localActionQueue.shift FIRST and
only then pumped TCP events. So when the host did anything that
didn't require guest input (forced switch, etc.), the host's TCP
events sat in the buffer indefinitely while guest waited for a
local action that would never come. Symptom: "host 쪽 포켓몬
교체하면 바로 guest 쪽으로 신호가 안감, guest 는 계속 대기만 함".
Fix: split the guest path into two independent coroutines:
- tcpPump continuously drains incoming battle events into the
local event queue regardless of action timing
- actionPump continuously pulls local actions and submits them
via TCP regardless of incoming events
They share a battleFinished flag so actionPump exits cleanly via
localActionQueue.fail() when tcpPump sees battle_finished.
2. liveState was carrying the host's pre-resolved name strings
(displayName, nameKo) so guests rendered HOST locale names even
for their own pokemon and moves. Fix:
- contracts.ts now also carries moveId on each move entry and
pokemonId on the active pokemon + each party entry
- battle-adapter.ts populates the IDs from BattlePokemon.id and
BattleMove.data.id
- daemon.ts new localizeMoveName / localizePokemonName helpers
consult getLoadedMovesDB / getPokemonDisplayName + getLocale to
render in the LOCAL client's language; legacy wire name is
preserved as a fallback when ID lookup misses
Full suite: 1200/1200 pass.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er model bin/run-friendly-battle-turn.sh prefers dist/cli/friendly-battle-turn.js when present (saves ~700ms tsx cold start per per-action call) and falls back to tsx on src for dev iteration. SKILL.md now invokes the helper instead of tsx-resolve directly, and the open/join flow tells the user they can drop to /model haiku or sonnet during the battle since turn dispatch is mostly mechanical. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
H1 (HIGH): Guest tcpPump now routes battle_finished through eventStatus() so peer disconnect / voluntary leave (winner=null, reason='disconnect'| 'cancelled') maps to 'aborted' instead of being silently downgraded to 'defeat'. The leave integration test is tightened to require status== 'aborted' on the non-leaving side. H2 (HIGH): shutdown(exitCode!=0) now arms queueClosedEnvelope BEFORE failing localEventQueue, so a wait_next_event caller already blocked in shift() falls through to the sentinel path in the IPC catch branch and receives an aborted envelope instead of a generic 'handler_error'. M1 (MEDIUM): The queueClosedEnvelope fallback message is now derived from record.status (victory / aborted / else), so a voluntary leave no longer flips "You left the battle." into a contradictory "You lost!" on a subsequent poll. Aborted states render as "Battle ended." L1 (LOW): parseCliOptions now re-validates sessionId / generation / sessionCode against the safe-id regex after decoding TKM_FB_OPTIONS_B64, so launching the daemon directly with a crafted env can't slip a bad segment into socket / crash-log paths. Also: SKILL.md now recommends /model sonnet instead of haiku. Haiku is too weak to keep the AskUserQuestion loop stable during a battle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1) DAEMON_ENTRY now resolves daemon.js when running from dist/ (plain node) and daemon.ts when running under tsx. The old hardcoded .ts path meant the compiled CLI spawned a nonexistent file, forcing users back onto the slow tsx path or crashing outright. daemonSpawnArgs() also drops the `--import tsx` shim for the compiled case. 2) DEFAULT_PLUGIN_ROOT in friendly-battle/snapshot.ts now walks up from import.meta.url until it finds package.json. The old `../..` hop pointed at `dist/` in compiled form, so loadMovesData() silently failed to find data/moves.json, the moves DB stayed null, and localizeMoveName / localizePokemonName fell back to English wire names even for Korean clients. Walking to package.json handles both src/ and dist/ layouts without hardcoding either tree. 3) battle_initialized and turn_resolved events now carry liveState from the host's battle-adapter runtime. Guest daemons render these events via buildEnvelopeFieldsFromLiveState instead of falling back to buildBattleContext, which previously showed species-base HP (100/100 for Dialga) from ownSnapshot.baseStats.hp. The test stripLiveState helper now strips liveState from all three event types; three battle-adapter assertions that compared raw turn_resolved events are updated to use the strippers. Net effect: running /tkm:friendly-battle from the compiled dist/ now shows real HP and locale-correct names on both sides. The battle state flow is fully host-authoritative for every event that carries state, matching the "host computes, guest receives" architecture. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
core/paths.ts PLUGIN_ROOT is the canonical data-root used by pokemon-data (species names), battle-setup (move names), sprites, cries, and i18n dictionaries. The old `join(import.meta.dirname, '..', '..')` pointed at `dist/` when compiled, so every data file load silently fell through to empty defaults — which is why Pokemon species names rendered as raw IDs or English even for Korean clients. Now walks up from the module directory until a package.json is found, matching the snapshot.ts fix from 740690f so both src/ (tsx) and dist/ (node) layouts work. friendly-battle-turn.ts --wait-next-event now survives ENOENT / ECONNREFUSED on the daemon socket. When the host daemon exits cleanly after the final turn, the skill's next poll used to crash with a socket connect error; the skill then hallucinated a plausible-sounding Korean recap instead of displaying the actual record. The CLI now reads the frozen session record from disk, synthesizes a terminal envelope with "You won!" / "You lost!" / "Battle ended." based on record.status, and writes it to stdout so the skill's turn-loop can exit cleanly. Matches the "status never fails" guarantee used by the --status subcommand. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The daemon process is a standalone node entry spawned by the CLI and never called initLocale(), so i18n/index.ts stayed at its default currentLocale='en'. Every getPokemonName / getGameI18n / localizeMoveName / localizePokemonName / t() lookup returned English even when the user's tokenmon config was 'ko', which is why species and move names kept rendering as "Dialga / Aura Sphere / Roar of Time" instead of "디아루가 / 파동탄 / 시간의포효" after the PLUGIN_ROOT fix in 0389713 already made the i18n JSON files loadable. runDaemon() now reads the global config and calls initLocale() once at startup, mirroring the pattern used by cli/battle-turn.ts main(). Wrapped in try/catch so a missing or corrupt config still lets the daemon come up (the battle will just show English names). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The CLI and transport already supported binding to any address via --listen-host; only the skill gated it to 127.0.0.1. Now `/tkm:friendly-battle open lan` flips the bind to 0.0.0.0 and resolves the host's first non-loopback IPv4 to advertise in the share strings. - Step 0 dispatch recognizes the optional `lan` second token on open. - Step 1a parameterizes LISTEN_HOST and ADVERTISED_HOST; loopback mode stays 127.0.0.1 for safety by default. - Share strings now interpolate ADVERTISED_HOST instead of hardcoding 127.0.0.1, so the guest sees the real LAN IP. - LAN mode also prints a firewall warning (WSL2 users need Windows firewall + WSL2 port forwarding; internet/NAT routing is NOT supported — same LAN only). - Help text updated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Same-machine testing is the edge case; real PvP across different machines on the same network is the expected flow. Default now binds 0.0.0.0 and advertises the detected LAN IP. The explicit `local` second token switches back to 127.0.0.1 for loopback testing. Help text + Step 0 dispatch updated accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scaffolds the evidence PR for PR42 gap doc §4-4. LAN mode itself already landed in PR46 (8631e4c), so this PR is documentation + convenience script only. New: - scripts/friendly-battle-smoke.sh — host / host local / guest / lan-ip subcommands wrapping bin/run-friendly-battle-turn.sh for copy-paste-free smoke setup on two real machines - docs/friendly-battle/validation/two-machine-smoke.md — §1-§9 with prerequisites, LAN IP discovery, firewall setup, walkthrough, success log sample, 3 failure scenarios (guest timeout / bad code / peer disconnect), troubleshooting, smoke script reference. Real two-machine logs left as USER ACTION placeholders; §6-1 includes a real loopback reference capture from this branch. - docs/friendly-battle/roadmap/pr47-smoke-evidence-plan.md — formal plan doc, matches the pr43-46 plan pattern in the same directory Updated: - docs/friendly-battle/roadmap/pr-stack-after-remote-snapshot-handshake.md — note that PR47 scope narrowed after LAN mode landed in PR46 Script bug fixed inline: the host subcommand's PORT parser raced --init-host's fast exit. Fixed by waiting for the child to fully exit before re-parsing stderr — same commit. Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove global --test-concurrency=1 from package.json; scope serial
execution to daemon test files via { concurrency: false }
- Delete docs/pvp/ speculative server specs (unimplemented, out of scope)
- Extract duplicated AsyncQueue into shared src/friendly-battle/async-queue.ts
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ThunderConch
changed the base branch from
feat/friendly-battle-pvp-fainted
to
master
This was referenced Apr 16, 2026
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
1 participant
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
User can now explicitly abort a friendly battle mid-flow via `/tkm:friendly-battle leave`. Both daemons shut down cleanly with a synthetic `battle_finished` event so the skill renders a clean "you left" / "opponent left" message instead of timing out.
What landed
Approach (no protocol change)
Voluntary leave uses the existing TCP disconnect path — the leaving daemon closes its transport, which causes EOF on the peer. No `peer_left` wire message is added to `tcp-direct.ts`, keeping the protocol layer stable. The peer's turn loop already catches transport errors; PR46 just augments that catch to push a synthetic `battle_finished{reason:'disconnect'}` event so the skill sees a clean envelope instead of a timeout.
Stacked PR
`/tkm:friendly-battle leave` is a terminal-path output (no AskUserQuestion), but the turn loop's new aborted handling IS visual. Merging requires:
This PR stays draft until visual QA passes.
Verification
Out of scope
Commits
🤖 Generated with Claude Code via the autopilot skill