From 2bb7b9f3fe45cf80ed7913e181eee647b344a286 Mon Sep 17 00:00:00 2001 From: Sangwon Lee Date: Mon, 20 Apr 2026 16:56:53 +0900 Subject: [PATCH 1/3] fix: unblock friendly-battle open and LAN bind Three stacked issues made `/tkm:friendly-battle open` unusable when the host's Claude Code session runs outside the plugin directory and wants to accept LAN guests. - Daemon spawn inherits Claude Code's cwd, which breaks Node ESM's cwd-relative `--import tsx` resolution. Pin the spawn cwd to PLUGIN_ROOT (derived from import.meta.url) so the detached daemon always finds tsx in the plugin's node_modules. - The TCP transport rejects wildcard listen hosts (0.0.0.0 / ::) unless an advertise host is provided. Thread a new `--join-host` flag from the CLI through DaemonOptions into createFriendlyBattleSpikeHost so LAN mode can bind 0.0.0.0 and still publish a concrete address. - Remove the 5-minute foreground polling loop that kept the host's Claude Code session blocked after open. Both `open` and `join` now run a single non-blocking --status probe, then detach. A new `/tkm:friendly-battle resume` subcommand (Step 1c) picks up the turn loop once the peer is connected, and a stand-by rule lets free-form check keywords trigger an implicit resume. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/friendly-battle/SKILL.md | 86 +++++++++++++++++++++++---------- src/cli/friendly-battle-turn.ts | 41 ++++++++++++++-- src/friendly-battle/daemon.ts | 7 ++- 3 files changed, 103 insertions(+), 31 deletions(-) diff --git a/skills/friendly-battle/SKILL.md b/skills/friendly-battle/SKILL.md index 8eb4adf4..44037869 100644 --- a/skills/friendly-battle/SKILL.md +++ b/skills/friendly-battle/SKILL.md @@ -12,6 +12,7 @@ Read the first token of `$ARGUMENTS`: - `open` → go to **Step 1a** (open flow). **Default is LAN mode** — bind `0.0.0.0` and advertise the machine's detected LAN IP. If the **second** token is `local`, run in loopback mode instead — bind `127.0.0.1` (same-machine only). - `join` → go to **Step 1b** (join flow); the second token must be `@:` +- `resume` → go to **Step 1c** (resume flow — picks up the turn loop after `open` detaches while waiting for the guest) - `status` → go to **Step 7** (status flow) - `leave` → go to **Step 9** (leave flow) - `help` or empty → go to **Step 8** (help flow) @@ -26,27 +27,28 @@ Read the first token of `$ARGUMENTS`: - Default (no second token, or anything besides `local`): **LAN mode** — bind `0.0.0.0`, advertise the machine's detected LAN IP. Works for two players on the **same local network** (e.g. same WiFi). The host's firewall must allow inbound TCP on the chosen port. - Second token `local`: **loopback mode** — bind `127.0.0.1`, advertise `127.0.0.1:`. Only works when host and guest are on the **same machine**. -**Initialize the host daemon:** +**Resolve plugin root, generation, and — in LAN mode — the machine's LAN IP first:** ```bash P="${CLAUDE_PLUGIN_ROOT:-$(ls -d ~/.claude/plugins/marketplaces/tkm 2>/dev/null || ls -d ~/.claude/plugins/cache/tkm/tkm/*/ 2>/dev/null | sort -V | tail -1)}" GEN=$(node -e "try{const g=JSON.parse(require('fs').readFileSync(require('path').join(require('os').homedir(),'.claude/tokenmon/global-config.json'),'utf-8'));console.log(g.active_generation||'gen1')}catch{console.log('gen1')}") SESSION_CODE=$(node -e "process.stdout.write(require('crypto').randomBytes(3).toString('hex'))") -# Default LAN mode binds 0.0.0.0 so same-network peers can reach this host; -# only the explicit `local` second token switches back to 127.0.0.1. -LISTEN_HOST=0.0.0.0 # set to 127.0.0.1 when the second token is `local` -"$P/bin/run-friendly-battle-turn.sh" --init-host --session-code "$SESSION_CODE" --generation "$GEN" --listen-host "$LISTEN_HOST" --port 0 --timeout-ms 300000 --player-name Host +LAN_IP=$(node -e "const i=require('os').networkInterfaces();for(const k of Object.keys(i))for(const a of i[k]||[])if(a.family==='IPv4'&&!a.internal){process.stdout.write(a.address);process.exit(0)}process.stdout.write('127.0.0.1')") ``` -Parse the JSON envelope on stdout. Store `sessionId` and `questionContext`. Also read the `PORT:` line from stderr to get the bound port. - -**In LAN mode (default), resolve the advertised host IP** (pick the first non-loopback IPv4 on the machine): +**Initialize the host daemon.** In LAN mode (default) bind `0.0.0.0` and advertise the detected `LAN_IP` via `--join-host` so guests learn the concrete address to connect to. In loopback mode (`local`) bind `127.0.0.1` and omit `--join-host`. ```bash -LAN_IP=$(node -e "const i=require('os').networkInterfaces();for(const k of Object.keys(i))for(const a of i[k]||[])if(a.family==='IPv4'&&!a.internal){process.stdout.write(a.address);process.exit(0)}process.stdout.write('127.0.0.1')") +# LAN mode (default): +"$P/bin/run-friendly-battle-turn.sh" --init-host --session-code "$SESSION_CODE" --generation "$GEN" --listen-host 0.0.0.0 --join-host "$LAN_IP" --port 0 --timeout-ms 300000 --player-name Host + +# Loopback mode (second $ARGUMENTS token == local): +# "$P/bin/run-friendly-battle-turn.sh" --init-host --session-code "$SESSION_CODE" --generation "$GEN" --listen-host 127.0.0.1 --port 0 --timeout-ms 300000 --player-name Host ``` -Use `ADVERTISED_HOST=$LAN_IP` in LAN mode (default), `ADVERTISED_HOST=127.0.0.1` in loopback mode. Use this value — NOT `127.0.0.1` — in the share strings below. +Parse the JSON envelope on stdout. Store `sessionId` — you MUST remember this for `/tkm:friendly-battle resume` and other subcommands later. Also read the `PORT:` line from stderr to get the bound port. + +Set `ADVERTISED_HOST=$LAN_IP` in LAN mode, `ADVERTISED_HOST=127.0.0.1` in loopback mode. Tell the user: - "세션 코드: ``" @@ -54,22 +56,22 @@ Tell the user: - "상대방에게 위 코드와 주소를 공유하고, 상대방이 `/tkm:friendly-battle join @:` 를 실행하도록 안내하세요." - **In LAN mode (default) only**, add: "⚠️ LAN 모드는 호스트 머신의 방화벽이 해당 TCP 포트(``)의 인바운드 연결을 허용해야 합니다. WSL2 호스트의 경우 Windows 방화벽도 확인하세요. 같은 WiFi/LAN 에 있는 상대방만 접속 가능합니다 (인터넷 경유 X). 같은 머신에서 두 터미널로 테스트하려면 `/tkm:friendly-battle open local` 을 사용하세요." - "💡 친선전 턴 진행은 거의 mechanical 한 작업이니 Opus 대신 Sonnet 을 쓰면 훨씬 빠릅니다. 배틀 속도가 답답하면 `/model sonnet` 으로 전환하세요. (배틀 끝난 뒤 `/model` 로 원래 모델 복구. Haiku는 UI 입력 파싱이 불안정해서 비추천.)" -- "게스트가 접속할 때까지 잠시 기다립니다..." - -Then enter the **wait-for-guest polling loop**: +- "💤 게스트 접속 대기는 백그라운드 데몬이 알아서 처리합니다. (핸드셰이크 제한 시간은 5분이며, 그 전에 resume 하지 않으면 세션이 만료됩니다.)" -All subsequent friendly-battle-turn invocations use the same launcher: -`"$P/bin/run-friendly-battle-turn.sh" ` — keep `$P`, `GEN`, and `sessionId` in scope. - -Poll loop: +**Then immediately probe with `--status` once** (non-blocking, never fails) so that if the opponent has already connected we skip straight to the turn loop: ```bash -"$P/bin/run-friendly-battle-turn.sh" --wait-next-event --session "$SESSION_ID" --generation "$GEN" --timeout-ms 60000 +"$P/bin/run-friendly-battle-turn.sh" --status --session "$SESSION_ID" --generation "$GEN" ``` -- If the returned envelope has `phase === 'battle'`: transition to **Step 2** (turn loop). -- If the returned envelope has `phase === 'aborted'`: read the REASON from stderr, show it to the user, and stop. -- Otherwise (still `waiting_for_guest`): repeat the poll (up to 5 times total, then stop with a "게스트가 응답하지 않습니다" message). +Dispatch on the returned envelope's `phase`: +- `phase === 'battle'` → transition to **Step 2** (turn loop) now. +- `phase === 'aborted'` → show `questionContext` (or REASON from stderr) and stop. +- anything else (`waiting_for_guest` / `handshake` / `connecting` / `ready`) → tell the user "아직 상대가 접속하지 않았습니다. 상대방이 붙었다 싶으면 `/tkm:friendly-battle resume` 을 실행하거나 저에게 '확인' / 'check' / 'resume' 이라고 말씀하세요. 그때 제가 다시 확인해서 배틀을 시작하겠습니다." and end your turn. + +**Stand-by rule (follow-up turns in the same conversation):** while the session is in a pre-battle phase (sessionId still in scope but Step 2 not yet entered), treat any subsequent free-form user message that looks like a check request — exact matches or fuzzy Korean/English equivalents of `확인`, `상태`, `체크`, `check`, `status`, `resume`, `왔어`, `접속`, `들어왔`, `ready?`, `joined?` — as an explicit `/tkm:friendly-battle resume` invocation and run Step 1c. Do not treat unrelated chit-chat as a check request; in that case answer the user's actual message and remind them once that they can say `확인` to probe the session. + +Do NOT poll or sleep-loop. Each status check is a single `--status` call that returns immediately. --- @@ -94,10 +96,41 @@ Parse the JSON envelope on stdout. Store `sessionId`. Tell the user: - "호스트에 접속 중입니다... 잠시 기다려 주세요." - "💡 친선전 턴 진행은 거의 mechanical 한 작업이니 Opus 대신 Sonnet 을 쓰면 훨씬 빠릅니다. 배틀 속도가 답답하면 `/model sonnet` 으로 전환하세요. (배틀 끝난 뒤 `/model` 로 원래 모델 복구. Haiku는 UI 입력 파싱이 불안정해서 비추천.)" +- "💤 호스트와의 핸드셰이크는 백그라운드 데몬이 처리합니다." + +**Then immediately probe with `--status` once** (non-blocking, never fails): + +```bash +"$P/bin/run-friendly-battle-turn.sh" --status --session "$SESSION_ID" --generation "$GEN" +``` + +Dispatch on the returned envelope's `phase`: +- `phase === 'battle'` → transition to **Step 2** (turn loop) now. +- `phase === 'aborted'` → show `questionContext` (or REASON from stderr) and stop. +- anything else (`handshake` / `connecting` / `ready`) → tell the user "호스트와의 핸드셰이크가 아직입니다. 배틀이 시작되었다 싶으면 `/tkm:friendly-battle resume` 을 실행하거나 저에게 '확인' / 'check' / 'resume' 이라고 말씀하세요. 그때 제가 다시 확인해서 배틀을 시작하겠습니다." and end your turn. + +**Stand-by rule (same as Step 1a):** while pre-battle, treat fuzzy check keywords (`확인` / `상태` / `체크` / `check` / `status` / `resume` / `왔어` / `접속` / `들어왔` / `ready?` / `joined?`) from the user as an implicit `/tkm:friendly-battle resume` invocation and run Step 1c. Do not sleep-loop. + +--- + +### Step 1c — Resume flow (host/guest, after `open`/`join` detached) + +Requires a stored `sessionId` from the current session (from Step 1a or Step 1b). If no session is active, tell the user "현재 대화에 friendly-battle 세션이 없습니다. 먼저 `/tkm:friendly-battle open` 또는 `/tkm:friendly-battle join` 을 실행하세요." and stop. + +**First, probe the daemon with `--status`** — this never blocks and never fails, so we can decide whether to enter the turn loop without waiting on an empty event queue. + +```bash +P="${CLAUDE_PLUGIN_ROOT:-$(ls -d ~/.claude/plugins/marketplaces/tkm 2>/dev/null || ls -d ~/.claude/plugins/cache/tkm/tkm/*/ 2>/dev/null | sort -V | tail -1)}" +GEN=$(node -e "try{const g=JSON.parse(require('fs').readFileSync(require('path').join(require('os').homedir(),'.claude/tokenmon/global-config.json'),'utf-8'));console.log(g.active_generation||'gen1')}catch{console.log('gen1')}") +"$P/bin/run-friendly-battle-turn.sh" --status --session "$SESSION_ID" --generation "$GEN" +``` -Then poll with `--wait-next-event` (same loop as Step 1a) until `phase === 'battle'`, then transition to **Step 2**. +Parse the JSON envelope on stdout. Dispatch on `phase`: -If `phase === 'aborted'`: show the REASON from stderr and stop. +- `phase === 'battle'` → the peer has joined and the battle is live. Transition to **Step 2** (turn loop) — call `--wait-next-event` with the normal 60000ms timeout and continue as usual. +- `phase === 'waiting_for_guest'`, `phase === 'handshake'`, `phase === 'connecting'`, or `phase === 'ready'` → the peer is not fully connected yet. Tell the user "아직 상대가 접속하지 않았습니다. 상대방에게 접속을 부탁하고, 잠시 후 다시 '확인' / `/tkm:friendly-battle resume` 으로 확인해 주세요. 상태가 바뀌면 바로 배틀을 시작하겠습니다." and stop. (Same stand-by rule as Step 1a/1b — treat fuzzy check keywords in the next user message as another implicit resume.) +- `phase === 'aborted'` → the session failed (timeout, handshake error, or `leave`). Show `questionContext` if informative, else tell the user "세션이 종료되었습니다. `/tkm:friendly-battle open` 또는 `/tkm:friendly-battle join` 으로 새 세션을 여세요." and stop. +- `phase === 'finished'` → the battle already ended. Show `questionContext` (e.g. "You won!" / "You lost!" / "Battle ended.") and stop. --- @@ -248,11 +281,13 @@ Show: /tkm:friendly-battle open — LAN 모드로 방 열기 (같은 네트워크의 다른 머신에서 접속 가능) /tkm:friendly-battle open local — loopback 모드 (같은 머신의 두 터미널 테스트용) /tkm:friendly-battle join @: — 호스트 방에 참가 +/tkm:friendly-battle resume — open/join 후 상대가 접속한 뒤 배틀 턴 진행 시작 /tkm:friendly-battle status — 현재 세션의 phase / status 확인 /tkm:friendly-battle leave — 배틀 도중 나가기 (상대방에게 통보) /tkm:friendly-battle help — 이 도움말 표시 /open 실행 후 출력된 세션 코드와 host:port 를 상대방과 공유하세요. +host/guest 모두 대기(handshake) 단계에서는 대화를 블로킹하지 않고 돌아옵니다 — 상대가 접속한 뒤 /tkm:friendly-battle resume 을 실행해 배틀을 시작하세요. LAN 모드 (기본) 는 호스트 방화벽이 해당 포트의 인바운드 연결을 허용해야 합니다. 교체(switch) / 항복(surrender)은 배틀 중 기술 선택 AskUserQuestion의 Other에 입력하세요. 배틀 도중 나가려면 /tkm:friendly-battle leave 를 실행하세요. @@ -293,8 +328,9 @@ LAN 모드 (기본) 는 호스트 방화벽이 해당 포트의 인바운드 연 | Command | Description | |---|---| -| `/tkm:friendly-battle open` | Open a friendly battle room | -| `/tkm:friendly-battle join @:` | Join an open room | +| `/tkm:friendly-battle open` | Open a friendly battle room (non-blocking — returns immediately after the daemon is ready) | +| `/tkm:friendly-battle join @:` | Join an open room (non-blocking — returns after daemon handshake) | +| `/tkm:friendly-battle resume` | Pick up the turn loop once the peer has connected | | `/tkm:friendly-battle status` | Check current session phase | | `/tkm:friendly-battle leave` | Leave the battle mid-flow (opponent is notified) | | `/tkm:friendly-battle help` | Show this help | diff --git a/src/cli/friendly-battle-turn.ts b/src/cli/friendly-battle-turn.ts index d296e853..bebb1a6f 100644 --- a/src/cli/friendly-battle-turn.ts +++ b/src/cli/friendly-battle-turn.ts @@ -31,6 +31,12 @@ const DAEMON_ENTRY: string = (() => { return tsEntry; })(); const DAEMON_USES_TSX = DAEMON_ENTRY.endsWith('.ts'); +// Plugin root — used as spawn() cwd so the daemon child can resolve the +// `tsx` package from the plugin's node_modules regardless of the parent's +// cwd. Node ESM's `--import tsx` specifier resolution is cwd-relative, so +// without this the daemon crashes with ERR_MODULE_NOT_FOUND whenever the +// user's shell cwd is outside the plugin dir. +const PLUGIN_ROOT = resolve(fileURLToPath(new URL('../..', import.meta.url))); function daemonSpawnArgs(role: 'host' | 'guest'): string[] { return DAEMON_USES_TSX ? ['--import', 'tsx', DAEMON_ENTRY, '--role', role] @@ -54,7 +60,7 @@ const USAGE = [ 'Usage: friendly-battle-turn [subcommand] [flags]', '', 'Subcommands:', - ' --init-host --session-code [--listen-host 127.0.0.1] [--port 0] [--timeout-ms 4000] [--generation gen4] [--player-name Host]', + ' --init-host --session-code [--listen-host 127.0.0.1] [--join-host ] [--port 0] [--timeout-ms 4000] [--generation gen4] [--player-name Host]', ' --init-join --session-code --host --port [--timeout-ms 4000] [--generation gen4] [--player-name Guest]', ' --wait-next-event --session --generation [--timeout-ms 60000]', ' --action --session --generation ', @@ -76,6 +82,7 @@ const CLI_FLAG_SCHEMA = { 'session': { type: 'string' as const }, 'host': { type: 'string' as const }, 'listen-host': { type: 'string' as const }, + 'join-host': { type: 'string' as const }, 'port': { type: 'string' as const }, 'timeout-ms': { type: 'string' as const }, 'generation': { type: 'string' as const }, @@ -146,6 +153,20 @@ function validateSafeId(value: string, name: string): string { return value; } +// Host args (listen-host, join-host, --host) are fed straight to Node's net +// APIs, which do their own parsing. We only strip control chars and cap length +// so a malformed value is rejected early with a clean REASON instead of +// crashing deep inside the TCP layer. +const SAFE_HOST = /^[A-Za-z0-9._:\-\[\]%]{1,253}$/; +function validateHostArg(value: string | undefined, name: string): string | undefined { + if (value === undefined || value === '') return undefined; + if (!SAFE_HOST.test(value)) { + process.stderr.write(`REASON: --${name} must match /^[A-Za-z0-9._:\\-\\[\\]%]{1,253}$/, got ${JSON.stringify(value)}\n`); + process.exit(1); + } + return value; +} + // --------------------------------------------------------------------------- function printUsage(): void { @@ -248,6 +269,10 @@ async function runInitHost(flags: Record): // --- Input validation (must run before forking daemon) --- const sessionCode = validateSessionCode(requireFlag(flags, 'session-code')); const listenHost = asStringFlag(flags, 'listen-host') ?? '127.0.0.1'; + // --join-host is the address guests will actually connect to. Required + // whenever --listen-host is a wildcard (0.0.0.0, ::) because the daemon's + // TCP transport rejects wildcard-without-advertise combinations upfront. + const advertiseHost = validateHostArg(asStringFlag(flags, 'join-host'), 'join-host'); const port = requirePositiveInt(asStringFlag(flags, 'port'), 'port', 0); const timeoutMs = requirePositiveInt(asStringFlag(flags, 'timeout-ms'), 'timeout-ms', 4000); const generation = validateGeneration(asStringFlag(flags, 'generation')); @@ -261,6 +286,7 @@ async function runInitHost(flags: Record): sessionId, sessionCode, host: listenHost, + advertiseHost, port, generation, playerName, @@ -268,7 +294,9 @@ async function runInitHost(flags: Record): }; const optionsB64 = Buffer.from(JSON.stringify(daemonOptions), 'utf8').toString('base64'); - // Fork the daemon as a detached child + // Fork the daemon as a detached child. cwd is pinned to PLUGIN_ROOT so + // `--import tsx` can resolve tsx from the plugin's node_modules even when + // the parent Claude Code session runs in an unrelated project directory. const child = spawn( process.execPath, daemonSpawnArgs('host'), @@ -276,6 +304,7 @@ async function runInitHost(flags: Record): detached: true, stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env, TKM_FB_OPTIONS_B64: optionsB64 }, + cwd: PLUGIN_ROOT, }, ); @@ -304,7 +333,7 @@ async function runInitHost(flags: Record): sessionCode, phase: 'aborted', status: 'aborted', - transport: { host: listenHost, port }, + transport: { host: advertiseHost ?? listenHost, port }, opponent: null, pid: process.pid, daemonPid: 0, @@ -349,7 +378,7 @@ async function runInitHost(flags: Record): sessionCode, phase: 'waiting_for_guest', status: 'waiting_for_guest', - transport: { host: listenHost, port: boundPort }, + transport: { host: advertiseHost ?? listenHost, port: boundPort }, opponent: null, pid: process.pid, daemonPid, @@ -399,7 +428,8 @@ async function runInitJoin(flags: Record): }; const optionsB64 = Buffer.from(JSON.stringify(daemonOptions), 'utf8').toString('base64'); - // Fork the daemon as a detached child + // Fork the daemon as a detached child. See init-host spawn for why cwd is + // pinned to PLUGIN_ROOT. const child = spawn( process.execPath, daemonSpawnArgs('guest'), @@ -407,6 +437,7 @@ async function runInitJoin(flags: Record): detached: true, stdio: ['ignore', 'pipe', 'pipe'], env: { ...process.env, TKM_FB_OPTIONS_B64: optionsB64 }, + cwd: PLUGIN_ROOT, }, ); diff --git a/src/friendly-battle/daemon.ts b/src/friendly-battle/daemon.ts index 0b2d787b..2e348724 100644 --- a/src/friendly-battle/daemon.ts +++ b/src/friendly-battle/daemon.ts @@ -71,6 +71,10 @@ interface DaemonOptions { sessionId: string; sessionCode: string; host: string; // listenHost for role=host, remote host for role=guest + // Advertise host (role=host only). Passed through to the TCP transport so a + // wildcard --listen-host (0.0.0.0, ::) can still publish a concrete address + // to guests. Ignored for the guest role. + advertiseHost?: string; port: number; generation: string; playerName: string; @@ -467,7 +471,7 @@ function serializeDaemonAction(action: DaemonAction): string { // --------------------------------------------------------------------------- async function runDaemon(role: FriendlyBattleRole, options: DaemonOptions): Promise { - const { sessionId, sessionCode, host, port, generation, playerName, timeoutMs } = options; + const { sessionId, sessionCode, host, advertiseHost, port, generation, playerName, timeoutMs } = options; // Initialize locale from the user's global config so getPokemonName, // getGameI18n, localizeMoveName, and localizePokemonName all render in @@ -751,6 +755,7 @@ async function runDaemon(role: FriendlyBattleRole, options: DaemonOptions): Prom if (role === 'host') { const host_transport = await createFriendlyBattleSpikeHost({ host, + advertiseHost, port, sessionCode, hostPlayerName: playerName, From 453da0b363f0dfedb11ed3116f526e127a3a79c4 Mon Sep 17 00:00:00 2001 From: Sangwon Lee Date: Mon, 20 Apr 2026 18:27:58 +0900 Subject: [PATCH 2/3] fix: apply validateHostArg to listen-host and join path --host; add CLI regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #56 review flagged that validateHostArg was only wired to --join-host. Thread it through --listen-host (host init) and --host (join init) so all three host-shaped flags fail the same way with a clean REASON: line before we spawn a daemon or open a socket. Also add `test/friendly-battle-turn-host-validation.test.ts` with three regression scenarios: - malformed value on each of --listen-host / --join-host / --host is rejected with a uniform REASON: ... must match … message; - wildcard --listen-host 0.0.0.0 with --join-host 10.77.77.1 writes the advertised host into the session record (not the wildcard); - the CLI invoked via bin/run-friendly-battle-turn.sh from a cwd outside the plugin root still reaches DAEMON_READY (emits PORT:) and never produces `Cannot find package 'tsx'` / ERR_MODULE_NOT_FOUND. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/cli/friendly-battle-turn.ts | 9 +- ...iendly-battle-turn-host-validation.test.ts | 214 ++++++++++++++++++ 2 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 test/friendly-battle-turn-host-validation.test.ts diff --git a/src/cli/friendly-battle-turn.ts b/src/cli/friendly-battle-turn.ts index bebb1a6f..9e459ad8 100644 --- a/src/cli/friendly-battle-turn.ts +++ b/src/cli/friendly-battle-turn.ts @@ -268,7 +268,10 @@ function readLineUntil( async function runInitHost(flags: Record): Promise { // --- Input validation (must run before forking daemon) --- const sessionCode = validateSessionCode(requireFlag(flags, 'session-code')); - const listenHost = asStringFlag(flags, 'listen-host') ?? '127.0.0.1'; + // Run every host-shaped flag through validateHostArg so bad input fails + // with a consistent `REASON:` line before we spawn a daemon or open a + // socket. --listen-host defaults to 127.0.0.1 when omitted. + const listenHost = validateHostArg(asStringFlag(flags, 'listen-host'), 'listen-host') ?? '127.0.0.1'; // --join-host is the address guests will actually connect to. Required // whenever --listen-host is a wildcard (0.0.0.0, ::) because the daemon's // TCP transport rejects wildcard-without-advertise combinations upfront. @@ -406,7 +409,9 @@ async function runInitHost(flags: Record): async function runInitJoin(flags: Record): Promise { // --- Input validation (must run before forking daemon) --- const sessionCode = validateSessionCode(requireFlag(flags, 'session-code')); - const hostAddr = requireFlag(flags, 'host'); + // --host is required; validateHostArg returns undefined only for empty input, + // which requireFlag already rejects, so the `!` assertion is safe. + const hostAddr = validateHostArg(requireFlag(flags, 'host'), 'host')!; const portStr = asStringFlag(flags, 'port') ?? requireFlag(flags, 'port'); const port = requirePositiveInt(portStr, 'port'); const timeoutMs = requirePositiveInt(asStringFlag(flags, 'timeout-ms'), 'timeout-ms', 4000); diff --git a/test/friendly-battle-turn-host-validation.test.ts b/test/friendly-battle-turn-host-validation.test.ts new file mode 100644 index 00000000..68875c05 --- /dev/null +++ b/test/friendly-battle-turn-host-validation.test.ts @@ -0,0 +1,214 @@ +// test/friendly-battle-turn-host-validation.test.ts +// +// Regression coverage for PR #56: +// 1. --listen-host / --join-host / --host all run through validateHostArg, +// not just --join-host (reviewer-flagged). +// 2. Wildcard --listen-host + --join-host writes the advertise host into +// the on-disk session record (not the wildcard). +// 3. The CLI can be invoked from a cwd outside the plugin root without +// tripping Node's cwd-relative `--import tsx` resolution (i.e. the +// daemon still reaches DAEMON_READY, so `PORT:` appears on stderr). + +import { after, describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; + +const REPO_ROOT = resolve(import.meta.dirname, '..'); +// Use the shell wrapper so tsx is resolved via absolute paths (the same +// entry point the skill invokes). This mirrors real-world invocation and +// lets the cwd-outside-plugin test exercise the actual code path that the +// bug report.md hit. +const LAUNCHER = resolve(REPO_ROOT, 'bin/run-friendly-battle-turn.sh'); +const GENERATION = 'gen4'; + +function makeClaudeConfigDir(tag: string): string { + const dir = mkdtempSync(join(tmpdir(), `tkm-fb-turn-host-val-${tag}-`)); + const genDir = join(dir, 'tokenmon', GENERATION); + mkdirSync(genDir, { recursive: true }); + writeFileSync( + join(dir, 'tokenmon', 'global-config.json'), + JSON.stringify({ active_generation: GENERATION, language: 'en', voice_tone: 'claude' }), + 'utf8', + ); + writeFileSync( + join(genDir, 'config.json'), + JSON.stringify({ party: ['387'], starter_chosen: true }), + 'utf8', + ); + writeFileSync( + join(genDir, 'state.json'), + JSON.stringify({ + pokemon: { + '387': { id: 387, xp: 100, level: 16, friendship: 0, ev: 0, moves: [33, 45] }, + }, + }), + 'utf8', + ); + return dir; +} + +function runCli( + args: string[], + options: { configDir: string; cwd?: string; timeoutMs?: number }, +) { + return spawnSync(LAUNCHER, args, { + cwd: options.cwd ?? REPO_ROOT, + encoding: 'utf8', + timeout: options.timeoutMs ?? 8000, + env: { + ...process.env, + CLAUDE_PLUGIN_ROOT: REPO_ROOT, + CLAUDE_CONFIG_DIR: options.configDir, + TSX_DISABLE_CACHE: '1', + }, + }); +} + +function readLatestSessionRecord(configDir: string): Record { + const sessionsDir = join(configDir, 'tokenmon', GENERATION, 'friendly-battle', 'sessions'); + const entries = readdirSync(sessionsDir).filter((name) => name.endsWith('.json')); + assert.ok(entries.length > 0, `expected at least one session record in ${sessionsDir}`); + // Deterministic pick: most recently written file. + const sorted = entries + .map((name) => ({ name, path: join(sessionsDir, name) })) + .sort((a, b) => readFileSync(b.path, 'utf8').localeCompare(readFileSync(a.path, 'utf8'))); + return JSON.parse(readFileSync(sorted[0].path, 'utf8')); +} + +describe('friendly-battle-turn — host-arg validation is uniform across flags', () => { + const badHost = '";rm -rf /"'; + + it('rejects a malformed --listen-host with a clean REASON line', () => { + const profile = makeClaudeConfigDir('bad-listen'); + after(() => rmSync(profile, { recursive: true, force: true })); + + const result = runCli( + [ + '--init-host', + '--session-code', 'alpha-listen-valid', + '--generation', GENERATION, + '--listen-host', badHost, + '--port', '0', + '--timeout-ms', '200', + '--player-name', 'Host', + ], + { configDir: profile }, + ); + + assert.notEqual(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); + assert.match(result.stderr, /REASON: --listen-host must match/); + }); + + it('rejects a malformed --join-host with a clean REASON line', () => { + const profile = makeClaudeConfigDir('bad-join'); + after(() => rmSync(profile, { recursive: true, force: true })); + + const result = runCli( + [ + '--init-host', + '--session-code', 'alpha-join-valid', + '--generation', GENERATION, + '--listen-host', '127.0.0.1', + '--join-host', badHost, + '--port', '0', + '--timeout-ms', '200', + '--player-name', 'Host', + ], + { configDir: profile }, + ); + + assert.notEqual(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); + assert.match(result.stderr, /REASON: --join-host must match/); + }); + + it('rejects a malformed --host on the join path with a clean REASON line', () => { + const profile = makeClaudeConfigDir('bad-host'); + after(() => rmSync(profile, { recursive: true, force: true })); + + const result = runCli( + [ + '--init-join', + '--session-code', 'alpha-host-valid', + '--generation', GENERATION, + '--host', badHost, + '--port', '12345', + '--timeout-ms', '200', + '--player-name', 'Guest', + ], + { configDir: profile }, + ); + + assert.notEqual(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); + assert.match(result.stderr, /REASON: --host must match/); + }); +}); + +describe('friendly-battle-turn — wildcard listen + advertise host', () => { + it('records the advertised host in the session transport, not the wildcard', () => { + const profile = makeClaudeConfigDir('advertise'); + after(() => rmSync(profile, { recursive: true, force: true })); + + // Short timeout so the daemon aborts on guest-join wait — we only need + // the session record to be written once init-host finishes its setup. + const advertise = '10.77.77.1'; + const result = runCli( + [ + '--init-host', + '--session-code', 'alpha-advertise-123', + '--generation', GENERATION, + '--listen-host', '0.0.0.0', + '--join-host', advertise, + '--port', '0', + '--timeout-ms', '150', + '--player-name', 'Host', + ], + { configDir: profile }, + ); + + // init-host emits the PORT: line once the daemon binds, then writes the + // session record. Even if the guest-join wait later times out, the + // record is on disk and the transport host must be the advertise host. + assert.match(result.stderr, /PORT: \d+/, `stderr=${result.stderr}`); + const record = readLatestSessionRecord(profile); + const transport = record.transport as { host: string; port: number }; + assert.equal(transport.host, advertise, `record=${JSON.stringify(record)}`); + assert.notEqual(transport.host, '0.0.0.0'); + assert.ok(typeof transport.port === 'number' && transport.port > 0, 'port should be bound'); + }); +}); + +describe('friendly-battle-turn — cwd outside the plugin root', () => { + it('spawns the daemon successfully (tsx resolves via PLUGIN_ROOT cwd)', () => { + const profile = makeClaudeConfigDir('cwd-outside'); + const foreignCwd = mkdtempSync(join(tmpdir(), 'tkm-fb-foreign-cwd-')); + after(() => { + rmSync(profile, { recursive: true, force: true }); + rmSync(foreignCwd, { recursive: true, force: true }); + }); + + const result = runCli( + [ + '--init-host', + '--session-code', 'alpha-cwd-outside-123', + '--generation', GENERATION, + '--listen-host', '127.0.0.1', + '--port', '0', + '--timeout-ms', '150', + '--player-name', 'Host', + ], + { configDir: profile, cwd: foreignCwd }, + ); + + // The key assertion: the daemon child must NOT have died with + // `Cannot find package 'tsx'` because its cwd is now pinned to + // PLUGIN_ROOT. The daemon reaching DAEMON_READY means the CLI + // emits a PORT: line on stderr before the guest-join timeout. + const combined = `${result.stdout}\n${result.stderr}`; + assert.doesNotMatch(combined, /Cannot find package 'tsx'/, combined); + assert.doesNotMatch(combined, /ERR_MODULE_NOT_FOUND/, combined); + assert.match(result.stderr, /PORT: \d+/, combined); + }); +}); From a59976088ddd4137917ab8f3d2f1680d7734807b Mon Sep 17 00:00:00 2001 From: Sangwon Lee Date: Mon, 20 Apr 2026 22:35:58 +0900 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20Codex=20adversarial=20revi?= =?UTF-8?q?ew=20=E2=80=94=20durable=20resume=20+=20honest=20host=20sanitiz?= =?UTF-8?q?er?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the adversarial review on PR #56. [high] The stand-by/resume flow is no longer orphaned when conversational memory drops the `sessionId`. - New `--list-active --generation ` subcommand in `src/cli/friendly-battle-turn.ts` enumerates session records whose daemon PID is still alive and whose phase is non-terminal, emitting a compact JSON array sorted by `updatedAt` descending. Dead daemons and finished/aborted records are filtered out. - `skills/friendly-battle/SKILL.md` Step 1c now calls `--list-active` when `sessionId` is missing from conversation scope: empty → tell the user to `open`/`join` again; exactly one → adopt it; multiple → AskUserQuestion to pick. The existing `--status` probe runs after sessionId is resolved. [medium] Renamed `validateHostArg` → `sanitizeHostArg` to match what it actually does. The helper was never semantic host validation — it was always a shell-safety character filter; the old name/comment/REASON copy overclaimed. Updated the doc comment and REASON message to say "contains characters outside the shell-safe set" so malformed-but-clean hosts like `::::` or `[::1` that Node's net layer still rejects are honestly described as deferred-to-transport rather than pre-validated at the CLI. Tests: extended `test/friendly-battle-turn-host-validation.test.ts` with two `--list-active` cases (empty sessions dir → `[]`; live init-host run → one entry whose sessionId/role/phase matches). The new cases use a SIGKILL-then-poll helper so the detached daemon is reaped before the temp dir is removed, avoiding an ENOTEMPTY race. Full suite: 1210/1210 passing. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/friendly-battle/SKILL.md | 15 ++- src/cli/friendly-battle-turn.ts | 74 +++++++++++--- ...iendly-battle-turn-host-validation.test.ts | 99 ++++++++++++++++++- 3 files changed, 167 insertions(+), 21 deletions(-) diff --git a/skills/friendly-battle/SKILL.md b/skills/friendly-battle/SKILL.md index 44037869..e36081f1 100644 --- a/skills/friendly-battle/SKILL.md +++ b/skills/friendly-battle/SKILL.md @@ -115,13 +115,22 @@ Dispatch on the returned envelope's `phase`: ### Step 1c — Resume flow (host/guest, after `open`/`join` detached) -Requires a stored `sessionId` from the current session (from Step 1a or Step 1b). If no session is active, tell the user "현재 대화에 friendly-battle 세션이 없습니다. 먼저 `/tkm:friendly-battle open` 또는 `/tkm:friendly-battle join` 을 실행하세요." and stop. - -**First, probe the daemon with `--status`** — this never blocks and never fails, so we can decide whether to enter the turn loop without waiting on an empty event queue. +**Recover `sessionId` first.** If you already have `sessionId` in conversation scope (from the same Step 1a / Step 1b call), use it and skip this block. Otherwise — e.g. the conversation was compacted, handed off, or simply a while ago — ask the CLI which sessions are still alive: ```bash P="${CLAUDE_PLUGIN_ROOT:-$(ls -d ~/.claude/plugins/marketplaces/tkm 2>/dev/null || ls -d ~/.claude/plugins/cache/tkm/tkm/*/ 2>/dev/null | sort -V | tail -1)}" GEN=$(node -e "try{const g=JSON.parse(require('fs').readFileSync(require('path').join(require('os').homedir(),'.claude/tokenmon/global-config.json'),'utf-8'));console.log(g.active_generation||'gen1')}catch{console.log('gen1')}") +"$P/bin/run-friendly-battle-turn.sh" --list-active --generation "$GEN" +``` + +The output is a JSON array (possibly empty) of non-terminal sessions whose daemon PID is still alive, sorted by `updatedAt` descending. Each entry includes `sessionId`, `role`, `phase`, `status`, `transport`, `updatedAt`. Dispatch on the array size: +- `[]` (empty) → tell the user "현재 활성화된 friendly-battle 세션이 없습니다. 먼저 `/tkm:friendly-battle open` 또는 `/tkm:friendly-battle join` 을 실행하세요." and stop. +- one entry → adopt its `sessionId` and proceed with the `--status` probe below. +- two or more → use AskUserQuestion to let the user pick which session to resume, labelling each option as `role= code= updated=`. After the pick, adopt that `sessionId`. + +**Then probe the daemon with `--status`** — this never blocks and never fails, so we can decide whether to enter the turn loop without waiting on an empty event queue. + +```bash "$P/bin/run-friendly-battle-turn.sh" --status --session "$SESSION_ID" --generation "$GEN" ``` diff --git a/src/cli/friendly-battle-turn.ts b/src/cli/friendly-battle-turn.ts index 9e459ad8..ec88ed0a 100644 --- a/src/cli/friendly-battle-turn.ts +++ b/src/cli/friendly-battle-turn.ts @@ -9,6 +9,7 @@ import { type FriendlyBattleSessionRecord, writeFriendlyBattleSessionRecord, readFriendlyBattleSessionRecord, + listFriendlyBattleSessionRecords, isPidAlive, } from '../friendly-battle/session-store.js'; import { formatFriendlyBattleTurnJson } from '../friendly-battle/turn-json.js'; @@ -49,7 +50,8 @@ type Subcommand = | 'wait-next-event' | 'action' | 'status' - | 'leave'; + | 'leave' + | 'list-active'; interface ParsedCliArgs { subcommand: Subcommand; @@ -65,6 +67,7 @@ const USAGE = [ ' --wait-next-event --session --generation [--timeout-ms 60000]', ' --action --session --generation ', ' --status --session --generation ', + ' --list-active --generation ', '', ].join('\n'); @@ -75,6 +78,7 @@ const SUBCOMMAND_FLAGS = new Set([ // '--action' is intentionally absent: it carries a value and is parsed by parseArgs directly '--status', '--leave', + '--list-active', ]); const CLI_FLAG_SCHEMA = { @@ -153,15 +157,19 @@ function validateSafeId(value: string, name: string): string { return value; } -// Host args (listen-host, join-host, --host) are fed straight to Node's net -// APIs, which do their own parsing. We only strip control chars and cap length -// so a malformed value is rejected early with a clean REASON instead of -// crashing deep inside the TCP layer. +// Host args (listen-host, join-host, --host) are shell-safety filtered only. +// This is NOT semantic host validation: inputs like `::::`, `foo..bar`, or +// `[::1` (unmatched bracket) will pass this check and fail later inside +// Node's net layer with its own error. The purpose here is strictly to +// reject shell metacharacters and control bytes so a malicious value can't +// traverse into downstream command paths — semantic parsing (IPv4/IPv6 +// literal, hostname labels) is intentionally deferred to `net.listen` / +// `net.connect`, which already has the right behavior and error messages. const SAFE_HOST = /^[A-Za-z0-9._:\-\[\]%]{1,253}$/; -function validateHostArg(value: string | undefined, name: string): string | undefined { +function sanitizeHostArg(value: string | undefined, name: string): string | undefined { if (value === undefined || value === '') return undefined; if (!SAFE_HOST.test(value)) { - process.stderr.write(`REASON: --${name} must match /^[A-Za-z0-9._:\\-\\[\\]%]{1,253}$/, got ${JSON.stringify(value)}\n`); + process.stderr.write(`REASON: --${name} contains characters outside the shell-safe set [A-Za-z0-9._:\\-\\[\\]%], got ${JSON.stringify(value)}\n`); process.exit(1); } return value; @@ -180,6 +188,7 @@ function resolveSubcommand(argv: string[]): Subcommand | null { if (argv.includes('--action')) return 'action'; if (argv.includes('--status')) return 'status'; if (argv.includes('--leave')) return 'leave'; + if (argv.includes('--list-active')) return 'list-active'; return null; } @@ -268,14 +277,15 @@ function readLineUntil( async function runInitHost(flags: Record): Promise { // --- Input validation (must run before forking daemon) --- const sessionCode = validateSessionCode(requireFlag(flags, 'session-code')); - // Run every host-shaped flag through validateHostArg so bad input fails - // with a consistent `REASON:` line before we spawn a daemon or open a - // socket. --listen-host defaults to 127.0.0.1 when omitted. - const listenHost = validateHostArg(asStringFlag(flags, 'listen-host'), 'listen-host') ?? '127.0.0.1'; + // Run every host-shaped flag through sanitizeHostArg so shell-unsafe + // input is rejected with a consistent `REASON:` line before we spawn a + // daemon or open a socket. Semantic validity of the host is left to + // Node's net layer. --listen-host defaults to 127.0.0.1 when omitted. + const listenHost = sanitizeHostArg(asStringFlag(flags, 'listen-host'), 'listen-host') ?? '127.0.0.1'; // --join-host is the address guests will actually connect to. Required // whenever --listen-host is a wildcard (0.0.0.0, ::) because the daemon's // TCP transport rejects wildcard-without-advertise combinations upfront. - const advertiseHost = validateHostArg(asStringFlag(flags, 'join-host'), 'join-host'); + const advertiseHost = sanitizeHostArg(asStringFlag(flags, 'join-host'), 'join-host'); const port = requirePositiveInt(asStringFlag(flags, 'port'), 'port', 0); const timeoutMs = requirePositiveInt(asStringFlag(flags, 'timeout-ms'), 'timeout-ms', 4000); const generation = validateGeneration(asStringFlag(flags, 'generation')); @@ -409,9 +419,9 @@ async function runInitHost(flags: Record): async function runInitJoin(flags: Record): Promise { // --- Input validation (must run before forking daemon) --- const sessionCode = validateSessionCode(requireFlag(flags, 'session-code')); - // --host is required; validateHostArg returns undefined only for empty input, + // --host is required; sanitizeHostArg returns undefined only for empty input, // which requireFlag already rejects, so the `!` assertion is safe. - const hostAddr = validateHostArg(requireFlag(flags, 'host'), 'host')!; + const hostAddr = sanitizeHostArg(requireFlag(flags, 'host'), 'host')!; const portStr = asStringFlag(flags, 'port') ?? requireFlag(flags, 'port'); const port = requirePositiveInt(portStr, 'port'); const timeoutMs = requirePositiveInt(asStringFlag(flags, 'timeout-ms'), 'timeout-ms', 4000); @@ -705,6 +715,39 @@ async function runLeave(flags: Record): Pr } } +/** + * List sessions whose daemon is still alive. Used by the skill's `resume` + * flow when the conversation has lost the in-memory `sessionId` (e.g. the + * conversation was compacted, restarted, or handed off mid-battle). The + * output is a single JSON array on stdout — one entry per live session, + * sorted by `updatedAt` descending so the caller can trivially pick the + * most recent one. + * + * Terminal phases (`finished` / `aborted`) and dead daemons are filtered + * out so a stale file never gets adopted as an "active" session. + */ +function runListActive(flags: Record): void { + const generation = validateGeneration(asStringFlag(flags, 'generation')); + const all = listFriendlyBattleSessionRecords(generation); + const active = all + .filter((r) => r.phase !== 'finished' && r.phase !== 'aborted') + .filter((r) => typeof r.daemonPid === 'number' && r.daemonPid > 0 && isPidAlive(r.daemonPid)) + .sort((a, b) => b.updatedAt.localeCompare(a.updatedAt)); + // Keep the payload small and skill-friendly: only the fields the resume + // flow actually needs. Full records are still readable via --status. + const payload = active.map((r) => ({ + sessionId: r.sessionId, + role: r.role, + generation: r.generation, + sessionCode: r.sessionCode, + phase: r.phase, + status: r.status, + transport: r.transport, + updatedAt: r.updatedAt, + })); + process.stdout.write(`${JSON.stringify(payload)}\n`); +} + function requireFlag(flags: Record, name: string): string { const v = flags[name]; if (typeof v === 'string') return v; @@ -733,6 +776,9 @@ async function main(argv: string[]): Promise { case 'leave': await runLeave(parsed.flags); return; + case 'list-active': + runListActive(parsed.flags); + return; } } diff --git a/test/friendly-battle-turn-host-validation.test.ts b/test/friendly-battle-turn-host-validation.test.ts index 68875c05..2bc28f5c 100644 --- a/test/friendly-battle-turn-host-validation.test.ts +++ b/test/friendly-battle-turn-host-validation.test.ts @@ -1,13 +1,17 @@ // test/friendly-battle-turn-host-validation.test.ts // // Regression coverage for PR #56: -// 1. --listen-host / --join-host / --host all run through validateHostArg, +// 1. --listen-host / --join-host / --host all run through sanitizeHostArg, // not just --join-host (reviewer-flagged). // 2. Wildcard --listen-host + --join-host writes the advertise host into // the on-disk session record (not the wildcard). // 3. The CLI can be invoked from a cwd outside the plugin root without // tripping Node's cwd-relative `--import tsx` resolution (i.e. the // daemon still reaches DAEMON_READY, so `PORT:` appears on stderr). +// 4. --list-active returns a JSON array that the skill's `resume` flow +// can use to recover the sessionId when conversational memory is +// lost. An active init-host run is discoverable; an empty sessions +// dir yields an empty array. import { after, describe, it } from 'node:test'; import assert from 'node:assert/strict'; @@ -99,7 +103,7 @@ describe('friendly-battle-turn — host-arg validation is uniform across flags', ); assert.notEqual(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); - assert.match(result.stderr, /REASON: --listen-host must match/); + assert.match(result.stderr, /REASON: --listen-host contains characters outside the shell-safe set/); }); it('rejects a malformed --join-host with a clean REASON line', () => { @@ -121,7 +125,7 @@ describe('friendly-battle-turn — host-arg validation is uniform across flags', ); assert.notEqual(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); - assert.match(result.stderr, /REASON: --join-host must match/); + assert.match(result.stderr, /REASON: --join-host contains characters outside the shell-safe set/); }); it('rejects a malformed --host on the join path with a clean REASON line', () => { @@ -142,7 +146,7 @@ describe('friendly-battle-turn — host-arg validation is uniform across flags', ); assert.notEqual(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); - assert.match(result.stderr, /REASON: --host must match/); + assert.match(result.stderr, /REASON: --host contains characters outside the shell-safe set/); }); }); @@ -212,3 +216,90 @@ describe('friendly-battle-turn — cwd outside the plugin root', () => { assert.match(result.stderr, /PORT: \d+/, combined); }); }); + +describe('friendly-battle-turn — --list-active recovers sessions when sessionId is lost', () => { + async function killAndWait(pid: number | undefined, timeoutMs = 2000): Promise { + if (typeof pid !== 'number' || pid <= 0) return; + try { process.kill(pid, 'SIGKILL'); } catch { return; /* already dead */ } + // Poll until the daemon stops writing to the sessions dir so rmSync + // doesn't race with a still-flushing crash log or the final record + // write and fail with ENOTEMPTY. + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + try { process.kill(pid, 0); } catch { return; } + await new Promise((r) => setTimeout(r, 25)); + } + } + + function rmTreeQuietly(path: string): void { + // Retry loop protects against the daemon's last-gasp crash-log write + // arriving after killAndWait returns (kernel delivery + fs flush can + // still complete a fraction of a millisecond after the PID is gone). + for (let attempt = 0; attempt < 5; attempt++) { + try { rmSync(path, { recursive: true, force: true, maxRetries: 3 }); return; } catch { /* retry */ } + } + } + + it('returns an empty JSON array when no sessions exist', () => { + const profile = makeClaudeConfigDir('list-empty'); + after(() => rmTreeQuietly(profile)); + + const result = runCli( + ['--list-active', '--generation', GENERATION], + { configDir: profile }, + ); + + assert.equal(result.status, 0, `stdout=${result.stdout}\nstderr=${result.stderr}`); + const parsed: unknown = JSON.parse(result.stdout.trim()); + assert.ok(Array.isArray(parsed), 'expected a JSON array'); + assert.equal(parsed.length, 0); + }); + + it('surfaces an active init-host daemon so resume can adopt its sessionId', () => { + const profile = makeClaudeConfigDir('list-active'); + // init-host detaches the daemon and returns quickly after DAEMON_READY. + // Give the daemon a generous guest-join timeout so it is still alive + // when --list-active probes the sessions dir. + const init = runCli( + [ + '--init-host', + '--session-code', 'alpha-list-active-123', + '--generation', GENERATION, + '--listen-host', '127.0.0.1', + '--port', '0', + '--timeout-ms', '5000', + '--player-name', 'Host', + ], + { configDir: profile }, + ); + assert.equal(init.status, 0, `init stderr=${init.stderr}`); + + // Pull the daemonPid off the on-disk record so we can reap it after + // the assertion even if the test body throws. + const record = readLatestSessionRecord(profile) as { daemonPid?: number; sessionId?: string }; + const daemonPid = record.daemonPid; + after(async () => { + await killAndWait(daemonPid); + rmTreeQuietly(profile); + }); + + const list = runCli( + ['--list-active', '--generation', GENERATION], + { configDir: profile }, + ); + assert.equal(list.status, 0, `list stderr=${list.stderr}`); + const parsed = JSON.parse(list.stdout.trim()) as Array<{ + sessionId: string; + role: string; + phase: string; + sessionCode: string; + }>; + assert.ok(Array.isArray(parsed), 'expected a JSON array'); + assert.equal(parsed.length, 1, `expected exactly one active session, got ${JSON.stringify(parsed)}`); + assert.equal(parsed[0].sessionId, record.sessionId); + assert.equal(parsed[0].role, 'host'); + assert.notEqual(parsed[0].phase, 'finished'); + assert.notEqual(parsed[0].phase, 'aborted'); + assert.equal(parsed[0].sessionCode, 'alpha-list-active-123'); + }); +});