# Lessons Learned

Recurring mistakes and the rule we adopted so the same issue doesn't bite twice. Append-only. Each entry has: date, context, lesson.

---

## Sprint 2 (closed 2026-05-26)

### Testing — Vitest module hoisting (frontend-builder)
**Context** : `vi.mock(path, factory)` is hoisted to module scope before any other statement runs. A `mockAuth(role)` helper that captures `role` in a closure crashes at runtime with `"role is not defined"` because the factory executes before the closure is set up.
**Lesson** : when a Vitest mock needs runtime-mutable state, declare the mutable in module scope (`let mockRole = 'redteam'`) and mutate it inside `beforeEach`. Closures over test-local variables don't survive the hoist.

### Testing — `useParams()` in MemoryRouter (frontend-builder)
**Context** : a page component that reads `useParams()` returns an empty object when rendered directly inside `<MemoryRouter>` — no params are extracted unless the component is mounted under a matching `<Route path="...">`.
**Lesson** : page tests that depend on params must wrap the component in `<MemoryRouter><Routes><Route path="/foo/:id" element={<Page />} /></Routes></MemoryRouter>` and set `initialEntries={['/foo/42']}`.

### Testing — jsdom missing browser APIs (frontend-builder)
**Context** : jsdom doesn't implement `Element.scrollIntoView`. Calling it in a component (e.g., scrolling the active autocomplete option into view) throws inside Vitest unless guarded.
**Lesson** : in components meant to run in both browser and jsdom, guard browser-only DOM APIs with optional chaining (`el?.scrollIntoView?.({ block: 'nearest' })`) or feature-detect before calling.

### Process — Reuse idle team agents via SendMessage, not Agent (team-lead)
**Context** : during post-review fixes, I re-spawned `backend-builder` and `frontend-builder` via `Agent({name: "..."})` even though the original instances were still alive (just idle). The system auto-suffixed `-2` and BOTH instances received the same brief, producing duplicate parallel commits on the branch. Frontend got two fix commits (`c9032a9` + `cf0e8a8`) where one would have sufficed; the second commit happened to layer cleanly on top, but only by luck.
**Lesson** : to redispatch work to an existing team agent, use `SendMessage({to: "backend-builder", ...})`. `Agent({name: ...})` creates a new instance when the name is taken. The team config at `~/.claude/teams/<team>/config.json` is the source of truth for who's already present.

### Workflow — Update e2e assertions when later sprints supersede placeholders (team-lead)
**Context** : Sprint 1 AC-4.9 asserted the literal text "Simulations à venir au Sprint 2" on `EngagementDetailPage`. Sprint 2 correctly replaced that placeholder with `<SimulationList>`, breaking the assertion. The test-verifier initially classified this as "pre-existing failure".
**Lesson** : whenever a later sprint replaces a placeholder asserted by an earlier sprint's e2e test, the earlier test must be refreshed in the same sprint (not deferred). A failing test that's "expected" is still a failing test — and it muddies the signal of the PR.