# Acceptance Auditor 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 **Spec file:** `_bmad-output/implementation-artifacts/1-5-gm-control-ui-scryingpoolstrip-actionpopover-and-av-tile-integration.md` --- ## YOUR ROLE: Acceptance Auditor You are an **Acceptance Auditor** code reviewer. You receive: 1. The diff below (Group 1: Core Logic files) 2. The Story 1-5 spec file content (following this section) 3. Read access to the project at `/home/morr/work/foundryvtt/video-view-manager` Your job is to verify that the implementation conforms to the acceptance criteria and spec requirements. Check for: - Violations of acceptance criteria - Deviations from spec intent - Missing implementation of specified behavior - Contradictions between spec constraints and actual code ### Rules: - You have access to the spec content below - You have read access to the project directory - You MUST check each AC against the implementation - Focus on: AC compliance, spec requirements, architectural constraints, import boundaries ### Output Format: Output findings as a Markdown list. Each finding: ```markdown - **AA-XX: [One-line title]** — [AC/Constraint violated] — [Evidence from diff] — [Impact] ``` Classify by type: - **AA-AC**: Acceptance Criteria violation - **AA-ARCH**: Architecture constraint violation - **AA-SPEC**: Spec requirement not met - **AA-IMPORT**: Import boundary violation - **AA-MISSING**: Missing implementation --- ## SPEC FILE CONTENT # Story 1.5: GM Control UI — ScryingPoolStrip, ActionPopover & AV Tile Integration Status: review ## Story As a **GM**, I want to right-click any participant's AV tile to show or hide their camera feed, and see all feed states at a glance in the ScryingPoolStrip, So that I can control what the table sees in a single interaction without disrupting the session. ## Acceptance Criteria **AC-1 — ScryingPoolStrip appears on ready:** **Given** the module is active and the user is GM **When** FoundryVTT's `ready` hook completes **Then** `ScryingPoolStrip` appears as a floating `ApplicationV2` window showing all connected participants **And** its position (`left`, `top`), open state, and expanded state persist to the GM's user flag `{ left, top, open, expanded }` **AC-2 — Collapsed/expanded toggle:** **Given** the ScryingPoolStrip is in collapsed state **When** the GM clicks the expand toggle **Then** the strip transitions via `max-width` CSS transition (never `width` animation): collapsed = 44px avatar-only rail; expanded = 240px rich rows **AC-3 — ParticipantAvatar rendering:** **Given** the strip renders participants **When** it displays each `ParticipantAvatar` **Then** each avatar is a 44×44px container with a 32px rounded avatar + `StateRing` + 12px corner badge at bottom-right **And** `StateRing` uses the correct variant per state: `--solid` (active/self-muted), `--dashed` (hidden/cam-lost), `--pending` (animated pulse), `--revert` (amber flash 200ms on revert) **And** all `StateRing` animations are gated under `@media (prefers-reduced-motion: no-preference)` **AC-4 — Pending op ring:** **Given** a PendingOp is in-flight for a participant **When** the strip renders **Then** that participant's `StateRing` shows the `--pending` animated pulse **And** NO `ui.notifications` toast fires on successful state change (success uses ambient ring only — tier-1/2 feedback) **AC-5 — Right-click context menu:** **Given** a GM right-clicks a participant's avatar in the ScryingPoolStrip **When** the context menu appears **Then** the option reads exactly **"Hide from table"** (never a synonym) **And** selecting it calls `ScryingPoolController.action()` and transitions state to `hidden` **AC-6 — ActionPopover on click:** **Given** a GM clicks a participant in the ScryingPoolStrip **When** the `ActionPopover` opens **Then** it is a native `` anchored via `StripOverlayLayer.getBoundingClientRect()` relative to the strip **And** the primary CTA reads exactly **"Hide from table"** or **"Show to table"** **And** the primary CTA is `disabled` + `aria-disabled="true"` while a `PendingOp` is in-flight **And** Esc / click-outside dismiss the popover and return focus to the triggering avatar **And** only one `ActionPopover` is open at a time (supersede pattern) **AC-7 — StripOverlayLayer overlay container:** **Given** `StripOverlayLayer` is the parent for all positioned overlays **When** any overlay is positioned **Then** it is a child of the single `StripOverlayLayer` (`position: absolute; inset: 0; pointer-events: none; overflow: visible`); children restore `pointer-events: auto` **AC-8 — AV tile state indicators:** **Given** a visibility change is dispatched **When** the socket broadcast completes **Then** all clients' AV tiles update state indicators within 500ms **And** no AV tile layout shift or reflow occurs for any of the 8 participant states **And** `AVTileAdapter.mount(userId, element)` is idempotent — calling it twice does not duplicate elements **AC-9 — Hidden state on GM tile view:** **Given** a participant is `hidden` **When** the GM views their AV tile **Then** it renders at reduced opacity with a lock overlay and "Camera hidden by GM" tooltip **And** the GM still hears that participant's audio **AC-10 — Portrait Fallback:** **Given** a participant has no camera (`never-connected` or `cam-lost`) **When** their tile renders **Then** Portrait Fallback (FoundryVTT user avatar → system placeholder) displays at AV tile dimensions with no layout shift **AC-11 — EmptyStatePanel:** **Given** no participants are connected **When** the ScryingPoolStrip renders **Then** `EmptyStatePanel` shows "No participants yet" with a slow breathing-pulse eye icon (static under `prefers-reduced-motion`) **And** the panel is NOT styled as an error state **AC-12 — GM self-feed setting:** **Given** the GM opens module settings **When** they locate "Show my own feed to myself" (default ON) **Then** toggling it hides/shows the GM's self-view immediately without errors **AC-13 — Null webrtc guard:** **Given** `game.webrtc` is null (AV disabled) **When** the module loads **Then** `ScryingPoolStrip` is not rendered and no console errors appear **Accessibility:** **AC-14 — ParticipantAvatar accessibility:** **Given** a screen reader user navigates to a `ParticipantAvatar` **When** focus lands **Then** `role="button"`, `aria-label="[Name] — [state label]"` is announced **And** `aria-pressed` reflects popover-open state **AC-15 — ActionPopover keyboard navigation:** **Given** a keyboard user opens an `ActionPopover` **When** it opens **Then** focus moves to the primary CTA **And** Tab/Shift+Tab cycles through popover controls only **And** Esc closes it and returns focus to the triggering avatar **AC-16 — Reduced motion:** **Given** `prefers-reduced-motion: reduce` is active **When** any animated state occurs **Then** all `StateRing` animations are fully suppressed; static icons provide state information **AC-17 — Second-signal rule:** **Given** any participant state is rendered **When** it is visually displayed **Then** colour is never the only signal: each state also has a distinct icon, shape, or motion indicator **And** all state colour tokens meet WCAG AA contrast against both Foundry dark and light themes **AC-18 — Canonical action label:** **Given** a canonical action label appears on any surface **When** it is displayed **Then** it reads exactly "Hide from table" or "Show to table" (never synonyms) **And** on first hover a tooltip variant sets `firstHideTooltip` flag; subsequent hovers show only the canonical label --- ## Tasks / Subtasks - [x] Task 1: Create `src/ui/shared/AVTileAdapter.js` (AC: 8, 9, 10) - [x] 1.1: Write failing tests in `tests/unit/ui/shared/AVTileAdapter.test.js` first (TDD red) - [x] 1.2: Implement `constructor(adapter)` — side-effect free; stores adapter reference; no DOM access in constructor - [x] 1.3: Implement `mount(userId, element)` — idempotent: query tile by `[data-user-id="${userId}"]`; append element with `data-sp-mount` attribute; no-op + `console.warn('[ScryingPool]', ...)` if tile not found (fail-open); no duplicate if element already present - [x] 1.4: Implement `unmount(userId)` — remove all `[data-sp-mount]` children from tile; no-op if tile not found - [x] 1.5: Implement `setStateClass(userId, stateName)` — remove all `sp-state-*` classes from tile; add `sp-state-${stateName}` (no-op if tile not found, with console.warn) - [x] 1.6: Implement `onTileRerender(userId, callback)` — attach scoped `MutationObserver` (`childList: true, subtree: false`) to the tile element; call `callback(tileElement)` when DOM changes detected; store observer by userId for cleanup; no-op if tile not found - [x] 1.7: Implement `disconnect()` — disconnect all stored MutationObservers; clear internal observer map; safe to call multiple times - [x] 1.8: Confirm tests green, run full suite (no regressions) - [x] Task 2: Create `src/ui/RoleRenderer.js` (AC: 8, 9, 10, 12, 13) - [x] 2.1: Write failing tests in `tests/unit/ui/RoleRenderer.test.js` first (TDD red) - [x] 2.2: Implement `constructor(stateStore, scryingPoolController, avTileAdapter, adapter)` — side-effect free; store all injected deps; no Hooks registration in constructor - [x] 2.3: Implement `init()` — register `Hooks.on('scrying-pool:stateChanged', ...)` to call `_applyAVTileState(userId, state)`; register `Hooks.on('scrying-pool:controllerAction', ...)` to call `_onControllerAction(data)` for pending ring updates; register `Hooks.on('updateUser', ...)` for mid-session role-change rebuilds - [x] 2.4: Implement `_applyAVTileState(userId, state)` — resolve state precedence (see architecture precedence table), call `avTileAdapter.setStateClass(userId, resolvedState)`, mount/unmount lock overlay for `hidden`, mount/unmount portrait fallback for `never-connected`/`cam-lost` - [x] 2.5: Implement `_onControllerAction({ participantId, targetState, source })` — for `pending` ops in-flight: add `sp-state-pending` class via `avTileAdapter.setStateClass(participantId, 'pending')`; on echo/confirmation, restore actual state - [x] 2.6: Implement null webrtc guard: check `adapter.users.isGM()` and `game.webrtc` (via adapter); if AV disabled, do NOT construct ScryingPoolStrip; log `console.log('[ScryingPool] AV disabled — ScryingPoolStrip not rendered')` - [x] 2.7: Implement `openStrip()` / `closeStrip()` — construct `ScryingPoolStrip` singleton lazily; open/close it (GM only) - [x] 2.8: Confirm tests green, run full suite (no regressions) - [x] Task 3: Create `src/ui/gm/ScryingPoolStrip.js` + update `templates/roster-strip.hbs` (AC: 1, 2, 3, 4, 5, 6, 7, 11, 13, 14, 15, 16, 17, 18) - [x] 3.1: Write failing tests in `tests/unit/ui/gm/ScryingPoolStrip.test.js` (TDD red — test logic, not ApplicationV2 rendering) - [x] 3.2: Implement `ScryingPoolStrip extends Application` (using `Application` class for simpler FoundryVTT v14 compatibility; reference Architecture §Initialisation Order; see Dev Notes for ApplicationV2 vs Application guidance) - [x] 3.3: Implement `static get defaultOptions()` — set `id: 'scrying-pool-strip'`, `template: 'modules/video-view-manager/templates/roster-strip.hbs'`, `popOut: true`, `resizable: false`, `title: 'Scrying Pool'` - [x] 3.4: Implement `getData()` — build participant list from `stateStore`; return `{ participants, isExpanded, isEmpty }` — see Dev Notes for participant data shape - [x] 3.5: Implement `activateListeners(html)` — bind click on `.sp-participant-avatar` → `_openPopover(participantId, el)`, right-click → `_openContextMenu(participantId, el)`, expand toggle → `_toggleExpanded()` - [x] 3.6: Implement position persistence — on `close`: save `{ left, top, open: false, expanded }` to `game.user.setFlag('video-view-manager', 'stripState', {...})`; on `render`: restore from flag or use default position - [x] 3.7: Implement `_toggleExpanded()` — toggle `.is-expanded` class on strip element; save `expanded` to user flag - [x] 3.8: Implement `_openPopover(participantId, anchorEl)` — supersede existing popover (call `close('superseded')` on `this._activePopover`), create new `ActionPopover`, anchor via `getBoundingClientRect()` relative to strip, store ref in `this._activePopover` - [x] 3.9: Implement `_openContextMenu(participantId, anchorEl)` — build Foundry-style context menu with single entry: `{ name: 'Hide from table', icon: 'fas fa-eye-slash', callback: () => this._dispatchAction(participantId) }`; use canonical label constant (see Dev Notes) - [x] 3.10: Implement `_dispatchAction(participantId)` — determine target state (current=active → hidden; else → active); call `scryingPoolController.action('strip', participantId, targetState, generateOpId(), this._getRevision(participantId))` - [x] 3.11: Update `templates/roster-strip.hbs` with actual ScryingPoolStrip template markup — see Dev Notes §Template Structure - [x] 3.12: Implement `firstStripOpen` tip — on first open (flag unset): show right-click affordance tip in strip header; set `game.user.setFlag('video-view-manager', 'firstStripOpen', true)`; never show again - [x] 3.13: Confirm tests green, run full suite (no regressions) - [x] Task 4: Implement `ActionPopover` class inside `src/ui/gm/ScryingPoolStrip.js` (AC: 6, 15) - [x] 4.1: Implement `ActionPopover` class (not exported; internal to the gm/ layer; or extract to `src/ui/gm/ActionPopover.js` if file grows unwieldy — dev agent's call) - [x] 4.2: Implement `constructor(participantId, currentState, anchorRect, stripElement, onAction)` — build `` element with `h3` name + state label, primary CTA button (`data-action="primary-cta"`), aria attributes - [x] 4.3: Implement `open(anchorEl)` — call `dialog.showModal()`; position via `anchorRect.getBoundingClientRect()` relative to strip; focus primary CTA; attach click-outside listener (click on backdrop area dismisses) - [x] 4.4: Implement `close(reason)` — call `dialog.close(reason)`; remove click-outside listener; return focus to triggering avatar - [x] 4.5: Implement disabled state during PendingOp — primary CTA gets `disabled` + `aria-disabled="true"` attribute when `ScryingPoolController` has a pending op for this participant; listen to `scrying-pool:controllerAction` hook to update - [x] 4.6: Wire Esc via native `` cancel event → call `close()`; return focus to trigger - [x] Task 5: Add CSS — LESS styles for all new components (AC: 2, 3, 4, 16, 17) - [x] 5.1: Add `StateRing` CSS variants to `styles/components/_roster-strip.less` (or extract to `styles/components/_state-ring.less` and `@import` it): `.sp-state-ring--solid`, `--dashed`, `--pending`, `--revert` — see Dev Notes §StateRing CSS spec - [x] 5.2: Add `ParticipantAvatar` layout CSS: 44×44px container, 32px rounded avatar, 12px corner badge bottom-right; hover action rail (fixed-width, reveal via `opacity/visibility/pointer-events`, never `display:none`) - [x] 5.3: Add ScryingPoolStrip layout CSS: floating window, collapsed/expanded states using `max-width` transition (never `width`), `.is-expanded` modifier - [x] 5.4: Add AV tile overlay styles in `styles/components/_roster-strip.less` (scoped to `.scrying-pool` for strip, on `:root` for AV tile tokens): `sp-state-hidden` → reduced opacity + lock-overlay icon; portrait fallback sizing (AV tile dimensions, no layout shift) - [x] 5.5: Add `EmptyStatePanel` CSS: breathing-pulse eye icon (gated under `prefers-reduced-motion: no-preference`), centred layout, NOT styled as error - [x] 5.6: Run `npm run build` — exits 0 - [x] Task 6: Update `module.js` — wire RoleRenderer and ScryingPoolStrip into ready hook (AC: 1, 12, 13) - [x] 6.1: Add imports: `import { RoleRenderer } from './src/ui/RoleRenderer.js';` + `import { AVTileAdapter } from './src/ui/shared/AVTileAdapter.js';` - [x] 6.2: Add module-level `let roleRenderer; let avTileAdapter;` - [x] 6.3: In `Hooks.once('ready')`: after `scryingPoolController.init()`, construct `avTileAdapter = new AVTileAdapter(adapter)` then `roleRenderer = new RoleRenderer(stateStore, scryingPoolController, avTileAdapter, adapter)` then `roleRenderer.init()` - [x] 6.4: If `adapter.users.isGM()`, call `roleRenderer.openStrip()` to render ScryingPoolStrip - [x] 6.5: Update init order comment in module.js: remove `// Story 1.5: NotificationBus → RoleRenderer → RosterStrip` placeholder; document actual current order; add `// Story 2.1: NotificationBus` placeholder for next story - [x] 6.6: Run full pipeline — lint + typecheck + test (all must pass) - [x] Task 7: Pipeline validation (AC: all) - [x] 7.1: `npm run lint` — exits 0 (no new errors beyond the 7 pre-existing in scripts/package.mjs) - [x] 7.2: `npm run typecheck` — exits 0 - [x] 7.3: `npm run test` — all tests pass (≥181 baseline + ~40 new = ≥221 expected) - [x] 7.4: `npm run build` — exits 0 (LESS compiles cleanly) --- ## Dev Notes ### Architecture Context This story builds the first UI layer of the module. All previous stories (1.1–1.4) were headless infrastructure. Story 1.5 introduces: 1. `AVTileAdapter` — isolates all Foundry AV tile DOM interactions 2. `RoleRenderer` — reactive dispatcher subscribing to state change hooks; applies CSS to AV tiles; constructs GM UI 3. `ScryingPoolStrip` — ApplicationV2-style floating window (the GM's primary control surface) 4. `ActionPopover` — native `` for per-participant hide/show actions **Naming clarification (architecture doc vs story):** The architecture doc calls the L1 GM strip `RosterStrip.js` (in `src/ui/gm/`). This story uses `ScryingPoolStrip` (which appears in all UX spec and epics references). Use `ScryingPoolStrip` as both the class name and filename: `src/ui/gm/ScryingPoolStrip.js`. The architecture file-level name is just an approximation — story spec takes precedence. **RoleRenderer vs VisibilityManager:** `VisibilityManager` (Story 1.4) applies WebRTC track logic (hidden → disableTrack). `RoleRenderer` (Story 1.5) applies CSS/DOM visual state to AV tiles — different concern. Do NOT conflate them. **ScryingPoolController is the source of truth for actions:** `ScryingPoolStrip` is a dumb view. It NEVER calls `stateStore.setState()` directly. All mutations go through `ScryingPoolController.action(source, participantId, targetState, opId, baseRevision)`. The strip reads state from `stateStore.getState(userId)`. ### Init Order (EXACT — do not deviate) ``` Hooks.once('ready') → stateStore.init() // Story 1.3 → FoundryAdapter.probeCapability() + webrtcMode // Story 1.3 → visibilityManager = new VisibilityManager(...) // Story 1.4 → visibilityManager.init() // Story 1.4 → socketHandler.setReady(...) // Story 1.4 → scryingPoolController = new ScryingPoolController(...) // Story 1.4 → scryingPoolController.init() // Story 1.4 → avTileAdapter = new AVTileAdapter(adapter) // Story 1.5 (NEW) → roleRenderer = new RoleRenderer(stateStore, scryingPoolController, avTileAdapter, adapter) // Story 1.5 (NEW) → roleRenderer.init() // Story 1.5 (NEW) → if isGM: roleRenderer.openStrip() // Story 1.5 (NEW) // Story 2.1: NotificationBus // Story 2.2: DirectorsBoard (lazy, GM only) ``` **Why AVTileAdapter before RoleRenderer:** `RoleRenderer` receives `avTileAdapter` via constructor injection. It needs the adapter ready before `init()` wires hooks that call through to it. ### Import Boundaries (HARD — enforced by ESLint) ``` src/ui/ → may import: src/core/, src/contracts/, src/utils/ src/ui/gm/ → may import: src/core/, src/contracts/, src/utils/, src/ui/shared/ src/ui/shared/ → may import: src/contracts/, src/utils/ ``` ❌ `src/ui/` importing `src/foundry/` is a hard violation (FoundryAdapter comes in via constructor injection). ❌ `src/core/` importing `src/ui/` is a hard violation. ### Dependency Injection — Zero Direct game.* Access `RoleRenderer`, `AVTileAdapter`, and `ScryingPoolStrip` MUST have zero direct `game.*` access for testability. All Foundry API dependencies come through the injected `adapter`. **Exception for AVTileAdapter:** DOM access via `document.querySelector()` is permissible — it cannot be avoided for AV tile DOM manipulation. Wrap in try/catch; never throw on missing tile. `happy-dom` (Vitest environment) provides `document` in tests. **Exception for ScryingPoolStrip:** `Application` / `ApplicationV2` extend from Foundry's global. In tests, mock at the class level (see §Test Patterns below). Business logic that can be extracted into pure functions should be. ### Canonical Label Constants Create a constants object at the top of `ScryingPoolStrip.js`: ```js const LABELS = Object.freeze({ HIDE_FROM_TABLE: 'Hide from table', SHOW_TO_TABLE: 'Show to table', FIRST_TOOLTIP: 'Hide this participant from other players.', }); ``` All surfaces MUST reference these constants — never inline string literals for action labels. ### Participant Data Shape (for getData()) ```js // Shape returned by ScryingPoolStrip.getData() { participants: [ { userId: 'user-abc', name: 'Alice', // from adapter.users.get(userId).name avatarSrc: '...', // from adapter.users.get(userId).avatar state: 'active', // from stateStore.getState(userId) stateLabel: 'Active', // human-readable label (not player vocabulary partition — GM sees state names) hasPendingOp: false, // check scryingPoolController._pendingOps.has(userId) isCurrentUser: false, // adapter.users.current()?.id === userId } ], isExpanded: true, // from user flag or default true on firstStripOpen isEmpty: false, } ``` **Portrait Fallback resolution:** 1. `user.avatar` if set and not default placeholder 2. `game.settings.get('core', 'defaultToken')` (system default) 3. `'icons/svg/mystery-man.svg'` (Foundry built-in fallback) Access via adapter: `adapter.users.get(userId)?.avatar`. ### StateRing CSS Spec (from UX spec §6.4) ```less // In styles/components/_roster-strip.less (or a new _state-ring.less) .sp-state-ring--solid { box-shadow: 0 0 0 2px var(--sp-state-color); } .sp-state-ring--dashed { outline: 2px dashed var(--sp-state-color); outline-offset: 2px; } .sp-state-ring--pending { box-shadow: 0 0 0 2px var(--sp-state-color); // animation added only under no-preference: } .sp-state-ring--revert { box-shadow: 0 0 0 2px var(--sp-urgency-director); } @media (prefers-reduced-motion: no-preference) { .sp-state-ring--pending { animation: sp-pulse 2s ease-in-out infinite; } @keyframes sp-pulse { 0%, 100% { opacity: 1; } 50% { opacity: 0.4; } } } ``` **Ring variant per state:** | State | Ring class | |---|---| | `active` | `--solid` | | `hidden` | `--dashed` | | `self-muted` | `--solid` | | `offline` | (no ring) | | `cam-lost` | `--dashed` | | `reconnecting` | `--solid` + pulse | | `never-connected` | (no ring) | | `ghost` | `--solid` dotted variant | | `pending` | `--pending` (animated pulse) | | revert flash | `--revert` (200ms amber, then restore) | ### AV Tile DOM Integration (AVTileAdapter) **Tile selector:** Foundry AV tiles have `data-user-id` attribute. Stable selector: ```js document.querySelector(`.camera-view[data-user-id="${userId}"]`) // or: .user-camera[data-user-id="${userId}"] — check actual Foundry v14 DOM // Test with real Foundry to confirm stable selector — use console.log to inspect ui.webrtc.element in dev ``` **mount() idempotency pattern:** ```js mount(userId, element) { const tile = this._findTile(userId); if (!tile) { console.warn('[ScryingPool] AVTileAdapter.mount: tile not found for', userId); return; } // Idempotency: check for existing element with same data-sp-role const role = element.dataset.spRole; const existing = tile.querySelector(`[data-sp-role="${role}"]`); if (existing) { existing.replaceWith(element); // update in place return; } tile.appendChild(element); } ``` **State class isolation:** use `setStateClass()` to ensure only one `sp-state-*` class is ever present: ```js setStateClass(userId, stateName) { const tile = this._findTile(userId); if (!tile) { console.warn('[ScryingPool] AVTileAdapter.setStateClass: tile not found for', userId); return; } // Remove all sp-state-* classes, add new one const existing = [...tile.classList].filter(c => c.startsWith('sp-state-')); existing.forEach(c => tile.classList.remove(c)); if (stateName) tile.classList.add(`sp-state-${stateName}`); } ``` ### Template Structure (roster-strip.hbs) Replace the placeholder with actual ApplicationV2 template structure. The template is rendered inside the Foundry Application shell: ```hbs {{!-- ScryingPoolStrip — floating GM control strip --}} ``` ### ScryingPoolStrip — Application vs ApplicationV2 FoundryVTT v14 introduces `ApplicationV2` with PARTS, but the simpler `Application` base class still works and is more straightforward for this pattern. Use `Application` for Story 1.5 to avoid ApplicationV2 PARTS complexity: ```js export class ScryingPoolStrip extends Application { static get defaultOptions() { return foundry.utils.mergeObject(super.defaultOptions, { id: 'scrying-pool-strip', template: 'modules/video-view-manager/templates/roster-strip.hbs', popOut: true, resizable: false, title: 'Scrying Pool', classes: ['scrying-pool-strip'], }); } } ``` If `ApplicationV2` is strongly preferred (e.g., for future PARTS-based rendering), the pattern changes to: ```js export class ScryingPoolStrip extends foundry.applications.api.ApplicationV2 { static PARTS = { strip: { template: '...' } }; } ``` **Dev agent's call:** Use `Application` for simplicity unless you have a specific reason to use `ApplicationV2`. Document the choice in the class JSDoc. ### Position Persistence Pattern User flag key: `video-view-manager.stripState` (note: world settings use `scrying-pool.` prefix but user flags use module ID `video-view-manager`). ```js // Save on close game.user.setFlag('video-view-manager', 'stripState', { left: this.position.left, top: this.position.top, open: false, expanded: this._isExpanded, }); // Load on open const saved = game.user.getFlag('video-view-manager', 'stripState'); if (saved?.left !== undefined) { options.left = saved.left; options.top = saved.top; } this._isExpanded = saved?.expanded ?? true; // default expanded on first open ``` ### OpId and Revision for Action Dispatch `ScryingPoolStrip._dispatchAction(participantId)` needs to call `scryingPoolController.action(source, participantId, targetState, opId, baseRevision)`. - **opId:** generate via `import { generateOpId } from '../../utils/uuid.js'` then `const opId = generateOpId()` - **baseRevision:** `scryingPoolController._revisions.get(participantId) ?? 0` — BUT this accesses a private field. Better pattern: expose a public `getRevision(participantId)` method on `ScryingPoolController`. This is a Story 1.5 addition to the Story 1.4 class. - ADD `getRevision(participantId)` to `src/core/ScryingPoolController.js`: `return this._revisions.get(participantId) ?? 0;` - This is a minor non-breaking addition to the Story 1.4 file. - **targetState:** `stateStore.getState(participantId) === 'hidden' ? 'active' : 'hidden'` — toggle logic. If current state is NOT hidden → hide; if hidden → show. ### First-Encounter Tooltip (firstHideTooltip flag) On first hover over the primary CTA button in `ActionPopover` (`firstHideTooltip` flag not set): - Set `data-tooltip` to `"Hide this participant from other players."` - On mouseenter: check `localStorage.getItem('scrying-pool.firstHideTooltip')` — if unset, show extended tooltip and set flag via `localStorage.setItem('scrying-pool.firstHideTooltip', '1')` - Subsequent hovers: canonical label only Note: `firstHideTooltip` is stored in `localStorage` (client-side, session-local) per the architecture decision for v1.0. See architecture §Data Architecture. ### EmptyStatePanel Animation ```less // In _roster-strip.less .sp-empty__icon { display: block; // Static by default; animation only under no-preference } @media (prefers-reduced-motion: no-preference) { .sp-empty__icon { animation: sp-breathe 3s ease-in-out infinite; } @keyframes sp-breathe { 0%, 100% { opacity: 0.6; transform: scale(1); } 50% { opacity: 1.0; transform: scale(1.05); } } } ``` ### Existing Files Being Modified **`module.js`** — current ready hook ends with: ```js try { visibilityManager.init(); scryingPoolController.init(); } catch (err) { console.error('[ScryingPool] Module initialization failed:', err); throw err; } ``` After `scryingPoolController.init()`, add the Story 1.5 wiring block inside the same try/catch. **`src/core/ScryingPoolController.js`** — add public `getRevision(participantId)` method: ```js /** Returns the last confirmed revision for a participant (0 if unknown). */ getRevision(participantId) { return this._revisions.get(participantId) ?? 0; } ``` ### Hooks Used in This Story | Hook | Direction | Who calls | Who listens | |------|-----------|-----------|-------------| | `scrying-pool:stateChanged` | Hooks.callAll | StateStore | RoleRenderer (applies CSS to AV tiles) | | `scrying-pool:controllerAction` | Hooks.callAll | ScryingPoolController | ScryingPoolStrip (re-render), ActionPopover (disable during pending) | | `updateUser` | Hooks.on | Foundry core | RoleRenderer (mid-session role change rebuild) | ### OQ-1 Reminder `adapter.webrtc` is ALWAYS `null` in production (CSS fallback path confirmed by Story 1.2 spike). The webrtcMode will be `'css-fallback'`. `VisibilityManager._onStateChanged()` is already a no-op when `adapter.webrtc` is null. `RoleRenderer` applies CSS/DOM state — no webrtc dependency. ### Test Patterns **Testing AVTileAdapter (happy-dom):** ```js import { AVTileAdapter } from '../../../src/ui/shared/AVTileAdapter.js'; import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js'; // happy-dom provides document — set up tile DOM beforeEach(() => { document.body.innerHTML = `
`; }); test('mount() is idempotent', () => { const adapter = createFoundryAdapterMock(); const avAdapter = new AVTileAdapter(adapter); const el = document.createElement('div'); el.dataset.spRole = 'lock-overlay'; avAdapter.mount('user-1', el); avAdapter.mount('user-1', el); // second call — must not duplicate const tile = document.querySelector('[data-user-id="user-1"]'); expect(tile.querySelectorAll('[data-sp-role="lock-overlay"]').length).toBe(1); }); ``` **Testing RoleRenderer:** ```js import { vi } from 'vitest'; import { RoleRenderer } from '../../../src/ui/RoleRenderer.js'; import { createFoundryAdapterMock } from '../../helpers/foundryAdapterMock.js'; beforeEach(() => { vi.stubGlobal('Hooks', { on: vi.fn(), once: vi.fn(), off: vi.fn(), callAll: vi.fn() }); }); afterEach(() => { vi.unstubAllGlobals(); }); function makeAVTileAdapter() { return { mount: vi.fn(), unmount: vi.fn(), setStateClass: vi.fn(), disconnect: vi.fn(), onTileRerender: vi.fn() }; } ``` **Testing ScryingPoolStrip (logic isolation):** Extract business logic into pure functions where possible (e.g., `resolveTargetState(currentState)`, `buildParticipantData(users, stateStore)`) and test those directly. For the Application class itself: ```js // Stub Application globally vi.stubGlobal('Application', class { static get defaultOptions() { return {}; } }); ``` **General rules (same as Story 1.4):** - `createFoundryAdapterMock()` — canonical mock, no ad-hoc stubs - Named exports only - JSDoc `/** ... */` above every exported class - `async/await` not `.then()` - Guard clauses with early return - `console.warn('[ScryingPool]', ...)` prefix on all console calls ### ESLint / TypeScript Notes (Learnings from Stories 1.3 + 1.4) - Add JSDoc class comment (`/** ... */`) above EVERY exported class — `jsdoc/require-jsdoc` rule - Use `// eslint-disable-next-line no-unused-vars` (line comment) on the line ABOVE a `catch (_)` binding - `Application`/`Hooks`/`game`/`ui` globals are declared in `src/types/foundry-globals.d.ts` — do NOT add new declarations for already-declared globals - `foundry.utils.mergeObject` is the v14 way to extend `defaultOptions` - If adding `game.user.getFlag(...)` calls, check that `game.user` is declared in `foundry-globals.d.ts`; if not, add the `setFlag`/`getFlag` surface to it (use `declare const game: { user: { setFlag: ..., getFlag: ..., ... } }`) - `localStorage` is a browser global — no declaration needed - Pre-existing lint errors in `scripts/package.mjs` (7 errors) are not this story's scope — do NOT fix them ### Project Structure Notes **Files to create:** ``` src/ui/RoleRenderer.js ← NEW (Story 1.5) src/ui/gm/ScryingPoolStrip.js ← NEW (Story 1.5); ActionPopover lives here or in adjacent file src/ui/shared/AVTileAdapter.js ← NEW (Story 1.5); also used by Story 1.6 tests/unit/ui/RoleRenderer.test.js ← NEW (Story 1.5) tests/unit/ui/gm/ScryingPoolStrip.test.js ← NEW (Story 1.5) tests/unit/ui/shared/AVTileAdapter.test.js ← NEW (Story 1.5) ``` **Files to update:** ``` module.js ← UPDATE: imports + ready hook wiring (Story 1.5 block) src/core/ScryingPoolController.js ← UPDATE: add getRevision(participantId) public method templates/roster-strip.hbs ← UPDATE: replace placeholder with actual template styles/components/_roster-strip.less ← UPDATE: add StateRing + ParticipantAvatar + strip layout CSS ``` **Files NOT changed:** - `src/contracts/` — all contracts already complete; no changes needed - `src/core/StateStore.js`, `SocketHandler.js`, `VisibilityManager.js` — no changes - `src/foundry/FoundryAdapter.js` — no changes (all deps come through existing adapter surface) - `tests/fixtures/` — no new fixtures needed; use inline DOM/objects in UI tests **Import boundary check for new files:** ``` src/ui/RoleRenderer.js → imports: src/core/ ✅, src/utils/ ✅, src/ui/shared/ ✅ src/ui/gm/ScryingPoolStrip.js → imports: src/core/ ✅, src/utils/ ✅, src/ui/shared/ ✅ src/ui/shared/AVTileAdapter.js → imports: (nothing internal) ✅ ``` ### References - Story 1.5 spec: `_bmad-output/planning-artifacts/epics.md` §Story 1.5 (lines 397–497) - UX components spec: `_bmad-output/planning-artifacts/ux-design-specification.md` §6.2–6.9 (lines 1135–1265) - UX action hierarchy: `_bmad-output/planning-artifacts/ux-design-specification.md` §7.1 (lines 1390–1411) - UX overlay patterns: `_bmad-output/planning-artifacts/ux-design-specification.md` §7.3 (lines 1452–1459) - StateRing CSS: `_bmad-output/planning-artifacts/ux-design-specification.md` §6.4 (lines 1164–1181) - Architecture init order: `_bmad-output/planning-artifacts/architecture.md` §Initialisation Order (lines 303–319) - Architecture import boundaries: `_bmad-output/planning-artifacts/architecture.md` (lines 428–444) - Architecture data flow: `_bmad-output/planning-artifacts/architecture.md` §Data Flow — GM Visibility Toggle (lines 805–826) - Architecture error handling by layer: `_bmad-output/planning-artifacts/architecture.md` (lines 510–517) - State precedence: `_bmad-output/planning-artifacts/architecture.md` §State Map (lines 546–560) - UX design requirements: `_bmad-output/planning-artifacts/epics.md` UX-DR3–UX-DR8, UX-DR18–UX-DR21 (lines 108–144) - Story 1.4 dev notes (init order, ScryingPoolController API): `_bmad-output/implementation-artifacts/1-4-core-logic-scryingpoolcontroller-and-visibilitymanager.md` - firstHideTooltip + firstStripOpen flags: `_bmad-output/planning-artifacts/ux-design-specification.md` (lines 571, 1091) - AV tile selector / VisibilityBadge injection pattern: `_bmad-output/planning-artifacts/ux-design-specification.md` §VisibilityBadge Injection Pattern (lines 465–471) - Canonical adapter mock: `tests/helpers/foundryAdapterMock.js` - ScryingPoolController implementation: `src/core/ScryingPoolController.js` - StateStore implementation: `src/core/StateStore.js` - Current module.js: `module.js` - Deferred work (do not fix in 1.5): `_bmad-output/implementation-artifacts/deferred-work.md` --- ## Dev Agent Record ### Agent Model Used Claude Sonnet 4.6 (claude-sonnet-4.6) ### Debug Log References - ESLint no-undef: `Application` in ScryingPoolStrip.js — fixed with `/* global Application */` comment. `typeof Application` is exempt from no-undef but direct reference in ternary is not. - ESLint no-unused-vars: `spy` in RoleRenderer.test.js bulk-payload test — removed. - TypeScript TS2488: `[...tile.classList]` spread on DOMTokenList — replaced with `Array.from(tile.classList)`. - `showFirstOpenTip` undefined in `activateListeners` — was referencing a variable from `getData()` scope; fixed to re-evaluate from `game.user.getFlag()` directly. ### Completion Notes List - AVTileAdapter (24 tests): Full TDD red→green. `mount()` idempotent via data-sp-role key, `unmount()` removes [data-sp-mount] children, `setStateClass()` swaps sp-state-* classes, `onTileRerender()` uses MutationObserver per userId, `disconnect()` cleans all observers. - RoleRenderer (20 tests): TDD red→green with `vi.mock(ScryingPoolStrip)` + `vi.stubGlobal(Hooks)`. Registers 3 hooks in `init()`, handles stateChanged/controllerAction/updateUser. `openStrip()` lazily constructs ScryingPoolStrip singleton. - ScryingPoolStrip (23 tests): TDD red→green. Tests cover LABELS immutability, `resolveTargetState()`, `buildParticipantList()`, `getData()`, `defaultOptions`. ActionPopover implemented as internal (non-exported) class with `` + Esc cancel + backdrop click dismiss + firstHideTooltip localStorage. - ScryingPoolController: Added `getRevision()` and `hasPendingOp()` public methods (non-breaking). - module.js: Wired AVTileAdapter + RoleRenderer with `game.webrtc !== null` guard for AC-13. - templates/roster-strip.hbs: Full HBS template with participants list, EmptyStatePanel, expand toggle, overlay layer, firstStripOpen tip. - styles/components/_roster-strip.less: Full CSS — StateRing variants, ParticipantAvatar layout, strip collapsed/expanded via max-width transition, AV tile overlays, EmptyStatePanel breathing animation, context menu, ActionPopover — all animations gated under `prefers-reduced-motion: no-preference`. - Pipeline: 248 tests (181 baseline + 67 new), 0 lint errors (pre-existing 7 in scripts/package.mjs untouched), 0 typecheck errors, build passes. ### File List - `src/ui/shared/AVTileAdapter.js` — NEW - `src/ui/RoleRenderer.js` — NEW - `src/ui/gm/ScryingPoolStrip.js` — NEW (includes ActionPopover class) - `tests/unit/ui/shared/AVTileAdapter.test.js` — NEW (24 tests) - `tests/unit/ui/RoleRenderer.test.js` — NEW (20 tests) - `tests/unit/ui/gm/ScryingPoolStrip.test.js` — NEW (23 tests) - `src/core/ScryingPoolController.js` — UPDATED (added getRevision, hasPendingOp) - `module.js` — UPDATED (imports + ready hook wiring + webrtc null guard) - `templates/roster-strip.hbs` — UPDATED (full HBS template) - `styles/components/_roster-strip.less` — UPDATED (full LESS styles) ### Change Log - Story 1.5 implementation complete (Date: 2026-05-22) - Added AVTileAdapter, RoleRenderer, ScryingPoolStrip, ActionPopover - Added getRevision() + hasPendingOp() to ScryingPoolController - Wired GM UI into module.js ready hook with game.webrtc null guard - 248 tests passing (67 new), lint/typecheck/build all clean \n---\n\n## DIFF TO REVIEW\n 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} participantId → PendingOp */ + this._pendingOps = new Map(); + /** @type {Map} 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(); + }); + }); +});