Files
scrying-pool/_bmad-output/implementation-artifacts/1-4-core-logic-scryingpoolcontroller-and-visibilitymanager.md
2026-05-23 18:23:48 +02:00

25 KiB
Raw Permalink Blame History

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<string, PendingOp> (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

  • Task 1: Create src/core/ScryingPoolController.js (AC: 1, 2, 3, 4, 5, 11)

    • 1.1: Write failing tests in tests/unit/core/ScryingPoolController.test.js first (TDD red)
    • 1.2: Implement constructor(stateStore, socketHandler, adapter) — side-effect free; initialise _pendingOps: Map<string, PendingOp>, _revisions: Map<string, number> (per-participant baseRevision tracker), _handler = null
    • 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)
    • 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', ...)
    • 1.5: Implement _onEcho(payload) — destructure { opId, userId, state }, call socketHandler.confirmPendingOp(opId), stateStore.setVisibility(userId, state), Hooks.callAll('scrying-pool:controllerAction', { source: 'echo', ... })
    • 1.6: Confirm tests green, run full suite (no regressions)
  • Task 2: Create src/core/VisibilityManager.js (AC: 6, 7, 9, 10)

    • 2.1: Write failing tests in tests/unit/core/VisibilityManager.test.js first (TDD red)
    • 2.2: Implement constructor(stateStore, adapter) — side-effect free; no Hooks registration in constructor
    • 2.3: Implement init() — registers Hooks.on('scrying-pool:stateChanged', ...) listener — called from module.js ready hook (NOT from constructor)
    • 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
    • 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')
    • 2.6: Confirm tests green, run full suite (no regressions)
  • Task 3: Update module.js ready hook (AC: 8)

    • 3.1: Import ScryingPoolController and VisibilityManager at top of module.js
    • 3.2: Add module-level let visibilityManager; let scryingPoolController;
    • 3.3: In Hooks.once('ready'): construct VisibilityManager(stateStore, adapter), call visibilityManager.init(), then call socketHandler.setReady(visibilityManager) immediately after
    • 3.4: In Hooks.once('ready'): construct ScryingPoolController(stateStore, socketHandler, adapter), call scryingPoolController.init()
    • 3.5: Remove the // Story 1.4: placeholder comment from the existing ready hook
    • 3.6: Run full pipeline — lint + typecheck + test (all must pass)
  • Task 4: Pipeline validation (AC: 12)

    • 4.1: npm run lint — exits 0
    • 4.2: npm run typecheck — exits 0
    • 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:

  • ScryingPoolControllerstate orchestration: authorizes actions, manages _pendingOps, fires change events, handles socket echo reconciliation. This is the layer UI (Strip, Board) calls.
  • VisibilityManagerstrategy 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:

this._pendingOps = new Map();      // participantId → PendingOp
this._revisions  = new Map();      // participantId → last-confirmed baseRevision

action() algorithm:

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:

_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:

Hooks.on('scrying-pool:stateChanged', (data) => this._onStateChanged(data));

_onStateChanged({ userId, state }) algorithm:

_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:

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):

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:

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:

const gmAdapter = createFoundryAdapterMock({ users: { isGM: () => true } });
const playerAdapter = createFoundryAdapterMock({ users: { isGM: () => false } });

Stub Hooks global for controllerAction assertions:

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):

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:

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 357394)
  • UX-DR16 (ScryingPoolController spec): _bmad-output/planning-artifacts/epics.md line 134
  • Architecture init order: _bmad-output/planning-artifacts/architecture.md §Initialisation Order (lines 303319)
  • Architecture data flow: _bmad-output/planning-artifacts/architecture.md §Data Flow — GM Visibility Toggle (lines 805826)
  • Import boundaries: _bmad-output/planning-artifacts/architecture.md lines 430440
  • Error handling by layer: _bmad-output/planning-artifacts/architecture.md lines 510517
  • Constructor side-effect rule: _bmad-output/planning-artifacts/architecture.md lines 487492
  • Test patterns: _bmad-output/planning-artifacts/architecture.md lines 527540
  • 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

  • [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]
  • [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]
  • [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]
  • [Review][Patch] Missing input validation in action() — No validation for participantId, targetState, opId, baseRevision parameters; invalid inputs cause downstream errors. [ScryingPoolController.js:50-75]
  • [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

  • [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
  • [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

  • [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
  • [Review][Dismiss] AC-1 "Missing visibilityMatrix Map" — Spec states visibilityMatrix: Map<string, PendingOp> (pendingOps); code correctly implements _pendingOps per the parenthetical clarification. — dismissed, spec naming inconsistency
  • [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
  • [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 15 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).