9ace9ac0d8
- README: status bump to sprint 2, blueprints + workflow + MITRE section, test counts refreshed (131/63/68) - CHANGELOG: sprint 2 entry under [Unreleased]; sprint 1 moved to its own [Sprint 1] section - tasks/lessons.md: 5 lessons captured (3 frontend testing gotchas, agent-reuse via SendMessage, e2e refresh on placeholder supersession) - tasks/todo.md: status flipped to 🟢 SPRINT COMPLET, execution sequence ticks updated with commit hashes Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
28 lines
3.1 KiB
Markdown
28 lines
3.1 KiB
Markdown
# 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.
|