merge: story — multiplayer E2E MVP vertical slice

_bmad-output/stories/multiplayer-e2e-mvp.md

@@ -0,0 +1,158 @@
+# Story: Multiplayer Live E2E — MVP Vertical Slice (2-player-bot live run)
+
+Status: ready-for-dev
+PRD: [`_bmad-output/prds/prd-multiplayer-e2e-second-bot-2026-06-22/prd.md`](../prds/prd-multiplayer-e2e-second-bot-2026-06-22/prd.md) (FR-1/2/3/4/5/6/7/8/10)
+Epic (PRD §9): multiplayer live E2E harness — this is stories (1)+(2)+(3)+(4) rolled into one vertical slice.
+
+## Story
+
+As the engine maintainer,
+I want a live E2E test that runs a real 2-player group encounter — two player-bots join a lobby, post real chat turns that route through the live `messageRouter`, and roll in a group skill check whose scoreboard finalizes with the real `successRule` —
+so that the multiplayer surfaces (lobby gating, multi-player message routing, group checks) are covered by automated live E2E against the real Discord gateway, not unit tests alone.
+
+## Scope
+
+**IN (this story):** config + anti-loop allowlist + unit test (PRD §9.1); `liveBots` N-player extension (§9.2); `minPlayers: 2` fixture spec (§9.3); the MVP live test — lobby gating, 2 real chat turns, group check N=2 (§9.4 / FR-6/7/8/10).
+
+**OUT (follow-up stories, do NOT implement here):** the 4 gap-case ACs (FR-11–14: simultaneous fan-out, `successRule` N>1 matrix, per-user ephemeral, second-claimant rejection); the live-E2E doc update (FR-17); 3+ player bots; real gateway button clicks; real per-user ephemeral delivery to a bot client.
+
+## Acceptance Criteria
+
+1. **AC-1 (config flag):** `src/config.ts` adds `E2E_ALLOW_PLAYER_BOTS: z.coerce.boolean().default(false)`. When false, production behavior is byte-for-byte unchanged.
+2. **AC-2 (anti-loop allowlist):** `src/bot/handlers/messageRouter.ts:33` — when `config.E2E_ALLOW_PLAYER_BOTS === true` and the author's id is in the runtime player-bot allowlist, a bot-authored message is routed as a player turn instead of being skipped. Otherwise the current `return` behavior is unchanged.
+3. **AC-3 (unit test):** a unit test covers both branches without going live — flag off → bot message skipped (no turn); flag on + id allowlisted → message routed as a player turn. Green in `npx vitest run tests/unit`.
+4. **AC-4 (liveBots N-player):** `connectLiveBots()` connects player1 (`E2E_DRIVER_TOKEN`, re-roled from "driver") + player2 (`E2E_PLAYER2_TOKEN`) and returns `{ botClient, players: Client[], guild, channel }` with `players.length ≥ 2`. A `driverBot` alias to `players[0]` is preserved so existing AC2–AC4 tests stay unchanged. Throws clearly if `E2E_PLAYER2_TOKEN` is missing when `RUN_FULL_E2E=1`.
+5. **AC-5 (auto-populate allowlist):** After each player client logs in, its `client.user.id` is added to the runtime allowlist (so `messageRouter` routes its messages as player turns). No manual userId env var required.
+6. **AC-6 (fixture spec):** `tests/fixtures/spec-group-e2e.yaml` with `minPlayers: 2`, a group skill check, and a passive reveal. Loads via `EncounterSpecSchema` (`loadSpec` round-trip / specLoader test).
+7. **AC-7 (live test gate):** New `tests/integration/graphmcp/group-encounter-live.test.ts`, gated by `RUN_FULL_E2E=1` AND `E2E_PLAYER2_TOKEN` present (`describe.skipIf`), skipped by default → CI-safe.
+8. **AC-8 (lobby gating):** Start the fixture spec via `fakeInteraction` (starter userId); assert **Start** disabled while < 2 joined; player1 + player2 **Join** (fakeInteraction, each bot's real userId); assert **Start** enables; starter starts; assert opening narrative + a passive reveal fire for a qualifying player.
+9. **AC-9 (2 real chat turns + regulation):** Player1 and player2 each post a **real gateway message** in the thread; assert both route through `messageRouter` as player turns (session history grows with both authors). A message from a non-joined, non-allowlisted author is **auto-deleted**; assert no false-positive deletion of joined players' messages.
+10. **AC-10 (group check N=2):** A group skill check is emitted; player1 + player2 **Roll** (fake button, each bot's real userId); assert the central scoreboard updates and finalizes with the real `successRule`; assert a `[GROUP CHECK RESULT]` system message is appended to history.
+11. **AC-11 (no secret leak):** No token value in any committed file; `.env` stays gitignored; tests/PRD reference env-var names only.
+12. **AC-12 (no regressions):** `npx vitest run tests/unit` stays green (527+ tests). Existing live AC2–AC4 tests are unchanged and still skip/pass under `RUN_FULL_E2E`.
+
+## Tasks / Subtasks
+
+- [ ] **T1 — config flag + allowlist module + anti-loop branch (AC: 1,2,3)**
+  - [ ] `src/config.ts`: add `E2E_ALLOW_PLAYER_BOTS: z.coerce.boolean().default(false)` to `EnvSchema`.
+  - [ ] NEW `src/bot/lib/e2ePlayerAllowlist.ts`: module-level `Set<string>` + `addPlayerBotIds(ids: string[])`, `isE2EPlayerBot(id: string): boolean`, `clearE2EPlayerBots()`. (Isolates E2E-specific prod code; gated by the config flag so prod never routes bot messages.)
+  - [ ] `src/bot/handlers/messageRouter.ts:33`: change `if (message.author.bot) return;` to skip only when NOT (`config.E2E_ALLOW_PLAYER_BOTS` AND `isE2EPlayerBot(message.author.id)`); otherwise fall through as a player turn.
+  - [ ] NEW `tests/unit/e2ePlayerAllowlist.test.ts` (or extend `messageRouterRunLLMTurn.test.ts`): mock `config.E2E_ALLOW_PLAYER_BOTS` true/false; assert routing vs skip. Use `ioredis-mock` + a stub session; no live Discord.
+- [ ] **T2 — liveBots N-player extension (AC: 4,5)**
+  - [ ] `tests/integration/graphmcp/support/liveBots.ts`: require `E2E_PLAYER2_TOKEN`; connect `player1` (`E2E_DRIVER_TOKEN`) + `player2` (`E2E_PLAYER2_TOKEN`); return `{ botClient, players: [player1, player2], guild, channel }`; keep `driverBot` = `players[0]` alias on the returned object for back-compat.
+  - [ ] After both `login()`s resolve, call `addPlayerBotIds(players.map(p => p.user!.id))` so the allowlist is populated before any message is posted.
+  - [ ] `disconnectLiveBots`: destroy all three clients; call `clearE2EPlayerBots()`.
+- [ ] **T3 — fixture spec (AC: 6)**
+  - [ ] NEW `tests/fixtures/spec-group-e2e.yaml`: `encounterId: "mardonar-group-e2e-008"` (unique), `minPlayers: 2`, 2 NPCs, 2 primary + 1 secondary goals, a `skillChecks` entry for a group Stealth check (note in `_note`: emit as `skill_check_group_emit`, `successRule: majority`), a `passiveReveals` entry (e.g. Insight/15), `tools:` including `skill_check_emit`, `skill_check_group_emit`, `encounter_resolve`, `context_recall`. Follow `specs/SPEC_FORMAT.md` + the velvet-auction pattern.
+  - [ ] Validate: `DISCORD_TOKEN=x DISCORD_CLIENT_ID=x npx vitest run tests/unit/specLoader.test.ts tests/unit/specsToolsConsistency.test.ts` stays green (the fixture is NOT in `specs/`, so the consistency test won't scan it — confirm `SPECS_DIR` resolution; if the test only reads `./specs`, the fixture in `tests/fixtures` is fine).
+- [ ] **T4 — MVP live test (AC: 7,8,9,10)**
+  - [ ] NEW `tests/integration/graphmcp/group-encounter-live.test.ts`: `import './support/env.js'`; `const runE2E = process.env.RUN_FULL_E2E === '1' && !!process.env.E2E_PLAYER2_TOKEN`; `describe.skipIf(!runE2E)`.
+  - [ ] `beforeAll`: `connectLiveBots()`; `flushRedisForGuild(bots.guild.id)`; load the fixture spec via `loadSpec('group-e2e')` — **note:** `loadSpec` reads `config.SPECS_DIR` (default `./specs`); either place the fixture in `specs/` OR point `SPECS_DIR` at `tests/fixtures` for this run OR load+parse the fixture path directly. Pick one and document it (see Dev Notes).
+  - [ ] `afterAll`: delete threads, `disconnectRedis()`, `disconnectLiveBots(bots)`.
+  - [ ] AC-8: start via `fakeInteraction({ subcommand:'start', stringOptions:{spec:'group-e2e'}, channel: bots.channel, guildId: bots.guild.id, userId: <starter> })`; drive Join for player1 + player2 via the lobby button fakes (see Dev Notes on `fakeButton`); assert Start gating + opening + passive reveal.
+  - [ ] AC-9: `await bots.players[0].send(...)` / `players[1]` into the thread (real gateway messages); `waitFor` session history to contain both authors; post a non-allowlisted message and assert deletion.
+  - [ ] AC-10: drive the group check Roll for both players via fake buttons carrying each player's userId; `waitFor` scoreboard finalization + `[GROUP CHECK RESULT]` in history.
+- [ ] **T5 — verify (AC: 11,12)**
+  - [ ] `npx vitest run tests/unit` → green.
+  - [ ] `git grep -niE "token|MTA|GMXw"` across new files → only env-var NAME references, no values.
+  - [ ] Existing live tests still `skip` by default (no `RUN_FULL_E2E`).
+
+## Dev Notes
+
+### ⚠️ config.ts parses env ONCE at import
+`src/config.ts` ends with `export const config = EnvSchema.parse(process.env)` — a frozen, module-level parse. **You cannot auto-populate a player-bot id list through `config`**, because `connectLiveBots` logs in (and learns `client.user.id`) long after `config` is parsed. That is why:
+- Only the on/off **flag** `E2E_ALLOW_PLAYER_BOTS` lives in `config` (set in `.env`, loaded by `support/env.ts`'s `import 'dotenv/config'` before `config` parses).
+- The **id list** is a runtime `Set` in `src/bot/lib/e2ePlayerAllowlist.ts`, populated by `connectLiveBots` after login and read by `messageRouter` at call time.
+Do NOT add `E2E_PLAYER_BOT_IDS` to `config.ts` and expect to mutate it post-parse.
+
+### Anti-loop guard — the only production-code touch
+`src/bot/handlers/messageRouter.ts:32-33`:
+```ts
+export async function handleMessage(message: Message, client: Client): Promise<void> {
+  if (message.author.bot) return;   // ← this line
+```
+Change to:
+```ts
+import { isE2EPlayerBot } from '../lib/e2ePlayerAllowlist.js';
+// ...
+  if (message.author.bot) {
+    if (!(config.E2E_ALLOW_PLAYER_BOTS && isE2EPlayerBot(message.author.id))) return;
+    // fall through: treat as a player turn (E2E only)
+  }
+```
+Everything else in `handleMessage` stays the same. Default `E2E_ALLOW_PLAYER_BOTS=false` → the branch is dead in prod → no behavior change. The unit test (AC-3) must prove both branches.
+
+### `fakeButton` needs a userId for group-check Roll
+`tests/integration/graphmcp/support/fakes.ts:111` `fakeButton(channel, customId)` builds a `ButtonInteraction` with **no `user`**. `rollHandler.submitResult` / `groupCheckManager` identify the roller from the interaction's user — so for player1/player2 Roll you need the clicker's real userId. Extend `fakeButton` to accept an optional `userId`/`username` (default to the existing behavior for back-compat), or add `fakeButtonForUser(channel, customId, userId, username)`. **Read `src/bot/handlers/rollHandler.ts` and `src/harness/groupCheckManager.ts` to confirm exactly which `interaction.user` / `interaction.member` fields they read** before finalizing the fake's shape. The existing `fakeInteraction` (line 41) shows the captured-reply pattern to follow.
+
+### Lobby Join is also a button interaction
+Lobby **Join**/**Start** go through `lobbyHandler` (`src/bot/handlers/lobbyHandler.ts`) via button `customId`s. Read `lobbyHandler.ts` + `src/bot/embeds/lobby.ts` to learn the customId prefixes and what the handler reads from the interaction (likely `interaction.user.id`, `interaction.channel`, `interaction.guildId`). The same `fakeButton`-with-userId extension drives Join for both players.
+
+### Fixture spec location vs `loadSpec`
+`loadSpec(name)` reads `${config.SPECS_DIR}/${name}.yaml` (`SPECS_DIR` defaults to `./specs`). The fixture is `tests/fixtures/spec-group-e2e.yaml`. Options (pick one, document in the test):
+- **(a)** Put the fixture in `specs/spec-group-e2e.yaml` — simplest, `loadSpec('spec-group-e2e')` works, but it joins the production corpus (the `specsToolsConsistency` test will scan it — fine, it'll pass, but it's an E2E-only spec in the prod dir).
+- **(b)** Keep it in `tests/fixtures/` and set `SPECS_DIR=./tests/fixtures` for the live run (env), then `loadSpec('spec-group-e2e')`.
+- **(c)** Bypass `loadSpec`; parse the fixture directly with `parseYaml` + `EncounterSpecSchema.parse` (matches `specsToolsConsistency`'s pattern).
+**Recommend (c)** — keeps the E2E fixture out of the prod corpus and avoids env-juggling. Confirm `EncounterSpecSchema` is exported from `src/spec/loader.ts` (it is).
+
+### Real messages vs `runLLMTurn`
+The existing AC2 (`encounter-lifecycle.test.ts` S2.2) drives turns by appending to history + calling `runLLMTurn` directly, because bot-authored messages were filtered. **This story's whole point is that player-bot messages now route for real** — so AC-9 must post via `bots.players[i].send(...)` (real gateway) and assert the turn flows through `handleMessage` → `processEncounterMessage` → history. Do NOT fall back to the `runLLMTurn` shortcut for the chat-turn AC. (You may still use `runLLMTurn` directly if needed to nudge the LLM to emit the group check, but the chat turns themselves must be real gateway posts.)
+
+### Test discipline (from the existing harness)
+- `import './support/env.js'` FIRST (stubs `DISCORD_TOKEN`/`DISCORD_CLIENT_ID` only if absent; a real `.env` wins).
+- Assert on STRUCTURAL outcomes (session state, thread existence, history roles/lengths, GraphMCP read-after-write) — never exact narrative text.
+- Use `waitFor` from `./support/poll.js` with generous timeouts (60–150s) for gateway/LLM/Redis async.
+- `flushRedisForGuild` + `deleteThread` in cleanup; destroy all clients.
+- Serialized runs — do not add `concurrent` to the multiplayer suite.
+
+### Anti-patterns to avoid
+- Do NOT add `E2E_PLAYER2_TOKEN` to `config.ts` (it's a test-only secret; read it from `process.env` in `liveBots.ts`, like `E2E_DRIVER_TOKEN`).
+- Do NOT hardcode bot userIds anywhere.
+- Do NOT touch the 4 gap-case ACs (FR-11–14) — separate follow-up story.
+- Do NOT lower `velvet-auction`'s `minPlayers` — the fixture spec is the E2E target; velvet-auction stays `minPlayers: 3`.
+- Do NOT commit `.env` or any token value.
+
+## Project Structure Notes
+
+New files:
+- `src/bot/lib/e2ePlayerAllowlist.ts` — runtime player-bot id set (gated by config flag).
+- `tests/unit/e2ePlayerAllowlist.test.ts` — unit cover for the allowlist branch.
+- `tests/fixtures/spec-group-e2e.yaml` — `minPlayers: 2` E2E group spec.
+- `tests/integration/graphmcp/group-encounter-live.test.ts` — MVP live multiplayer test.
+
+Updated files:
+- `src/config.ts` — `E2E_ALLOW_PLAYER_BOTS` (default false).
+- `src/bot/handlers/messageRouter.ts` — anti-loop allowlist branch (line 33).
+- `tests/integration/graphmcp/support/liveBots.ts` — N-player + `driverBot` alias.
+- `tests/integration/graphmcp/support/fakes.ts` — `fakeButton` carries userId.
+
+Naming follows existing conventions: `src/bot/lib/` for bot-layer helpers (cf. `welcomeDM.ts`), `tests/integration/graphmcp/support/` for live fixtures, `tests/fixtures/` for spec fixtures.
+
+## References
+
+- PRD: `_bmad-output/prds/prd-multiplayer-e2e-second-bot-2026-06-22/prd.md` (FR-1…FR-10, NFR-1…NFR-6).
+- Existing live harness: `tests/integration/graphmcp/encounter-lifecycle.test.ts` (AC2 — the pattern to extend), `support/liveBots.ts`, `support/fakes.ts`, `support/env.ts`, `support/cleanup.ts`, `support/poll.ts`.
+- Anti-loop guard: `src/bot/handlers/messageRouter.ts:32-33`.
+- Config parse-once: `src/config.ts:90` (`export const config = EnvSchema.parse(process.env)`).
+- Group-encounters PRD FR-15 (`successRule` semantics: `≥ ceil(N/2)`), FR-43 (single player-locked Roll), FR-44 (group-check Redis), FR-45 (idempotency lock): `_bmad-output/prds/prd-mardonar-encounter-engine-2026-06-20/prd.md`.
+- Roll/lobby handlers to read before faking buttons: `src/bot/handlers/rollHandler.ts`, `src/bot/handlers/lobbyHandler.ts`, `src/harness/groupCheckManager.ts`.
+- Spec format: `specs/SPEC_FORMAT.md`; schema source: `src/spec/loader.ts` (`EncounterSpecSchema`).
+- FU-9 playtest checklist this complements: `docs/release-playtest-checklist.md`.
+
+## Dev Agent Record
+
+### Agent Model Used
+_(filled by dev agent)_
+
+### Debug Log References
+_(filled by dev agent)_
+
+### Completion Notes List
+_(filled by dev agent)_
+
+### File List
+_(filled by dev agent)_
+
+---
+
+_Implementation note: this story was created directly (not via the full BMad sprint workflow) because the project has no `_bmad/` sprint-tracking infrastructure (no `sprint-status.yaml`, no `epics.md` in the BMad layout). It follows the BMad story template and is grounded in the actual files touched. Scope is the MVP vertical slice; the 4 gap-case ACs and the doc update are follow-up stories._