# Retrospective — Epic 1: Core Camera Visibility Control

**Date:** 2026-05-22  
**Facilitator:** Amelia (Developer)  
**Project Lead:** Morr  
**Status:** Complete

---

## Epic Summary

| Metric | Value |
|--------|-------|
| Stories Completed | 6 / 6 (100%) |
| Test Count at Close | 296 tests, all passing |
| Technical Debt Items | 8 open (deferred-work.md) |
| Major Blockers | 1 (OQ-1 WebRTC spike — resolved as css-fallback) |
| Architecture Deviations Corrected | 3 |
| Previous Retrospective | None (first epic) |

**Stories:**
- 1.1 — Module Scaffold, CI/CD & Design Token System ✅
- 1.2 — WebRTC Spike — Track Disabling API Validation ✅
- 1.3 — Data Layer — FoundryAdapter, StateStore & Socket Infrastructure ✅
- 1.4 — Core Logic — ScryingPoolController & VisibilityManager ✅
- 1.5 — GM Control UI — ScryingPoolStrip, ActionPopover & AV Tile Integration ✅
- 1.6 — Player Camera Status Badge ✅

---

## Successes & Strengths

1. **TDD discipline held across all 6 stories.** Started at 49 tests (Story 1.1), ended at 296 — every story delivered a passing pipeline with zero regressions.

2. **Import boundary enforcement worked.** ESLint `import/no-restricted-paths` wired in Story 1.1 held for the full epic. Zero cross-layer violations slipped through.

3. **Accessibility built into ACs, not added later.** `prefers-reduced-motion` gates, native `<dialog>` for focus trapping, `aria-live="polite"` on the VisibilityBadge — all specified upfront and delivered.

4. **UI pattern reuse was excellent.** `ActionPopover` (Story 1.5) was directly reused as the pattern for `FirstEncounterPanel` and `VisibilityDetailsPanel` (Story 1.6) — zero wheel reinvention.

5. **Architecture guardrails saved multiple errors.** Three corrections were caught and applied mid-story rather than shipping as bugs.

6. **Morr's assessment:** Everything runs fine. Clean green on all tests.

---

## Challenges & Growth Areas

### 1. Epic/Architecture Spec Drift (3 occurrences)

Mismatches between the epics file and architecture document caught mid-story:
- Story 1.1: `.eslintrc.js` spec vs ESLint 9 flat config (`eslint.config.js`)
- Story 1.2: Epic path `src/adapters/foundry-adapter.js` vs architecture-canonical `src/foundry/FoundryAdapter.js`
- Story 1.2: Setting namespace `video-view-manager.*` in spec vs correct `scrying-pool.*`

**Root cause:** Architecture doc written before epics were finalized; paths/namespaces drifted.  
**Resolution:** Dev agent caught all three. No shipped bugs, but correction cycles add overhead.

### 2. Spike Story AC Format

Story 1.2's ACs assumed `track-disable` would be the spike outcome. The spike proved it wasn't — `css-fallback` is the FoundryVTT v14 reality. The dev agent correctly updated test expectations, but the story file had wrong expectations going in.

**Root cause:** Spike ACs written as implementation expectations, not hypothesis format.

### 3. Vitest Fake Timer Nested Pattern Not Documented

Story 1.6 discovered that `vi.advanceTimersByTime(N)` requires two separate advance calls when a timer callback schedules a nested timer. The first advance fires the outer timer; the second fires the inner one. This cost debugging time.

**Pattern discovered:** 
```js
vi.advanceTimersByTime(10_001); // fires 10s collapse timeout
vi.advanceTimersByTime(301);    // fires 300ms CSS transition replacement timer
```

### 4. Story 1.1 Review Depth

The Story 1.1 code review generated 3 separate review prompt files and 7 deferred CI/CD items. While appropriate for a scaffold story, the depth felt heavy relative to the scope.

---

## Key Insights & Lessons

1. **Spike stories need hypothesis-format ACs.** Write: "Given X, when spike runs, then outcome will be documented as one of [A/B/C]" — not pre-determined implementation expectations.

2. **Vitest nested timer testing requires two-step advance.** Always use two `vi.advanceTimersByTime()` calls when the callback under test schedules a subsequent timer. Document this pattern visibly in dev notes for any story with timer-based logic.

3. **Architecture doc is canonical over epics.** When epics and architecture disagree, architecture wins. Path inconsistencies should be resolved in epics review before stories are created.

4. **Foundation story reviews deserve more scrutiny.** Story 1.1 review depth was appropriate; the overhead is proportional to its cross-cutting nature. Non-foundation stories can use lighter review.

5. **Reuse patterns establish themselves within 1-2 stories.** ActionPopover → FirstEncounterPanel/VisibilityDetailsPanel shows that patterns spread naturally when the first implementation is clean.

---

## Deferred Technical Debt (from deferred-work.md)

These items are tracked, not forgotten, and should be addressed in Epic 2:

**HIGH PRIORITY (should fold into Story 2.1 or 2.2):**
- [ ] Memory leak: `_revisions` Map in `ScryingPoolController.js:31` — no cleanup on user disconnect
- [ ] Listener cleanup: `init()` socket + Hooks listeners never unregistered in `ScryingPoolController.js` and `VisibilityManager.js`
- [ ] Echo revision validation: No guard against NaN/Infinity in `ScryingPoolController.js:164`

**MEDIUM PRIORITY:**
- [ ] Echo accepts non-finite revisions (same file, revision ?? 0 doesn't validate type)
- [ ] CVE-2023-43645 in flat-cache-4.0.1 (needs verification — may be transitive dev dep only)

**LOW PRIORITY (CI/CD, pre-existing):**
- [ ] Missing failure notifications, missing build artifact upload, no Node matrix testing
- [ ] Missing concurrency control, missing i18n schema validation, ubuntu-latest not pinned

---

## Epic 2 Preview & Readiness

**Epic 2: Player Notifications & Director's Board**  
Stories: 2.1 NotificationBus, 2.2 Director's Board Core, 2.3 Director's Board Bulk Actions

**Dependencies on Epic 1 work:**
- `ScryingPoolController.action()` + event system ✅ solid
- `SocketHandler` echo/broadcast ✅ solid
- `AVTileAdapter` pattern available for Director's Board card tiles ✅
- `StateStore` visibility matrix ✅

**Readiness: CLEAR TO PROCEED ✅**  
No blockers. Recommended: fold memory leak + listener cleanup into Story 2.1 dev tasks.

**Preparation notes for Story 2.1:**
- NotificationBus introduces a 3s coalescing timer — explicitly include the vitest fake timer two-advance pattern in dev notes
- Include deferred debt items (_revisions cleanup, listener unregistration) as Story 2.1 technical tasks
- NotificationBus subscribes to ScryingPoolController events — the listener cleanup debt becomes higher priority once another subscriber is added

---

## Action Items

### Technical (carry into Epic 2)

| Item | Owner | Priority | Target Story |
|------|-------|----------|--------------|
| Fix `_revisions` Map leak (`ScryingPoolController.js:31`) | Dev | High | 2.1 or 2.2 |
| Add listener cleanup to ScryingPoolController + VisibilityManager | Dev | High | 2.1 or 2.2 |
| Add echo revision type validation (reject NaN/Infinity) | Dev | Medium | 2.1 or 2.2 |
| Verify CVE-2023-43645 flat-cache scope | Dev | Low | Backlog |

### Process Improvements

| Item | Action |
|------|--------|
| Spec drift | Review epic paths/namespaces against architecture doc before story creation |
| Spike ACs | Write spike ACs as hypothesis format, not expected outcomes |
| Fake timer pattern | Add vitest nested timer two-advance pattern to story dev notes whenever coalescing timers are involved |

---

## Next Steps

1. ✅ Retrospective saved
2. ✅ `epic-1-retrospective` marked `done` in sprint-status.yaml
3. → Create Story 2.1 (NotificationBus & Notification Verbosity) using `bmad-create-story`
4. → Fold deferred debt items into Story 2.1 dev tasks

---

*Amelia (Developer): "Great session, Morr. Epic 1 delivered a solid, tested foundation. Epic 2 is clear to go."*

*Winston (System Architect): "The architecture held. Import boundaries, constructor rules, token system — all intact."*

*Sally (UX Designer): "Accessibility-first delivered. Epic 2 has its own UX challenges with the Director's Board — looking forward to it."*