Story 3.2 done

2026-05-23 18:23:48 +02:00
parent d175f92806
commit a1e8886fce
66 changed files with 18258 additions and 1650 deletions
@@ -0,0 +1,841 @@
+# Blind Hunter Review Prompt - Story 1-5 Group 1 (Core Logic)
+
+**Story:** 1-5-gm-control-ui-scryingpoolstrip-actionpopover-and-av-tile-integration  
+**Group:** Core Logic (Group 1 of 4)  
+**Files:** ScryingPoolController.js, VisibilityManager.js, and their tests  
+**Diff lines:** 804
+
+---
+
+## YOUR ROLE: Blind Hunter
+
+You are a **Blind Hunter** code reviewer. You receive **ONLY** the diff below — no spec, no context, no project access. Your job is to find security issues, bugs, anti-patterns, performance problems, and code quality issues using ONLY what you see in the diff.
+
+### Rules:
+- You have NO access to the spec file
+- You have NO access to the project repository
+- You have NO context about the project's goals
+- You MUST find at least 5 issues
+- Be adversarial: assume the code has problems and find them
+- Focus on: security vulnerabilities, race conditions, error handling gaps, performance issues, code smells, test coverage gaps
+
+### Output Format:
+Output findings as a Markdown list. Each finding:
+```markdown
+- **SEV-XX: [One-line title]** — [Evidence from diff] — [Impact]
+```
+
+Classify severity:
+- **SEV-CRITICAL**: Security vulnerability, data loss, crash
+- **SEV-HIGH**: Race condition, resource leak, incorrect behavior
+- **SEV-MEDIUM**: Code smell, maintainability issue
+- **SEV-LOW**: Style, minor improvement
+
+---
+
+## DIFF TO REVIEW
+
+diff --git a/src/core/ScryingPoolController.js b/src/core/ScryingPoolController.js
+new file mode 100644
+index 0000000..fc013f0
+--- /dev/null
+++ b/src/core/ScryingPoolController.js
+@@ -0,0 +1,181 @@
+/**
+ * ScryingPoolController — Orchestrates GM visibility actions with optimistic state updates.
+ *
+ * Handles: GM authorization, latest-revision-wins guard, last-intent guard, PendingOp
+ * lifecycle, optimistic setVisibility, socket emit, and echo reconciliation.
+ *
+ * Import rule: may only import from src/contracts/ and src/utils/.
+ * Constructors are side-effect free — call init() from module.js Hooks.once('ready').
+ *
+ * @module core/ScryingPoolController
+ */
+
+import { createPendingOp } from '../contracts/pending-op.js';
+import { createSocketIntentMessage, SOCKET_EVENTS } from '../contracts/socket-message.js';
+
+/**
+ * Orchestrates GM visibility actions: auth, optimistic state, socket emit, echo reconciliation.
+ */
+export class ScryingPoolController {
+  /**
+   * @param {import('./StateStore.js').StateStore} stateStore
+   * @param {{ emit(event: string, payload: object): void, registerPendingOp(op: object, event: string, payload: object): void, confirmPendingOp(opId: string): void, setReady(handler: object): void }} socketHandler
+   * @param {{ users: { isGM(): boolean }, socket: { on(event: string, handler: (...args: unknown[]) => void): void }, hooks: { on(event: string, handler: (...args: unknown[]) => void): void, callAll(event: string, data: unknown): void } }} adapter
+   */
+  constructor(stateStore, socketHandler, adapter) {
+    this._stateStore = stateStore;
+    this._socketHandler = socketHandler;
+    this._adapter = adapter;
+    /** @type {Map<string, import('../contracts/pending-op.js').PendingOp>} participantId → PendingOp */
+    this._pendingOps = new Map();
+    /** @type {Map<string, number>} participantId → last-confirmed revision */
+    this._revisions = new Map();
+  }
+
+  /**
+   * Registers the socket echo listener.
+   * Called from module.js Hooks.once('ready') — NOT from constructor.
+   */
+  init() {
+    this._adapter.socket.on(
+      SOCKET_EVENTS.VISIBILITY_UPDATED,
+      (payload) => this._onEcho(/** @type {any} */ (payload))
+    );
+  }
+
+  /**
+   * Returns the last confirmed revision for a participant (0 if unknown).
+   * @param {string} participantId
+   * @returns {number}
+   */
+  getRevision(participantId) {
+    return this._revisions.get(participantId) ?? 0;
+  }
+
+  /**
+   * Returns true if a pending op is currently in-flight for the given participant.
+   * @param {string} participantId
+   * @returns {boolean}
+   */
+  hasPendingOp(participantId) {
+    return this._pendingOps.has(participantId);
+  }
+
+  /**
+   * Cleans up a pending operation by userId.
+   * Called by SocketHandler timeout callback via composite handler in module.js.
+   * @param {string} userId
+   */
+  cleanupPendingOp(userId) {
+    this._pendingOps.delete(userId);
+  }
+
+  /**
+   * Processes a GM visibility toggle request.
+   * Guards: isGM, latest-revision-wins, last-intent (idempotent).
+   *
+   * @param {string} source - Who triggered the action (e.g. 'ui', 'preset').
+   * @param {string} participantId - Target userId.
+   * @param {string} targetState - Desired VisibilityState.
+   * @param {string} opId - Unique operation ID (supplied by caller — Story 1.5 UI).
+   * @param {number} baseRevision - StateStore revision at time of intent.
+   */
+  action(source, participantId, targetState, opId, baseRevision) {
+    // 0. Input validation
+    if (!participantId || typeof participantId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid participantId');
+      return;
+    }
+    if (!targetState || typeof targetState !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid targetState');
+      return;
+    }
+    if (!opId || typeof opId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid opId');
+      return;
+    }
+    if (typeof baseRevision !== 'number' || !Number.isFinite(baseRevision) || baseRevision < 0) {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid baseRevision');
+      return;
+    }
+
+    // 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;
+
+    // 3. Last-intent guard (idempotent)
+    const currentState = this._stateStore.getState(participantId);
+    if (currentState === targetState) return;
+
+    // 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. Start acknowledgement timer
+    this._socketHandler.registerPendingOp(pendingOp, msg.event, msg.payload);
+
+    // 8. Notify UI subscribers
+    try {
+      this._adapter.hooks.callAll('scrying-pool:controllerAction', { participantId, targetState, source, opId });
+    } catch (hookErr) {
+      console.error('[ScryingPool] ScryingPoolController.action: hook emission failed', hookErr);
+    }
+  }
+
+  /**
+   * Processes an authoritative echo from the socket server.
+   * Confirms the pending op, updates revision, and sets the authoritative state.
+   * @private
+   * @param {{ opId: string, userId: string, state: string, revision?: number }} payload
+   */
+  _onEcho(payload) {
+    // Validate payload fields
+    if (!payload || typeof payload !== 'object') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: invalid payload');
+      return;
+    }
+    const { opId, userId, state, revision } = payload;
+    if (!opId || typeof opId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: missing or invalid opId');
+      return;
+    }
+    if (!userId || typeof userId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: missing or invalid userId');
+      return;
+    }
+    if (!state || typeof state !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: missing or invalid state');
+      return;
+    }
+
+    this._socketHandler.confirmPendingOp(opId);
+    this._revisions.set(userId, revision ?? 0);
+    this._pendingOps.delete(userId);
+    this._stateStore.setVisibility(userId, state);
+
+    try {
+      this._adapter.hooks.callAll('scrying-pool:controllerAction', {
+        participantId: userId,
+        targetState: state,
+        source: 'echo',
+        opId,
+      });
+    } catch (hookErr) {
+      console.error('[ScryingPool] ScryingPoolController._onEcho: hook emission failed', hookErr);
+    }
+  }
+}
+diff --git a/src/core/VisibilityManager.js b/src/core/VisibilityManager.js
+new file mode 100644
+index 0000000..0e465f2
+--- /dev/null
+++ b/src/core/VisibilityManager.js
+@@ -0,0 +1,104 @@
+/**
+ * VisibilityManager — WebRTC strategy applier and SocketHandler revert handler.
+ *
+ * Listens to `scrying-pool:stateChanged` hook events (emitted by StateStore) and
+ * applies the appropriate webrtcMode strategy:
+ *   - 'track-disable' + non-null adapter.webrtc → call disableTrack / enableTrack
+ *   - 'css-fallback' / 'unsupported' / null webrtc → no-op (CSS handled by RoleRenderer)
+ *
+ * Also implements onRevert(pendingOp) for SocketHandler timeout callbacks.
+ *
+ * Import rule: may only import from src/contracts/ and src/utils/.
+ * Constructors are side-effect free — call init() from module.js Hooks.once('ready').
+ *
+ * @module core/VisibilityManager
+ */
+
+/**
+ * Applies webrtcMode strategy on state changes and reverts failed operations.
+ */
+export class VisibilityManager {
+  /**
+   * @param {import('./StateStore.js').StateStore} stateStore
+   * @param {{ settings: { get(key: string): unknown }, webrtc: { disableTrack(userId: string): void, enableTrack(userId: string): void } | null, notifications: { warn(msg: string): void }, hooks: { on(event: string, handler: (...args: unknown[]) => void): void } }} adapter
+   */
+  constructor(stateStore, adapter) {
+    this._stateStore = stateStore;
+    this._adapter = adapter;
+  }
+
+  /**
+   * Registers the Hooks.on('scrying-pool:stateChanged') listener.
+   * Called from module.js Hooks.once('ready') — NOT from constructor.
+   */
+  init() {
+    this._adapter.hooks.on('scrying-pool:stateChanged', (data) => this._onStateChanged(/** @type {any} */ (data)));
+  }
+
+  /**
+   * Handles a state change by applying the webrtcMode strategy.
+   * css-fallback / unsupported → no-op (CSS applied by RoleRenderer in Story 1.5).
+   * track-disable + non-null webrtc → disable/enable the participant's track.
+   * Always safe with null adapter.webrtc (OQ-1 spike result for v14).
+   *
+   * @private
+   * @param {{ userId: string, state: string }} data
+   */
+  _onStateChanged(data) {
+    const { userId, state } = data;
+    // Input validation
+    if (!userId || typeof userId !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager._onStateChanged: invalid userId');
+      return;
+    }
+    if (!state || typeof state !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager._onStateChanged: invalid state');
+      return;
+    }
+
+    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);
+    }
+  }
+
+  /**
+   * Called by SocketHandler after retry exhaustion — reverts the optimistic state
+   * and notifies the GM that the operation could not be confirmed.
+   *
+   * @param {{ userId: string, previousState: string, opId: string }} pendingOp
+   */
+  onRevert(pendingOp) {
+    // Input validation
+    if (!pendingOp || typeof pendingOp !== 'object') {
+      console.warn('[ScryingPool]', 'VisibilityManager.onRevert: invalid pendingOp');
+      return;
+    }
+    const { userId, previousState } = pendingOp;
+    if (!userId || typeof userId !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager.onRevert: invalid userId in pendingOp');
+      return;
+    }
+    if (!previousState || typeof previousState !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager.onRevert: invalid previousState in pendingOp');
+      return;
+    }
+
+    try {
+      this._stateStore.setVisibility(userId, previousState);
+    } catch (err) {
+      console.error('[ScryingPool] VisibilityManager.onRevert: setVisibility failed', err);
+    }
+
+    try {
+      this._adapter.notifications.warn(
+        `[ScryingPool] Visibility change for ${userId} could not be confirmed — reverting to ${previousState}`
+      );
+    } catch (err) {
+      console.error('[ScryingPool] VisibilityManager.onRevert: notification failed', err);
+    }
+  }
+}
+diff --git a/tests/unit/core/ScryingPoolController.test.js b/tests/unit/core/ScryingPoolController.test.js
+new file mode 100644
+index 0000000..eb4f4ad
+--- /dev/null
+++ b/tests/unit/core/ScryingPoolController.test.js
+@@ -0,0 +1,277 @@
+// @ts-nocheck
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { ScryingPoolController } from '../../../src/core/ScryingPoolController.js';
+import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js';
+import { StateStore } from '../../../src/core/StateStore.js';
+
+/** @returns {{ emit: Function, registerPendingOp: Function, confirmPendingOp: Function, setReady: Function }} */
+function makeSocketHandler() {
+  return {
+    emit: vi.fn(),
+    registerPendingOp: vi.fn(),
+    confirmPendingOp: vi.fn(),
+    setReady: vi.fn(),
+  };
+}
+
+/** @returns {StateStore} */
+function makeStateStore() {
+  const settingsMock = {
+    get: vi.fn().mockReturnValue({ _version: 1, matrix: {} }),
+    set: vi.fn().mockResolvedValue(undefined),
+    register: vi.fn(),
+  };
+  return new StateStore(settingsMock);
+}
+
+describe('ScryingPoolController', () => {
+  let adapter;
+  let stateStore;
+  let socketHandler;
+  let controller;
+  let hooksStub;
+
+  beforeEach(() => {
+    hooksStub = { callAll: vi.fn(), on: vi.fn(), once: vi.fn(), off: vi.fn() };
+    vi.stubGlobal('Hooks', hooksStub);
+
+    adapter = createFoundryAdapterMock({ 
+      users: { isGM: () => true },
+      hooks: hooksStub
+    });
+    adapter.socket.on = vi.fn();
+
+    stateStore = makeStateStore();
+    socketHandler = makeSocketHandler();
+    controller = new ScryingPoolController(stateStore, socketHandler, adapter);
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.clearAllMocks();
+  });
+
+  // ── AC-1: Construction ────────────────────────────────────────────────────
+
+  describe('constructor (AC-1)', () => {
+    it('initialises _pendingOps as an empty Map', () => {
+      expect(controller._pendingOps).toBeInstanceOf(Map);
+      expect(controller._pendingOps.size).toBe(0);
+    });
+
+    it('initialises _revisions as an empty Map', () => {
+      expect(controller._revisions).toBeInstanceOf(Map);
+      expect(controller._revisions.size).toBe(0);
+    });
+
+    it('does NOT register socket listener in constructor (side-effect free)', () => {
+      expect(adapter.socket.on).not.toHaveBeenCalled();
+    });
+  });
+
+  // ── AC-1: init() ─────────────────────────────────────────────────────────
+
+  describe('init() (AC-1)', () => {
+    it('registers socket echo listener for scrying-pool.visibility.updated', () => {
+      controller.init();
+      expect(adapter.socket.on).toHaveBeenCalledWith(
+        'scrying-pool.visibility.updated',
+        expect.any(Function)
+      );
+    });
+  });
+
+  // ── AC-2: action() happy path ─────────────────────────────────────────────
+
+  describe('action() happy path (AC-2)', () => {
+    it('stores a PendingOp in _pendingOps keyed by participantId', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(controller._pendingOps.has('user-1')).toBe(true);
+      expect(controller._pendingOps.get('user-1')).toMatchObject({
+        opId: 'op-1',
+        userId: 'user-1',
+        targetState: 'hidden',
+      });
+    });
+
+    it('calls stateStore.setVisibility with the target state (optimistic update)', () => {
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'hidden');
+    });
+
+    it('calls socketHandler.emit with VISIBILITY_SET event and correct payload', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(socketHandler.emit).toHaveBeenCalledWith(
+        'scrying-pool.visibility.set',
+        expect.objectContaining({ opId: 'op-1', userId: 'user-1', targetState: 'hidden', baseRevision: 0 })
+      );
+    });
+
+    it('calls socketHandler.registerPendingOp with the PendingOp, event, and payload', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(socketHandler.registerPendingOp).toHaveBeenCalledWith(
+        expect.objectContaining({ opId: 'op-1', userId: 'user-1', targetState: 'hidden' }),
+        'scrying-pool.visibility.set',
+        expect.objectContaining({ opId: 'op-1' })
+      );
+    });
+
+    it('fires Hooks.callAll scrying-pool:controllerAction with correct payload', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(hooksStub.callAll).toHaveBeenCalledWith(
+        'scrying-pool:controllerAction',
+        expect.objectContaining({ participantId: 'user-1', targetState: 'hidden', source: 'ui', opId: 'op-1' })
+      );
+    });
+
+    it('sets previousState to null-coalesced "never-connected" when participant is new', () => {
+      controller.action('ui', 'new-user', 'hidden', 'op-1', 0);
+      const op = controller._pendingOps.get('new-user');
+      expect(op.previousState).toBe('never-connected');
+    });
+  });
+
+  // ── AC-5: non-GM authorization ────────────────────────────────────────────
+
+  describe('action() non-GM authorization (AC-5)', () => {
+    it('warns and silently drops the action when adapter.users.isGM() is false', () => {
+      const nonGmAdapter = createFoundryAdapterMock({ 
+        users: { isGM: () => false },
+        hooks: hooksStub
+      });
+      nonGmAdapter.socket.on = vi.fn();
+      const playerController = new ScryingPoolController(stateStore, socketHandler, nonGmAdapter);
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+      playerController.action('ui', 'user-1', 'hidden', 'op-1', 0);
+
+      expect(warnSpy).toHaveBeenCalledWith('[ScryingPool]', expect.stringContaining('non-GM'));
+      expect(setSpy).not.toHaveBeenCalled();
+      expect(socketHandler.emit).not.toHaveBeenCalled();
+      expect(socketHandler.registerPendingOp).not.toHaveBeenCalled();
+      expect(hooksStub.callAll).not.toHaveBeenCalled();
+
+      warnSpy.mockRestore();
+    });
+  });
+
+  // ── AC-3: latest-revision-wins guard ─────────────────────────────────────
+
+  describe('action() latest-revision-wins guard (AC-3)', () => {
+    it('silently drops action when baseRevision < confirmed revision', () => {
+      controller._revisions.set('user-1', 5);
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-2', 3); // 3 < 5 → stale
+
+      expect(setSpy).not.toHaveBeenCalled();
+      expect(socketHandler.emit).not.toHaveBeenCalled();
+      expect(hooksStub.callAll).not.toHaveBeenCalled();
+    });
+
+    it('allows action when baseRevision equals confirmed revision (not stale)', () => {
+      controller._revisions.set('user-1', 5);
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-2', 5); // 5 == 5 → not stale
+
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'hidden');
+    });
+
+    it('allows action with baseRevision=0 when no revision confirmed yet', () => {
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+
+      expect(setSpy).toHaveBeenCalled();
+    });
+  });
+
+  // ── AC-4: last-intent guard ───────────────────────────────────────────────
+
+  describe('action() last-intent guard (AC-4)', () => {
+    it('silently drops action when participant is already in targetState', () => {
+      // Seed the state store with the current state
+      stateStore.setVisibility('user-1', 'hidden');
+      vi.clearAllMocks(); // reset all mock call counts
+
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-2', 0);
+
+      expect(setSpy).not.toHaveBeenCalled();
+      expect(socketHandler.emit).not.toHaveBeenCalled();
+    });
+
+    it('allows action when targetState differs from current state', () => {
+      stateStore.setVisibility('user-1', 'active');
+      vi.clearAllMocks();
+
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-3', 0);
+
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'hidden');
+    });
+  });
+
+  // ── AC-11: echo reconciliation (_onEcho) ──────────────────────────────────
+
+  describe('_onEcho() echo reconciliation (AC-11)', () => {
+    // Helper: call init() and return the captured echo handler
+    function getEchoHandler() {
+      controller.init();
+      return adapter.socket.on.mock.calls[0][1];
+    }
+
+    it('calls socketHandler.confirmPendingOp with the opId', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 1 });
+      expect(socketHandler.confirmPendingOp).toHaveBeenCalledWith('op-1');
+    });
+
+    it('stores the echo revision in _revisions for the userId', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 7 });
+      expect(controller._revisions.get('user-1')).toBe(7);
+    });
+
+    it('calls stateStore.setVisibility with the authoritative state', () => {
+      const echoHandler = getEchoHandler();
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'active', revision: 2 });
+
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'active');
+    });
+
+    it('fires Hooks.callAll scrying-pool:controllerAction with source: echo', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 1 });
+
+      expect(hooksStub.callAll).toHaveBeenCalledWith(
+        'scrying-pool:controllerAction',
+        expect.objectContaining({ source: 'echo', participantId: 'user-1', targetState: 'hidden', opId: 'op-1' })
+      );
+    });
+
+    it('removes the participant from _pendingOps after echo', () => {
+      // Register a pending op first
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(controller._pendingOps.has('user-1')).toBe(true);
+
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 1 });
+
+      expect(controller._pendingOps.has('user-1')).toBe(false);
+    });
+
+    it('defaults revision to 0 when echo payload omits revision field', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden' }); // no revision
+      expect(controller._revisions.get('user-1')).toBe(0);
+    });
+  });
+});
+diff --git a/tests/unit/core/VisibilityManager.test.js b/tests/unit/core/VisibilityManager.test.js
+new file mode 100644
+index 0000000..36df70f
+--- /dev/null
+++ b/tests/unit/core/VisibilityManager.test.js
+@@ -0,0 +1,218 @@
+// @ts-nocheck
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { VisibilityManager } from '../../../src/core/VisibilityManager.js';
+import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js';
+import { StateStore } from '../../../src/core/StateStore.js';
+
+/** @returns {StateStore} */
+function makeStateStore() {
+  const settingsMock = {
+    get: vi.fn().mockReturnValue({ _version: 1, matrix: {} }),
+    set: vi.fn().mockResolvedValue(undefined),
+    register: vi.fn(),
+  };
+  return new StateStore(settingsMock);
+}
+
+describe('VisibilityManager', () => {
+  let adapter;
+  let stateStore;
+  let manager;
+  let hooksStub;
+
+  beforeEach(() => {
+    hooksStub = { callAll: vi.fn(), on: vi.fn(), once: vi.fn(), off: vi.fn() };
+    vi.stubGlobal('Hooks', hooksStub);
+
+    adapter = createFoundryAdapterMock({ hooks: hooksStub });
+    stateStore = makeStateStore();
+    manager = new VisibilityManager(stateStore, adapter);
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.clearAllMocks();
+  });
+
+  // ── AC-1 (construction side-effect free) ─────────────────────────────────
+
+  describe('constructor (side-effect free)', () => {
+    it('does NOT register Hooks.on listener in constructor', () => {
+      expect(hooksStub.on).not.toHaveBeenCalled();
+    });
+  });
+
+  // ── init() ────────────────────────────────────────────────────────────────
+
+  describe('init()', () => {
+    it('registers Hooks.on for scrying-pool:stateChanged', () => {
+      manager.init();
+      expect(hooksStub.on).toHaveBeenCalledWith(
+        'scrying-pool:stateChanged',
+        expect.any(Function)
+      );
+    });
+  });
+
+  // ── AC-6: _onStateChanged — track-disable strategy ────────────────────────
+
+  describe('_onStateChanged() track-disable strategy (AC-6)', () => {
+    let webrtcMock;
+
+    beforeEach(() => {
+      webrtcMock = { disableTrack: vi.fn(), enableTrack: vi.fn() };
+      const trackDisableAdapter = createFoundryAdapterMock({
+        webrtc: webrtcMock,
+        settings: { get: (key) => (key === 'webrtcMode' ? 'track-disable' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, trackDisableAdapter);
+      manager.init();
+    });
+
+    it('calls disableTrack(userId) when state is hidden', () => {
+      const handler = hooksStub.on.mock.calls[0][1];
+      handler({ userId: 'user-1', state: 'hidden' });
+      expect(webrtcMock.disableTrack).toHaveBeenCalledWith('user-1');
+      expect(webrtcMock.enableTrack).not.toHaveBeenCalled();
+    });
+
+    it('calls enableTrack(userId) when state is active', () => {
+      const handler = hooksStub.on.mock.calls[0][1];
+      handler({ userId: 'user-1', state: 'active' });
+      expect(webrtcMock.enableTrack).toHaveBeenCalledWith('user-1');
+      expect(webrtcMock.disableTrack).not.toHaveBeenCalled();
+    });
+  });
+
+  // ── AC-7: _onStateChanged — css-fallback / unsupported ────────────────────
+
+  describe('_onStateChanged() css-fallback strategy (AC-7)', () => {
+    it('performs no webrtc call and throws no error when mode is css-fallback', () => {
+      const cssFallbackAdapter = createFoundryAdapterMock({
+        settings: { get: (key) => (key === 'webrtcMode' ? 'css-fallback' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, cssFallbackAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'hidden' })).not.toThrow();
+    });
+
+    it('performs no webrtc call and throws no error when mode is unsupported', () => {
+      const unsupportedAdapter = createFoundryAdapterMock({
+        settings: { get: (key) => (key === 'webrtcMode' ? 'unsupported' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, unsupportedAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'hidden' })).not.toThrow();
+    });
+  });
+
+  // ── AC-10: null webrtc guard ──────────────────────────────────────────────
+
+  describe('_onStateChanged() null webrtc guard (AC-10)', () => {
+    it('does not throw when adapter.webrtc is null in track-disable mode', () => {
+      const nullWebrtcAdapter = createFoundryAdapterMock({
+        webrtc: null,
+        settings: { get: (key) => (key === 'webrtcMode' ? 'track-disable' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, nullWebrtcAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'hidden' })).not.toThrow();
+    });
+
+    it('does not throw when adapter.webrtc is null with state active', () => {
+      const nullWebrtcAdapter = createFoundryAdapterMock({
+        webrtc: null,
+        settings: { get: (key) => (key === 'webrtcMode' ? 'track-disable' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, nullWebrtcAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'active' })).not.toThrow();
+    });
+  });
+
+  // ── AC-9: onRevert() ─────────────────────────────────────────────────────
+
+  describe('onRevert() (AC-9)', () => {
+    /** @type {import('../../../src/contracts/pending-op.js').PendingOp} */
+    const pendingOp = {
+      opId: 'op-1',
+      userId: 'user-1',
+      targetState: 'hidden',
+      previousState: 'active',
+      issuedAt: 1000000,
+      timeoutId: null,
+    };
+
+    it('calls stateStore.setVisibility with previousState to revert', () => {
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+      manager.onRevert(pendingOp);
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'active');
+    });
+
+    it('calls adapter.notifications.warn with a [ScryingPool]-prefixed message', () => {
+      const warnMock = vi.fn();
+      const warnAdapter = createFoundryAdapterMock({
+        notifications: { warn: warnMock, info: () => {}, error: () => {} },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, warnAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(warnMock).toHaveBeenCalledOnce();
+      expect(warnMock.mock.calls[0][0]).toMatch(/^\[ScryingPool\]/);
+    });
+
+    it('includes userId in the warning message', () => {
+      const warnMock = vi.fn();
+      const warnAdapter = createFoundryAdapterMock({
+        notifications: { warn: warnMock, info: () => {}, error: () => {} },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, warnAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(warnMock.mock.calls[0][0]).toContain('user-1');
+    });
+
+    it('does NOT call notifications.info (no success notification on revert)', () => {
+      const infoMock = vi.fn();
+      const noInfoAdapter = createFoundryAdapterMock({
+        notifications: { warn: () => {}, info: infoMock, error: () => {} },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, noInfoAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(infoMock).not.toHaveBeenCalled();
+    });
+
+    it('does NOT call notifications.error', () => {
+      const errorMock = vi.fn();
+      const noErrorAdapter = createFoundryAdapterMock({
+        notifications: { warn: () => {}, info: () => {}, error: errorMock },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, noErrorAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(errorMock).not.toHaveBeenCalled();
+    });
+  });
+});
@@ -0,0 +1,850 @@
+# Edge Case Hunter Review Prompt - Story 1-5 Group 1 (Core Logic)
+
+**Story:** 1-5-gm-control-ui-scryingpoolstrip-actionpopover-and-av-tile-integration  
+**Group:** Core Logic (Group 1 of 4)  
+**Files:** ScryingPoolController.js, VisibilityManager.js, and their tests  
+**Diff lines:** 804  
+**Project path:** /home/morr/work/foundryvtt/video-view-manager
+
+---
+
+## YOUR ROLE: Edge Case Hunter
+
+You are an **Edge Case Hunter** code reviewer. You receive the diff below AND read access to the project repository at `/home/morr/work/foundryvtt/video-view-manager`. Your job is to find edge cases, boundary conditions, and unusual scenarios that the code doesn't handle properly.
+
+### Rules:
+- You have read access to the project directory
+- You have access to the diff below
+- You have NO access to the spec file (blind to requirements)
+- You MUST find at least 5 edge case issues
+- Focus on: null/undefined handling, empty collections, concurrent access, extreme values, error paths, timing issues, state transitions
+
+### Method:
+1. Read the diff carefully
+2. Explore the project codebase to understand the broader context
+3. Walk every branching path in the code
+4. Check boundary conditions for all inputs and loops
+5. Identify unhandled edge cases
+
+### Output Format:
+Output findings as a Markdown list. Each finding:
+```markdown
+- **EC-XX: [One-line title]** — [Code location] — [Edge case scenario] — [Impact]
+```
+
+Classify by type:
+- **EC-NULL**: Missing null/undefined checks
+- **EC-BOUNDARY**: Off-by-one, empty collections, extreme values
+- **EC-CONCURRENCY**: Race conditions, async issues
+- **EC-STATE**: Invalid state transitions, inconsistent state
+- **EC-ERROR**: Unhandled exceptions, error swallowing
+- **EC-TIMING**: Timeout issues, ordering problems
+
+---
+
+## DIFF TO REVIEW
+
+diff --git a/src/core/ScryingPoolController.js b/src/core/ScryingPoolController.js
+new file mode 100644
+index 0000000..fc013f0
+--- /dev/null
+++ b/src/core/ScryingPoolController.js
+@@ -0,0 +1,181 @@
+/**
+ * ScryingPoolController — Orchestrates GM visibility actions with optimistic state updates.
+ *
+ * Handles: GM authorization, latest-revision-wins guard, last-intent guard, PendingOp
+ * lifecycle, optimistic setVisibility, socket emit, and echo reconciliation.
+ *
+ * Import rule: may only import from src/contracts/ and src/utils/.
+ * Constructors are side-effect free — call init() from module.js Hooks.once('ready').
+ *
+ * @module core/ScryingPoolController
+ */
+
+import { createPendingOp } from '../contracts/pending-op.js';
+import { createSocketIntentMessage, SOCKET_EVENTS } from '../contracts/socket-message.js';
+
+/**
+ * Orchestrates GM visibility actions: auth, optimistic state, socket emit, echo reconciliation.
+ */
+export class ScryingPoolController {
+  /**
+   * @param {import('./StateStore.js').StateStore} stateStore
+   * @param {{ emit(event: string, payload: object): void, registerPendingOp(op: object, event: string, payload: object): void, confirmPendingOp(opId: string): void, setReady(handler: object): void }} socketHandler
+   * @param {{ users: { isGM(): boolean }, socket: { on(event: string, handler: (...args: unknown[]) => void): void }, hooks: { on(event: string, handler: (...args: unknown[]) => void): void, callAll(event: string, data: unknown): void } }} adapter
+   */
+  constructor(stateStore, socketHandler, adapter) {
+    this._stateStore = stateStore;
+    this._socketHandler = socketHandler;
+    this._adapter = adapter;
+    /** @type {Map<string, import('../contracts/pending-op.js').PendingOp>} participantId → PendingOp */
+    this._pendingOps = new Map();
+    /** @type {Map<string, number>} participantId → last-confirmed revision */
+    this._revisions = new Map();
+  }
+
+  /**
+   * Registers the socket echo listener.
+   * Called from module.js Hooks.once('ready') — NOT from constructor.
+   */
+  init() {
+    this._adapter.socket.on(
+      SOCKET_EVENTS.VISIBILITY_UPDATED,
+      (payload) => this._onEcho(/** @type {any} */ (payload))
+    );
+  }
+
+  /**
+   * Returns the last confirmed revision for a participant (0 if unknown).
+   * @param {string} participantId
+   * @returns {number}
+   */
+  getRevision(participantId) {
+    return this._revisions.get(participantId) ?? 0;
+  }
+
+  /**
+   * Returns true if a pending op is currently in-flight for the given participant.
+   * @param {string} participantId
+   * @returns {boolean}
+   */
+  hasPendingOp(participantId) {
+    return this._pendingOps.has(participantId);
+  }
+
+  /**
+   * Cleans up a pending operation by userId.
+   * Called by SocketHandler timeout callback via composite handler in module.js.
+   * @param {string} userId
+   */
+  cleanupPendingOp(userId) {
+    this._pendingOps.delete(userId);
+  }
+
+  /**
+   * Processes a GM visibility toggle request.
+   * Guards: isGM, latest-revision-wins, last-intent (idempotent).
+   *
+   * @param {string} source - Who triggered the action (e.g. 'ui', 'preset').
+   * @param {string} participantId - Target userId.
+   * @param {string} targetState - Desired VisibilityState.
+   * @param {string} opId - Unique operation ID (supplied by caller — Story 1.5 UI).
+   * @param {number} baseRevision - StateStore revision at time of intent.
+   */
+  action(source, participantId, targetState, opId, baseRevision) {
+    // 0. Input validation
+    if (!participantId || typeof participantId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid participantId');
+      return;
+    }
+    if (!targetState || typeof targetState !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid targetState');
+      return;
+    }
+    if (!opId || typeof opId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid opId');
+      return;
+    }
+    if (typeof baseRevision !== 'number' || !Number.isFinite(baseRevision) || baseRevision < 0) {
+      console.warn('[ScryingPool]', 'ScryingPoolController.action: invalid baseRevision');
+      return;
+    }
+
+    // 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;
+
+    // 3. Last-intent guard (idempotent)
+    const currentState = this._stateStore.getState(participantId);
+    if (currentState === targetState) return;
+
+    // 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. Start acknowledgement timer
+    this._socketHandler.registerPendingOp(pendingOp, msg.event, msg.payload);
+
+    // 8. Notify UI subscribers
+    try {
+      this._adapter.hooks.callAll('scrying-pool:controllerAction', { participantId, targetState, source, opId });
+    } catch (hookErr) {
+      console.error('[ScryingPool] ScryingPoolController.action: hook emission failed', hookErr);
+    }
+  }
+
+  /**
+   * Processes an authoritative echo from the socket server.
+   * Confirms the pending op, updates revision, and sets the authoritative state.
+   * @private
+   * @param {{ opId: string, userId: string, state: string, revision?: number }} payload
+   */
+  _onEcho(payload) {
+    // Validate payload fields
+    if (!payload || typeof payload !== 'object') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: invalid payload');
+      return;
+    }
+    const { opId, userId, state, revision } = payload;
+    if (!opId || typeof opId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: missing or invalid opId');
+      return;
+    }
+    if (!userId || typeof userId !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: missing or invalid userId');
+      return;
+    }
+    if (!state || typeof state !== 'string') {
+      console.warn('[ScryingPool]', 'ScryingPoolController._onEcho: missing or invalid state');
+      return;
+    }
+
+    this._socketHandler.confirmPendingOp(opId);
+    this._revisions.set(userId, revision ?? 0);
+    this._pendingOps.delete(userId);
+    this._stateStore.setVisibility(userId, state);
+
+    try {
+      this._adapter.hooks.callAll('scrying-pool:controllerAction', {
+        participantId: userId,
+        targetState: state,
+        source: 'echo',
+        opId,
+      });
+    } catch (hookErr) {
+      console.error('[ScryingPool] ScryingPoolController._onEcho: hook emission failed', hookErr);
+    }
+  }
+}
+diff --git a/src/core/VisibilityManager.js b/src/core/VisibilityManager.js
+new file mode 100644
+index 0000000..0e465f2
+--- /dev/null
+++ b/src/core/VisibilityManager.js
+@@ -0,0 +1,104 @@
+/**
+ * VisibilityManager — WebRTC strategy applier and SocketHandler revert handler.
+ *
+ * Listens to `scrying-pool:stateChanged` hook events (emitted by StateStore) and
+ * applies the appropriate webrtcMode strategy:
+ *   - 'track-disable' + non-null adapter.webrtc → call disableTrack / enableTrack
+ *   - 'css-fallback' / 'unsupported' / null webrtc → no-op (CSS handled by RoleRenderer)
+ *
+ * Also implements onRevert(pendingOp) for SocketHandler timeout callbacks.
+ *
+ * Import rule: may only import from src/contracts/ and src/utils/.
+ * Constructors are side-effect free — call init() from module.js Hooks.once('ready').
+ *
+ * @module core/VisibilityManager
+ */
+
+/**
+ * Applies webrtcMode strategy on state changes and reverts failed operations.
+ */
+export class VisibilityManager {
+  /**
+   * @param {import('./StateStore.js').StateStore} stateStore
+   * @param {{ settings: { get(key: string): unknown }, webrtc: { disableTrack(userId: string): void, enableTrack(userId: string): void } | null, notifications: { warn(msg: string): void }, hooks: { on(event: string, handler: (...args: unknown[]) => void): void } }} adapter
+   */
+  constructor(stateStore, adapter) {
+    this._stateStore = stateStore;
+    this._adapter = adapter;
+  }
+
+  /**
+   * Registers the Hooks.on('scrying-pool:stateChanged') listener.
+   * Called from module.js Hooks.once('ready') — NOT from constructor.
+   */
+  init() {
+    this._adapter.hooks.on('scrying-pool:stateChanged', (data) => this._onStateChanged(/** @type {any} */ (data)));
+  }
+
+  /**
+   * Handles a state change by applying the webrtcMode strategy.
+   * css-fallback / unsupported → no-op (CSS applied by RoleRenderer in Story 1.5).
+   * track-disable + non-null webrtc → disable/enable the participant's track.
+   * Always safe with null adapter.webrtc (OQ-1 spike result for v14).
+   *
+   * @private
+   * @param {{ userId: string, state: string }} data
+   */
+  _onStateChanged(data) {
+    const { userId, state } = data;
+    // Input validation
+    if (!userId || typeof userId !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager._onStateChanged: invalid userId');
+      return;
+    }
+    if (!state || typeof state !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager._onStateChanged: invalid state');
+      return;
+    }
+
+    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);
+    }
+  }
+
+  /**
+   * Called by SocketHandler after retry exhaustion — reverts the optimistic state
+   * and notifies the GM that the operation could not be confirmed.
+   *
+   * @param {{ userId: string, previousState: string, opId: string }} pendingOp
+   */
+  onRevert(pendingOp) {
+    // Input validation
+    if (!pendingOp || typeof pendingOp !== 'object') {
+      console.warn('[ScryingPool]', 'VisibilityManager.onRevert: invalid pendingOp');
+      return;
+    }
+    const { userId, previousState } = pendingOp;
+    if (!userId || typeof userId !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager.onRevert: invalid userId in pendingOp');
+      return;
+    }
+    if (!previousState || typeof previousState !== 'string') {
+      console.warn('[ScryingPool]', 'VisibilityManager.onRevert: invalid previousState in pendingOp');
+      return;
+    }
+
+    try {
+      this._stateStore.setVisibility(userId, previousState);
+    } catch (err) {
+      console.error('[ScryingPool] VisibilityManager.onRevert: setVisibility failed', err);
+    }
+
+    try {
+      this._adapter.notifications.warn(
+        `[ScryingPool] Visibility change for ${userId} could not be confirmed — reverting to ${previousState}`
+      );
+    } catch (err) {
+      console.error('[ScryingPool] VisibilityManager.onRevert: notification failed', err);
+    }
+  }
+}
+diff --git a/tests/unit/core/ScryingPoolController.test.js b/tests/unit/core/ScryingPoolController.test.js
+new file mode 100644
+index 0000000..eb4f4ad
+--- /dev/null
+++ b/tests/unit/core/ScryingPoolController.test.js
+@@ -0,0 +1,277 @@
+// @ts-nocheck
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { ScryingPoolController } from '../../../src/core/ScryingPoolController.js';
+import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js';
+import { StateStore } from '../../../src/core/StateStore.js';
+
+/** @returns {{ emit: Function, registerPendingOp: Function, confirmPendingOp: Function, setReady: Function }} */
+function makeSocketHandler() {
+  return {
+    emit: vi.fn(),
+    registerPendingOp: vi.fn(),
+    confirmPendingOp: vi.fn(),
+    setReady: vi.fn(),
+  };
+}
+
+/** @returns {StateStore} */
+function makeStateStore() {
+  const settingsMock = {
+    get: vi.fn().mockReturnValue({ _version: 1, matrix: {} }),
+    set: vi.fn().mockResolvedValue(undefined),
+    register: vi.fn(),
+  };
+  return new StateStore(settingsMock);
+}
+
+describe('ScryingPoolController', () => {
+  let adapter;
+  let stateStore;
+  let socketHandler;
+  let controller;
+  let hooksStub;
+
+  beforeEach(() => {
+    hooksStub = { callAll: vi.fn(), on: vi.fn(), once: vi.fn(), off: vi.fn() };
+    vi.stubGlobal('Hooks', hooksStub);
+
+    adapter = createFoundryAdapterMock({ 
+      users: { isGM: () => true },
+      hooks: hooksStub
+    });
+    adapter.socket.on = vi.fn();
+
+    stateStore = makeStateStore();
+    socketHandler = makeSocketHandler();
+    controller = new ScryingPoolController(stateStore, socketHandler, adapter);
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.clearAllMocks();
+  });
+
+  // ── AC-1: Construction ────────────────────────────────────────────────────
+
+  describe('constructor (AC-1)', () => {
+    it('initialises _pendingOps as an empty Map', () => {
+      expect(controller._pendingOps).toBeInstanceOf(Map);
+      expect(controller._pendingOps.size).toBe(0);
+    });
+
+    it('initialises _revisions as an empty Map', () => {
+      expect(controller._revisions).toBeInstanceOf(Map);
+      expect(controller._revisions.size).toBe(0);
+    });
+
+    it('does NOT register socket listener in constructor (side-effect free)', () => {
+      expect(adapter.socket.on).not.toHaveBeenCalled();
+    });
+  });
+
+  // ── AC-1: init() ─────────────────────────────────────────────────────────
+
+  describe('init() (AC-1)', () => {
+    it('registers socket echo listener for scrying-pool.visibility.updated', () => {
+      controller.init();
+      expect(adapter.socket.on).toHaveBeenCalledWith(
+        'scrying-pool.visibility.updated',
+        expect.any(Function)
+      );
+    });
+  });
+
+  // ── AC-2: action() happy path ─────────────────────────────────────────────
+
+  describe('action() happy path (AC-2)', () => {
+    it('stores a PendingOp in _pendingOps keyed by participantId', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(controller._pendingOps.has('user-1')).toBe(true);
+      expect(controller._pendingOps.get('user-1')).toMatchObject({
+        opId: 'op-1',
+        userId: 'user-1',
+        targetState: 'hidden',
+      });
+    });
+
+    it('calls stateStore.setVisibility with the target state (optimistic update)', () => {
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'hidden');
+    });
+
+    it('calls socketHandler.emit with VISIBILITY_SET event and correct payload', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(socketHandler.emit).toHaveBeenCalledWith(
+        'scrying-pool.visibility.set',
+        expect.objectContaining({ opId: 'op-1', userId: 'user-1', targetState: 'hidden', baseRevision: 0 })
+      );
+    });
+
+    it('calls socketHandler.registerPendingOp with the PendingOp, event, and payload', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(socketHandler.registerPendingOp).toHaveBeenCalledWith(
+        expect.objectContaining({ opId: 'op-1', userId: 'user-1', targetState: 'hidden' }),
+        'scrying-pool.visibility.set',
+        expect.objectContaining({ opId: 'op-1' })
+      );
+    });
+
+    it('fires Hooks.callAll scrying-pool:controllerAction with correct payload', () => {
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(hooksStub.callAll).toHaveBeenCalledWith(
+        'scrying-pool:controllerAction',
+        expect.objectContaining({ participantId: 'user-1', targetState: 'hidden', source: 'ui', opId: 'op-1' })
+      );
+    });
+
+    it('sets previousState to null-coalesced "never-connected" when participant is new', () => {
+      controller.action('ui', 'new-user', 'hidden', 'op-1', 0);
+      const op = controller._pendingOps.get('new-user');
+      expect(op.previousState).toBe('never-connected');
+    });
+  });
+
+  // ── AC-5: non-GM authorization ────────────────────────────────────────────
+
+  describe('action() non-GM authorization (AC-5)', () => {
+    it('warns and silently drops the action when adapter.users.isGM() is false', () => {
+      const nonGmAdapter = createFoundryAdapterMock({ 
+        users: { isGM: () => false },
+        hooks: hooksStub
+      });
+      nonGmAdapter.socket.on = vi.fn();
+      const playerController = new ScryingPoolController(stateStore, socketHandler, nonGmAdapter);
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
+
+      playerController.action('ui', 'user-1', 'hidden', 'op-1', 0);
+
+      expect(warnSpy).toHaveBeenCalledWith('[ScryingPool]', expect.stringContaining('non-GM'));
+      expect(setSpy).not.toHaveBeenCalled();
+      expect(socketHandler.emit).not.toHaveBeenCalled();
+      expect(socketHandler.registerPendingOp).not.toHaveBeenCalled();
+      expect(hooksStub.callAll).not.toHaveBeenCalled();
+
+      warnSpy.mockRestore();
+    });
+  });
+
+  // ── AC-3: latest-revision-wins guard ─────────────────────────────────────
+
+  describe('action() latest-revision-wins guard (AC-3)', () => {
+    it('silently drops action when baseRevision < confirmed revision', () => {
+      controller._revisions.set('user-1', 5);
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-2', 3); // 3 < 5 → stale
+
+      expect(setSpy).not.toHaveBeenCalled();
+      expect(socketHandler.emit).not.toHaveBeenCalled();
+      expect(hooksStub.callAll).not.toHaveBeenCalled();
+    });
+
+    it('allows action when baseRevision equals confirmed revision (not stale)', () => {
+      controller._revisions.set('user-1', 5);
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-2', 5); // 5 == 5 → not stale
+
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'hidden');
+    });
+
+    it('allows action with baseRevision=0 when no revision confirmed yet', () => {
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+
+      expect(setSpy).toHaveBeenCalled();
+    });
+  });
+
+  // ── AC-4: last-intent guard ───────────────────────────────────────────────
+
+  describe('action() last-intent guard (AC-4)', () => {
+    it('silently drops action when participant is already in targetState', () => {
+      // Seed the state store with the current state
+      stateStore.setVisibility('user-1', 'hidden');
+      vi.clearAllMocks(); // reset all mock call counts
+
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-2', 0);
+
+      expect(setSpy).not.toHaveBeenCalled();
+      expect(socketHandler.emit).not.toHaveBeenCalled();
+    });
+
+    it('allows action when targetState differs from current state', () => {
+      stateStore.setVisibility('user-1', 'active');
+      vi.clearAllMocks();
+
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      controller.action('ui', 'user-1', 'hidden', 'op-3', 0);
+
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'hidden');
+    });
+  });
+
+  // ── AC-11: echo reconciliation (_onEcho) ──────────────────────────────────
+
+  describe('_onEcho() echo reconciliation (AC-11)', () => {
+    // Helper: call init() and return the captured echo handler
+    function getEchoHandler() {
+      controller.init();
+      return adapter.socket.on.mock.calls[0][1];
+    }
+
+    it('calls socketHandler.confirmPendingOp with the opId', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 1 });
+      expect(socketHandler.confirmPendingOp).toHaveBeenCalledWith('op-1');
+    });
+
+    it('stores the echo revision in _revisions for the userId', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 7 });
+      expect(controller._revisions.get('user-1')).toBe(7);
+    });
+
+    it('calls stateStore.setVisibility with the authoritative state', () => {
+      const echoHandler = getEchoHandler();
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'active', revision: 2 });
+
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'active');
+    });
+
+    it('fires Hooks.callAll scrying-pool:controllerAction with source: echo', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 1 });
+
+      expect(hooksStub.callAll).toHaveBeenCalledWith(
+        'scrying-pool:controllerAction',
+        expect.objectContaining({ source: 'echo', participantId: 'user-1', targetState: 'hidden', opId: 'op-1' })
+      );
+    });
+
+    it('removes the participant from _pendingOps after echo', () => {
+      // Register a pending op first
+      controller.action('ui', 'user-1', 'hidden', 'op-1', 0);
+      expect(controller._pendingOps.has('user-1')).toBe(true);
+
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden', revision: 1 });
+
+      expect(controller._pendingOps.has('user-1')).toBe(false);
+    });
+
+    it('defaults revision to 0 when echo payload omits revision field', () => {
+      const echoHandler = getEchoHandler();
+      echoHandler({ opId: 'op-1', userId: 'user-1', state: 'hidden' }); // no revision
+      expect(controller._revisions.get('user-1')).toBe(0);
+    });
+  });
+});
+diff --git a/tests/unit/core/VisibilityManager.test.js b/tests/unit/core/VisibilityManager.test.js
+new file mode 100644
+index 0000000..36df70f
+--- /dev/null
+++ b/tests/unit/core/VisibilityManager.test.js
+@@ -0,0 +1,218 @@
+// @ts-nocheck
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { VisibilityManager } from '../../../src/core/VisibilityManager.js';
+import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js';
+import { StateStore } from '../../../src/core/StateStore.js';
+
+/** @returns {StateStore} */
+function makeStateStore() {
+  const settingsMock = {
+    get: vi.fn().mockReturnValue({ _version: 1, matrix: {} }),
+    set: vi.fn().mockResolvedValue(undefined),
+    register: vi.fn(),
+  };
+  return new StateStore(settingsMock);
+}
+
+describe('VisibilityManager', () => {
+  let adapter;
+  let stateStore;
+  let manager;
+  let hooksStub;
+
+  beforeEach(() => {
+    hooksStub = { callAll: vi.fn(), on: vi.fn(), once: vi.fn(), off: vi.fn() };
+    vi.stubGlobal('Hooks', hooksStub);
+
+    adapter = createFoundryAdapterMock({ hooks: hooksStub });
+    stateStore = makeStateStore();
+    manager = new VisibilityManager(stateStore, adapter);
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.clearAllMocks();
+  });
+
+  // ── AC-1 (construction side-effect free) ─────────────────────────────────
+
+  describe('constructor (side-effect free)', () => {
+    it('does NOT register Hooks.on listener in constructor', () => {
+      expect(hooksStub.on).not.toHaveBeenCalled();
+    });
+  });
+
+  // ── init() ────────────────────────────────────────────────────────────────
+
+  describe('init()', () => {
+    it('registers Hooks.on for scrying-pool:stateChanged', () => {
+      manager.init();
+      expect(hooksStub.on).toHaveBeenCalledWith(
+        'scrying-pool:stateChanged',
+        expect.any(Function)
+      );
+    });
+  });
+
+  // ── AC-6: _onStateChanged — track-disable strategy ────────────────────────
+
+  describe('_onStateChanged() track-disable strategy (AC-6)', () => {
+    let webrtcMock;
+
+    beforeEach(() => {
+      webrtcMock = { disableTrack: vi.fn(), enableTrack: vi.fn() };
+      const trackDisableAdapter = createFoundryAdapterMock({
+        webrtc: webrtcMock,
+        settings: { get: (key) => (key === 'webrtcMode' ? 'track-disable' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, trackDisableAdapter);
+      manager.init();
+    });
+
+    it('calls disableTrack(userId) when state is hidden', () => {
+      const handler = hooksStub.on.mock.calls[0][1];
+      handler({ userId: 'user-1', state: 'hidden' });
+      expect(webrtcMock.disableTrack).toHaveBeenCalledWith('user-1');
+      expect(webrtcMock.enableTrack).not.toHaveBeenCalled();
+    });
+
+    it('calls enableTrack(userId) when state is active', () => {
+      const handler = hooksStub.on.mock.calls[0][1];
+      handler({ userId: 'user-1', state: 'active' });
+      expect(webrtcMock.enableTrack).toHaveBeenCalledWith('user-1');
+      expect(webrtcMock.disableTrack).not.toHaveBeenCalled();
+    });
+  });
+
+  // ── AC-7: _onStateChanged — css-fallback / unsupported ────────────────────
+
+  describe('_onStateChanged() css-fallback strategy (AC-7)', () => {
+    it('performs no webrtc call and throws no error when mode is css-fallback', () => {
+      const cssFallbackAdapter = createFoundryAdapterMock({
+        settings: { get: (key) => (key === 'webrtcMode' ? 'css-fallback' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, cssFallbackAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'hidden' })).not.toThrow();
+    });
+
+    it('performs no webrtc call and throws no error when mode is unsupported', () => {
+      const unsupportedAdapter = createFoundryAdapterMock({
+        settings: { get: (key) => (key === 'webrtcMode' ? 'unsupported' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, unsupportedAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'hidden' })).not.toThrow();
+    });
+  });
+
+  // ── AC-10: null webrtc guard ──────────────────────────────────────────────
+
+  describe('_onStateChanged() null webrtc guard (AC-10)', () => {
+    it('does not throw when adapter.webrtc is null in track-disable mode', () => {
+      const nullWebrtcAdapter = createFoundryAdapterMock({
+        webrtc: null,
+        settings: { get: (key) => (key === 'webrtcMode' ? 'track-disable' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, nullWebrtcAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'hidden' })).not.toThrow();
+    });
+
+    it('does not throw when adapter.webrtc is null with state active', () => {
+      const nullWebrtcAdapter = createFoundryAdapterMock({
+        webrtc: null,
+        settings: { get: (key) => (key === 'webrtcMode' ? 'track-disable' : null) },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, nullWebrtcAdapter);
+      manager.init();
+
+      const handler = hooksStub.on.mock.calls[0][1];
+      expect(() => handler({ userId: 'user-1', state: 'active' })).not.toThrow();
+    });
+  });
+
+  // ── AC-9: onRevert() ─────────────────────────────────────────────────────
+
+  describe('onRevert() (AC-9)', () => {
+    /** @type {import('../../../src/contracts/pending-op.js').PendingOp} */
+    const pendingOp = {
+      opId: 'op-1',
+      userId: 'user-1',
+      targetState: 'hidden',
+      previousState: 'active',
+      issuedAt: 1000000,
+      timeoutId: null,
+    };
+
+    it('calls stateStore.setVisibility with previousState to revert', () => {
+      const setSpy = vi.spyOn(stateStore, 'setVisibility');
+      manager.onRevert(pendingOp);
+      expect(setSpy).toHaveBeenCalledWith('user-1', 'active');
+    });
+
+    it('calls adapter.notifications.warn with a [ScryingPool]-prefixed message', () => {
+      const warnMock = vi.fn();
+      const warnAdapter = createFoundryAdapterMock({
+        notifications: { warn: warnMock, info: () => {}, error: () => {} },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, warnAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(warnMock).toHaveBeenCalledOnce();
+      expect(warnMock.mock.calls[0][0]).toMatch(/^\[ScryingPool\]/);
+    });
+
+    it('includes userId in the warning message', () => {
+      const warnMock = vi.fn();
+      const warnAdapter = createFoundryAdapterMock({
+        notifications: { warn: warnMock, info: () => {}, error: () => {} },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, warnAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(warnMock.mock.calls[0][0]).toContain('user-1');
+    });
+
+    it('does NOT call notifications.info (no success notification on revert)', () => {
+      const infoMock = vi.fn();
+      const noInfoAdapter = createFoundryAdapterMock({
+        notifications: { warn: () => {}, info: infoMock, error: () => {} },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, noInfoAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(infoMock).not.toHaveBeenCalled();
+    });
+
+    it('does NOT call notifications.error', () => {
+      const errorMock = vi.fn();
+      const noErrorAdapter = createFoundryAdapterMock({
+        notifications: { warn: () => {}, info: () => {}, error: errorMock },
+        hooks: hooksStub,
+      });
+      manager = new VisibilityManager(stateStore, noErrorAdapter);
+
+      manager.onRevert(pendingOp);
+
+      expect(errorMock).not.toHaveBeenCalled();
+    });
+  });
+});
@@ -0,0 +1,77 @@
+# Code Review - Story 1.5 Group 1 (Core Logic)
+
+**Generated:** 2026-05-22  
+**Story:** 1-5-gm-control-ui-scryingpoolstrip-actionpopover-and-av-tile-integration  
+**Review Group:** 1 of 4 (Core Logic)  
+**Status:** Awaiting parallel reviewer execution
+
+---
+
+## REVIEW WORKFLOW
+
+This directory contains prompt files for parallel code review layers. Each layer must be executed in a **separate session** (ideally with a different LLM model) to ensure independent analysis.
+
+### Review Layers
+
+1. **🔴 Blind Hunter** (`01-blind-hunter-prompt.md`)
+   - **Skill:** `bmad-review-adversarial-general`
+   - **Input:** Diff only (no spec, no context, no project access)
+   - **Focus:** Security issues, bugs, anti-patterns, performance problems
+   - **Output:** Adversarial findings (minimum 5)
+
+2. **🟡 Edge Case Hunter** (`02-edge-case-hunter-prompt.md`)
+   - **Skill:** `bmad-review-edge-case-hunter`
+   - **Input:** Diff + project read access
+   - **Focus:** Edge cases, boundary conditions, null handling, race conditions
+   - **Output:** Edge case findings (minimum 5)
+
+3. **🟢 Acceptance Auditor** (`03-acceptance-auditor-prompt.md`)
+   - **Input:** Diff + spec file + context docs + project read access
+   - **Focus:** AC compliance, spec violations, missing implementation
+   - **Output:** Acceptance audit findings
+
+---
+
+## FILES IN THIS GROUP
+
+### New Files (4)
+- `src/core/ScryingPoolController.js` (181 lines)
+- `src/core/VisibilityManager.js` (104 lines)
+- `tests/unit/core/ScryingPoolController.test.js` (277 lines)
+- `tests/unit/core/VisibilityManager.test.js` (218 lines)
+
+### Diff Size
+- Total: **804 lines** (under 3000 line threshold ✓)
+
+---
+
+## INSTRUCTIONS FOR REVIEWERS
+
+### For Each Layer:
+1. Open the prompt file for that layer
+2. Read the entire file carefully
+3. Execute the review as described in the role section
+4. Return findings in the specified format
+5. Save findings to a separate file (e.g., `findings-blind-hunter.md`)
+
+### After All Layers Complete:
+1. Collect all findings files
+2. Return to this workflow at Step 3 (Triage)
+3. Paste all findings for consolidated triage
+
+---
+
+## REMINDER
+
+- All layers run **in parallel** — do not wait for one to finish before starting another
+- Each layer must use a **different session** to maintain independence
+- All findings are consolidated in Step 3 (Triage) before final presentation
+
+---
+
+## NEXT STEPS
+
+After generating findings from all three layers:
+1. Proceed to `step-03-triage.md` in the bmad-code-review workflow
+2. Triage and deduplicate findings
+3. Present consolidated results