Metamorph/tasks/lessons.md

---
type: lessons
project: Metamorph
---

# Metamorph — Lessons learned

> Capture session-level retrospectives here: surprises, traps avoided, decisions revisited. Keep entries short and actionable. Most recent first.

## 2026-05-08 — M0 bootstrap

- Spec finalisée d'abord (`tasks/spec.md`), 8 tours de questions ciblées avant tout code → 0 hypothèse latente avant M0. Pattern à reproduire pour les futurs projets greenfield.
- Choix `uv` pour le backend Python (rapidité de lock, image Docker plus mince qu'avec poetry).
- TLS terminé par reverse proxy externe (cf. spec §6 NF-network) → pas de Caddy/Traefik dans le compose, simplifie le M0.
- Bootstrap du 1er admin via token affiché dans les logs : retenu sur Token-in-logs plutôt que ENV pour éviter de mettre le password en clair dans `.env`.
- **Piège Dockerfile** : la process-substitution bash `<(...)` ne marche pas dans une instruction `RUN` Docker car le shell par défaut est `sh`, pas `bash`. Soit ajouter `SHELL ["/bin/bash", "-c"]`, soit refactor sans process-sub. Ici j'ai préféré refactor (plus portable) : `uv venv` + `uv pip install --python /opt/venv/bin/python .`. Quand un `uv.lock` existera, basculer sur `uv sync --frozen --no-dev`.
- Vérification d'un compose sans Docker installé : `python3 -c "import yaml; yaml.safe_load(open('docker-compose.yml'))"` valide la syntaxe YAML, et un script qui croise les `environment:` du compose avec `.env.example` détecte les variables manquantes côté docs.
- **Lancer le subagent `spec-reviewer` à chaque fin de milestone** (HARD RULE 4 du CLAUDE.md global). J'avais oublié à la fin de M0 ; le user me l'a rappelé. Le reviewer a remonté 6 défauts légitimes en quelques minutes (pre-commit absent, fonts via CDN, secrets par défaut non gardés, `make dev` no-op, `database_url` dead-code, Node engines non pinned). À automatiser dans le workflow de fin de milestone.
- **Spec §7 "pas de CDN runtime"** s'applique aussi aux fonts, pas seulement aux libs JS. Self-host via `@fontsource/<name>` plutôt que Google Fonts `<link>` — bonus OPSEC (pas de fingerprinting via fonts.googleapis.com).
- **Pattern de garde de secrets** : un `model_validator` Pydantic qui refuse de booter en `APP_ENV != "dev"` avec des secrets manquants ou égaux aux placeholders de `.env.example`. Coût quasi nul, élimine la classe entière des "oubli de set en prod".
- **Makefile portable docker/podman** : `ENGINE := $(shell command -v docker … podman …)`, puis sélection du compose driver en fonction (`docker compose` vs `podman compose` vs `podman-compose` legacy). Le piège classique `COMPOSE ?=` ne marche pas si on veut conditionner la valeur par défaut sur `ENGINE` — il faut `ifndef COMPOSE` + `ifeq ($(ENGINE),docker)`. Tous les targets restent compose-driven (`$(COMPOSE) exec`, etc.) ; seuls `volumes` / `inspect-health` / `logs-api` ont besoin de `$(ENGINE)` directement, et même là on évite les filtres par label projet (instables entre podman-compose et docker compose) en se reposant sur `container_name:` du compose file.

## 2026-05-10 — M0 DoD validation (réelle, pas paperware)

- **JE DOIS LANCER LE DoD MOI-MÊME avant de déclarer un milestone done.** L'utilisateur me l'a fait remonter ; le `make up` initial échouait sur 3 problèmes que la revue statique n'a pas vus. Règle : à chaque fin de milestone, exécuter le DoD localement (`make up` + smoke + e2e) en plus du spec-reviewer.
- **Podman + Fedora exige des FQDN d'image** (`docker.io/library/postgres:16-alpine`, pas `postgres:16-alpine`). Le mode `short-name-mode=enforcing` fail sans TTY pour prompter. Docker accepte le même préfixe transparente. → Dorénavant tous les `image:` et `FROM …` des projets cross-engine sont qualifiés.
- **`.dockerignore` qui exclut `*.md` casse `pyproject.toml` qui référence `readme = "README.md"`** : hatchling lit le README au build pour valider les métadonnées. Soit on copie le README explicitement, soit on n'exclut pas les `*.md`, soit on retire la clé `readme`. J'ai retiré la clé pour découpler.
- **`extends HTMLAttributes<HTMLDivElement>` clash sur `title`** : la prop native est `string`, donc redéfinir `title?: ReactNode` produit TS2430. Pattern à retenir : `Omit<HTMLAttributes<…>, 'title'>` quand on overload `title`/`color`/`autoFocus` etc.
- **Podman-compose 1.x ne surfait pas les `HEALTHCHECK` du Dockerfile dans `podman inspect`** : il faut redéclarer le healthcheck dans le `docker-compose.yml` pour que `make inspect-health` voie réellement l'état. Bonus : c'est aussi plus portable.
- **Piège shell : `make up 2>&1 | tail -80` bloque** quand la sortie est petite, parce que `tail` bufferise jusqu'à recevoir SIGPIPE en fin de pipeline ; quand le build est lent, on n'a aucune sortie pendant des minutes. Fix : rediriger vers fichier (`>/tmp/log 2>&1`) puis `tail` séparément, ou utiliser le `Monitor` tool pour streamer.
- **`PODMAN_COMPOSE_WARNING_LOGS=false`** masque le banner "Executing external compose provider …" qui spamme chaque commande. À exporter depuis le Makefile.

## 2026-05-10 — M1 schéma DB & migrations

- **Compose pioche le DERNIER stage du Dockerfile par défaut.** En ajoutant un stage `test` après `runtime`, le container `api` s'est mis à exécuter `python -m pytest` au lieu de `gunicorn`, en boucle (exit 1 → restart → exit 1). Fix : `target: runtime` explicite dans `docker-compose.yml`. Règle : **toujours préciser `target:` quand un Dockerfile a >1 stage final viable.**
- **Snapshot vs référence (spec §11)** : pour qu'un snapshot survive à un re-sync de la référence (ex : MITRE qui retire une technique), il faut **dénormaliser les champs descriptifs** dans la table snapshot (ici `mitre_external_id`, `mitre_name`, `mitre_url`) et **ne pas mettre de FK** vers la table source. Si on garde une FK, la cascade détruit la donnée historique (CASCADE) ou bloque le sync (RESTRICT). La dénormalisation est le bon trade-off pour un état figé en lecture après archivage.
- **`SoftDeleteMixin.__table_args__` est silencieusement écrasé** par la classe enfant qui déclare son propre `__table_args__`. Pattern à éviter pour les mixins qui veulent ajouter des contraintes/index. Soit ne rien mettre dans `__table_args__` du mixin (et imposer aux classes de déclarer l'index), soit utiliser `event.listens_for("after_parent_attach", ...)`. J'ai choisi la 1re option : explicite > magique.
- **Workflow Alembic en container** : `alembic revision --autogenerate` crée le fichier dans le container, qu'il faut `podman cp` vers l'host avant rebuild. Sinon perdu. Ajouter ce détail dans la doc M1 (et envisager un bind mount `dev` plus tard).
- **Bypass `APP_ENV` doit couvrir `dev` ET `test`** : un container test légitime ne doit pas avoir besoin de secrets prod-grade. `if self.APP_ENV in ("dev", "test"): return self`.
- **`pytest` dans le runtime image, c'est non.** Faire un stage `test` dédié (multi-stage `--target test`) qui étend `deps` + `dev extras` + `tests/`, lancé via `podman run --rm --network <project>_<network>` en éphémère. Le runtime reste minimal en prod.
- **Le test d'intégration "expected tables/FK/CHECK" est le bon filet de sécurité** pour M1+ : il a immédiatement attrapé les fixes du reviewer (le retrait de `ck_mission_test_mitre_tags_exactly_one_mitre_fk` aurait été un oubli silencieux sinon).
- **Lancer le DoD avant de dire "M1 done"** : règle gravée à M0, respectée ici. `make clean && make up && make migrate && make test-api && make e2e` est la séquence canonique de fin de milestone.

## 2026-05-11 — M3 RBAC, groupes, users, invitations

- **`logging.LogRecord` réserve `name`** comme attribut interne (en plus de `message`, `levelname`, `pathname`, `filename`, `module`, `funcName`, `lineno`, `asctime`, `process`, `thread`, `args`). Donc `log.info("metamorph.x.created", extra={"name": entity.name})` lève `KeyError: "Attempt to overwrite 'name' in LogRecord"`. Patron : préfixer toute clé risquée par l'entité (`group_name`, `user_name`, `template_name`). À documenter dans le style guide quand on en aura un.
- **Pattern "sentinel pour distinguer absent vs null"** : Pydantic ne sait pas distinguer `{}` de `{"display_name": null}` quand le champ est `str | None = None`. Solution : lire `raw = request.get_json()` puis tester `"display_name" in raw` dans la couche API, passer un sentinel `...` au service, qui distingue "ne pas toucher" de "set à None". Lourd mais explicite. Si ça revient souvent, encapsuler dans un helper `triState(raw, key, payload)`.
- **`limiter.reset()` flask-limiter** est public et clean — pas besoin de toucher à `limiter._storage`. À appeler dans `/diag/reset` quand le limiter est `enabled`. Toujours guarder avec `if limiter.enabled` pour ne pas planter en `APP_ENV=test`.
- **Rate-limit scope `APP_ENV in ("prod", "staging")`** : meilleure granularité que prod-only. La spec NF-security est *operator-facing*, pas dev. Trade-off réconcilié dans `app/core/rate_limit.py` avec un docstring explicite. Dev = ergonomics totale, prod/staging = limiter actif, test = désactivé.
- **Playwright `workers: 1` + `fullyParallel: false`** quand chaque spec file fait du `/diag/reset` (DB partagée). Avec parallélisme, les workers se truncate mutuellement entre eux → install token consumé, etc. Pattern simple et robuste : un seul worker pour les e2e, parallélisme intra-file laissé à `test.describe.configure({ mode: 'serial' })`.
- **Sessions Playwright entre tests** : chaque `test()` reçoit une `page` neuve (BrowserContext fresh). Pas de partage de session entre tests du même `describe`. Helper `loginViaSpa()` à appeler au début de chaque test SPA-driven (les tests purement API peuvent partager via une variable de spec mais c'est rare). Alternative : `storageState` global, mais ça complique le truncate workflow.
- **Dual seed = boot + bootstrap** : seeder les perms au boot ET dans `bootstrap_admin()` n'est pas redondant. Sur DB fraîchement migrée vide, le boot suffit. Mais après `/diag/reset` (qui TRUNCATE `permissions` + `group_permissions` + `groups`), seul `/setup` re-déclenche le chemin de seed via `bootstrap_admin → seed_all`. Sans ce 2e appel, l'admin créé aurait `is_admin=True` mais le catalogue serait vide.
- **Snapshot UserView/GroupView détachés** : retourner des `@dataclass(frozen=True)` au lieu de l'ORM permet de fermer le `session_scope` immédiatement. Plus simple que `s.expunge()` pour chaque champ, et la couche API peut sérialiser sans lazy-loading. Patron à reproduire pour tous les services.
- **Invariant "admin a toutes les perms"** : même si le décorateur bypass via `is_admin = "admin" in group_names` (et pas via le perm set), garder l'invariant côté API en refusant `set_group_permissions(admin_group, !=all_codes)`. Future-proof : si on bouge le bypass à un check perm-based plus tard, l'invariant tient déjà. `SystemGroupProtected` réutilisé pour le 409.
- **Toujours rebuild front + recreate containers** : `make rebuild` ne recrée pas les containers, donc le bundle nginx reste l'ancien. Patron canonique : `make down && make up`. Documenté pour la 2e fois dans M3 ; à faire passer en runbook au prochain `tasks/testing-m<N>.md`.

## 2026-05-10 — M2 auth, JWT, invitations

- **`pydantic.EmailStr` rejette les TLD réservés** (`.local`, `.corp`, `.test`, …) via `email-validator` `globally_deliverable=True`. Pour un outil red-team utilisé en lab/intranet, créer un type custom permissif (`Annotated[str, AfterValidator(...)]`) avec une regex RFC-shape. À garder en tête pour tout futur projet "internal".
- **Cookies `Secure=True` sur `localhost` HTTP** : modern browsers (Chrome ≥89, Firefox ≥75) traitent `localhost` comme un secure context et acceptent les cookies `Secure` même servis en HTTP. Donc on peut respecter la spec strictement (`Secure` toujours) sans casser le dev — pas besoin de gating par `APP_ENV`.
- **`getByLabel` de Playwright** prend le **nom accessible** de l'input. Quand un `<label>` enveloppe `input` + `<span>` hint + `<span>` error, le hint et l'error polluent le nom et `getByLabel('Password', exact: true)` ne matche plus. Pattern correct : `<div>` parent, `<label htmlFor>` séparé du `<input id>`, hint et error en `<p>` siblings hors du `<label>`.
- **`flask-limiter` doit être désactivé en `APP_ENV=test`** sinon les tests qui font 10+ logins de suite rate-limit. `Limiter(..., enabled=settings.APP_ENV != "test")` règle le cas globalement.
- **`pydantic[email]` extra** est REQUIS dès qu'on utilise `EmailStr`. Ne pas s'en rendre compte donne un crash gunicorn worker au boot avec `ImportError: email-validator is not installed`. À dupliquer dans le starter pyproject pour les futurs projets.
- **Compose `target:` est OBLIGATOIRE** quand un Dockerfile a un stage après le runtime — par défaut compose builde le DERNIER stage. J'ai été mordu deux fois (M1 puis M2). Désormais : tout Dockerfile multi-stage avec un stage de test/dev → `target: runtime` explicite dans `docker-compose.yml`.
- **Refresh token rotation + chain revoke** : à chaque `/auth/refresh`, on marque l'ancien token `revoked_at` + `replaced_by_id`. Si quelqu'un re-présente un token déjà rotaté, on cascade-revoke toute la chaîne (compromise probable). Pattern à reproduire pour tout système JWT à long terme.
- **`make rebuild` ne recrée pas les containers** — il faut `make down && make up` après un changement front pour que nginx serve le nouveau bundle. Important quand on debug un test e2e qui attend un selecteur récemment ajouté côté React.
- **`podman compose stop api` puis `up -d api` casse les dépendances** entre containers (`db` healthy → `api` depends on it) : podman-compose ne résout pas la chaîne de deps quand on cible un seul service. Pour un override d'env, mieux vaut `make down && APP_ENV=test make up`.
- **`/diag/reset` test-only** : exposer un endpoint qui truncate la DB est tentant pour les e2e mais ouvre une grosse surface en cas de fuite. Compromise actuel : autorisé en `dev` ET `test` (pas en prod), avec un log `WARNING` à chaque appel. Si jamais on déploie une stack dev publique, **désactiver** l'endpoint via env var.

<!--
Template for future entries:

## YYYY-MM-DD — short title
- bullet
- bullet
-->