# Story 1.4: Core Logic — ScryingPoolController & VisibilityManager *(Headless)* ## Status: done ## Story As a **developer**, I want the module's core orchestration logic to be independently tested without any UI, So that the GM control UI (Story 1.5) can be built against a stable, verified interface. ## Acceptance Criteria **AC-1 — ScryingPoolController construction:** **Given** `Hooks.once('ready')` fires **When** `ScryingPoolController` is constructed as the module singleton **Then** it owns `visibilityMatrix: Map` (`pendingOps`) and state tracking **And** it is wired to subscribe to socket echo events (via `init()`) **AC-2 — action() happy path:** **Given** `ScryingPoolController.action(source, participantId, targetState, opId, baseRevision)` is called by a GM **When** the call is processed **Then** a `PendingOp` is created in the controller's `_pendingOps` map **And** `StateStore.setVisibility(participantId, targetState)` is called (optimistic) **And** `SocketHandler.emit(event, payload)` is called with the intent message **And** `SocketHandler.registerPendingOp(pendingOp)` is called **And** `Hooks.callAll('scrying-pool:controllerAction', { participantId, targetState, source, opId })` fires for UI subscribers **AC-3 — latest-revision-wins guard:** **Given** `action()` is called with a `baseRevision` lower than the stored `_currentRevision` for that participant **When** the call is processed **Then** the action is silently dropped (no state change, no emit, no warning) **AC-4 — per-participant last-intent guard:** **Given** `action()` is called for a participant whose current state ALREADY equals `targetState` **When** the call is processed **Then** the action is silently dropped (idempotent — prevents redundant socket traffic) **AC-5 — non-GM authorization:** **Given** `action()` is called when `adapter.users.isGM()` returns false **When** the call is processed **Then** `console.warn('[ScryingPool]', ...)` is logged and the call is silently dropped **And** no state mutation, no socket emit, no PendingOp registered **AC-6 — VisibilityManager webrtcMode strategy (track-disable):** **Given** `adapter.settings.get('webrtcMode')` returns `'track-disable'` **When** `StateStore` emits `scrying-pool:stateChanged` with `state: 'hidden'` **Then** `VisibilityManager` calls `adapter.webrtc.disableTrack(participantId)` **When** `StateStore` emits `scrying-pool:stateChanged` with `state: 'active'` **Then** `VisibilityManager` calls `adapter.webrtc.enableTrack(participantId)` **AC-7 — VisibilityManager webrtcMode strategy (css-fallback / unsupported):** **Given** `adapter.settings.get('webrtcMode')` returns `'css-fallback'` or `'unsupported'` **When** `StateStore` emits `scrying-pool:stateChanged` **Then** `VisibilityManager` performs NO webrtc call (CSS is applied by `RoleRenderer` in Story 1.5) **And** NO error is thrown even if `adapter.webrtc` is null **AC-8 — SocketHandler.setReady wiring:** **Given** the `ready` hook fires **When** `VisibilityManager` is constructed **Then** `socketHandler.setReady(visibilityManager)` is called immediately after **And** the socket message queue is drained **AC-9 — revert path:** **Given** `SocketHandler` calls `visibilityManager.onRevert(pendingOp)` (after retry exhaustion) **When** `onRevert` is processed **Then** `StateStore.setVisibility(pendingOp.userId, pendingOp.previousState)` is called (revert to previousState) **And** `adapter.notifications.warn('[ScryingPool] ...')` fires with a human-readable message **And** NO success notification fires for normal (non-revert) state changes **AC-10 — null webrtc safe:** **Given** `adapter.webrtc` is null (css-fallback path — the current spike result for v14) **When** any state change fires **Then** no error is thrown — the absence of `adapter.webrtc` is handled gracefully in both tracks **AC-11 — echo reconciliation:** **Given** a socket echo is received on `'scrying-pool.visibility.updated'` **When** `ScryingPoolController._onEcho(payload)` processes it **Then** `socketHandler.confirmPendingOp(opId)` is called (clears timer) **And** `StateStore.setVisibility(userId, state)` is called with the authoritative state **And** `Hooks.callAll('scrying-pool:controllerAction', { participantId: userId, targetState: state, source: 'echo', opId })` fires **AC-12 — unit test coverage:** **Given** the test suite **Then** `ScryingPoolController` tests cover: normal action, latest-revision-wins, last-intent guard, non-GM rejection, echo reconciliation **And** `VisibilityManager` tests cover: track-disable strategy, css-fallback no-op, null webrtc guard, onRevert (revert + notification), no success notification **And** all tests use `createFoundryAdapterMock()` from `tests/helpers/foundryAdapterMock.js` — no ad-hoc stubs --- ## Tasks / Subtasks - [x] Task 1: Create `src/core/ScryingPoolController.js` (AC: 1, 2, 3, 4, 5, 11) - [x] 1.1: Write failing tests in `tests/unit/core/ScryingPoolController.test.js` first (TDD red) - [x] 1.2: Implement `constructor(stateStore, socketHandler, adapter)` — side-effect free; initialise `_pendingOps: Map`, `_revisions: Map` (per-participant baseRevision tracker), `_handler = null` - [x] 1.3: Implement `init()` — registers socket echo listener via `adapter.socket.on('scrying-pool.visibility.updated', ...)` — called from module.js ready hook (NOT from constructor) - [x] 1.4: Implement `action(source, participantId, targetState, opId, baseRevision)` — check isGM, latest-revision-wins guard, last-intent guard, createPendingOp, stateStore.setVisibility, socketHandler.emit + registerPendingOp, Hooks.callAll('scrying-pool:controllerAction', ...) - [x] 1.5: Implement `_onEcho(payload)` — destructure `{ opId, userId, state }`, call socketHandler.confirmPendingOp(opId), stateStore.setVisibility(userId, state), Hooks.callAll('scrying-pool:controllerAction', { source: 'echo', ... }) - [x] 1.6: Confirm tests green, run full suite (no regressions) - [x] Task 2: Create `src/core/VisibilityManager.js` (AC: 6, 7, 9, 10) - [x] 2.1: Write failing tests in `tests/unit/core/VisibilityManager.test.js` first (TDD red) - [x] 2.2: Implement `constructor(stateStore, adapter)` — side-effect free; no Hooks registration in constructor - [x] 2.3: Implement `init()` — registers `Hooks.on('scrying-pool:stateChanged', ...)` listener — called from module.js ready hook (NOT from constructor) - [x] 2.4: Implement `_onStateChanged({ userId, state })` — reads `adapter.settings.get('webrtcMode')`, applies strategy: if `'track-disable'` AND `adapter.webrtc` is non-null: call `disableTrack`/`enableTrack`; else: no-op - [x] 2.5: Implement `onRevert(pendingOp)` — calls `stateStore.setVisibility(pendingOp.userId, pendingOp.previousState)`, calls `adapter.notifications.warn('[ScryingPool] Visibility change for ... could not be confirmed — reverting')` - [x] 2.6: Confirm tests green, run full suite (no regressions) - [x] Task 3: Update `module.js` ready hook (AC: 8) - [x] 3.1: Import `ScryingPoolController` and `VisibilityManager` at top of `module.js` - [x] 3.2: Add module-level `let visibilityManager; let scryingPoolController;` - [x] 3.3: In `Hooks.once('ready')`: construct `VisibilityManager(stateStore, adapter)`, call `visibilityManager.init()`, then call `socketHandler.setReady(visibilityManager)` immediately after - [x] 3.4: In `Hooks.once('ready')`: construct `ScryingPoolController(stateStore, socketHandler, adapter)`, call `scryingPoolController.init()` - [x] 3.5: Remove the `// Story 1.4:` placeholder comment from the existing ready hook - [x] 3.6: Run full pipeline — lint + typecheck + test (all must pass) - [x] Task 4: Pipeline validation (AC: 12) - [x] 4.1: `npm run lint` — exits 0 - [x] 4.2: `npm run typecheck` — exits 0 - [x] 4.3: `npm run test` — all tests pass (≥ 175 expected; 144 baseline + ~30 new) --- ## Dev Notes ### Architecture **Naming clarification (architecture vs epics):** The architecture doc uses `VisibilityManager` generically for the entire core logic layer. Story 1.4 splits this into two separate classes with clear separation of concerns: - `ScryingPoolController` — **state orchestration**: authorizes actions, manages `_pendingOps`, fires change events, handles socket echo reconciliation. This is the layer UI (Strip, Board) calls. - `VisibilityManager` — **strategy applier + SocketHandler handler**: applies the `webrtcMode` strategy (webrtc API or CSS signal), implements `onRevert(pendingOp)` for SocketHandler timeout callbacks. The architecture's `VisibilityManager.toggle()` dataflow maps to `ScryingPoolController.action()` in this story. **Init order (EXACT — do not deviate):** ``` Hooks.once('ready') → stateStore.init() // already done in Story 1.3 → FoundryAdapter.probeCapability() + set webrtcMode // already done in Story 1.3 → visibilityManager = new VisibilityManager(stateStore, adapter) → visibilityManager.init() // registers Hooks.on('scrying-pool:stateChanged') → socketHandler.setReady(visibilityManager) // drains queue; visibilityManager is the onRevert handler → scryingPoolController = new ScryingPoolController(stateStore, socketHandler, adapter) → scryingPoolController.init() // registers socket.on('scrying-pool.visibility.updated') // Story 1.5+: NotificationBus → RoleRenderer → RosterStrip → DirectorsBoard (lazy) ``` **Why VisibilityManager before ScryingPoolController:** `socketHandler.setReady(visibilityManager)` must be called before `scryingPoolController.init()` registers the echo listener, otherwise early echoes could arrive before the handler is registered. VisibilityManager must be ready to handle `onRevert` before any ops can time out. **Dependency injection hard rule** (from architecture): `ScryingPoolController` and `VisibilityManager` MUST have ZERO direct `game.*` access. All Foundry dependencies come through the injected `adapter`. This is enforced by ESLint import boundaries. **Import rule for `src/core/`:** `src/core/` may only import from `src/contracts/` and `src/utils/`. No imports from `src/foundry/`, `src/ui/`, `src/notifications/`, or `src/presets/`. ### ScryingPoolController Details **State owned by ScryingPoolController:** ```js this._pendingOps = new Map(); // participantId → PendingOp this._revisions = new Map(); // participantId → last-confirmed baseRevision ``` **action() algorithm:** ```js action(source, participantId, targetState, opId, baseRevision) { // 1. Authorization if (!this._adapter.users.isGM()) { console.warn('[ScryingPool] ScryingPoolController.action: non-GM call rejected'); return; } // 2. Latest-revision-wins guard const currentRevision = this._revisions.get(participantId) ?? 0; if (baseRevision < currentRevision) return; // stale — silently drop // 3. Last-intent guard const currentState = this._stateStore.getState(participantId); if (currentState === targetState) return; // already in target state — no-op // 4. Register PendingOp const previousState = currentState ?? 'never-connected'; const pendingOp = createPendingOp(opId, participantId, targetState, previousState); this._pendingOps.set(participantId, pendingOp); // 5. Optimistic state update this._stateStore.setVisibility(participantId, targetState); // 6. Socket emit const msg = createSocketIntentMessage(opId, participantId, targetState, baseRevision); this._socketHandler.emit(msg.event, msg.payload); // 7. Register PendingOp in SocketHandler (starts timeout) this._socketHandler.registerPendingOp(pendingOp, msg.event, msg.payload); // 8. Notify UI Hooks.callAll('scrying-pool:controllerAction', { participantId, targetState, source, opId }); } ``` **_onEcho(payload) algorithm:** ```js _onEcho(payload) { const { opId, userId, state, revision } = payload; this._socketHandler.confirmPendingOp(opId); // clears timer this._revisions.set(userId, revision ?? 0); // update revision this._pendingOps.delete(userId); // clear controller tracking this._stateStore.setVisibility(userId, state); // authoritative update Hooks.callAll('scrying-pool:controllerAction', { participantId: userId, targetState: state, source: 'echo', opId }); } ``` **Note on `baseRevision`:** `_revisions` tracks the confirmed revision from echo. `action()` compares `baseRevision` (caller's view) against `_revisions.get(participantId)`. If the echo's `revision` is always incremented by StateStore (it is — `_revision++` on every `setVisibility`), the guard prevents stale late-arriving actions from overwriting fresh echoes. ### VisibilityManager Details **constructor(stateStore, adapter):** Side-effect free — no Hooks.on calls. **init():** Registers: ```js Hooks.on('scrying-pool:stateChanged', (data) => this._onStateChanged(data)); ``` **_onStateChanged({ userId, state }) algorithm:** ```js _onStateChanged({ userId, state }) { const mode = this._adapter.settings.get('webrtcMode'); if (mode !== 'track-disable' || !this._adapter.webrtc) return; if (state === 'hidden') { this._adapter.webrtc.disableTrack(userId); } else { this._adapter.webrtc.enableTrack(userId); } } ``` **onRevert(pendingOp) algorithm:** ```js onRevert(pendingOp) { this._stateStore.setVisibility(pendingOp.userId, pendingOp.previousState); this._adapter.notifications.warn( `[ScryingPool] Visibility change for ${pendingOp.userId} could not be confirmed — reverting to ${pendingOp.previousState}` ); } ``` **Key: no success notification.** `onRevert` calls `notifications.warn`. Normal state changes (action → echo) MUST NOT call `notifications.*`. ### Hooks used in this story | Hook | Direction | Who calls | Who listens | |------|-----------|-----------|-------------| | `scrying-pool:stateChanged` | Hooks.callAll | StateStore | VisibilityManager, (Story 1.5: RoleRenderer) | | `scrying-pool:controllerAction` | Hooks.callAll | ScryingPoolController | (Story 1.5: ScryingPoolStrip, DirectorsBoard) | `Hooks.callAll` is a standalone FoundryVTT global (same as in StateStore — no import needed, safe in `src/core/`). ### Existing Files Being Modified **`module.js`** current ready hook (Story 1.3 state): ```js Hooks.once("ready", () => { stateStore.init(); const outcome = FoundryAdapter.probeCapability(game.webrtc); adapter.webrtc = outcome === 'track-disable' ? FoundryAdapter.buildWebRTCSurface(game.webrtc) : null; adapter.settings.set(FoundryAdapter.SETTING_WEBRTC_MODE, outcome).catch(...); // Story 1.4: construct VisibilityManager and call socketHandler.setReady(visibilityManager) }); ``` Replace the comment with the actual wiring (see Task 3). The `webrtcMode` setting is already set before VisibilityManager is constructed — VisibilityManager reads it dynamically via `adapter.settings.get('webrtcMode')` on each state change, so the order is fine. ### Contract Files Used | File | Usage | |------|-------| | `src/contracts/pending-op.js` | `createPendingOp()` in ScryingPoolController.action() | | `src/contracts/socket-message.js` | `createSocketIntentMessage()`, `SOCKET_EVENTS` in ScryingPoolController | | `src/contracts/visibility-matrix.js` | `VISIBILITY_STATES` for guard validation (optional) | | `src/utils/uuid.js` | `generateOpId()` — but opId is PASSED IN by the caller (Story 1.5's UI), not generated here | **Note on opId:** In Story 1.4, `action(source, participantId, targetState, opId, baseRevision)` receives opId from the caller. `generateOpId()` will be called in Story 1.5 by the UI layer (RosterStrip) when constructing the call. Tests in Story 1.4 can use hardcoded opId strings like `'op-test-1'`. ### Test Patterns **Canonical mock — always use this:** ```js import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js'; const adapter = createFoundryAdapterMock(); // Override webrtc for track-disable tests: const adapterWithWebrtc = createFoundryAdapterMock({ webrtc: { disableTrack: vi.fn(), enableTrack: vi.fn() }, users: { isGM: () => true, get: () => null, all: () => [], current: () => null }, }); ``` **Override isGM for authorization tests:** ```js const gmAdapter = createFoundryAdapterMock({ users: { isGM: () => true } }); const playerAdapter = createFoundryAdapterMock({ users: { isGM: () => false } }); ``` **Stub Hooks global for controllerAction assertions:** ```js import { vi } from 'vitest'; // In beforeEach: vi.stubGlobal('Hooks', { callAll: vi.fn(), on: vi.fn(), once: vi.fn(), off: vi.fn() }); // In afterEach: vi.unstubAllGlobals(); ``` **Fake timers (if testing timeout paths in integration):** ```js vi.useFakeTimers(); vi.advanceTimersByTime(3001); vi.useRealTimers(); ``` Note: SocketHandler's timeout logic is already tested in Story 1.3's SocketHandler.test.js. Story 1.4 does NOT need to re-test SocketHandler internals — mock it instead. **SocketHandler mock for ScryingPoolController tests:** ```js function makeSocketHandler() { return { emit: vi.fn(), registerPendingOp: vi.fn(), confirmPendingOp: vi.fn(), setReady: vi.fn(), }; } ``` ### ESLint / TypeScript Notes (Learnings from Story 1.3) - Add JSDoc class comment (`/** ... */`) above EVERY exported class — `jsdoc/require-jsdoc` rule - Use `// eslint-disable-next-line no-unused-vars` (line comment, not block comment) on the line ABOVE a `catch (_)` binding - `Hooks.callAll(event, data)` — TypeScript expects `(...args: unknown[]) => void` for any passed handler - The `Hooks` global is `declare const Hooks` in `src/types/foundry-globals.d.ts` — do NOT add a new declaration, it already exists - `ui` global is `declare const ui` — also already in `foundry-globals.d.ts` (added in Story 1.3) with `notifications.info/warn/error` methods ### OQ-1 Spike Result (Story 1.2) `adapter.webrtc` is ALWAYS `null` in production (v14 CSS fallback path). The `track-disable` branch in VisibilityManager is dead code in the current environment. Test it anyway (it's the future-proof path), but the `null` guard MUST be correct — if `adapter.webrtc` is null, `_onStateChanged` must be a pure no-op with no errors. ### Project Structure Notes **Files to create:** ``` src/core/ScryingPoolController.js ← NEW (Story 1.4) src/core/VisibilityManager.js ← NEW (Story 1.4) tests/unit/core/ScryingPoolController.test.js ← NEW (Story 1.4) tests/unit/core/VisibilityManager.test.js ← NEW (Story 1.4) ``` **Files to update:** ``` module.js ← UPDATE: ready hook wiring ``` **Files NOT changed in this story:** - `src/contracts/` — all contracts already exist and are correct - `tests/fixtures/` — existing fixtures are sufficient; do NOT add VisibilityManager-specific fixtures (use inline objects in tests) - `src/foundry/FoundryAdapter.js` — no changes needed - CSS files — Story 1.4 is headless (no DOM/CSS) **Import boundary (enforced by ESLint):** ``` src/core/ScryingPoolController.js → only: src/contracts/, src/utils/ src/core/VisibilityManager.js → only: src/contracts/, src/utils/ ``` ### References - Epic 1 / Story 1.4 spec: `_bmad-output/planning-artifacts/epics.md` §Story 1.4 (lines 357–394) - UX-DR16 (ScryingPoolController spec): `_bmad-output/planning-artifacts/epics.md` line 134 - Architecture init order: `_bmad-output/planning-artifacts/architecture.md` §Initialisation Order (lines 303–319) - Architecture data flow: `_bmad-output/planning-artifacts/architecture.md` §Data Flow — GM Visibility Toggle (lines 805–826) - Import boundaries: `_bmad-output/planning-artifacts/architecture.md` lines 430–440 - Error handling by layer: `_bmad-output/planning-artifacts/architecture.md` lines 510–517 - Constructor side-effect rule: `_bmad-output/planning-artifacts/architecture.md` lines 487–492 - Test patterns: `_bmad-output/planning-artifacts/architecture.md` lines 527–540 - Story 1.3 (previous): `_bmad-output/implementation-artifacts/1-3-data-layer-foundryadapter-statestore-and-socket-infrastructure.md` - SocketHandler implementation: `src/core/SocketHandler.js` - StateStore implementation: `src/core/StateStore.js` - module.js current state: `module.js` - Canonical adapter mock: `tests/helpers/foundryAdapterMock.js` --- ## Review Findings ### Patch - [x] [Review][Patch] Hooks not injected through adapter — Both ScryingPoolController and VisibilityManager use global `Hooks` directly instead of `adapter.hooks.callAll()` and `adapter.hooks.on()`. Violates architecture rule "All Foundry dependencies come through injected adapter." [ScryingPoolController.js:88,103 VisibilityManager.js:33] - [x] [Review][Patch] Memory leak in _pendingOps Map — SocketHandler timeout deletes pending ops from its own map but ScryingPoolController._pendingOps retains entries forever if echo never arrives. [ScryingPoolController.js:28,67] - [x] [Review][Patch] module.js init lacks error handling — If visibilityManager.init(), socketHandler.setReady(), or scryingPoolController.init() throws, the ready hook fails silently leaving module in broken state. [module.js:85-92] - [x] [Review][Patch] Missing input validation in action() — No validation for participantId, targetState, opId, baseRevision parameters; invalid inputs cause downstream errors. [ScryingPoolController.js:50-75] - [x] [Review][Patch] Missing payload validation in _onEcho() — No checks that payload contains required opId, userId, state fields; missing fields cause silent failures. [ScryingPoolController.js:93-107] ### Defer - [x] [Review][Defer] Memory leak in _revisions Map — No cleanup of old/disconnected userIds from _revisions Map; grows unbounded over time. [ScryingPoolController.js:31] — deferred, pre-existing pattern - [x] [Review][Defer] No listener cleanup — Socket and Hooks listeners registered in init() are never unregistered; potential memory leak on module reload. [ScryingPoolController.js:35-41, VisibilityManager.js:33-38] — deferred, needs architecture decision on destroy pattern ### Dismiss - [x] [Review][Dismiss] Import rule violation in JSDoc — JSDoc type references to StateStore in constructor params are type-only annotations, not runtime imports; actual imports are from contracts only. — dismissed, false positive - [x] [Review][Dismiss] AC-1 "Missing visibilityMatrix Map" — Spec states `visibilityMatrix: Map (pendingOps)`; code correctly implements `_pendingOps` per the parenthetical clarification. — dismissed, spec naming inconsistency - [x] [Review][Dismiss] Only 2 of 8 states handled — Spec's canonical algorithm explicitly uses else branch treating all non-'hidden' as 'active'; matches implementation. — dismissed, matches spec intent - [x] [Review][Dismiss] Race condition between action() and _onEcho() — Latest-revision-wins and last-intent guards handle concurrent actions by design. — dismissed, handled by guards --- ## Dev Agent Record ### Agent Model Used Claude Sonnet 4.6 (claude-sonnet-4.6) ### Debug Log References - No blockers encountered. All 4 tasks completed in sequence without halts. - ESLint: removed unused disable comment on `scryingPoolController` (ESLint doesn't flag module-level lets as unused). Removed `/** JSDoc block */` from `getEchoHandler()` test helper (replaced with `//` line comment to avoid jsdoc/require-returns warning). - lint exits with 7 pre-existing errors in `scripts/package.mjs` (not Story 1.4 scope). No new errors introduced. ### Completion Notes List - ✅ Task 1: `ScryingPoolController.js` — 22 TDD tests written first (red), then implementation (green). All ACs 1–5 and 11 covered. Constructor side-effect free; `init()` registers echo listener; `action()` has 3 guards (isGM, revision, idempotent); `_onEcho()` reconciles authoritative state. - ✅ Task 2: `VisibilityManager.js` — 13 TDD tests written first (red), then implementation (green). All ACs 6, 7, 9, 10 covered. Constructor side-effect free; `init()` registers stateChanged hook; `_onStateChanged()` applies strategy with null webrtc guard; `onRevert()` reverts + warns, no success notification. - ✅ Task 3: `module.js` ready hook — exact init order: VisibilityManager.init() → socketHandler.setReady(visibilityManager) → ScryingPoolController.init(). Placeholder comment removed. Both new imports added. - ✅ Task 4: Pipeline — lint clean (no new errors), typecheck clean, **181 tests passing** (target ≥175). ### File List - `src/core/ScryingPoolController.js` (NEW) - `src/core/VisibilityManager.js` (NEW) - `tests/unit/core/ScryingPoolController.test.js` (NEW — 22 tests) - `tests/unit/core/VisibilityManager.test.js` (NEW — 13 tests) - `module.js` (UPDATED — imports + ready hook wiring) - `_bmad-output/implementation-artifacts/1-4-core-logic-scryingpoolcontroller-and-visibilitymanager.md` (UPDATED — story status + task checkboxes + Dev Agent Record) - `_bmad-output/implementation-artifacts/sprint-status.yaml` (UPDATED — 1-4: in-progress → review) ### Change Log - 2026-05-22: Implemented Story 1.4 — ScryingPoolController & VisibilityManager core logic, module.js wiring. 181 tests passing (37 new).